[feat] Extend the delivery check to rules, agents and MCP #624

Activity

1. SaulMoro commented on Sep 18, 2026

   

   SaulMoro

   on Sep 18, 2026

   Last edited by SaulMoro

   CollaboratorAuthor

   More actions

   Implementing the skills half in #625 turned up a seam this issue will need, so recording it here rather than as its own issue: it has no user-visible symptom left, and whoever extends the check to rules and agents is the person who will want it.

   What happened

   doctor's <tool> is installed probed the tool root, while skill delivery asks skillTargetForTool. Those disagree for OpenClaw, which keeps its skills under a workspace directory rather than under ~/.openclaw. A tool root on its own passed the check while delivery skipped the tool and its delivery check vanished with it, which is the failure this whole line of work exists to catch, reproduced inside the command written to catch it.

   Fixed in #625 by having the probe ask the same resolver the sync uses. That fix needed this:

   const INSTALL_PROBE_SKILL = '__teamai_probe__';
   
   export async function skillsReachTool(tool, configuredSkillsPath, localConfig): Promise<boolean> {
     return await skillTargetForTool(tool, configuredSkillsPath, localConfig, INSTALL_PROBE_SKILL) !== null;
   }

   A yes/no question has to invent a skill name, because skillTargetForTool fuses two questions: whether the tool can receive skills at all, and where a given skill lands. Only a comment says the name cannot affect the first one.

   Why it matters here

   Rules and agents each need the same gate, and today the codebase answers it three different ways for skills alone:

   skillTargetForTool            openclaw -> workspace dir, hermes -> hermes home,
     (src/resources/skills.ts)   else isToolInstalledForConfig
   isToolInstalledForConfig      copilot -> enabledAgents, else tool root
     (src/resources/base.ts)     used by agents.ts:420,501,559 and pull.ts:550,602
   ResourceHandler.isToolInstalled   tool root only, no copilot, no openclaw, no hermes
     (pull.ts:432)               inactive-skill cleanup, against resolveBaseDir
   

   So agents already answers the question with the generic probe, which is the spelling that was wrong for OpenClaw's skills. Adding a delivery check for agents on top of that would bake the same disagreement in, one resource type further along. The third spelling at pull.ts:432 judges installation for the inactive-skill cleanup differently again, and for OpenClaw it looks under a directory delivery never writes to.

   Suggested shape

   Correcting myself: this is not a separate proposal. deliveryTargets(teamConfig, localConfig, item) in the issue body already answers "which tools receive this item", so the gate below is that contract restricted to a yes/no, and the two should land as one piece of work rather than compete.

   Split the gate from the path, so the gate can be asked on its own and a new caller cannot get it wrong by omission:

   +/** Whether `tool` receives this resource type at all on this machine. */
   +export async function toolReceives(
   +  type: ResourceType,
   +  tool: string,
   +  configuredPath: string | undefined,
   +  localConfig: LocalConfig,
   +): Promise<boolean>
   +
    export async function skillTargetForTool(tool, configuredPath, localConfig, skillName, sourcePath?) {
   -  if (!configuredPath) return null;
   -  if (tool === 'openclaw') { ... }
   -  if (tool === 'hermes') { ... }
   -  if (!await isToolInstalledForConfig(tool, configuredPath, localConfig)) return null;
   +  if (!await toolReceives('skills', tool, configuredPath, localConfig)) return null;
      ...
    }

   Then doctor's installed check, the delivery checks for skills, rules and agents, the cleanup at pull.ts:432, and the agents gates all read one function, and skillsReachTool and its invented probe name go away. If deliveryTargets lands first, toolReceives is one line on top of it and does not need to exist as its own function.

   Worth doing before the rules and agents checks rather than after: added afterwards it is a change across five call sites plus three new ones, and each of those is a place where the check and the sync can drift apart again.

   Related: #625 also left resolveSkillDestination with two behaviours depending on whether sourcePath is passed, held together by a comment. That one is narrower and can ride along with this.
2. jeff-r2026

   added a commit that references this issue on Sep 20, 2026

   feat(doctor): extend the delivery check to rules, agents, MCP and env (…

   ...

   52525a9
3. jeff-r2026

   closed this as completedin #669on Sep 20, 2026