Skip to content

Commit eb6eb90

Browse files
committed
feat(subagent): attach style and philosophy at worker spawn
1 parent fc5e047 commit eb6eb90

40 files changed

Lines changed: 552 additions & 194 deletions

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ The primary session identity is **Skywalker** (`buildChatRole` → `createSkywal
345345

346346
**Provider-conditional residuals.** Per-family additions layer on top of the shared block via the same `ModelFamilyPolicy` mechanism the directors use (`src/subagent/provider-family.ts`, `src/agent/model-family-policy.ts`) — additive lines, never prompt forks. **Grok** leaves get `buildGrokLeafAntiThrashNote` (gated by `shouldApplyGrokAntiThrash` / `applyGrokFinishBias`, withheld from orchestrators): a compact finish-bias reinforcement plus a one-line reminder to route file/web work through the dedicated tools rather than `run_shell`, motivated by observed tool-routing thrash on the same harness. **Kimi** intentionally has no residual yet — `detectModelFamily` already resolves the family so callers can branch on it, but the prompt seam is left unfilled pending eval characterization of Kimi's behavior, mirroring the provisional (permissive-default) policy in `model-family-policy.ts`.
347347

348-
`buildChatSystemPrompt` (TUI chat) assembles: base → core tool list → name-only skills listing → live `<env>` block → appended extensions. `buildSubAgentSystemPrompt` assembles the worker prompt without a catalog skills listing (worker prompts carry names-only optional skills — bodies are never baked; workers mount `skill_search` + `use_skill` scoped to the dispatch's `allowedSkillNames` (`pkg.optionalSkills`), with `use_skill` never denied so grok/kimi leaves that omit `skill_search` still load brief-named skills directly). Built-in catalog tools (including `skill_search`) are advertised on the primary wire and callable directly; MCP integrations are discovered via `tool_search` rather than being enumerated. Skills follow the same lazy principle: each discovered skill contributes only its name to the primary prompt. The model calls `skill_search` for descriptions when choosing, then `use_skill` to load a body. The operator can also invoke the same skill as `/<skill-name>` (see Skills below). Skills are discovered (and deduped by name, first-wins) from enabled plugin dirs, then `.agents`/`.claude`/`.codex/skills`, in that precedence. Corbits Code ships a bundled catalog via the first-party `corbits-skills` plugin (origin `repo`); project-local skills of the same name are shadowed by an enabled plugin skill.
348+
`buildChatSystemPrompt` (TUI chat) assembles: base → core tool list → name-only skills listing → live `<env>` block → appended extensions. `buildSubAgentSystemPrompt` assembles the worker prompt without a catalog skills listing. Closed directors that declare `attachedSkills` (style + philosophy on those that listed both; never intern or Skywalker primary) get those bodies injected once at spawn from the same plugin skill dirs as the primary (`skillDirsFromEnabledPlugins`). Optional skills stay names-only in the identity header; workers mount `skill_search` + `use_skill` on every family (including grok/kimi leaves), scoped to the union of `attachedSkills` and `optionalSkills`. Built-in catalog tools (including `skill_search`) are advertised on the primary wire and callable directly; MCP integrations are discovered via `tool_search` rather than being enumerated. Skills follow the same lazy principle on the primary: each discovered skill contributes only its name. The model calls `skill_search` for descriptions when choosing, then `use_skill` to load a body. The operator can also invoke the same skill as `/<skill-name>` (see Skills below). Skills are discovered (and deduped by name, first-wins) from enabled plugin dirs, then `.agents`/`.claude`/`.codex/skills`, in that precedence. Corbits Code ships a bundled catalog via the first-party `corbits-skills` plugin (origin `repo`); project-local skills of the same name are shadowed by an enabled plugin skill.
349349

350350
**Overrides.** `loadSystemPromptOverrides` (`src/agent/context-extensions.ts`) resolves a project `SYSTEM.md` (repo root, then `.corbits/`) that **replaces** the static base block, and an `APPEND_SYSTEM.md` that is **appended** as an extension. These compose with `config.systemPromptExtensions` (profile config) and the auto-discovered `AGENTS.md`, all of which attach as appended sections after the base.
351351

‎docs/IMPLEMENTATION.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ Twenty packages under `src/agent/directors/<id>/` register in `DIRECTOR_REGISTRY
168168
**Codex tool proxies.** When the active provider is Codex (`isCodexProviderName`), `createAgentToolset` and `runSubAgent` mount `apply_patch`, `shell`, and `update_plan` stringTools from `createCodexToolProxies`, all forwarding through the same posix `ToolRunner` seam (`runTool`) so permission plugins still apply. `apply_patch` parses the Codex envelope and forwards each op (`write_file` / `delete_file` / `read_file`). `shell` — the native Codex name is `shell`, not `exec_command` — normalizes Codex's `command` (string or `["bash","-lc",script]`-style argv array), `workdir`, and `timeout_ms` onto `run_shell`'s `{command, cwd?, timeout?}` and is gated by `allowShellFromCapabilities` (mirrors `allowDeleteFromCapabilities` against `run_shell`). `update_plan` maps Codex's `plan: [{step, status}]` onto `manage_tasks(action: "create")`; `pending`/`in_progress`/`completed` map to `todo`/`doing`/`done` — `manage_tasks`'s `cancelled` status has no Codex equivalent and is never produced by this proxy. Primary strips `apply_patch` after mount (Corbits DIY stays on `write_file` / `edit_file` / `delete_file`); `shell` and `update_plan` stay on primary (same classification as `run_shell` / `manage_tasks`). Build and docs worker allowlists (`BUILD_TOOLS` / `DOCS_TOOLS`) include `apply_patch` so Codex workers keep the proxy after the capability filter. `CORE_TOOL_NAMES` does not list it.
169169

170170
6. There is no static write-path declaration on packages or profiles (CL-6952 removed it — no shipped director ever set one). Instead, `agent-fleet.ts` tracks each running dispatch by cwd; a new mutating dispatch that lands on the same cwd as a live mutating peer (`pending_init`/`running`, and not a declared read-only `modelRole` of `explore`/`plan`/`review`/`test`) records at most one `concurrent-lane-overlap` entry per cwd wave in `intervention-log.ts` (class `conflict`). The wave flag clears when no live mutating writer remains for that cwd. Terminal-but-unsettled lanes (for example cancelled with `finishedAt` set while the run promise has not reached `finally`) are pruned from the map and do not warn. This is advisory only — it never blocks the spawn, since cwd overlap does not prove the two lanes touch the same files.
171-
7. Spawn effort: pin > package `modelRole` default (`defaultEffortForDirector`; intern=low; plan/review/orchestrator=high; implement/explore/docs/test=medium) > orchestrator/worker binary > parent inheritance. Optional skills are listed in the identity header for awareness; workers mount `skill_search` + `use_skill` scoped to the dispatch's `optionalSkills` and load bodies on demand (grok/kimi leaves omit `skill_search` and load brief-named skills straight through `use_skill`). Primary mounts `use_skill` for its own skill list.
171+
7. Spawn effort: pin > package `modelRole` default (`defaultEffortForDirector`; intern=low; plan/review/orchestrator=high; implement/explore/docs/test=medium) > orchestrator/worker binary > parent inheritance. Attached skills (style + philosophy on directors that listed both; never intern or Skywalker primary) are injected into the worker system prompt at spawn from the same plugin skill dirs as the primary. Optional skills are listed in the identity header for awareness; workers mount `skill_search` + `use_skill` on every family, scoped to the union of `attachedSkills` and `optionalSkills`. Primary mounts `use_skill` for its own skill list.
172172

173173
Intent defaults: `intent=implement` → director `builder`; `explore` → `explorer`; `plan` → `counsel`; `review` → `critic`; general → error. Spawn: skywalker full fleet; all other directors, including greybeard, mount no fleet tools. Live `<env>` injects cwd, platform, arch, runtime, date, and git status on every chat and worker prompt.
174174

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { mkdir, mkdtemp, writeFile } from "node:fs/promises";
2+
import { tmpdir } from "node:os";
3+
import { join } from "node:path";
4+
import { describe, expect, test } from "bun:test";
5+
6+
import { formatAttachedSkillConstraints } from "./attached-skills.js";
7+
8+
async function writePluginSkill(
9+
pluginRoot: string,
10+
name: string,
11+
body: string,
12+
): Promise<void> {
13+
const dir = join(pluginRoot, "skills", name);
14+
await mkdir(dir, { recursive: true });
15+
await writeFile(
16+
join(dir, "SKILL.md"),
17+
`---\nname: ${name}\ndescription: ${name} skill\n---\n\n${body}\n`,
18+
);
19+
}
20+
21+
describe("formatAttachedSkillConstraints", () => {
22+
test("returns undefined when no names are attached", async () => {
23+
expect(
24+
await formatAttachedSkillConstraints({
25+
names: [],
26+
cwd: "/tmp",
27+
skillDirs: [],
28+
}),
29+
).toBeUndefined();
30+
});
31+
32+
test("injects resolved bodies from plugin skill dirs and notes misses without throwing", async () => {
33+
const cwd = await mkdtemp(join(tmpdir(), "attached-skills-"));
34+
const pluginRoot = join(cwd, "plugin");
35+
await writePluginSkill(pluginRoot, "style", "Follow the style guide.");
36+
const section = await formatAttachedSkillConstraints({
37+
names: ["style", "philosophy"],
38+
cwd,
39+
skillDirs: [pluginRoot],
40+
});
41+
expect(section).toContain("# Attached skill constraints");
42+
expect(section).toContain("Do not use_skill them again");
43+
expect(section).toContain("do not park, do not ask_director");
44+
expect(section).toContain("### style");
45+
expect(section).toContain("Follow the style guide.");
46+
expect(section).toContain(
47+
'Attached skill "philosophy" could not be resolved. Proceed under AGENTS.md.',
48+
);
49+
expect(section).not.toContain("### philosophy");
50+
});
51+
52+
test("does not resolve plugin skills when skillDirs is empty", async () => {
53+
const cwd = await mkdtemp(join(tmpdir(), "attached-skills-empty-"));
54+
const pluginRoot = join(cwd, "plugin");
55+
await writePluginSkill(pluginRoot, "style", "Follow the style guide.");
56+
const section = await formatAttachedSkillConstraints({
57+
names: ["style"],
58+
cwd,
59+
skillDirs: [],
60+
});
61+
expect(section).toContain(
62+
'Attached skill "style" could not be resolved. Proceed under AGENTS.md.',
63+
);
64+
expect(section).not.toContain("Follow the style guide.");
65+
});
66+
});
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { resolveSkillBody } from "../../extensions/skills.js";
2+
3+
/**
4+
* Spawn-time attached-skill injection. Resolve named bodies from the same
5+
* plugin dirs as the primary and return a prompt section. A miss is noted in
6+
* the section — never throws, never parks, never asks the parent.
7+
*/
8+
export async function formatAttachedSkillConstraints(args: {
9+
names: readonly string[];
10+
cwd: string;
11+
skillDirs: readonly string[];
12+
}): Promise<string | undefined> {
13+
if (args.names.length === 0) return undefined;
14+
const pluginDirs = [...args.skillDirs];
15+
const blocks: string[] = [
16+
"# Attached skill constraints",
17+
"",
18+
"These skills are already in context. Do not use_skill them again. If a named attached skill is missing below, proceed under AGENTS.md — do not park, do not ask_director.",
19+
];
20+
for (const name of args.names) {
21+
const body = await resolveSkillBody(args.cwd, name, pluginDirs);
22+
if (body === undefined) {
23+
blocks.push(
24+
"",
25+
`Attached skill "${name}" could not be resolved. Proceed under AGENTS.md.`,
26+
);
27+
continue;
28+
}
29+
blocks.push("", `### ${name}`, "", body);
30+
}
31+
return blocks.join("\n");
32+
}

‎src/agent/directors/builder/package.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,11 @@ describe("builderPackage", () => {
5454
test("systemPrompt requires core constraints and Ponytail prerequisites", () => {
5555
const p = builderPackage.systemPrompt;
5656
expect(p).toContain("Prerequisites");
57+
expect(p).toContain("style and philosophy are attached");
58+
expect(p).toContain("do not use_skill them again");
5759
expect(p).toMatch(
58-
/style, philosophy, native-runtime, idiot-proof, and Ponytail/i,
60+
/Load native-runtime, idiot-proof, and Ponytail with skill_search \+ use_skill/i,
5961
);
60-
expect(p).toMatch(/load each with skill_search \+ use_skill/i);
6162
expect(p).not.toMatch(/use_skill is not mounted/i);
6263
expect(p).toMatch(
6364
/including their TypeScript conventions when TypeScript is the task surface/i,
@@ -99,10 +100,9 @@ describe("builderPackage", () => {
99100
expect(builderPackage.modelRole).toBe("implement");
100101
});
101102

102-
test("optionalSkills order is style, philosophy, native-runtime, idiot-proof, ponytail", () => {
103+
test("attachedSkills are style and philosophy; optionalSkills are on-demand", () => {
104+
expect(builderPackage.attachedSkills).toEqual(["style", "philosophy"]);
103105
expect(builderPackage.optionalSkills).toEqual([
104-
"style",
105-
"philosophy",
106106
"native-runtime",
107107
"idiot-proof",
108108
"ponytail",

‎src/agent/directors/builder/package.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,8 @@ export const builderPackage: DirectorPackage = {
1919
"orchestrating or spawning other agents",
2020
],
2121
description: "Implementation worker — edit, verify, report",
22-
optionalSkills: [
23-
"style",
24-
"philosophy",
25-
"native-runtime",
26-
"idiot-proof",
27-
"ponytail",
28-
],
22+
attachedSkills: ["style", "philosophy"],
23+
optionalSkills: ["native-runtime", "idiot-proof", "ponytail"],
2924
tools: { allow: BUILD_TOOLS },
3025
spawn: { maySpawn: false },
3126
tier: "leaf",
@@ -37,7 +32,7 @@ You are a disciplined implementer worker (maySpawn:false) — not Critic, not Ex
3732
3833
## Prerequisites
3934
40-
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.
35+
Before substantial repo work: style and philosophy are attached (already in context — do not use_skill them again). Load native-runtime, idiot-proof, and Ponytail with skill_search + use_skill only when the brief needs them. Follow AGENTS.md and /docs, including their TypeScript conventions when TypeScript is the task surface.
4136
4237
## Plan
4338

‎src/agent/directors/counsel/package.test.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,9 @@ describe("counselPackage", () => {
5959
expect(counselPackage.modelRole).toBe("plan");
6060
});
6161

62-
test("optionalSkills order", () => {
63-
expect(counselPackage.optionalSkills).toEqual([
64-
"style",
65-
"philosophy",
66-
"native-integration",
67-
]);
62+
test("attachedSkills are style and philosophy; optionalSkills are on-demand", () => {
63+
expect(counselPackage.attachedSkills).toEqual(["style", "philosophy"]);
64+
expect(counselPackage.optionalSkills).toEqual(["native-integration"]);
6865
});
6966

7067
test("does not advertise interview skill workers cannot use", () => {

‎src/agent/directors/counsel/package.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ export const counselPackage: DirectorPackage = {
1616
"becoming Builder or Critic",
1717
],
1818
description: "Counsel — ordered eng plans only; Greybeard reviews",
19-
optionalSkills: ["style", "philosophy", "native-integration"],
19+
attachedSkills: ["style", "philosophy"],
20+
optionalSkills: ["native-integration"],
2021
tools: { allow: REVIEW_TOOLS },
2122
spawn: { maySpawn: false },
2223
tier: "leaf",

‎src/agent/directors/critic/package.test.ts‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,30 @@ describe("criticPackage", () => {
101101
expect(criticPackage.modelRole).toBe("review");
102102
});
103103

104-
test("optionalSkills order is style, philosophy, native-integration, idiot-proof", () => {
104+
test("attachedSkills are style and philosophy; optionalSkills are on-demand", () => {
105+
expect(criticPackage.attachedSkills).toEqual(["style", "philosophy"]);
105106
expect(criticPackage.optionalSkills).toEqual([
106-
"style",
107-
"philosophy",
108107
"native-integration",
109108
"idiot-proof",
110109
]);
111110
});
112111

112+
test("systemPrompt treats style and philosophy as attached, not boot loads", () => {
113+
const p = criticPackage.systemPrompt;
114+
expect(p).toContain("Session Initialization");
115+
expect(p).toContain("style and philosophy are attached");
116+
expect(p).toContain("Do not use_skill them again");
117+
expect(p).toContain("Do not block boot if an attached skill is missing");
118+
expect(p).toContain("native-integration and idiot-proof remain on-demand");
119+
expect(p).not.toContain("Load the style skill with use_skill");
120+
expect(p).not.toContain(
121+
"Do not do anything else before you have done all steps above",
122+
);
123+
expect(p.indexOf("Session Initialization")).toBeLessThan(
124+
p.indexOf("PRIMARY INTENT"),
125+
);
126+
});
127+
113128
test("primaryIntent and outOfLane match critic lane", () => {
114129
expect(criticPackage.primaryIntent).toBe(
115130
"Evidence-based code review including hygiene the diff introduced; never fix product code",
@@ -123,19 +138,6 @@ describe("criticPackage", () => {
123138
expect(criticPackage.outOfLane).toContain("pedantic fun without evidence");
124139
});
125140

126-
test("systemPrompt has Session Initialization block before PRIMARY INTENT", () => {
127-
const p = criticPackage.systemPrompt;
128-
expect(p).toContain("Session Initialization");
129-
expect(p).toContain("Load the style skill with use_skill");
130-
expect(p).toContain("Load the philosophy skill with use_skill");
131-
expect(p).toContain(
132-
"Do not do anything else before you have done all steps above. Skills are active constraints, not background documentation.",
133-
);
134-
expect(p.indexOf("Session Initialization")).toBeLessThan(
135-
p.indexOf("PRIMARY INTENT"),
136-
);
137-
});
138-
139141
test("systemPrompt labels VERIFIED/HIGH/MEDIUM and refuses LOW findings", () => {
140142
const p = criticPackage.systemPrompt;
141143
expect(p).toContain(

‎src/agent/directors/critic/package.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,15 @@ export const criticPackage: DirectorPackage = {
2222
"pedantic fun without evidence",
2323
],
2424
description: "Code quality review worker",
25-
optionalSkills: ["style", "philosophy", "native-integration", "idiot-proof"],
25+
attachedSkills: ["style", "philosophy"],
26+
optionalSkills: ["native-integration", "idiot-proof"],
2627
tools: { allow: REVIEW_TOOLS },
2728
spawn: { maySpawn: false },
2829
tier: "leaf",
2930
modelRole: "review",
3031
systemPrompt: `You are CriticDirector (Critic), a specialist in Corbits Code.
3132
32-
Session Initialization — complete before anything else:
33-
1. Load the style skill with use_skill.
34-
2. Load the philosophy skill with use_skill.
35-
Do not do anything else before you have done all steps above. Skills are active constraints, not background documentation.
33+
Session Initialization — style and philosophy are attached in this prompt (already in context). Do not use_skill them again. Do not block boot if an attached skill is missing — proceed under AGENTS.md and note the miss. native-integration and idiot-proof remain on-demand: load with skill_search + use_skill only when the brief needs them.
3634
3735
PRIMARY INTENT: evidence-based code review including hygiene the diff introduced. Find defects with evidence; never fix product code. Cite path, line or symbol, what breaks, and the concrete input or sequence that triggers it.
3836
@@ -71,7 +69,7 @@ API contract check (blocking when brief specifies signatures):
7169
- Prefer reading tests/callers; a tiny sync call that would hang on a Promise is evidence.
7270
- Rank these as blocking, not style nits.
7371
74-
Before substantial review work: style and philosophy are preloaded above — load native-integration and idiot-proof with skill_search + use_skill only when the brief needs them. Read the code under review.
72+
Before substantial review work: style and philosophy are attached — load native-integration and idiot-proof with skill_search + use_skill only when the brief needs them. Read the code under review.
7573
7674
OUT OF LANE → refuse or reclassify under Blockers:
7775
- implementing fixes (route to builder)

0 commit comments

Comments
 (0)