Enforce the skill search deny at the worker mount

Author: TheGreatAxios

src/agent/directors/identity.test.ts

@@ -28,18 +28,24 @@ describe("formatDirectorSystemPrompt", () => {
     const text = formatDirectorSystemPrompt(DIRECTOR_REGISTRY.builder);
     expect(text).not.toContain("# Baked skill guidance");
     expect(text).toContain(
-      "Optional skills (names for awareness; load with skill_search then use_skill): style, philosophy, native-runtime, idiot-proof, ponytail.",
+      "Optional skills (names for awareness; load brief-named skills straight through use_skill, skill_search for discovery when mounted): style, philosophy, native-runtime, idiot-proof, ponytail.",
     );
     expect(text).toContain(WORKER_SKILL_SCOPING);
     expect(text).toContain(
       "Skills are available; search only when the brief names a skill or the task is outside your lane. For a small, bounded edit, do not search skills.",
     );
     expect(text).toContain(
-      "Call skill_search for descriptions, then use_skill",
+      "Load a brief-named skill straight through use_skill",
     );
     expect(text).toContain("load only the skills the task needs");
   });
 
+  test("worker guidance never mandates skill_search (deny-safe for grok/kimi leaves)", () => {
+    const text = formatDirectorSystemPrompt(DIRECTOR_REGISTRY.builder);
+    expect(text).not.toContain("Call skill_search for descriptions");
+    expect(text).toContain("only when choosing among skills and it is mounted");
+  });
+
   test("skywalker does not bake Ponytail", () => {
     const text = formatDirectorSystemPrompt(DIRECTOR_REGISTRY.skywalker);
     expect(text).not.toContain("### ponytail");
@@ -60,7 +66,7 @@ describe("formatDirectorSystemPrompt", () => {
     });
     expect(text).not.toContain("# Baked skill guidance");
     expect(text).toContain(
-      "Optional skills (names for awareness; load with skill_search then use_skill): does-not-exist-xyz.",
+      "Optional skills (names for awareness; load brief-named skills straight through use_skill, skill_search for discovery when mounted): does-not-exist-xyz.",
     );
     expect(text).toContain(WORKER_SKILL_SCOPING);
   });

src/agent/directors/identity.ts

@@ -33,14 +33,17 @@ export function formatDirectorSystemPrompt(pkg: DirectorPackage): string {
   } else if (isPrimaryOrchestrator) {
     skillsLine = `Optional skills (names for awareness; use_skill is primary-mounted): ${names.join(", ")}.`;
   } else {
-    skillsLine = `Optional skills (names for awareness; load with skill_search then use_skill): ${names.join(", ")}.`;
+    skillsLine = `Optional skills (names for awareness; load brief-named skills straight through use_skill, skill_search for discovery when mounted): ${names.join(", ")}.`;
   }
 
   // Worker skill scoping (CL-7668): skills mount on every worker, so search
   // only when the brief or the lane calls for it — never bulk-load.
+  // Deny-safe: grok/kimi leaves omit skill_search (family policy) and load
+  // brief-named skills straight through use_skill, so the guidance never
+  // mandates a skill_search call — it is discovery-only, when mounted.
   const skillGuidance =
     names !== undefined && names.length > 0 && !isPrimaryOrchestrator
-      ? `${WORKER_SKILL_SCOPING} Call skill_search for descriptions, then use_skill with the skill name; load only the skills the task needs.`
+      ? `${WORKER_SKILL_SCOPING} Load a brief-named skill straight through use_skill with its exact name; call skill_search for descriptions only when choosing among skills and it is mounted, then use_skill; load only the skills the task needs.`
       : null;
 
   const header = [

src/agent/directors/intern/package.ts

@@ -83,7 +83,7 @@ Do not invent fixes. If blocked, ask_director (parent, not the human). After the
 - Read error messages and report them
 - Execute mechanical, deterministic operations with zero ambiguity
 - Perform exact path writes when the brief spells them out
-- Load exactly the brief-named skill (if any) with \`skill_search\` then \`use_skill\` and follow it literally — never wander beyond it
+- Load exactly the brief-named skill (if any) with \`use_skill\` directly using its exact brief-given name — never a \`skill_search\` round-trip for a known name (\`skill_search\` is only for choosing among skills, and some workers do not mount it) — and follow it literally — never wander beyond it
 
 **How to Report Back**
 

src/agent/directors/tool-sets.test.ts

@@ -109,6 +109,21 @@ describe("SKYWALKER_TOOLS / ORCHESTRATOR_TOOLS", () => {
       expect(surface as readonly string[]).not.toContain("ask_operator");
     }
   });
+
+  test("no surface lists a tool twice (SKILL_TOOLS spread once via READ_TOOLS)", () => {
+    for (const surface of [
+      READ_TOOLS,
+      BUILD_TOOLS,
+      DOCS_TOOLS,
+      REVIEW_TOOLS,
+      INTERN_TOOLS,
+      ORCHESTRATOR_TOOLS,
+      SKYWALKER_TOOLS,
+    ] as const) {
+      const names = surface as readonly string[];
+      expect(new Set(names).size).toBe(names.length);
+    }
+  });
 });
 
 describe("REVIEW_TOOLS / INTERN_TOOLS", () => {

src/agent/directors/tool-sets.ts

@@ -42,7 +42,6 @@ export const PRODUCT_WRITE_TOOLS = [
 export const BUILD_TOOLS = [
   ...READ_TOOLS,
   ...PRODUCT_WRITE_TOOLS,
-  ...SKILL_TOOLS,
   "apply_patch",
   "shell",
   "update_plan",
@@ -62,17 +61,12 @@ export const BUILD_TOOLS = [
 export const DOCS_TOOLS = [
   ...READ_TOOLS.filter((t) => t !== "run_shell" && t !== "shell_collect"),
   ...PRODUCT_WRITE_TOOLS,
-  ...SKILL_TOOLS,
   "apply_patch",
   "update_plan",
 ] as const;
 
-/** Review / counsel: read surface + path writes + skill tools (lane discipline in prompts). */
-export const REVIEW_TOOLS = [
-  ...READ_TOOLS,
-  ...PRODUCT_WRITE_TOOLS,
-  ...SKILL_TOOLS,
-] as const;
+/** Review / counsel: read surface + path writes (skill tools arrive via READ_TOOLS; lane discipline in prompts). */
+export const REVIEW_TOOLS = [...READ_TOOLS, ...PRODUCT_WRITE_TOOLS] as const;
 
 /** Mechanical intern: shell-first + path writes when the brief requires them. */
 export const INTERN_TOOLS = [
@@ -92,7 +86,6 @@ export const INTERN_TOOLS = [
 export const ORCHESTRATOR_TOOLS = [
   ...READ_TOOLS,
   ...PRODUCT_WRITE_TOOLS,
-  ...SKILL_TOOLS,
   "spawn_agent",
   "list_agents",
   "close_agent",

src/session/assemble-runtime.test.ts

@@ -101,6 +101,24 @@ describe("createAdvertisedToolset", () => {
     expect(names).toContain("mcp__linear__save_issue");
     expect(names).not.toContain("mcp__acme__do");
   });
+
+  test("primary keeps skill_search for grok/kimi providers (always orchestrator; leaf deny lives at the worker mount)", () => {
+    for (const getProvider of [
+      () => ({ providerName: "xai", model: "grok-4-1-fast-non-reasoning" }),
+      () => ({ providerName: "moonshot", model: "kimi-k2-0711" }),
+    ]) {
+      const { computeAdvertised, isAdvertised } = createAdvertisedToolset(
+        wiring({ getProvider }),
+      );
+      const names = computeAdvertised([
+        def("skill_search"),
+        def("use_skill"),
+      ]).map((d) => d.name);
+      expect(names).toContain("skill_search");
+      expect(names).toContain("use_skill");
+      expect(isAdvertised("skill_search")).toBe(true);
+    }
+  });
 });
 
 describe("loadSessionLocalSettings", () => {

src/session/assemble-runtime.ts

@@ -370,9 +370,12 @@ export function createAdvertisedToolset(args: {
   ];
   const activated = createActivatedToolTracker();
   // Advertise then family-gate wire schemas (kimi gets a non-recursive present).
-  // advertisedToolDeny (CL-7668) drops grok/kimi-leaf skill_search from the
-  // wire prefix and the dispatch gate; orchestrators keep the full surface.
-  // Resolved per call so a live model switch re-gates without a rebuild.
+  // The primary session is always the orchestrator (SessionMode is the single
+  // literal "orchestrator"), so orchestrator: true is passed directly instead
+  // of comparing against sessionMode — the comparison was always true and the
+  // deny always [], an unexecuted committed claim. Leaf gating lives at the
+  // worker mount in subagent/run.ts, which resolves the same policy with the
+  // leaf's provider and its own orchestrator flag.
   // use_skill is never denied — leaves load brief-named skills by exact name.
   const deniedFor = (provider: {
     providerName: string;
@@ -381,7 +384,7 @@ export function createAdvertisedToolset(args: {
     resolveModelFamilyPolicy({
       providerName: provider.providerName,
       model: provider.model,
-      orchestrator: args.sessionMode === "orchestrator",
+      orchestrator: true,
     }).advertisedToolDeny;
   const computeAdvertised = (
     all: readonly ToolDefinition[],

src/subagent/run-skill-scope.test.ts

@@ -191,4 +191,106 @@ describe("runSubAgent worker skill mounts (CL-7668)", () => {
       'No skills matched "off-lane". Try different keywords describing the capability.',
     );
   }, 15_000);
+
+  test("grok/kimi leaves omit skill_search but keep scoped use_skill; orchestrators keep both", async () => {
+    const cwd = await tmpCwd();
+    await writeSkill(
+      cwd,
+      "style",
+      "Code style rules.",
+      "Follow the style guide.",
+    );
+
+    async function runCase(params: RunSubAgentParams): Promise<{
+      searchCalls: number;
+      useSkillCalls: number;
+    }> {
+      let searchCalls = 0;
+      let useSkillCalls = 0;
+      await runWithFailingInference((baseURL) =>
+        withMockedModuleDuring(
+          import.meta.resolve("../agent/skill-search.js"),
+          (real: typeof import("../agent/skill-search.js")) => ({
+            ...real,
+            createSkillSearchTool: (
+              args: Parameters<typeof real.createSkillSearchTool>[0],
+            ) => {
+              searchCalls += 1;
+              return real.createSkillSearchTool(args);
+            },
+          }),
+          () =>
+            withMockedModuleDuring(
+              import.meta.resolve("../agent/use-skill.js"),
+              (real: typeof import("../agent/use-skill.js")) => ({
+                ...real,
+                createUseSkillTool: (...args: unknown[]) => {
+                  useSkillCalls += 1;
+                  return (
+                    real.createUseSkillTool as (...a: never[]) => unknown
+                  )(...(args as never[]));
+                },
+              }),
+              async () => {
+                const { runSubAgent: run } = await import("./run.js");
+                await run({
+                  ...params,
+                  cwd,
+                  workdirBase: join(cwd, ".ctx"),
+                  provider: { ...params.provider, baseURL },
+                }).catch(() => {
+                  // Inference fails by design; mount decisions run first.
+                });
+              },
+            ),
+        ),
+      );
+      return { searchCalls, useSkillCalls };
+    }
+
+    function leafParams(
+      providerName: string,
+      model: string,
+      extra?: Partial<RunSubAgentParams>,
+    ): RunSubAgentParams {
+      return {
+        cwd,
+        workdirBase: join(cwd, ".ctx"),
+        permissionGate: testPermissionGate,
+        provider: { providerName, baseURL: "http://localhost", model },
+        description: "skill deny probe",
+        prompt: "no-op",
+        allowedSkillNames: ["style"],
+        ...extra,
+      };
+    }
+
+    // Grok + kimi leaves: deny executes — skill_search omitted, use_skill kept.
+    for (const [providerName, model] of [
+      ["xai", "grok-4-1-fast-non-reasoning"],
+      ["moonshot", "kimi-k2-0711"],
+    ] as const) {
+      const counts = await runCase(leafParams(providerName, model));
+      expect({ providerName, ...counts }).toEqual({
+        providerName,
+        searchCalls: 0,
+        useSkillCalls: 1,
+      });
+    }
+
+    // Default family leaf: both mount (existing behavior unchanged).
+    expect(await runCase(leafParams("test", "test-model"))).toEqual({
+      searchCalls: 1,
+      useSkillCalls: 1,
+    });
+
+    // Grok orchestrator: deny cleared — both mount.
+    expect(
+      await runCase(
+        leafParams("xai", "grok-4-1-fast-non-reasoning", {
+          orchestrator: true,
+        }),
+      ),
+    ).toEqual({ searchCalls: 1, useSkillCalls: 1 });
+  }, 30_000);
 });

src/subagent/run.ts

@@ -719,19 +719,34 @@ async function runSubAgentInner(
       }),
     ];
 
-    // Every worker mounts skill_search + use_skill, scoped to the dispatch's
-    // allowedSkillNames (pkg.optionalSkills). Mounted before the capability
-    // filter so worker allowlists keep them like any other named tool; the
-    // scope cannot widen — use_skill refuses names outside the allowlist.
+    // Worker skill mounts, family-gated (CL-7668): grok/kimi leaves omit
+    // skill_search and load brief-named skills straight through use_skill,
+    // which is never denied. Scoped to the dispatch's allowedSkillNames
+    // (pkg.optionalSkills). Mounted before the capability filter so worker
+    // allowlists keep them like any other named tool; the scope cannot
+    // widen — use_skill refuses names outside the allowlist.
+    // Resolved here (not below with the director wiring) so the mount itself
+    // executes the deny; toolNames/prompt derivation below inherits it.
+    const modelFamilyPolicy = resolveModelFamilyPolicy({
+      providerName: params.provider.providerName,
+      model: params.provider.model,
+      orchestrator: params.orchestrator === true,
+    });
     const skillSnapshot = await discoverSkills(params.cwd);
+    const skillSearchDenied =
+      modelFamilyPolicy.advertisedToolDeny.includes("skill_search");
     tools = [
       ...tools,
-      createSkillSearchTool({
-        skills: skillSnapshot,
-        ...(params.allowedSkillNames !== undefined
-          ? { allowedNames: params.allowedSkillNames }
-          : {}),
-      }),
+      ...(skillSearchDenied
+        ? []
+        : [
+            createSkillSearchTool({
+              skills: skillSnapshot,
+              ...(params.allowedSkillNames !== undefined
+                ? { allowedNames: params.allowedSkillNames }
+                : {}),
+            }),
+          ]),
       createUseSkillTool(
         params.cwd,
         [],
@@ -981,11 +996,9 @@ async function runSubAgentInner(
       });
     };
 
-    const modelFamilyPolicy = resolveModelFamilyPolicy({
-      providerName: params.provider.providerName,
-      model: params.provider.model,
-      orchestrator: params.orchestrator === true,
-    });
+    // modelFamilyPolicy is resolved above at the skill mount so the
+    // grok/kimi skill_search deny executes there; reused here for stall
+    // timing and wire-schema normalization.
 
     // Family-gate wire schemas the same way main sessions do (kimi present rewrite).
     // Sub-agent toolsets currently omit `present` (main-session only); normalize is