fix(core): validate skill commands against the live provider to survive late attach

packages/core/src/config/config.ts

@@ -8361,6 +8361,11 @@ export class Config {
    * skills, user/project file commands, MCP prompts). Called by the CLI's
    * CommandService after initialisation so that the startup snapshot and
    * per-turn drain can include these in the `<available_skills>` listing.
+   *
+   * Unlike `disabledSkillNamesProvider`, late attachment (after
+   * `Config.initialize()` has warmed the tool registry) is supported:
+   * `SkillTool.validateToolParams` consults this provider live rather than
+   * relying on its construction-time snapshot (issue #9821).
    */
   setModelInvocableCommandsProvider(
     provider: () => ReadonlyArray<{ name: string; description: string }>,

packages/core/src/tools/skill-utils.ts

@@ -50,6 +50,8 @@ export interface CollectedAvailableSkills {
   pendingConditionalSkillNames: Set<string>;
   /** Model-invocable commands, deduped against file-based skill names. */
   modelInvocableCommands: ReadonlyArray<{ name: string; description: string }>;
+  /** File-based skills hidden from model invocation. */
+  hiddenSkillNames?: Set<string>;
   /** Normalized entries, ready for `renderAvailableSkillsBlock`. */
   entries: AvailableSkillEntry[];
 }
@@ -144,6 +146,9 @@ async function collectAvailableSkillEntriesUncached(
       skillManager.isSkillActive(s) &&
       !isDisabled(s.name),
   );
+  const hiddenSkillNames = new Set(
+    allSkills.filter((s) => s.disableModelInvocation).map((s) => s.name),
+  );
 
   // Track still-pending conditional skills so validation can emit a distinct
   // "gated by paths:" hint. Disabled conditional skills are excluded — no point
@@ -196,6 +201,7 @@ async function collectAvailableSkillEntriesUncached(
     availableSkills,
     pendingConditionalSkillNames,
     modelInvocableCommands,
+    hiddenSkillNames,
     entries,
   };
 }

packages/core/src/tools/skill.test.ts

@@ -1270,6 +1270,34 @@ describe('SkillTool', () => {
       expect(result).toBeNull();
     });
 
+    it('should fall back to cached commands when the live provider throws', () => {
+      vi.mocked(config.getModelInvocableCommandsProvider).mockReturnValue(
+        () => {
+          throw new Error('boom');
+        },
+      );
+
+      const result = skillTool.validateToolParams({ skill: 'mcp-prompt-a' });
+      expect(result).toBeNull();
+    });
+
+    it('should accept a command with the same name as a hidden file skill', async () => {
+      vi.mocked(mockSkillManager.listSkills).mockResolvedValue([
+        {
+          name: 'mcp-prompt-a',
+          description: 'Hidden file-based skill',
+          level: 'project',
+          filePath: '/test/project/.qwen/skills/mcp-prompt-a/SKILL.md',
+          body: 'Hidden body',
+          disableModelInvocation: true,
+        },
+      ]);
+      await skillTool.refreshSkills();
+
+      const result = skillTool.validateToolParams({ skill: 'mcp-prompt-a' });
+      expect(result).toBeNull();
+    });
+
     it('should reject a name not in skills or commands, listing both in error', () => {
       const result = skillTool.validateToolParams({ skill: 'unknown' });
       expect(result).toContain('"unknown" not found');
@@ -1278,6 +1306,83 @@ describe('SkillTool', () => {
     });
   });
 
+  // Regression for issue #9821. In interactive mode the
+  // modelInvocableCommands provider is only registered once the CLI's
+  // CommandService finishes initialising — AFTER `Config.initialize()` →
+  // `toolRegistry.warmAll()` has already constructed SkillTool, whose
+  // constructor `refreshSkills()` therefore read a still-null provider and
+  // cached an empty command set. Nothing re-notifies SkillTool when the
+  // provider is attached, so validation kept rejecting every command until
+  // an unrelated SkillManager change event happened to re-run
+  // `refreshSkills()` — the source of the reported ~50% flakiness.
+  describe('late-attached modelInvocableCommands provider (issue #9821)', () => {
+    it('validates a command registered after construction with no SkillManager change event', () => {
+      // beforeEach already constructed skillTool with a null provider and
+      // drained the constructor's refreshSkills(). Register the provider
+      // late — deliberately WITHOUT firing a change listener — mirroring
+      // slashCommandProcessor's post-CommandService.create registration.
+      vi.mocked(config.getModelInvocableCommandsProvider).mockReturnValue(
+        () => [
+          { name: 'late-command', description: 'Registered after startup' },
+        ],
+      );
+
+      expect(
+        skillTool.validateToolParams({ skill: 'late-command' }),
+      ).toBeNull();
+    });
+
+    it('lists late-registered commands in the not-found error', () => {
+      vi.mocked(config.getModelInvocableCommandsProvider).mockReturnValue(
+        () => [
+          { name: 'late-command', description: 'Registered after startup' },
+        ],
+      );
+
+      const result = skillTool.validateToolParams({ skill: 'unknown' });
+      expect(result).toContain('"unknown" not found');
+      expect(result).toContain('late-command');
+    });
+
+    it('keeps path-gated skills gated when the provider is late-registered', async () => {
+      // The live read must apply the same file-based-skill name shadowing
+      // as collectAvailableSkillEntries, so a command named after a pending
+      // conditional skill cannot bypass the "gated by paths:" branch.
+      const conditionalSkill: SkillConfig = {
+        name: 'tsx-helper',
+        description: 'React TSX helper',
+        level: 'project',
+        filePath: '/test/project/.qwen/skills/tsx-helper/SKILL.md',
+        body: 'Body.',
+        paths: ['src/**/*.tsx'],
+      };
+      vi.mocked(mockSkillManager.listSkills).mockResolvedValue([
+        conditionalSkill,
+      ]);
+      vi.mocked(mockSkillManager.isSkillActive).mockImplementation(
+        (s: SkillConfig) => !s.paths || s.paths.length === 0,
+      );
+      const gatedTool = new SkillTool(config);
+      await vi.runAllTimersAsync();
+
+      // Late provider registration (SkillCommandLoader surfaces the skill).
+      vi.mocked(config.getModelInvocableCommandsProvider).mockReturnValue(
+        () => [{ name: 'tsx-helper', description: 'React TSX helper' }],
+      );
+
+      const result = gatedTool.validateToolParams({ skill: 'tsx-helper' });
+      expect(result).toMatch(/gated by path-based activation/);
+    });
+
+    it('still rejects unknown commands when no provider is ever registered', () => {
+      // SDK/headless mode without a provider: behavior must be unchanged.
+      const result = skillTool.validateToolParams({ skill: 'unknown' });
+      expect(result).toBe(
+        'Skill "unknown" not found. Available skills: code-review, testing',
+      );
+    });
+  });
+
   describe('commandExecutor fallback in execute()', () => {
     beforeEach(async () => {
       // Expose an MCP-only command that has no file-based skill
@@ -1417,6 +1522,43 @@ describe('SkillTool', () => {
   });
 
   describe('disabled-skill execute guard', () => {
+    it('runs the same-named MCP prompt instead of loading a hidden skill', async () => {
+      vi.mocked(mockSkillManager.listSkills).mockResolvedValue([
+        {
+          name: 'mcp-prompt-a',
+          description: 'Hidden file-based skill',
+          level: 'project',
+          filePath: '/test/project/.qwen/skills/mcp-prompt-a/SKILL.md',
+          body: 'HIDDEN skill body must not execute',
+          disableModelInvocation: true,
+        },
+      ]);
+      const hiddenAwareTool = new SkillTool(config);
+      await vi.runAllTimersAsync();
+      const executor = vi.fn().mockResolvedValue('MCP prompt body');
+      vi.mocked(config.getModelInvocableCommandsExecutor).mockReturnValue(
+        executor,
+      );
+      vi.mocked(mockSkillManager.loadSkillForRuntime).mockResolvedValue({
+        name: 'mcp-prompt-a',
+        description: 'Hidden file-based skill',
+        level: 'project',
+        filePath: '/test/project/.qwen/skills/mcp-prompt-a/SKILL.md',
+        body: 'HIDDEN skill body must not execute',
+        disableModelInvocation: true,
+      });
+
+      const invocation = (
+        hiddenAwareTool as SkillToolWithProtectedMethods
+      ).createInvocation({ skill: 'mcp-prompt-a' });
+      const result = await invocation.execute();
+
+      expect(mockSkillManager.loadSkillForRuntime).not.toHaveBeenCalled();
+      expect(executor).toHaveBeenCalledWith('mcp-prompt-a', '');
+      expect(partToString(result.llmContent)).toBe('MCP prompt body');
+      expect(result.returnDisplay).toBe('Delegated to command: mcp-prompt-a');
+    });
+
     it('runs the same-named MCP prompt instead of loading a disabled skill', async () => {
       // Regression: without the execute-side guard,
       // `loadSkillForRuntime` resolves the disabled skill from disk and

packages/core/src/tools/skill.ts

@@ -97,6 +97,7 @@ export class SkillTool extends BaseDeclarativeTool<SkillParams, ToolResult> {
     name: string;
     description: string;
   }> = [];
+  private hiddenSkillNames: Set<string> = new Set();
   private loadedSkillNames: Set<string> = new Set();
   // Cleanup function returned by `addChangeListener`. Stored so per-agent
   // SkillTool instances (subagents share the parent's SkillManager) can
@@ -184,11 +185,13 @@ export class SkillTool extends BaseDeclarativeTool<SkillParams, ToolResult> {
       this.pendingConditionalSkillNames =
         collected.pendingConditionalSkillNames;
       this.modelInvocableCommands = collected.modelInvocableCommands;
+      this.hiddenSkillNames = collected.hiddenSkillNames ?? new Set();
     } catch (error) {
       debugLogger.warn('Failed to load skills for Skills tool:', error);
       this.availableSkills = [];
       this.pendingConditionalSkillNames = new Set();
       this.modelInvocableCommands = [];
+      this.hiddenSkillNames = new Set();
     }
   }
 
@@ -211,8 +214,14 @@ export class SkillTool extends BaseDeclarativeTool<SkillParams, ToolResult> {
     );
     if (skillExists) return null;
 
-    // Check model-invocable commands (e.g. MCP prompts) listed in <available_skills>
-    const commandExists = this.modelInvocableCommands.some(
+    // Check model-invocable commands (e.g. MCP prompts) listed in
+    // <available_skills>. Consults the live provider — not just the cached
+    // snapshot — because in interactive mode the provider is only attached
+    // after CommandService initialisation resolves, which races SkillTool
+    // construction: the constructor's refreshSkills() then reads a still-null
+    // provider and caches an empty command set that is never refreshed unless
+    // an unrelated SkillManager change event happens to fire (issue #9821).
+    const commandExists = this.getModelInvocableCommands().some(
       (cmd) => cmd.name === params.skill,
     );
     if (commandExists) return null;
@@ -236,15 +245,57 @@ export class SkillTool extends BaseDeclarativeTool<SkillParams, ToolResult> {
     }
 
     const availableNames = [
-      ...this.availableSkills.map((s) => s.name),
-      ...this.modelInvocableCommands.map((c) => c.name),
+      ...new Set([
+        ...this.availableSkills.map((s) => s.name),
+        ...this.getModelInvocableCommands().map((c) => c.name),
+      ]),
     ];
     if (availableNames.length === 0) {
       return `Skill "${params.skill}" not found. No skills are currently available.`;
     }
     return `Skill "${params.skill}" not found. Available skills: ${availableNames.join(', ')}`;
   }
 
+  /**
+   * Returns the model-invocable commands to validate against, preferring a
+   * live read of the config provider over the cached snapshot from the last
+   * `refreshSkills()` (see `validateToolParams` for the late-attach race).
+   * Falls back to the cache when no provider is registered (e.g. SDK mode)
+   * or when the provider throws. The provider is synchronous, so the live
+   * read is cheap enough to run on every validation.
+   *
+   * Commands whose names collide with a file-based skill (active or pending
+   * path-activation) are dropped, mirroring the `fileBasedSkillNames` dedup
+   * in `collectAvailableSkillEntries` — without this, a command named after
+   * a path-gated skill would pass validation here and bypass the
+   * "gated by paths:" branch above.
+   */
+  private getModelInvocableCommands(): ReadonlyArray<{
+    name: string;
+    description: string;
+  }> {
+    let commands: ReadonlyArray<{ name: string; description: string }>;
+    const provider = this.config.getModelInvocableCommandsProvider();
+    if (provider) {
+      try {
+        commands = provider();
+      } catch (error) {
+        debugLogger.warn(
+          'Model-invocable commands provider threw; falling back to cached set:',
+          error,
+        );
+        commands = this.modelInvocableCommands;
+      }
+    } else {
+      commands = this.modelInvocableCommands;
+    }
+    const shadowedNames = new Set<string>([
+      ...this.availableSkills.map((skill) => skill.name),
+      ...this.pendingConditionalSkillNames,
+    ]);
+    return commands.filter((cmd) => !shadowedNames.has(cmd.name));
+  }
+
   protected createInvocation(params: SkillParams) {
     return new SkillToolInvocation(
       this.config,
@@ -253,6 +304,7 @@ export class SkillTool extends BaseDeclarativeTool<SkillParams, ToolResult> {
       (name: string) => this.loadedSkillNames.add(name),
       this.config.getModelInvocableCommandsExecutor(),
       (name: string) => this.loadedSkillNames.has(name),
+      (name: string) => this.hiddenSkillNames.has(name),
     );
   }
 
@@ -315,6 +367,7 @@ class SkillToolInvocation extends BaseToolInvocation<SkillParams, ToolResult> {
         ) => Promise<ModelInvocableCommandExecutorResult | null>)
       | null = null,
     private readonly isSkillLoaded: (name: string) => boolean = () => false,
+    private readonly isSkillHidden: (name: string) => boolean = () => false,
   ) {
     super(params);
   }
@@ -357,6 +410,33 @@ class SkillToolInvocation extends BaseToolInvocation<SkillParams, ToolResult> {
     _signal?: AbortSignal,
     _updateOutput?: (output: ToolResultDisplay) => void,
   ): Promise<ToolResult> {
+    if (this.isSkillHidden(this.params.skill)) {
+      if (this.commandExecutor) {
+        try {
+          const content = await this.commandExecutor(
+            this.params.skill,
+            this.params.args ?? '',
+          );
+          if (content && typeof content === 'object' && 'error' in content) {
+            return {
+              llmContent: content.error,
+              returnDisplay: content.error,
+            };
+          }
+          if (typeof content === 'string') {
+            return {
+              llmContent: [{ text: content }],
+              returnDisplay: `Delegated to command: ${this.params.skill}`,
+            };
+          }
+        } catch {
+          // Fall through to the generic not-found message.
+        }
+      }
+      const msg = `Skill "${this.params.skill}" not found.`;
+      return { llmContent: msg, returnDisplay: msg };
+    }
+
     // Disabled-skill guard. Mirrors validateToolParams's commandExists →
     // disabled ordering at the execution layer: when a skill is disabled
     // but a same-named non-skill command (MCP prompt, file command)