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

packages/core/src/config/config.ts

@@ -8501,6 +8501,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,138 @@ describe('SkillTool', () => {
   });
 
   describe('disabled-skill execute guard', () => {
+    const createHiddenSkillInvocation = async (
+      executor: ReturnType<Config['getModelInvocableCommandsExecutor']>,
+    ) => {
+      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();
+      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,
+      });
+
+      return (
+        hiddenAwareTool as SkillToolWithProtectedMethods
+      ).createInvocation({ skill: 'mcp-prompt-a' });
+    };
+
+    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('returns command executor errors for hidden skill command alternatives', async () => {
+      const executor = vi
+        .fn()
+        .mockResolvedValue({ error: 'MCP prompt failed' });
+      const invocation = await createHiddenSkillInvocation(executor);
+      const result = await invocation.execute();
+
+      expect(executor).toHaveBeenCalledWith('mcp-prompt-a', '');
+      expect(mockSkillManager.loadSkillForRuntime).not.toHaveBeenCalled();
+      expect(partToString(result.llmContent)).toBe('MCP prompt failed');
+      expect(result.returnDisplay).toBe('MCP prompt failed');
+      expect(recordSkillInvocation).not.toHaveBeenCalled();
+    });
+
+    it('falls through to not-found when hidden skill commandExecutor throws', async () => {
+      const executor = vi.fn().mockRejectedValue(new Error('MCP timeout'));
+      const invocation = await createHiddenSkillInvocation(executor);
+      const result = await invocation.execute();
+
+      expect(executor).toHaveBeenCalledWith('mcp-prompt-a', '');
+      expect(mockSkillManager.loadSkillForRuntime).not.toHaveBeenCalled();
+      expect(partToString(result.llmContent)).toBe(
+        'Skill "mcp-prompt-a" not found.',
+      );
+      expect(recordSkillInvocation).not.toHaveBeenCalled();
+    });
+
+    it('returns not-found when a hidden skill command alternative returns null', async () => {
+      const executor = vi.fn().mockResolvedValue(null);
+      const invocation = await createHiddenSkillInvocation(executor);
+      const result = await invocation.execute();
+
+      expect(executor).toHaveBeenCalledWith('mcp-prompt-a', '');
+      expect(mockSkillManager.loadSkillForRuntime).not.toHaveBeenCalled();
+      expect(partToString(result.llmContent)).toBe(
+        'Skill "mcp-prompt-a" not found.',
+      );
+      expect(recordSkillInvocation).not.toHaveBeenCalled();
+    });
+
+    it('returns not-found and records failure when no hidden skill command alternative exists', async () => {
+      const invocation = await createHiddenSkillInvocation(null);
+      invocation.setPromptId('prompt-123');
+      const result = await invocation.execute();
+
+      expect(mockSkillManager.loadSkillForRuntime).not.toHaveBeenCalled();
+      expect(partToString(result.llmContent)).toBe(
+        'Skill "mcp-prompt-a" not found.',
+      );
+      expect(logSkillLaunch).toHaveBeenCalledWith(
+        config,
+        expect.objectContaining({
+          skill_name: 'mcp-prompt-a',
+          success: false,
+          prompt_id: 'prompt-123',
+        }),
+      );
+      expect(recordSkillInvocation).toHaveBeenCalledWith(config, {
+        skillName: 'mcp-prompt-a',
+        success: false,
+      });
+    });
+
     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