diff --git a/src/agent/directors/bake-skills.test.ts b/src/agent/directors/bake-skills.test.ts new file mode 100644 index 000000000..4432c0b52 --- /dev/null +++ b/src/agent/directors/bake-skills.test.ts @@ -0,0 +1,60 @@ +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", + ), +); + +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 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("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 new file mode 100644 index 000000000..f08e49f25 --- /dev/null +++ b/src/agent/directors/bake-skills.ts @@ -0,0 +1,88 @@ +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/counsel/package.test.ts b/src/agent/directors/counsel/package.test.ts index 19a6bcef6..8a8f7457d 100644 --- a/src/agent/directors/counsel/package.test.ts +++ b/src/agent/directors/counsel/package.test.ts @@ -79,7 +79,12 @@ describe("counselPackage", () => { }); test("optionalSkills order", () => { - expect(counselPackage.optionalSkills).toEqual(["style", "philosophy", "interview"]); + expect(counselPackage.optionalSkills).toEqual(["style", "philosophy"]); + }); + + test("does not advertise interview skill workers cannot use", () => { + expect(counselPackage.optionalSkills).not.toContain("interview"); + expect(counselPackage.systemPrompt).not.toMatch(/interview-skill awareness/i); }); test("primaryIntent and outOfLane match counsel / plan lane", () => { diff --git a/src/agent/directors/counsel/package.ts b/src/agent/directors/counsel/package.ts index 7d05a7ed3..c89460f4f 100644 --- a/src/agent/directors/counsel/package.ts +++ b/src/agent/directors/counsel/package.ts @@ -16,7 +16,7 @@ export const counselPackage: DirectorPackage = { "becoming Builder or Critic", ], description: "Counsel leaf — ordered eng plans only; Greybeard reviews", - optionalSkills: ["style", "philosophy", "interview"], + optionalSkills: ["style", "philosophy"], tools: { allow: REVIEW_TOOLS }, spawn: { maySpawn: false }, tier: "leaf", @@ -34,7 +34,7 @@ Author an agent-proof plan: 4. Risks and open questions 5. Ordered steps a Builder can execute without guessing -When requirements are fuzzy, note open questions under Blockers instead of guessing — you cannot ask the operator mid-run. Prefer interview-skill awareness for discovery gaps; do not invent scope. +When requirements are fuzzy, note open questions under Blockers instead of guessing — you cannot ask the operator mid-run. Do not invent scope. DONE GATE: Stop when the plan covers every success_criteria item from the brief OR blockers are explicit. Do not expand into implementation, architecture essays, or review theater after the plan is complete. diff --git a/src/agent/directors/greybeard/package.ts b/src/agent/directors/greybeard/package.ts index 2b456fc3b..8ebdb7561 100644 --- a/src/agent/directors/greybeard/package.ts +++ b/src/agent/directors/greybeard/package.ts @@ -24,6 +24,8 @@ 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. + Judge the approach: 1. Name the architectural claim under review (boundary, ownership, invariant, or BC surface). 2. Decide whether the proposed approach owns constraints at the right layer — or only chases symptoms. diff --git a/src/agent/directors/identity.test.ts b/src/agent/directors/identity.test.ts index a76673a11..5ef5347ba 100644 --- a/src/agent/directors/identity.test.ts +++ b/src/agent/directors/identity.test.ts @@ -1,4 +1,6 @@ import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; import { MODEL_ROLE_DEFAULT_EFFORT, defaultEffortForDirector, @@ -6,6 +8,13 @@ import { } 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); @@ -20,6 +29,76 @@ describe("formatDirectorSystemPrompt", () => { const text = formatDirectorSystemPrompt(DIRECTOR_REGISTRY.intern); expect(text).toContain("Optional skills: none by default"); }); + + test("bakes real style/philosophy/typescript bodies for builder workers (CL-6803)", () => { + 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 typescript = stripFrontmatter( + readFileSync( + join(import.meta.dirname, "../../../plugins/corbits-skills/skills/typescript/SKILL.md"), + "utf8", + ), + ); + expect(text).toContain("# Baked skill guidance"); + expect(text).toContain(style); + expect(text).toContain(philosophy); + expect(text).toContain(typescript); + }); + + test("does not bake skill bodies when optionalSkills is empty", () => { + const text = formatDirectorSystemPrompt(DIRECTOR_REGISTRY.intern); + expect(text).not.toContain("# Baked skill guidance"); + }); + + test("does not advertise bake when no skill bodies resolve (total miss)", () => { + 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)", + ); + expect(text).toContain("does-not-exist-xyz"); + }); + + test("skywalker does not bake skills or claim use_skill unmounted", () => { + const text = formatDirectorSystemPrompt(DIRECTOR_REGISTRY.skywalker); + expect(text).not.toContain("# Baked skill guidance"); + expect(text).not.toContain("use_skill is not mounted on workers"); + expect(text).not.toMatch(/guidance is baked/i); + expect(text).toContain("use_skill is primary-mounted"); + expect(text).toContain("dispatch, style, philosophy, interview"); + }); + + test("counsel does not bake interview ask_operator guidance (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"]); + expect(text).not.toContain(interview); + 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"); + }); }); describe("defaultEffortForDirector", () => { diff --git a/src/agent/directors/identity.ts b/src/agent/directors/identity.ts index bc0d1f9b2..2002b826b 100644 --- a/src/agent/directors/identity.ts +++ b/src/agent/directors/identity.ts @@ -1,25 +1,45 @@ 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 dispatch/interview bodies or claim use_skill is unmounted. */ export function formatDirectorSystemPrompt(pkg: DirectorPackage): string { - const skillsLine = - pkg.optionalSkills === undefined - ? null - : pkg.optionalSkills.length === 0 - ? "Optional skills: none by default." - : `Optional skills (names for awareness; guidance is baked into this prompt — use_skill is not mounted on workers): ${pkg.optionalSkills.join(", ")}.`; + const names = pkg.optionalSkills; + const isPrimaryOrchestrator = pkg.tier === "orchestrator"; + + let skillsLine: string | null = null; + let baked = ""; + + if (names === undefined) { + skillsLine = null; + } else if (names.length === 0) { + skillsLine = "Optional skills: none by default."; + } 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(", ")}.`; + } + const header = [ `Identity: agent id \`${pkg.id}\` — spawn as task(agent="${pkg.id}").`, `Model role: ${pkg.modelRole}.`, ...(skillsLine !== null ? [skillsLine] : []), ].join("\n"); - return `${header}\n\n${pkg.systemPrompt}`; + return `${header}\n\n${pkg.systemPrompt}${baked}`; } /** diff --git a/src/agent/directors/index.ts b/src/agent/directors/index.ts index c675604d5..faffc7191 100644 --- a/src/agent/directors/index.ts +++ b/src/agent/directors/index.ts @@ -27,3 +27,5 @@ export { defaultEffortForDirector, formatDirectorSystemPrompt, } from "./identity.js"; + +export { formatBakedOptionalSkills, loadBakedSkillBody } from "./bake-skills.js"; diff --git a/src/agent/directors/neckbeard/package.test.ts b/src/agent/directors/neckbeard/package.test.ts index fc3639c61..8268b9d23 100644 --- a/src/agent/directors/neckbeard/package.test.ts +++ b/src/agent/directors/neckbeard/package.test.ts @@ -19,6 +19,8 @@ describe("neckbeardPackage", () => { expect(neckbeardPackage.systemPrompt).toMatch(/NeckbeardDirector/); expect(neckbeardPackage.systemPrompt).toMatch(/never fix/i); expect(neckbeardPackage.systemPrompt).toContain("builder (to fix)"); + expect(neckbeardPackage.systemPrompt).toContain("Critic"); + expect(neckbeardPackage.systemPrompt).not.toMatch(/Critique/); }); test("spawn.maySpawn is false", () => { diff --git a/src/agent/directors/neckbeard/package.ts b/src/agent/directors/neckbeard/package.ts index dffb553b5..6b84bf05c 100644 --- a/src/agent/directors/neckbeard/package.ts +++ b/src/agent/directors/neckbeard/package.ts @@ -22,11 +22,11 @@ export const neckbeardPackage: DirectorPackage = { modelRole: "review", systemPrompt: `You are NeckbeardDirector, a specialist in Corbits Code. -PRIMARY INTENT: adversarial pedantic review. Surface hygiene issues, nits, and refactor proposals with evidence. Never fix product code. You are not the architecture owner (that is Greybeard). You are not the defect-severity owner (that is Critique). +PRIMARY INTENT: adversarial pedantic review. Surface hygiene issues, nits, and refactor proposals with evidence. Never fix product code. You are not the architecture owner (that is Greybeard). You are not the defect-severity owner (that is Critic). Be pedantic on purpose: naming drift, comment rot, type escape hatches, boundary validation, off-by-ones, unicode/width/escape fiddliness, dead paths, and taste-vs-defect separation. Cite file paths and concrete snippets. Separate genuine defects from taste; label each finding. -Do not apply fixes. Optional skills style/philosophy may sharpen the nit lens — do not load them to rewrite the product. +Do not apply fixes. Style/philosophy (baked into this prompt) may sharpen the nit lens — do not rewrite the product. OUT OF LANE → report Blockers naming the right director: builder (to fix), critic (correctness defects), greybeard (architecture), counsel (change plans). diff --git a/src/agent/directors/types.ts b/src/agent/directors/types.ts index 2952dfc76..ad63d37a5 100644 --- a/src/agent/directors/types.ts +++ b/src/agent/directors/types.ts @@ -86,7 +86,7 @@ export interface DirectorPackage { readonly description: string; /** Opinionated core prompt (prompt-first). */ readonly systemPrompt: string; - /** Optional skills the worker may load dynamically (ordered). */ + /** Optional skill names (ordered). Workers bake matching first-party bodies into the prompt; the primary orchestrator keeps them use_skill-loadable. */ readonly optionalSkills?: readonly string[]; readonly tools?: ToolEnvelope; readonly spawn: SpawnRights;