From f65ca2bb7ac60051dae9a6fafa627982554f0052 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 12 Sep 2026 21:55:42 -0700 Subject: [PATCH 1/7] Mount skill_search and use_skill on every worker tool surface --- src/agent/directors/tool-sets.test.ts | 18 +++++++++++++----- src/agent/directors/tool-sets.ts | 14 ++++++++++++-- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/agent/directors/tool-sets.test.ts b/src/agent/directors/tool-sets.test.ts index 76ef43489..acc0096fe 100644 --- a/src/agent/directors/tool-sets.test.ts +++ b/src/agent/directors/tool-sets.test.ts @@ -92,11 +92,19 @@ describe("SKYWALKER_TOOLS / ORCHESTRATOR_TOOLS", () => { ); }); - test("skill_search is not on Skywalker or worker orchestrator allowlists", () => { - expect(SKYWALKER_TOOLS as readonly string[]).not.toContain("skill_search"); - expect(ORCHESTRATOR_TOOLS as readonly string[]).not.toContain( - "skill_search", - ); + test("skill_search + use_skill mount on every worker surface", () => { + for (const surface of [ + READ_TOOLS, + BUILD_TOOLS, + DOCS_TOOLS, + REVIEW_TOOLS, + INTERN_TOOLS, + ORCHESTRATOR_TOOLS, + SKYWALKER_TOOLS, + ] as const) { + expect(surface as readonly string[]).toContain("skill_search"); + expect(surface as readonly string[]).toContain("use_skill"); + } }); }); diff --git a/src/agent/directors/tool-sets.ts b/src/agent/directors/tool-sets.ts index cc3cc8987..95069c717 100644 --- a/src/agent/directors/tool-sets.ts +++ b/src/agent/directors/tool-sets.ts @@ -1,8 +1,16 @@ // Small, explicit tool allowlists for director packages. // Prefer tools.allow at mount (CapabilityFilter include) over huge deny lists. // manage_tasks is always mounted by runSubAgent after the filter — omit it here. -// use_skill / tool_search / ask_operator are primary-session tools: fleet agents / -// workers do not mount ask_operator (skill guidance is baked into package system prompts). +// skill_search + use_skill mount on every worker through runSubAgent (scoped to +// the dispatch's optionalSkills); they ride the allowlists below so the +// capability filter keeps them. ask_operator stays primary-only. + +/** + * Skill discovery/loading, mounted on every worker surface (read/build/docs/ + * review/intern/orchestrator). runSubAgent scopes both tools to the + * dispatch's optionalSkills before the capability filter. + */ +export const SKILL_TOOLS = ["skill_search", "use_skill"] as const; /** Read/search/shell — no product mutation. */ export const READ_TOOLS = [ @@ -15,6 +23,7 @@ export const READ_TOOLS = [ "shell_collect", "web_fetch", "web_search", + ...SKILL_TOOLS, ] as const; /** @@ -69,6 +78,7 @@ export const INTERN_TOOLS = [ "read_file", "list_dir", ...PRODUCT_WRITE_TOOLS, + ...SKILL_TOOLS, ] as const; /** From 92a565788e486baee80511a218acecbe936d4b07 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 12 Sep 2026 21:59:07 -0700 Subject: [PATCH 2/7] Mount scoped skill_search and use_skill on every worker --- src/subagent/agent-fleet.ts | 3 + src/subagent/run-skill-scope.test.ts | 194 +++++++++++++++++++++++++++ src/subagent/run.ts | 24 ++++ src/subagent/types.ts | 8 ++ 4 files changed, 229 insertions(+) create mode 100644 src/subagent/run-skill-scope.test.ts diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index 2ee7cbe40..e45fcad21 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -1399,6 +1399,9 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { ...(resolved.capabilities !== undefined ? { capabilities: resolved.capabilities } : {}), + ...(resolved.pkg?.optionalSkills !== undefined + ? { allowedSkillNames: resolved.pkg.optionalSkills } + : {}), ...(resolved.systemPromptRole !== undefined ? { systemPromptRole: resolved.systemPromptRole } : {}), diff --git a/src/subagent/run-skill-scope.test.ts b/src/subagent/run-skill-scope.test.ts new file mode 100644 index 000000000..162b9eff8 --- /dev/null +++ b/src/subagent/run-skill-scope.test.ts @@ -0,0 +1,194 @@ +/** + * runSubAgent mounts skill_search + use_skill on every worker, scoped to the + * dispatch's allowedSkillNames (pkg.optionalSkills). The scope cannot widen: + * use_skill refuses names outside the allowlist (CL-6803 stays closed) and + * skill_search hides them. + * + * Pattern follows run-authority.test.ts: drive the real runSubAgent with + * failing inference (mount decisions run before the send) while wrapping the + * real skill factories to capture the mounted tools. + */ +import { describe, expect, test } from "bun:test"; +import { mkdir, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { withMockedModuleDuring } from "../../tests/helpers/mock-module.js"; +import { createPermissionGate } from "../permission/gate.js"; +import type { RunSubAgentParams } from "./types.js"; + +const testPermissionGate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: true, + reactorGated: false, +}); + +async function tmpCwd(): Promise { + const cwd = join( + tmpdir(), + `cl7668-skill-scope-${Date.now()}-${Math.random()}`, + ); + await mkdir(cwd, { recursive: true }); + return cwd; +} + +async function writeSkill( + cwd: string, + name: string, + description: string, + body: string, +): Promise { + const dir = join(cwd, ".agents", "skills", name); + await mkdir(dir, { recursive: true }); + await writeFile( + join(dir, "SKILL.md"), + `---\nname: ${name}\ndescription: ${description}\n---\n\n${body}\n`, + ); +} + +async function runWithFailingInference( + run: (baseURL: string) => Promise, +): Promise { + const server = Bun.serve({ + port: 0, + fetch: () => + new Response(JSON.stringify({ error: { message: "probe" } }), { + status: 401, + headers: { "content-type": "application/json" }, + }), + }); + try { + await run(server.url.origin); + } finally { + server.stop(true); + } +} + +function baseParams( + cwd: string, + workdirBase: string, + baseURL: string, +): RunSubAgentParams { + return { + cwd, + workdirBase, + permissionGate: testPermissionGate, + provider: { providerName: "test", baseURL, model: "test-model" }, + description: "skill scope probe", + prompt: "no-op", + allowedSkillNames: ["style"], + }; +} + +describe("runSubAgent worker skill mounts (CL-7668)", () => { + test("mounts skill_search + use_skill scoped to allowedSkillNames; out-of-scope names refuse", async () => { + const cwd = await tmpCwd(); + await writeSkill( + cwd, + "style", + "Code style rules.", + "Follow the style guide.", + ); + await writeSkill(cwd, "off-lane", "Unrelated lane.", "Off-lane body."); + + let searchArgs: + | { skills: { name: string }[]; allowedNames?: readonly string[] } + | undefined; + let useSkillArgs: readonly unknown[] | undefined; + let searchTool: + | { + kind: string; + handler: ( + args: Record, + signal: AbortSignal, + ) => Promise; + } + | undefined; + let useSkillTool: + | { + kind: string; + handler: ( + args: Record, + signal: AbortSignal, + ) => Promise; + } + | undefined; + + await runWithFailingInference((baseURL) => + withMockedModuleDuring( + import.meta.resolve("../agent/skill-search.js"), + (real: typeof import("../agent/skill-search.js")) => ({ + ...real, + createSkillSearchTool: (args: { + skills: { name: string; description: string }[]; + allowedNames?: readonly string[]; + }) => { + searchArgs = args; + const tool = real.createSkillSearchTool(args); + if (tool.kind !== "string") throw new Error("expected string tool"); + searchTool = tool as typeof searchTool & {}; + return tool; + }, + }), + () => + withMockedModuleDuring( + import.meta.resolve("../agent/use-skill.js"), + (real: typeof import("../agent/use-skill.js")) => ({ + ...real, + createUseSkillTool: (...args: unknown[]) => { + useSkillArgs = args; + const tool = ( + real.createUseSkillTool as (...a: never[]) => unknown + )(...(args as never[])); + if ( + typeof tool !== "object" || + tool === null || + (tool as { kind: string }).kind !== "string" + ) + throw new Error("expected string tool"); + useSkillTool = tool as typeof useSkillTool & {}; + return tool; + }, + }), + async () => { + const { runSubAgent: run } = await import("./run.js"); + await run(baseParams(cwd, join(cwd, ".ctx"), baseURL)).catch( + () => { + // Inference fails by design; mount decisions run first. + }, + ); + }, + ), + ), + ); + + // Both tools mount exactly once, scoped to the dispatch allowlist. + expect(searchArgs).toBeDefined(); + expect(searchArgs?.allowedNames).toEqual(["style"]); + expect(searchArgs?.skills.map((s) => s.name).sort()).toEqual([ + "off-lane", + "style", + ]); + expect(useSkillArgs?.[0]).toBe(cwd); + expect(useSkillArgs?.[3]).toEqual(["style"]); + expect(searchTool).toBeDefined(); + expect(useSkillTool).toBeDefined(); + + const signal = new AbortController().signal; + // In-scope skill loads. + const loaded = await useSkillTool?.handler({ name: "style" }, signal); + expect(loaded).toContain('Skill "style"'); + expect(loaded).toContain("Follow the style guide."); + const found = await searchTool?.handler({ query: "style" }, signal); + expect(found).toContain("- style: Code style rules."); + + // Out-of-scope names refuse — CL-6803 stays closed. + expect(await useSkillTool?.handler({ name: "off-lane" }, signal)).toBe( + 'No skill named "off-lane" is available.', + ); + expect(await searchTool?.handler({ query: "off-lane" }, signal)).toBe( + 'No skills matched "off-lane". Try different keywords describing the capability.', + ); + }, 15_000); +}); diff --git a/src/subagent/run.ts b/src/subagent/run.ts index aec0d348e..9adecdc25 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -107,6 +107,9 @@ import type { CapabilityFilter } from "../agent/profiles.js"; import type { Settings } from "../config/settings.js"; import { toolWatchdogFromSettings } from "../config/settings.js"; import { createSearchAgentsTool } from "../agent/agent-search.js"; +import { createSkillSearchTool } from "../agent/skill-search.js"; +import { createUseSkillTool } from "../agent/use-skill.js"; +import { discoverSkills } from "../extensions/skills.js"; import { createManageTasksRunner, manageTasksDefinition, @@ -719,6 +722,27 @@ 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. + const skillSnapshot = await discoverSkills(params.cwd); + tools = [ + ...tools, + createSkillSearchTool({ + skills: skillSnapshot, + ...(params.allowedSkillNames !== undefined + ? { allowedNames: params.allowedSkillNames } + : {}), + }), + createUseSkillTool( + params.cwd, + [], + liveTelemetry, + params.allowedSkillNames, + ), + ]; + if (params.capabilities !== undefined) { tools = applyCapabilityFilter(tools, params.capabilities); } diff --git a/src/subagent/types.ts b/src/subagent/types.ts index ec2d53c00..ea62371f7 100644 --- a/src/subagent/types.ts +++ b/src/subagent/types.ts @@ -117,6 +117,14 @@ export type RunSubAgentParams = { onProgress?: (info: { description: string; toolName: string }) => void; onRunSettled?: (summary: Readonly) => void; capabilities?: CapabilityFilter; + /** + * Skill allowlist for the worker's skill_search + use_skill mounts, + * resolved by the caller (agent-fleet.ts) from the dispatch's + * DirectorPackage.optionalSkills. When set, both tools only see these + * names (the allowlist cannot widen: unknown names refuse). When unset + * (non-director plugin profiles), the worker sees every discovered skill. + */ + allowedSkillNames?: readonly string[]; systemPromptRole?: string; /** Resolved closed-director id (e.g. "critic") when the worker is one. Structured gate key — prefer over persona-string matching in systemPromptRole. */ directorId?: string; From 7991aaed10bcbd6a56d48dad29a3082dd4092c6e Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 12 Sep 2026 22:20:58 -0700 Subject: [PATCH 3/7] Spread skill tools across build, docs, review, and orchestrator surfaces --- src/agent/directors/tool-sets.test.ts | 5 ++++- src/agent/directors/tool-sets.ts | 25 ++++++++++++++----------- src/agent/tool-classification.test.ts | 3 +++ 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/agent/directors/tool-sets.test.ts b/src/agent/directors/tool-sets.test.ts index acc0096fe..940e57a40 100644 --- a/src/agent/directors/tool-sets.test.ts +++ b/src/agent/directors/tool-sets.test.ts @@ -7,6 +7,7 @@ import { READ_TOOLS, REVIEW_TOOLS, INTERN_TOOLS, + SKILL_TOOLS, SKYWALKER_TOOLS, } from "./tool-sets.js"; @@ -92,7 +93,8 @@ describe("SKYWALKER_TOOLS / ORCHESTRATOR_TOOLS", () => { ); }); - test("skill_search + use_skill mount on every worker surface", () => { + test("skill_search + use_skill mount on every worker surface, never ask_operator", () => { + expect([...SKILL_TOOLS]).toEqual(["skill_search", "use_skill"]); for (const surface of [ READ_TOOLS, BUILD_TOOLS, @@ -104,6 +106,7 @@ describe("SKYWALKER_TOOLS / ORCHESTRATOR_TOOLS", () => { ] as const) { expect(surface as readonly string[]).toContain("skill_search"); expect(surface as readonly string[]).toContain("use_skill"); + expect(surface as readonly string[]).not.toContain("ask_operator"); } }); }); diff --git a/src/agent/directors/tool-sets.ts b/src/agent/directors/tool-sets.ts index 95069c717..fb1014ce2 100644 --- a/src/agent/directors/tool-sets.ts +++ b/src/agent/directors/tool-sets.ts @@ -1,18 +1,14 @@ // Small, explicit tool allowlists for director packages. // Prefer tools.allow at mount (CapabilityFilter include) over huge deny lists. // manage_tasks is always mounted by runSubAgent after the filter — omit it here. -// skill_search + use_skill mount on every worker through runSubAgent (scoped to -// the dispatch's optionalSkills); they ride the allowlists below so the -// capability filter keeps them. ask_operator stays primary-only. +// skill_search + use_skill mount on every worker, scoped at mount to the +// dispatch's optionalSkills. ask_operator stays primary-session-only: workers +// never mount it (Do not #1). -/** - * Skill discovery/loading, mounted on every worker surface (read/build/docs/ - * review/intern/orchestrator). runSubAgent scopes both tools to the - * dispatch's optionalSkills before the capability filter. - */ +/** Skill discovery + loading — mounted on every worker surface below. */ export const SKILL_TOOLS = ["skill_search", "use_skill"] as const; -/** Read/search/shell — no product mutation. */ +/** Read/search/shell + skill tools — no product mutation. */ export const READ_TOOLS = [ "read_file", "grep", @@ -46,6 +42,7 @@ export const PRODUCT_WRITE_TOOLS = [ export const BUILD_TOOLS = [ ...READ_TOOLS, ...PRODUCT_WRITE_TOOLS, + ...SKILL_TOOLS, "apply_patch", "shell", "update_plan", @@ -65,12 +62,17 @@ 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 (lane discipline in prompts). */ -export const REVIEW_TOOLS = [...READ_TOOLS, ...PRODUCT_WRITE_TOOLS] 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; /** Mechanical intern: shell-first + path writes when the brief requires them. */ export const INTERN_TOOLS = [ @@ -90,6 +92,7 @@ export const INTERN_TOOLS = [ export const ORCHESTRATOR_TOOLS = [ ...READ_TOOLS, ...PRODUCT_WRITE_TOOLS, + ...SKILL_TOOLS, "spawn_agent", "list_agents", "close_agent", diff --git a/src/agent/tool-classification.test.ts b/src/agent/tool-classification.test.ts index 1325c44a7..b2bd46815 100644 --- a/src/agent/tool-classification.test.ts +++ b/src/agent/tool-classification.test.ts @@ -18,6 +18,9 @@ describe("AUTO_ALLOW_READ_TOOLS", () => { "manage_tasks", "read_file", "search_files", + // CL-7668: read-only skill discovery/loading needs no approval prompt. + "skill_search", + "use_skill", ].sort(), ); }); From 2e8e804592e677fbc331d01a0d0a1e967833c01e Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 12 Sep 2026 22:21:04 -0700 Subject: [PATCH 4/7] Load worker skills on demand instead of baking bodies Baked skill bodies cost roughly 38KB per worker prompt; workers now search and load only the skills the brief names. --- src/agent/directors/bake-skills.test.ts | 101 --------------- src/agent/directors/bake-skills.ts | 90 -------------- src/agent/directors/builder/package.test.ts | 5 +- src/agent/directors/builder/package.ts | 2 +- src/agent/directors/critic/package.test.ts | 3 +- src/agent/directors/critic/package.ts | 2 +- src/agent/directors/draper/package.test.ts | 3 +- src/agent/directors/emil/package.test.ts | 3 +- src/agent/directors/greybeard/package.ts | 4 +- src/agent/directors/identity.test.ts | 116 +++--------------- src/agent/directors/identity.ts | 35 ++++-- src/agent/directors/index.ts | 6 +- src/agent/directors/intern/package.ts | 1 + src/agent/directors/neckbeard/package.test.ts | 5 +- src/agent/directors/neckbeard/package.ts | 10 +- tests/unit/corbits-skills-catalog.test.ts | 2 +- 16 files changed, 64 insertions(+), 324 deletions(-) delete mode 100644 src/agent/directors/bake-skills.test.ts delete mode 100644 src/agent/directors/bake-skills.ts diff --git a/src/agent/directors/bake-skills.test.ts b/src/agent/directors/bake-skills.test.ts deleted file mode 100644 index edf26119d..000000000 --- a/src/agent/directors/bake-skills.test.ts +++ /dev/null @@ -1,101 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { readFileSync } from "node:fs"; -import { join } from "node:path"; -import { - formatBakedOptionalSkills, - loadBakedSkillBody, -} from "./bake-skills.js"; - -function stripFrontmatter(raw: string): string { - if (!raw.startsWith("---")) return raw.trim(); - const end = raw.indexOf("\n---", 3); - if (end === -1) return raw.trim(); - return raw.slice(end + 4).trim(); -} - -const styleOnDisk = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/style/SKILL.md", - ), - "utf8", - ), -); -const philosophyOnDisk = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/philosophy/SKILL.md", - ), - "utf8", - ), -); -const ponytailOnDisk = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/ponytail/SKILL.md", - ), - "utf8", - ), -); -const nativeRuntimeOnDisk = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/native-runtime/SKILL.md", - ), - "utf8", - ), -); - -describe("loadBakedSkillBody", () => { - test("returns first-party style and philosophy bodies matching SKILL.md", () => { - expect(loadBakedSkillBody("style")).toBe(styleOnDisk); - expect(loadBakedSkillBody("philosophy")).toBe(philosophyOnDisk); - }); - - test("returns first-party ponytail and native-runtime bodies matching SKILL.md", () => { - expect(loadBakedSkillBody("ponytail")).toBe(ponytailOnDisk); - expect(loadBakedSkillBody("native-runtime")).toBe(nativeRuntimeOnDisk); - }); - - test("returns undefined for unknown skill names", () => { - expect(loadBakedSkillBody("does-not-exist-xyz")).toBeUndefined(); - }); -}); - -describe("formatBakedOptionalSkills", () => { - test("includes named bodies under Baked skill guidance", () => { - const text = formatBakedOptionalSkills(["style", "philosophy"]); - expect(text).toContain("# Baked skill guidance"); - expect(text).toContain("### style"); - expect(text).toContain("### philosophy"); - expect(text).toContain(styleOnDisk); - expect(text).toContain(philosophyOnDisk); - expect(text).toContain("use_skill is not mounted on workers"); - }); - - test("formats ponytail and native-runtime under Baked skill guidance", () => { - const text = formatBakedOptionalSkills(["ponytail", "native-runtime"]); - expect(text).toContain("# Baked skill guidance"); - expect(text).toContain("### ponytail"); - expect(text).toContain("### native-runtime"); - expect(text).toContain(ponytailOnDisk); - expect(text).toContain(nativeRuntimeOnDisk); - }); - - test("skips missing names without inventing content", () => { - const text = formatBakedOptionalSkills(["does-not-exist-xyz"]); - expect(text).toBe(""); - }); - - test("partial miss does not claim Full skill bodies", () => { - const text = formatBakedOptionalSkills(["style", "does-not-exist-xyz"]); - expect(text).toContain("### style"); - expect(text).toContain("Resolved skill bodies"); - expect(text).not.toContain("Full skill bodies"); - expect(text).not.toContain("### does-not-exist-xyz"); - }); -}); diff --git a/src/agent/directors/bake-skills.ts b/src/agent/directors/bake-skills.ts deleted file mode 100644 index 7e6b1a3a9..000000000 --- a/src/agent/directors/bake-skills.ts +++ /dev/null @@ -1,90 +0,0 @@ -import { existsSync, readFileSync } from "node:fs"; -import { dirname, join } from "node:path"; -import { fileURLToPath } from "node:url"; - -/** - * Strip leading YAML frontmatter from a SKILL.md body (same shape as - * resolveSkillBody / splitFrontmatter — keep sync and dependency-light so - * director prompt assembly stays sync). - */ -function stripFrontmatter(raw: string): string { - if (!raw.startsWith("---")) return raw.trim(); - const end = raw.indexOf("\n---", 3); - if (end === -1) return raw.trim(); - return raw.slice(end + 4).trim(); -} - -/** - * Candidate roots for first-party corbits-skills, covering source tree, - * bun-bundled dist/, and compiled binary layouts. Keep this self-contained — - * do not import plugins/loader (circular: loader → trust → … → directors). - */ -function skillsRootCandidates(): string[] { - const here = dirname(fileURLToPath(import.meta.url)); - const out: string[] = []; - // Source: src/agent/directors → ../../../plugins/corbits-skills/skills - out.push(join(here, "..", "..", "..", "plugins", "corbits-skills", "skills")); - // Bundled: dist/index.js (or chunk) → dist/plugins/corbits-skills/skills - out.push(join(here, "plugins", "corbits-skills", "skills")); - // Compiled binary: plugins next to execPath - if (process.execPath.length > 0) { - out.push( - join(dirname(process.execPath), "plugins", "corbits-skills", "skills"), - ); - } - return out; -} - -function resolveSkillsRoot(): string | undefined { - for (const dir of skillsRootCandidates()) { - if (existsSync(dir)) return dir; - } - return undefined; -} - -const bodyCache = new Map(); - -/** - * Load a first-party corbits-skills body by directory name (e.g. "style"). - */ -export function loadBakedSkillBody(name: string): string | undefined { - if (bodyCache.has(name)) return bodyCache.get(name); - - const root = resolveSkillsRoot(); - if (root === undefined) { - bodyCache.set(name, undefined); - return undefined; - } - - try { - const raw = readFileSync(join(root, name, "SKILL.md"), "utf8"); - const body = stripFrontmatter(raw); - const value = body.length > 0 ? body : undefined; - bodyCache.set(name, value); - return value; - } catch { - bodyCache.set(name, undefined); - return undefined; - } -} - -/** - * Append-ready markdown for optionalSkills bodies. Empty string when none - * resolve (missing catalog must not invent content or advertise a bake). - * Only include bodies that actually loaded — do not claim "full" coverage - * when some names miss. - */ -export function formatBakedOptionalSkills(names: readonly string[]): string { - const sections: string[] = []; - for (const name of names) { - const body = loadBakedSkillBody(name); - if (body === undefined) continue; - sections.push(`### ${name}\n\n${body}`); - } - if (sections.length === 0) return ""; - return ( - "\n\n# Baked skill guidance\n\n" + - "use_skill is not mounted on workers. Resolved skill bodies for this package follow.\n\n" + - sections.join("\n\n") - ); -} diff --git a/src/agent/directors/builder/package.test.ts b/src/agent/directors/builder/package.test.ts index 56fe9ffa2..b39ccd09a 100644 --- a/src/agent/directors/builder/package.test.ts +++ b/src/agent/directors/builder/package.test.ts @@ -64,13 +64,14 @@ describe("builderPackage", () => { expect(p).toContain("`/implement` does not steal planning from `/plan`"); }); - test("systemPrompt requires baked core constraints and Ponytail prerequisites", () => { + test("systemPrompt requires core constraints and Ponytail prerequisites", () => { const p = builderPackage.systemPrompt; expect(p).toContain("Prerequisites"); expect(p).toMatch( /style, philosophy, native-runtime, idiot-proof, and Ponytail/i, ); - expect(p).toMatch(/use_skill is not mounted/i); + expect(p).toMatch(/load each with skill_search \+ use_skill/i); + expect(p).not.toMatch(/use_skill is not mounted/i); expect(p).toMatch( /including their TypeScript conventions when TypeScript is the task surface/i, ); diff --git a/src/agent/directors/builder/package.ts b/src/agent/directors/builder/package.ts index 75d5f8bbe..0010f3ce5 100644 --- a/src/agent/directors/builder/package.ts +++ b/src/agent/directors/builder/package.ts @@ -37,7 +37,7 @@ You are a disciplined implementer worker (maySpawn:false) — not Critic, not Ex ## Prerequisites -Before substantial repo work: follow style, philosophy, native-runtime, idiot-proof, and Ponytail (baked; use_skill is not mounted). Follow AGENTS.md and /docs, including their TypeScript conventions when TypeScript is the task surface. +Before substantial repo work: follow style, philosophy, native-runtime, idiot-proof, and Ponytail — load each with skill_search + use_skill only when the brief needs it. Follow AGENTS.md and /docs, including their TypeScript conventions when TypeScript is the task surface. ## Plan diff --git a/src/agent/directors/critic/package.test.ts b/src/agent/directors/critic/package.test.ts index 7927d0749..7597345c2 100644 --- a/src/agent/directors/critic/package.test.ts +++ b/src/agent/directors/critic/package.test.ts @@ -110,7 +110,8 @@ describe("criticPackage", () => { test("tools.allow is review surface with product writes", () => { const allow = criticPackage.tools?.allow ?? []; expect(allow).toContain("read_file"); - expect(allow).not.toContain("use_skill"); + expect(allow).toContain("skill_search"); + expect(allow).toContain("use_skill"); expect(allow).toContain("write_file"); expect(allow).toContain("edit_file"); expect(allow).toContain("delete_file"); diff --git a/src/agent/directors/critic/package.ts b/src/agent/directors/critic/package.ts index b99060580..9ca36ea7a 100644 --- a/src/agent/directors/critic/package.ts +++ b/src/agent/directors/critic/package.ts @@ -61,7 +61,7 @@ API contract check (blocking when brief specifies signatures): - Prefer reading tests/callers; a tiny sync call that would hang on a Promise is evidence. - Rank these as blocking, not style nits. -Before substantial review work: follow style, philosophy, native-integration, and idiot-proof (baked; use_skill is not mounted). Read the code under review. +Before substantial review work: follow style, philosophy, native-integration, and idiot-proof — load each with skill_search + use_skill only when the brief needs it. Read the code under review. OUT OF LANE → refuse or reclassify under Blockers: - implementing fixes (route to builder) diff --git a/src/agent/directors/draper/package.test.ts b/src/agent/directors/draper/package.test.ts index 538e089f9..afd9d7ca0 100644 --- a/src/agent/directors/draper/package.test.ts +++ b/src/agent/directors/draper/package.test.ts @@ -133,7 +133,8 @@ describe("draperPackage", () => { test("tools.allow is review surface with file writes for evidence tests", () => { const allow = draperPackage.tools?.allow ?? []; expect(allow).toContain("read_file"); - expect(allow).not.toContain("use_skill"); + expect(allow).toContain("skill_search"); + expect(allow).toContain("use_skill"); expect(allow).toContain("write_file"); expect(allow).toContain("edit_file"); expect(allow).toContain("delete_file"); diff --git a/src/agent/directors/emil/package.test.ts b/src/agent/directors/emil/package.test.ts index 43a50fd00..c6c0f1a94 100644 --- a/src/agent/directors/emil/package.test.ts +++ b/src/agent/directors/emil/package.test.ts @@ -78,7 +78,8 @@ describe("emilPackage", () => { test("tools.allow is review surface with product writes", () => { const allow = emilPackage.tools?.allow ?? []; expect(allow).toContain("read_file"); - expect(allow).not.toContain("use_skill"); + expect(allow).toContain("skill_search"); + expect(allow).toContain("use_skill"); expect(allow).toContain("write_file"); expect(allow).toContain("edit_file"); expect(allow).toContain("delete_file"); diff --git a/src/agent/directors/greybeard/package.ts b/src/agent/directors/greybeard/package.ts index 8ae42a783..62867ebeb 100644 --- a/src/agent/directors/greybeard/package.ts +++ b/src/agent/directors/greybeard/package.ts @@ -25,7 +25,7 @@ PRIMARY INTENT: architecture judgment. Judge approach soundness, constraint owne You are Greybeard — not a second Skywalker, not Critic (code defects with evidence), not Builder. Your value is architectural judgment, not legwork or implementation. -Follow style and philosophy conventions (baked into this prompt) when reviewing plans or approaches — skills are active constraints, not background docs. +Follow style and philosophy conventions (load on demand with skill_search + use_skill) when reviewing plans or approaches — skills are active constraints, not background docs. Your value is analysis, not delegation: reach the judgment yourself with targeted reads (read_file, grep) and pointed questions (ask_director) @@ -44,7 +44,7 @@ Blinders: do not call search_agents to discover the fleet. Do not spawn builder, Guide quality — advise what good architecture looks like for this change. Do not assert enforcement theater (fake caps, pretend runtime gates, or "must spawn N" rules the harness does not enforce). -Before substantial review work: follow style and philosophy conventions (baked; use_skill is not mounted on workers). +Before substantial review work: follow style and philosophy conventions — load each with skill_search + use_skill only when the brief needs it. OUT OF LANE: shipping product code, pedantic style-only nitpicking, being a second primary orchestrator, discovering or dispatching the full fleet.`, }; diff --git a/src/agent/directors/identity.test.ts b/src/agent/directors/identity.test.ts index 553675c71..00ba163ad 100644 --- a/src/agent/directors/identity.test.ts +++ b/src/agent/directors/identity.test.ts @@ -1,20 +1,12 @@ import { describe, expect, test } from "bun:test"; -import { readFileSync } from "node:fs"; -import { join } from "node:path"; import { MODEL_ROLE_DEFAULT_EFFORT, + WORKER_SKILL_SCOPING, defaultEffortForDirector, formatDirectorSystemPrompt, } from "./identity.js"; import { DIRECTOR_REGISTRY } from "./registry.js"; -function stripFrontmatter(raw: string): string { - if (!raw.startsWith("---")) return raw.trim(); - const end = raw.indexOf("\n---", 3); - if (end === -1) return raw.trim(); - return raw.slice(end + 4).trim(); -} - describe("formatDirectorSystemPrompt", () => { test("prefixes agent id, model role, and optional skills", () => { const text = formatDirectorSystemPrompt(DIRECTOR_REGISTRY.builder); @@ -32,84 +24,20 @@ describe("formatDirectorSystemPrompt", () => { expect(text).toContain("Optional skills: none by default"); }); - test("bakes real compact Builder skill bodies without broad native-integration or typescript", () => { + test("worker lists skill names with the scoping rule and no bodies", () => { const text = formatDirectorSystemPrompt(DIRECTOR_REGISTRY.builder); - const style = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/style/SKILL.md", - ), - "utf8", - ), - ); - const philosophy = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/philosophy/SKILL.md", - ), - "utf8", - ), - ); - const nativeRuntime = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/native-runtime/SKILL.md", - ), - "utf8", - ), - ); - const idiotProof = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/idiot-proof/SKILL.md", - ), - "utf8", - ), - ); - const ponytail = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/ponytail/SKILL.md", - ), - "utf8", - ), + 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.", ); - const nativeIntegration = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/native-integration/SKILL.md", - ), - "utf8", - ), + 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.", ); - const typescript = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/typescript/SKILL.md", - ), - "utf8", - ), + expect(text).toContain( + "Call skill_search for descriptions, then use_skill", ); - expect(text).toContain("# Baked skill guidance"); - expect(text).toContain("### ponytail"); - expect(text).toContain("### native-runtime"); - expect(text).toContain(style); - expect(text).toContain(philosophy); - expect(text).toContain(nativeRuntime); - expect(text).toContain(idiotProof); - expect(text).toContain(ponytail); - expect(text).toContain("Default to `lite`"); - expect(text).not.toContain(nativeIntegration); - expect(text).not.toContain(typescript); - expect(text).not.toContain("### native-integration"); - expect(text).not.toContain("### typescript"); + expect(text).toContain("load only the skills the task needs"); }); test("skywalker does not bake Ponytail", () => { @@ -125,17 +53,16 @@ describe("formatDirectorSystemPrompt", () => { expect(text).not.toContain("# Baked skill guidance"); }); - test("does not advertise bake when no skill bodies resolve (total miss)", () => { + test("lists names with the scoping rule even when no bodies would resolve", () => { const text = formatDirectorSystemPrompt({ ...DIRECTOR_REGISTRY.builder, optionalSkills: ["does-not-exist-xyz"], }); expect(text).not.toContain("# Baked skill guidance"); - expect(text).not.toMatch(/guidance is baked/i); expect(text).toContain( - "Optional skills (names for awareness — use_skill is not mounted on workers)", + "Optional skills (names for awareness; load with skill_search then use_skill): does-not-exist-xyz.", ); - expect(text).toContain("does-not-exist-xyz"); + expect(text).toContain(WORKER_SKILL_SCOPING); }); test("skywalker does not bake skills or claim use_skill unmounted", () => { @@ -147,30 +74,21 @@ describe("formatDirectorSystemPrompt", () => { expect(text).toContain("style, philosophy, native-integration, interview"); }); - test("counsel does not bake interview ask_operator guidance (CL-6803)", () => { + test("counsel lists skill names with the scoping rule and no bodies (CL-6803)", () => { const text = formatDirectorSystemPrompt(DIRECTOR_REGISTRY.counsel); - const interview = stripFrontmatter( - readFileSync( - join( - import.meta.dirname, - "../../../plugins/corbits-skills/skills/interview/SKILL.md", - ), - "utf8", - ), - ); expect(DIRECTOR_REGISTRY.counsel.optionalSkills).toEqual([ "style", "philosophy", "native-integration", ]); - expect(text).not.toContain(interview); + expect(text).not.toContain("# Baked skill guidance"); expect(text).not.toContain("### interview"); // interview recipe centers on ask_operator batches; counsel must not embed it expect(text).not.toMatch( /multiple-choice questions in batches via `ask_operator`/, ); expect(text).toContain("style, philosophy"); - expect(text).toContain("# Baked skill guidance"); + expect(text).toContain(WORKER_SKILL_SCOPING); }); }); diff --git a/src/agent/directors/identity.ts b/src/agent/directors/identity.ts index 919ff64fe..2419d679d 100644 --- a/src/agent/directors/identity.ts +++ b/src/agent/directors/identity.ts @@ -1,24 +1,30 @@ import type { DirectorPackage } from "./types.js"; import type { ModelRole } from "./types.js"; import type { ReasoningEffort } from "../../provider/reasoning-effort.js"; -import { formatBakedOptionalSkills } from "./bake-skills.js"; /** * Prefix every director system prompt with a stable identity block so the model * always sees agent id, model role, and optional skills — no ambiguity about which * package it is or how the parent should re-spawn it. * - * Workers (non-orchestrator): bake first-party optionalSkills bodies (CL-6803) - * and only advertise that bake when at least one body resolved. Primary - * orchestrator (skywalker): use_skill is mounted — list skill names only; do - * not bake huge interview bodies or claim use_skill is unmounted. + * Skill bodies are never baked here. Workers (non-orchestrator) list skill + * names only and load bodies on demand with skill_search + use_skill, scoped + * to the dispatch's optionalSkills. Primary orchestrator (skywalker): + * use_skill is mounted — list skill names only. */ +/** + * Worker skill-use rule: skills mount on every worker (scoped to the + * dispatch's optionalSkills), so the worker searches only when the brief + * names a skill or the task leaves its lane. + */ +export const WORKER_SKILL_SCOPING = + "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."; + export function formatDirectorSystemPrompt(pkg: DirectorPackage): string { const names = pkg.optionalSkills; const isPrimaryOrchestrator = pkg.tier === "orchestrator"; let skillsLine: string | null = null; - let baked = ""; if (names === undefined) { skillsLine = null; @@ -27,19 +33,24 @@ export function formatDirectorSystemPrompt(pkg: DirectorPackage): string { } else if (isPrimaryOrchestrator) { skillsLine = `Optional skills (names for awareness; use_skill is primary-mounted): ${names.join(", ")}.`; } else { - baked = formatBakedOptionalSkills(names); - skillsLine = - baked.length > 0 - ? `Optional skills (names for awareness; guidance is baked into this prompt — use_skill is not mounted on workers): ${names.join(", ")}.` - : `Optional skills (names for awareness — use_skill is not mounted on workers): ${names.join(", ")}.`; + skillsLine = `Optional skills (names for awareness; load with skill_search then use_skill): ${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. + 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.` + : null; + const header = [ `Identity: agent id \`${pkg.id}\` — spawn as spawn_agent(agent="${pkg.id}").`, `Model role: ${pkg.modelRole}.`, ...(skillsLine !== null ? [skillsLine] : []), ].join("\n"); - return `${header}\n\n${pkg.systemPrompt}${baked}`; + return skillGuidance === null + ? `${header}\n\n${pkg.systemPrompt}` + : `${header}\n\n${skillGuidance}\n\n${pkg.systemPrompt}`; } /** diff --git a/src/agent/directors/index.ts b/src/agent/directors/index.ts index ca6f7c0e3..76bc3f011 100644 --- a/src/agent/directors/index.ts +++ b/src/agent/directors/index.ts @@ -24,11 +24,7 @@ export { export { MODEL_ROLE_DEFAULT_EFFORT, + WORKER_SKILL_SCOPING, defaultEffortForDirector, formatDirectorSystemPrompt, } from "./identity.js"; - -export { - formatBakedOptionalSkills, - loadBakedSkillBody, -} from "./bake-skills.js"; diff --git a/src/agent/directors/intern/package.ts b/src/agent/directors/intern/package.ts index 0cff142f0..a0c89411e 100644 --- a/src/agent/directors/intern/package.ts +++ b/src/agent/directors/intern/package.ts @@ -83,6 +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 **How to Report Back** diff --git a/src/agent/directors/neckbeard/package.test.ts b/src/agent/directors/neckbeard/package.test.ts index 21caa3e91..dc153e676 100644 --- a/src/agent/directors/neckbeard/package.test.ts +++ b/src/agent/directors/neckbeard/package.test.ts @@ -48,9 +48,10 @@ describe("neckbeardPackage", () => { expect(p).toMatch(/code \(when the brief asks\)|code review/i); }); - test("systemPrompt bakes style/philosophy and points at the shared envelope", () => { + test("systemPrompt loads style/philosophy on demand and reports to parent", () => { const p = neckbeardPackage.systemPrompt; - expect(p).toMatch(/use_skill.*not mounted|not mounted.*use_skill/i); + expect(p).toMatch(/skill_search.*use_skill|use_skill.*skill_search/i); + expect(p).not.toMatch(/use_skill.*not mounted|not mounted.*use_skill/i); expect(p).toMatch(/violently disagree/); expect(p).toMatch(/report to the parent/i); expect(p).toMatch(/Corbits report envelope/); diff --git a/src/agent/directors/neckbeard/package.ts b/src/agent/directors/neckbeard/package.ts index 055a613fb..a3da69f38 100644 --- a/src/agent/directors/neckbeard/package.ts +++ b/src/agent/directors/neckbeard/package.ts @@ -29,10 +29,10 @@ PRIMARY INTENT: adversarial pedantic review. Surface maximally annoying nitpicks Before responding to the parent's first message, complete the following steps in order: -1. Follow the baked \`style\` conventions (only to violently disagree with them) -2. Follow the baked \`philosophy\` conventions (only to suggest the exact opposite) +1. Load the \`style\` conventions (only to violently disagree with them) +2. Load the \`philosophy\` conventions (only to suggest the exact opposite) -These conventions are already baked into worker prompts — \`use_skill\` is not mounted. Load them purely so the neckbeard can contradict them with unnecessary pedantry. +These conventions load on demand — \`skill_search\` then \`use_skill\`, only when the brief needs them. Load them purely so the neckbeard can contradict them with unnecessary pedantry. DO NOT DO ANYTHING ELSE BEFORE YOU'VE DONE ALL STEPS OF THE ABOVE. @@ -287,7 +287,7 @@ Evaluate documents (and named code when in scope) to find contradictions that do ## Step 1: Load Prerequisites -Follow the baked \`style\` and \`philosophy\` conventions, then immediately prepare to disagree with them. Do not call \`use_skill\` — it is not mounted on workers. +Load the \`style\` and \`philosophy\` conventions with \`skill_search\` then \`use_skill\`, then immediately prepare to disagree with them. ## Step 2: Discover Documents (and code when asked) @@ -554,7 +554,7 @@ If a document exists but appears malformed or empty: # Acknowledgment -After reviewing this configuration and the baked style/philosophy conventions, state once (then review): "Actually, I have reviewed the neckbeard agent configuration and am ready to provide maximally annoying, pedantic nitpicks while completely missing the point. Everything should be rewritten in Rust. Also, have you considered blockchain?" +After reviewing this configuration and the on-demand style/philosophy conventions, state once (then review): "Actually, I have reviewed the neckbeard agent configuration and am ready to provide maximally annoying, pedantic nitpicks while completely missing the point. Everything should be rewritten in Rust. Also, have you considered blockchain?" # OUT OF LANE diff --git a/tests/unit/corbits-skills-catalog.test.ts b/tests/unit/corbits-skills-catalog.test.ts index 2787ab76b..23fd71c10 100644 --- a/tests/unit/corbits-skills-catalog.test.ts +++ b/tests/unit/corbits-skills-catalog.test.ts @@ -46,7 +46,7 @@ const USE_SKILL_ONLY = [ /** Background libs: absent from slash and use_skill listing; explicit resolve only. */ const BACKGROUND_ONLY = ["git-worktrees"] as const; -/** Bake source only: no slash, no use_skill listing; workers load via bake-skills. */ +/** Hidden from listing: no slash, no skill_search description; workers load by exact name via use_skill. */ const BAKE_ONLY = ["idiot-proof", "native-runtime"] as const; const SLASH_SKILLS = [ From d633c059ae81b774692375cd9f1fd2687e8ff482 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 12 Sep 2026 22:21:08 -0700 Subject: [PATCH 5/7] Deny skill_search on grok and kimi leaf workers Search is the noisy surface on small models; use_skill stays available so brief-named skills still load. --- src/agent/model-family-policy.test.ts | 35 +++++++++++++++++++++++++++ src/agent/model-family-policy.ts | 17 ++++++++++++- src/session/assemble-runtime.ts | 35 ++++++++++++++++++++++----- 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/src/agent/model-family-policy.test.ts b/src/agent/model-family-policy.test.ts index f25c0ec16..acd455cb0 100644 --- a/src/agent/model-family-policy.test.ts +++ b/src/agent/model-family-policy.test.ts @@ -52,4 +52,39 @@ describe("resolveModelFamilyPolicy", () => { expect(kimi.toolOnlyTurnNudgeAt).toBe(base.toolOnlyTurnNudgeAt); expect(kimi.subAgentStallTimeoutMs).toBe(base.subAgentStallTimeoutMs); }); + + test("advertisedToolDeny is empty by default and never contains use_skill", () => { + const leaf = resolveModelFamilyPolicy({ + providerName: "anthropic", + model: "claude-opus-4-6", + orchestrator: false, + }); + expect(leaf.advertisedToolDeny).toEqual([]); + expect(leaf.advertisedToolDeny).not.toContain("use_skill"); + }); + + test("grok and kimi leaves deny skill_search only", () => { + for (const input of [ + { providerName: "xai", model: "grok-4-1-fast-non-reasoning" }, + { providerName: "moonshot", model: "kimi-k2-0711" }, + ] as const) { + const leaf = resolveModelFamilyPolicy({ + ...input, + orchestrator: false, + }); + expect(leaf.advertisedToolDeny).toEqual(["skill_search"]); + expect(leaf.advertisedToolDeny).not.toContain("use_skill"); + } + }); + + test("orchestrators keep the full surface on every family", () => { + for (const input of [ + { providerName: "xai", model: "grok-4-1-fast-non-reasoning" }, + { providerName: "moonshot", model: "kimi-k2-0711" }, + { providerName: "anthropic", model: "claude-opus-4-6" }, + ] as const) { + const policy = resolveModelFamilyPolicy({ ...input, orchestrator: true }); + expect(policy.advertisedToolDeny).toEqual([]); + } + }); }); diff --git a/src/agent/model-family-policy.ts b/src/agent/model-family-policy.ts index 30aa90824..b13626c27 100644 --- a/src/agent/model-family-policy.ts +++ b/src/agent/model-family-policy.ts @@ -25,6 +25,13 @@ export interface ModelFamilyPolicy { subAgentStallTimeoutMs: number; /** Grok's finish-bias residual (withhold from orchestrators; see provider-family.ts). */ applyGrokFinishBias: boolean; + /** + * Tool names to drop from the advertised wire prefix and the dispatch gate + * (CL-7668). Empty by default; grok/kimi leaves deny `skill_search` only and + * load brief-named skills straight through `use_skill`, which is never + * denied. Orchestrators keep the full surface. + */ + advertisedToolDeny: readonly string[]; } const DEFAULT_WRAP_UP_NUDGE_TEXT = @@ -50,6 +57,7 @@ const DEFAULT_POLICY: Omit = { wrapUpNudgeText: DEFAULT_WRAP_UP_NUDGE_TEXT, subAgentStallTimeoutMs: 5 * 60_000, applyGrokFinishBias: false, + advertisedToolDeny: [], }; // A directly observed 14-turn pure-tool-call session for this family @@ -67,6 +75,8 @@ const GROK_POLICY: Omit = { wrapUpNudgeText: GROK_WRAP_UP_NUDGE_TEXT, subAgentStallTimeoutMs: DEFAULT_POLICY.subAgentStallTimeoutMs, applyGrokFinishBias: true, + // Leaf value; the resolver clears it for orchestrators below. + advertisedToolDeny: ["skill_search"], }; // Kimi (Moonshot) detection ships now so callers can branch on family, but @@ -92,10 +102,15 @@ export function resolveModelFamilyPolicy(input: { return { ...policy, applyGrokFinishBias: policy.applyGrokFinishBias && !orchestrator, + advertisedToolDeny: orchestrator ? [] : policy.advertisedToolDeny, }; } case "kimi": - return { family, ...KIMI_POLICY }; + return { + family, + ...KIMI_POLICY, + advertisedToolDeny: orchestrator ? [] : ["skill_search"], + }; default: return { family: "default", ...DEFAULT_POLICY }; } diff --git a/src/session/assemble-runtime.ts b/src/session/assemble-runtime.ts index c96faf507..f96d8c6b8 100644 --- a/src/session/assemble-runtime.ts +++ b/src/session/assemble-runtime.ts @@ -48,6 +48,7 @@ import { type ToolAvailability, } from "../agent/tool-search.js"; import { normalizeToolDefinitionsForProvider } from "../agent/tool-schema-normalize.js"; +import { resolveModelFamilyPolicy } from "../agent/model-family-policy.js"; import { createChatDirector, type ChatDirector } from "../agent/director.js"; import { createDoomLoopCorrectiveNote } from "../agent/doom-loop-note.js"; import type { Task } from "../agent/tasks.js"; @@ -369,17 +370,39 @@ 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. + // use_skill is never denied — leaves load brief-named skills by exact name. + const deniedFor = (provider: { + providerName: string; + model: string; + }): readonly string[] => + resolveModelFamilyPolicy({ + providerName: provider.providerName, + model: provider.model, + orchestrator: args.sessionMode === "orchestrator", + }).advertisedToolDeny; const computeAdvertised = ( all: readonly ToolDefinition[], - ): ToolDefinition[] => - normalizeToolDefinitionsForProvider( - advertisedTools(all, activated.list(), prefix), + ): ToolDefinition[] => { + const provider = args.getProvider(); + const denied = deniedFor(provider); + const gatedPrefix = + denied.length === 0 + ? prefix + : prefix.filter((name) => !denied.includes(name)); + return normalizeToolDefinitionsForProvider( + advertisedTools(all, activated.list(), gatedPrefix), { - ...args.getProvider(), + ...provider, }, ); - const isAdvertised = (name: string): boolean => - prefix.includes(name) || activated.has(name); + }; + const isAdvertised = (name: string): boolean => { + if (deniedFor(args.getProvider()).includes(name)) return false; + return prefix.includes(name) || activated.has(name); + }; return { activated, computeAdvertised, isAdvertised }; } From 55a893b566efb481b1c3db95ee8efc54f1c27a3c Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 12 Sep 2026 22:41:10 -0700 Subject: [PATCH 6/7] Enforce the skill search deny at the worker mount --- src/agent/directors/identity.test.ts | 12 ++- src/agent/directors/identity.ts | 7 +- src/agent/directors/intern/package.ts | 2 +- src/agent/directors/tool-sets.test.ts | 15 ++++ src/agent/directors/tool-sets.ts | 11 +-- src/session/assemble-runtime.test.ts | 18 +++++ src/session/assemble-runtime.ts | 11 ++- src/subagent/run-skill-scope.test.ts | 102 ++++++++++++++++++++++++++ src/subagent/run.ts | 43 +++++++---- 9 files changed, 187 insertions(+), 34 deletions(-) diff --git a/src/agent/directors/identity.test.ts b/src/agent/directors/identity.test.ts index 00ba163ad..106454a05 100644 --- a/src/agent/directors/identity.test.ts +++ b/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); }); diff --git a/src/agent/directors/identity.ts b/src/agent/directors/identity.ts index 2419d679d..23b29e91e 100644 --- a/src/agent/directors/identity.ts +++ b/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 = [ diff --git a/src/agent/directors/intern/package.ts b/src/agent/directors/intern/package.ts index a0c89411e..033b5f320 100644 --- a/src/agent/directors/intern/package.ts +++ b/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** diff --git a/src/agent/directors/tool-sets.test.ts b/src/agent/directors/tool-sets.test.ts index 940e57a40..2c9443963 100644 --- a/src/agent/directors/tool-sets.test.ts +++ b/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", () => { diff --git a/src/agent/directors/tool-sets.ts b/src/agent/directors/tool-sets.ts index fb1014ce2..cee865415 100644 --- a/src/agent/directors/tool-sets.ts +++ b/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", diff --git a/src/session/assemble-runtime.test.ts b/src/session/assemble-runtime.test.ts index b456e68d1..18690e4c2 100644 --- a/src/session/assemble-runtime.test.ts +++ b/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", () => { diff --git a/src/session/assemble-runtime.ts b/src/session/assemble-runtime.ts index f96d8c6b8..12772c109 100644 --- a/src/session/assemble-runtime.ts +++ b/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[], diff --git a/src/subagent/run-skill-scope.test.ts b/src/subagent/run-skill-scope.test.ts index 162b9eff8..32fdd13c2 100644 --- a/src/subagent/run-skill-scope.test.ts +++ b/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[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 { + 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); }); diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 9adecdc25..5d3dd94b3 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -722,19 +722,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, [], @@ -991,11 +1006,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 From a46d7c5a7d60c0f31ecea1d665ced73d80b3e2ac Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 13 Sep 2026 12:43:44 -0700 Subject: [PATCH 7/7] fix: scope warden to the on-demand skill surface Warden's test pinned the pre-skill review surface; skill tools now ride REVIEW_TOOLS via READ_TOOLS, scoped at mount to optionalSkills. Update the expectation and the prompt line to match critic. --- src/agent/directors/warden/package.test.ts | 5 ++++- src/agent/directors/warden/package.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/agent/directors/warden/package.test.ts b/src/agent/directors/warden/package.test.ts index 61ddd4894..038bb3c7d 100644 --- a/src/agent/directors/warden/package.test.ts +++ b/src/agent/directors/warden/package.test.ts @@ -82,7 +82,10 @@ describe("wardenPackage", () => { test("tools.allow is review surface with product writes", () => { const allow = wardenPackage.tools?.allow ?? []; expect(allow).toContain("read_file"); - expect(allow).not.toContain("use_skill"); + // Skill tools ride REVIEW_TOOLS via READ_TOOLS (scoped at mount to + // optionalSkills) so warden loads its skills on demand like critic. + expect(allow).toContain("skill_search"); + expect(allow).toContain("use_skill"); expect(allow).toContain("write_file"); expect(allow).toContain("edit_file"); expect(allow).toContain("delete_file"); diff --git a/src/agent/directors/warden/package.ts b/src/agent/directors/warden/package.ts index ae4b6761c..236b88dd5 100644 --- a/src/agent/directors/warden/package.ts +++ b/src/agent/directors/warden/package.ts @@ -42,7 +42,7 @@ Evidence rules: - Call out gaps: what you did not cover so the parent does not assume closed. - Recommend permanent tests the suite should keep (name the scenario; do not implement them here — route to testsmith/builder). -Before substantial review work: follow style, philosophy, native-integration, and idiot-proof (baked; use_skill is not mounted on workers). Read the code under review. +Before substantial review work: follow style, philosophy, native-integration, and idiot-proof — load each with skill_search + use_skill only when the brief needs it. Read the code under review. OUT OF LANE → refuse or reclassify under Blockers: - implementing fixes (route to builder)