diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 94fed4e16..0b22b5ea3 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -237,11 +237,11 @@ Every shipped specialist is a **director package** — a prompt-first `DirectorP | Director | Owns | Does not own | | ----------- | ----------------------------------------------------------------------- | ---------------------------------- | -| build | Ship product code | Pure docs, pure review | -| explore | Map/read codebase | Product edits | -| plan | Eng change plan (steps, paths, tests, risks) | Arch gate, product discovery, code | +| builder | Ship product code | Pure docs, pure review | +| explorer | Map/read codebase | Product edits | +| counsel | Eng change plan (steps, paths, tests, risks) | Arch gate, product discovery, code | | intern | Mechanical commands only | Ambiguous or product-design work | -| critique | Evidence-based code review | Fixing product code | +| critic | Evidence-based code review | Fixing product code | | greybeard | Architecture/approach review of plans/docs; limited spawn | Authoring eng plans, implementing | | neckbeard | Adversarial hygiene / refactor stress | Real review substitute | | bruckheimer | Product discovery → PRODUCT/ARCHITECTURE/IMPLEMENTATION-oriented briefs | Eng plan, code | @@ -249,11 +249,11 @@ Every shipped specialist is a **director package** — a prompt-first `DirectorP **Design trio (dev perspective)** -| Director | Owns | -| -------------- | ----------------------------------------------------- | -| draper | Product visual / design-system critique | -| emil | Design-engineering + software laws on product UI/code | -| brand-reviewer | **DESIGN.md** create-if-missing + alignment gate | +| Director | Owns | +| -------- | ----------------------------------------------------- | +| draper | Product visual / design-system critique | +| emil | Design-engineering + software laws on product UI/code | +| rand | **DESIGN.md** create-if-missing + alignment gate | **Docs + QA** @@ -265,25 +265,25 @@ Every shipped specialist is a **director package** — a prompt-first `DirectorP **Intent → director** (`task(intent=…)` when `agent` is omitted) -| Intent | Default director | -| --------- | ---------------------------------- | -| implement | build | -| explore | explore | -| plan | plan | -| review | critique (override with `agent=…`) | -| general | **none** — reclassify only | +| Intent | Default director | +| --------- | --------------------------------- | +| implement | builder | +| explore | explorer | +| plan | counsel | +| review | critic (override with `agent=…`) | +| general | **none** — reclassify only | **Spawn matrix** -| Who | Spawn rights | -| --------------------------- | ------------------------------ | -| skywalker (primary session) | Full closed fleet | -| greybeard | intern, explore, critique only | -| All other directors | no `task` | +| Who | Spawn rights | +| --------------------------- | -------------------------------- | +| skywalker (primary session) | Full closed fleet | +| greybeard | intern, explorer, critic only | +| All other directors | no `task` | -**Tool envelopes** prefer small `tools.allow` mounts over deny-everything. Shipped docs/design directors (shakespeare, brand-reviewer, bruckheimer) mount write tools with no path-level lock. Lane routing is spawn policy (shakespeare = P/A/I docs, brand-reviewer = DESIGN.md, bruckheimer = product discovery), not a file lock. There is no static per-package write-path declaration (CL-6952 removed it — no shipped director ever set one); instead the task tool records, without blocking, when two concurrently running dispatches land on the same cwd (see `intervention-log.ts`'s `conflict` class). +**Tool envelopes** prefer small `tools.allow` mounts over deny-everything. Shipped docs/design directors (shakespeare, rand, bruckheimer) mount write tools with no path-level lock. Lane routing is spawn policy (shakespeare = P/A/I docs, rand = DESIGN.md, bruckheimer = product discovery), not a file lock. There is no static per-package write-path declaration (CL-6952 removed it — no shipped director ever set one); instead the task tool records, without blocking, when two concurrently running dispatches land on the same cwd (see `intervention-log.ts`'s `conflict` class). -**Typical chain:** bruckheimer → plan → greybeard → build (+ intern) → critique (+ optional neckbeard), with skywalker coordinating throughout. +**Typical chain:** bruckheimer → counsel → greybeard → builder (+ intern) → critic (+ optional neckbeard), with skywalker coordinating throughout. **Reasoning effort by role** (`src/provider/reasoning-effort.ts` → `resolveEffortForRole` / `defaultEffortForDirector`): package `modelRole` defaults are orchestrator/plan/review → `high`, implement/explore/docs/test → `medium`, with **intern** pinned to `low`. Spawn-time binary fallback is orchestrator → `high`, worker → `medium`, clamped to the model. Explicit profile inference pins win; parent session effort is only a fallback when the role default is unsupported. This keeps multi-agent fleets off the sol+high latency cliff — see `docs/plans/reasoning-effort-by-role.md`. @@ -298,7 +298,7 @@ Data-only agent plugins (`src/plugins/data-only-agent.ts`) synthesize `agentPlug The primary session identity is **Skywalker** (`buildChatRole` → `createSkywalkerSystemPrompt`). Product name remains Corbits Code; when asked its name, the primary answers Skywalker. Role: orchestrate — classify, DIY tiny/single-file/one-route product edits, dispatch closed directors via `task` for substantial work, track the fleet, synthesize. Product mutation tools (`write_file` / `edit_file` / `delete_file`) are mounted on the primary session (CORE and `SKYWALKER_TOOLS`) so Skywalker can DIY bounded edits; spawn remains the default for substantial, multi-file, parallel, or specialist work. Shell file-writes stay denied by auto-shell policy. MCP tools are not re-filtered by a product-write deny list (that list is gone). There is no static per-leaf write-path lock; concurrent lanes sharing a cwd are instead flagged (not blocked) as a `conflict` intervention. A frontier model already knows how to code; the static prompt carries harness-specific facts and the closed-fleet orchestration policy. The base is three individually-exported sections: - `buildChatRole` — Skywalker primary identity (orchestrate; DIY tiny/bounded product edits; spawn for substantial work). -- `buildHarnessFacts` — the non-derivable rules: shell file-writes are blocked, path tools are the DIY surface on primary (spawn build/docs directors for substantial work), dependency installs and off-limits paths need approval, images are native multimodal input, only core tools are resident (load the rest via `tool_search`; use `search_agents` before dispatching specialists), workflows run only from slash-command steps, and session memory lives at `.corbits/MEMORY.md`. +- `buildHarnessFacts` — the non-derivable rules: shell file-writes are blocked, path tools are the DIY surface on primary (spawn builder/docs directors for substantial work), dependency installs and off-limits paths need approval, images are native multimodal input, only core tools are resident (load the rest via `tool_search`; use `search_agents` before dispatching specialists), workflows run only from slash-command steps, and session memory lives at `.corbits/MEMORY.md`. - `buildGuidelines` — be concise, prefer `task` for substantial product work, DIY tiny/bounded edits on the parent, answer questions and diagnose visual/product feedback before editing, work autonomously for explicit coding tasks, use `lsp` for symbol work, and verify changes when practical. - `buildPromptDisciplineBlock` — a shared, prohibition-form section appended exactly once to every built prompt (chat and sub-agent, every provider family). Primary vs leaf wording differs for product writes: leaves are told to use `read_file`/`edit_file`/`write_file`; Skywalker is told to DIY tiny/bounded edits with those path tools and spawn directors for substantial work. Shared rules: never `cat`/`sed`/heredoc/`echo` for file work, no setting or exporting environment variables (recurring needs belong in project settings), `web_fetch`/`web_search` instead of `curl`/`wget`/hand-rolled queries, one operation per `run_shell` call, turn semantics (a tool-less reply is the final answer, no repeat searches, stop and change approach after three failed attempts, batch independent reads in parallel), and TTY output rules (short bold headers, one-line bullets, backticks for paths/commands, no wide tables). @@ -403,7 +403,7 @@ Corbits Code **ships a bundled catalog** as the first-party data-only plugin `pl `discoverRepoPlugins` locates `plugins/` next to the source root, at `dist/plugins`, or at `dirname(execPath)/plugins`. It never scans the session cwd for the bundled catalog. -Primary is Skywalker. Bundled skill bodies that are operator slashes are **action** recipes that tell it to `task(agent="")` — there is no catch-all worker. Default slashes: `/implement`, `/plan`, `/refactor`, `/review`, `/pull-request-review`, `/create-issue`, `/scribe`, `/interview`, `/ast-grep`. `/scribe` dispatches shakespeare; `/implement` spawns build / greybeard / critique as the recipe specifies; `/plan` dispatches plan director (eng change plan; does not implement; does not file tracker issues); `/review` is a code-review action (not a director name); `/create-issue` remains the tracker command — Linear MCP when available, otherwise `ask_operator` for the platform and persists `Preferred issue tracker` in `.corbits/MEMORY.md`. Dispatch is `use_skill` only, not a default slash. Draper and emil are closed directors via `task(agent=…)`, not slashes. The operator types the slash; Skywalker reads the body and dispatches. +Primary is Skywalker. Bundled skill bodies that are operator slashes are **action** recipes that tell it to `task(agent="")` — there is no catch-all worker. Default slashes: `/implement`, `/plan`, `/refactor`, `/review`, `/pull-request-review`, `/create-issue`, `/scribe`, `/interview`, `/ast-grep`. `/scribe` dispatches shakespeare; `/implement` spawns builder / greybeard / critic as the recipe specifies; `/plan` dispatches counsel director (eng change plan; does not implement; does not file tracker issues); `/review` is a code-review action (not a director name); `/create-issue` remains the tracker command — Linear MCP when available, otherwise `ask_operator` for the platform and persists `Preferred issue tracker` in `.corbits/MEMORY.md`. Dispatch is `use_skill` only, not a default slash. Draper and emil are closed directors via `task(agent=…)`, not slashes. The operator types the slash; Skywalker reads the body and dispatches. #### Discovery and precedence @@ -416,19 +416,20 @@ Primary is Skywalker. Bundled skill bodies that are operator slashes are **actio | `.claude/skills/` | Claude Code workspace skills | | `.codex/skills/` | Codex workspace skills | -Each `//SKILL.md` is one skill. Discovery dedupes by directory name: the first base dir that provides a given name wins, so an enabled plugin skill shadows a project-local skill of the same name. Plugin dirs are passed in discovery order (repo first), so a first-party catalog name wins over a later marketplace or project skill of the same name. `resolveSkillBody(cwd, ref, pluginDirs)` resolves a skill's body using the same ordered list (it accepts a bare name or a `plugin:name` ref, keying on the name). +Each `//SKILL.md` is one skill. Discovery dedupes by directory name: the first base dir that provides a given name wins, so an enabled plugin skill shadows a project-local skill of the same name. Plugin dirs are passed in discovery order (repo first), so a first-party catalog name wins over a later marketplace or project skill of the same name. Skills with `disable-model-invocation: true` are omitted from the returned listing but still claim the name (first-wins), so a lower-priority same-name skill cannot leak into the listing. `resolveSkillBody(cwd, ref, pluginDirs)` resolves a skill's body using the same ordered list (it accepts a bare name or a `plugin:name` ref, keying on the name) and **does not** hard-fail on `disable-model-invocation` — explicit `use_skill("name")` still loads background libraries. #### SKILL.md format -A skill file begins with a YAML frontmatter block, followed by the body that holds the instructions. Discovery parses `description`; `loadSkillCommands` also reads `user-invocable`. The skill's identifier (what `use_skill` and `/` take) is its directory name. A skill with no `SKILL.md` or an empty body is skipped. +A skill file begins with a YAML frontmatter block, followed by the body that holds the instructions. Discovery parses `description` and `disable-model-invocation`; `loadSkillCommands` also reads `user-invocable`. The skill's identifier (what `use_skill` and `/` take) is its directory name. A skill with no `SKILL.md` or an empty body is skipped. -| Field | Required | Description | -| ---------------- | ------------ | -------------------------------------------------------------------------------------------------------------------------------------------------- | -| `description` | yes | One-line summary shown in the prompt's lazy skills listing and the slash picker | -| `name` | conventional | Conventionally matches the directory name; the directory name is what is actually used as the identifier | -| `user-invocable` | no | When `false`, `loadSkillCommands` skips slash synthesis; the skill remains `use_skill` only. Untagged skills still become slashes (marketplace BC) | +| Field | Required | Description | +| --------------------------- | ------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `description` | yes | One-line summary shown in the prompt's lazy skills listing and the slash picker | +| `name` | conventional | Conventionally matches the directory name; the directory name is what is actually used as the identifier | +| `user-invocable` | no | When `false`, `loadSkillCommands` skips slash synthesis; the skill remains `use_skill` only. Untagged skills still become slashes (marketplace BC) | +| `disable-model-invocation` | no | When `true`, `discoverSkills` omits the skill from the lazy listing (but still claims the name for first-wins). Explicit `resolveSkillBody` / `use_skill("name")` still loads the body. Does not affect slash emission. | -There are no `type` or `disable-model-invocation` fields required for model invocation — a skill body is plain instruction text. `argument-hint` on frontmatter is preserved for the slash picker (greyed arg guidance). Multi-step orchestration is a separate mechanism (see Workflows above), not a skill `type`. +There is no skill `type` field required for model invocation — a skill body is plain instruction text. Background libraries (e.g. `git-worktrees`) set both `user-invocable: false` and `disable-model-invocation: true` so they are absent from slash and listing, yet recipes can still `use_skill("git-worktrees")`. `argument-hint` on frontmatter is preserved for the slash picker (greyed arg guidance). Multi-step orchestration is a separate mechanism (see Workflows above), not a skill `type`. #### Loading (model and operator) diff --git a/docs/IMPLEMENTATION.md b/docs/IMPLEMENTATION.md index 4be525468..b316de621 100644 --- a/docs/IMPLEMENTATION.md +++ b/docs/IMPLEMENTATION.md @@ -157,14 +157,14 @@ Sixteen packages under `src/agent/directors//` register in `DIRECTOR_REGISTR 2. `packageToProfile` maps envelope (`tools.allow`/`deny`) to `AgentProfile.capabilities` and `spawn.maySpawn` → `orchestrator`. System prompts are prefixed with a stable identity block (`formatDirectorSystemPrompt`: agent id, model role, optional skills). 3. Nested spawn: packages with `spawn.allowlist` forward that list into nested `task` (`spawnAllowlist` on nestedDispatch). Off-list `agent` is refused. `task(agent=skywalker)` is refused (primary is not a spawned worker). Primary omits the list so plugin profiles stay reachable. 4. `directorProfiles()` is the spawn catalog (`default-agents.ts`) — closed set minus skywalker; plugin agent profiles still load and can override by id. -5. Primary chat role is Skywalker: `buildChatRole()` → `createSkywalkerSystemPrompt()`. Product mutation tools (`write_file` / `edit_file` / `delete_file`) live in CORE (and `SKYWALKER_TOOLS`) so they are advertised on the primary without a `tool_search` round-trip. DIY tiny/bounded edits on the parent; spawn build/docs directors for substantial work — a prompt judgment call, not a toolset strip. `PRIMARY_DENIED_PRODUCT_TOOLS` is gone. Shell file-writes stay denied; MCP tools are not re-filtered by a product-write deny list. There is no static per-profile write-path lock (CL-6952). +5. Primary chat role is Skywalker: `buildChatRole()` → `createSkywalkerSystemPrompt()`. Product mutation tools (`write_file` / `edit_file` / `delete_file`) live in CORE (and `SKYWALKER_TOOLS`) so they are advertised on the primary without a `tool_search` round-trip. DIY tiny/bounded edits on the parent; spawn builder/docs directors for substantial work — a prompt judgment call, not a toolset strip. `PRIMARY_DENIED_PRODUCT_TOOLS` is gone. Shell file-writes stay denied; MCP tools are not re-filtered by a product-write deny list. There is no static per-profile write-path lock (CL-6952). **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`, per the pinned base-instructions text quoted in `codex-responses-adapter.ts`'s bridge message — 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 leaf 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. 6. There is no static write-path declaration on packages or profiles (CL-6952 removed it — no shipped director ever set one). Instead, `task-tool.ts` tracks each running dispatch by cwd; a new dispatch that lands on the same cwd as a still-running lane records a `concurrent-lane-overlap` entry in `intervention-log.ts` (class `conflict`). This is advisory only — it never blocks the spawn, since cwd overlap does not prove the two lanes touch the same files. 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 do not mount `use_skill` (guidance is baked into package system prompts). Primary mounts `use_skill` for its own skill list. -Intent defaults: `intent=implement` → director `build`; explore/plan → same-named director; review → critique; general → error. Spawn: skywalker full fleet; greybeard intern/explore/critique only; all other directors no `task`. Live `` injects cwd, platform, arch, runtime, date, and git status on every chat and worker prompt. +Intent defaults: `intent=implement` → director `builder`; `explore` → `explorer`; `plan` → `counsel`; `review` → `critic`; general → error. Spawn: skywalker full fleet; greybeard intern/explorer/critic only; all other directors no `task`. Live `` injects cwd, platform, arch, runtime, date, and git status on every chat and worker prompt. ### Auto Mode diff --git a/docs/PLUGINS.md b/docs/PLUGINS.md index ae347ee75..3848302e6 100644 --- a/docs/PLUGINS.md +++ b/docs/PLUGINS.md @@ -309,10 +309,14 @@ shape. become slashes (marketplace backward compatibility). Frontmatter `argument-hint` is preserved so the TUI can show greyed arg guidance (e.g. `/create-issue` → `[description] [--from-doc]`). This is an additional - surface: `discoverSkills` is unchanged, so the model can still auto-invoke any - skill via `use_skill` — including first-party recipes that are not operator - slashes (`dispatch`, `git-rebase`, `linear-issue-workflow`, `style`, - `philosophy`, `typescript`, `opsh`). The slash command is a direct user entry + surface: `discoverSkills` skips skills with `disable-model-invocation: true` + from the lazy listing (those stay loadable via explicit `use_skill` / + `resolveSkillBody`), so the model does not auto-suggest background libraries. + First-party recipes that are not operator slashes remain listed for + `use_skill` when they only set `user-invocable: false` (`dispatch`, + `git-rebase`, `linear-issue-workflow`, `style`, `philosophy`, `typescript`, + `opsh`). Background libs such as `git-worktrees` set both flags. The slash + command is a direct user entry point on top. - **First-party catalog.** `plugins/corbits-skills/` (id `corbits-skills`, kind `command`, `defaultEnabled: true`) is the bundled skill catalog. Origin diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index 1d58ecacf..1600cb905 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -98,7 +98,7 @@ is the direct, explicit resume path. The TUI has an extensible slash-command framework. Built-ins include `/help` (shortcut + command overlay), `/model` (models-only picker for connected accounts; **Alt+A** adds a provider), `/settings`, `/permissions`, `/plugins`, `/clear`, `/new`, `/mcp`, and `/yolo` (persists as the user-global skip-permissions default; `--dangerously-skip-permissions` still forces this process; secret-guard and authz still apply; `/yolo [on|off|toggle]`, bare `/yolo` toggles), plus a `/` command per available workflow. When a session starts with the persisted default already on, the TUI shows a startup notice ("Permission prompts are disabled by your saved default…") so the silent machine-wide default is never invisible; `corbits exec` prints the equivalent warning to stderr. Plugins can register additional commands. -**Default skills** exist out of the gate as first-party slash **actions**, not director names: `/implement`, `/plan`, `/refactor`, `/review`, `/pull-request-review`, `/create-issue`, `/scribe`, `/interview`, `/ast-grep`. Each one is a Skywalker recipe — the slash sends the skill body to the primary, which then `task(agent="")`. `/scribe` → shakespeare; `/implement` spawns build / greybeard / critique as the recipe specifies; `/plan` → plan director (eng change plan: files, AC, non-goals, risks, ordered steps; does not implement); `/review` is a code-review action. `/create-issue` remains the tracker command: Linear MCP when available; otherwise it `ask_operator`s for the platform (GitHub etc.) and persists `Preferred issue tracker` in `.corbits/MEMORY.md` (GitHub via `gh issue create`). Dispatch is not a default slash — it stays `use_skill` only, along with git-rebase, linear-issue-workflow, style, philosophy, typescript, and opsh (`user-invocable: false`). Draper and emil are not slashes; they remain closed directors via `task(agent=…)`. There is no catch-all worker. Slash names are also available to the model via `use_skill`. Disable the catalog in `/plugins` (`corbits-skills`) if you want them gone. +**Default skills** exist out of the gate as first-party slash **actions**, not director names: `/implement`, `/plan`, `/refactor`, `/review`, `/pull-request-review`, `/create-issue`, `/scribe`, `/interview`, `/ast-grep`. Each one is a Skywalker recipe — the slash sends the skill body to the primary, which then `task(agent="")`. `/scribe` → shakespeare; `/implement` spawns builder / greybeard / critic as the recipe specifies; `/plan` → counsel director (eng change plan: files, AC, non-goals, risks, ordered steps; does not implement); `/review` is a code-review action. `/create-issue` remains the tracker command: Linear MCP when available; otherwise it `ask_operator`s for the platform (GitHub etc.) and persists `Preferred issue tracker` in `.corbits/MEMORY.md` (GitHub via `gh issue create`). Dispatch is not a default slash — it stays `use_skill` only, along with git-rebase, linear-issue-workflow, style, philosophy, typescript, and opsh (`user-invocable: false`). Draper and emil are not slashes; they remain closed directors via `task(agent=…)`. There is no catch-all worker. Slash names are also available to the model via `use_skill`. Disable the catalog in `/plugins` (`corbits-skills`) if you want them gone. Providers are **models-first**: there is no standalone `/login` command. `/model` opens a **models-only list** (Recent, Favorites, then connected provider/model rows) — type-to-filter owns printable keys, so Connect is never a bare letter. **Alt+A** opens a dedicated add-provider selector over every first-class kind (OpenAI dual-path ChatGPT OAuth or API key, xAI, OpenCode Zen, Anthropic, Google, OpenCode Go, Z.AI Coding Plan, Custom), each annotated with its live account count and never filtered out for “already connected.” **Alt+F** toggles favorite on the highlighted model. **Alt+D** persists the highlighted pair as the default without switching the live session. Advanced provider drill-down (edit/delete/tiers) stays on the advanced surface, not a bare printable key while the model list is filtering. OAuth providers open their existing browser login with a named account step so multiple accounts per kind coexist (`codex/work`, …). API-key providers use the same named-instance step before the key (auth-only form: instance name + key + fixed catalog base URL), so personal and team keys land as distinct catalog rows (`openai/default`, `anthropic/work`, …); reusing a name re-keys that instance after confirm. Custom remains a free-form single endpoint (full manual form). Successful connect refreshes the catalog and reopens the model list focused on the new account’s default model. OpenCode Go routes each model by its protocol metadata (chat completions, OpenAI responses, or Anthropic messages) and can show subscription usage in the status bar when active (rolling 5h / weekly / monthly windows when the usage API responds; omitted on auth or network failure). When Go returns a quota or rate-limit error — including some HTTP 400 responses that carry limit payloads — Corbits classifies them so quota aborts cleanly and short provider rate limits remain retryable. On a free-tier or subscription quota hit, wait for the window to reset or use OpenCode Zen free models. @@ -144,14 +144,14 @@ Capabilities beyond the core toolset are opt-in plugins, enabled per workspace t The primary session is always **orchestrator** (single-agent mode is gone). Its identity is **Skywalker** (product name remains Corbits Code; when asked its name, answer Skywalker): classify work, DIY tiny/single-file/one-route product edits, dispatch a **closed fleet of 16 directors** for substantial work, track the fleet, and synthesize. Product mutation tools (`write_file` / `edit_file` / `delete_file`) are mounted on the primary (CORE / `SKYWALKER_TOOLS`) — path tools are the DIY surface; spawn remains the default for substantial, multi-file, parallel, or specialist work. Shell file-writes stay denied. MCP tools are not re-filtered by a product-write deny list (that list is gone). There is no static per-package write-path declaration (CL-6952 removed it — no shipped director ever set one). A concurrent dispatch landing on the same working directory as another still-running lane is recorded as a `conflict` intervention, not blocked. Operator slash recipes (`/implement`, `/plan`, `/refactor`, `/review`, `/pull-request-review`, `/create-issue`, `/scribe`, `/interview`, `/ast-grep`) tell Skywalker which directors to spawn for substantial work; tiny/bounded edits may run on the primary. -| Lane | Directors | -| --------- | ---------------------------------------------------------------------------------- | -| Primary | skywalker | -| Eng | build, explore, plan, intern, critique, greybeard, neckbeard, bruckheimer, gaasbot | -| Design | draper, emil, brand-reviewer | -| Docs / QA | shakespeare, testsmith, tester | +| Lane | Directors | +| --------- | ------------------------------------------------------------------------------------ | +| Primary | skywalker | +| Eng | builder, explorer, counsel, intern, critic, greybeard, neckbeard, bruckheimer, gaasbot | +| Design | draper, emil, rand | +| Docs / QA | shakespeare, testsmith, tester | -There is **no catch-all worker**. `task` requires `agent=…` or a non-general `intent` (implement/explore/plan/review→critique); bare dispatch and `intent=general` are refused. Named `task(agent=…)` selects a director package without requiring a plugin profile, except `skywalker` which is the primary session identity and is refused as a spawned worker. Nested spawn is runtime-enforced: only skywalker (full fleet allowlist) and greybeard (intern/explore/critique) may spawn; other workers have no `task`. Primary omits an allowlist so plugin profiles remain reachable from the main session. +There is **no catch-all worker**. `task` requires `agent=…` or a non-general `intent` (implement/explore/plan/review→critic); bare dispatch and `intent=general` are refused. Named `task(agent=…)` selects a director package without requiring a plugin profile, except `skywalker` which is the primary session identity and is refused as a spawned worker. Nested spawn is runtime-enforced: only skywalker (full fleet allowlist) and greybeard (intern/explorer/critic) may spawn; other workers have no `task`. Primary omits an allowlist so plugin profiles remain reachable from the main session. Corbits Code fans work out to short-lived **sub-agents** — child agents with their own loop, tools, and checklist — while the primary session stays focused. diff --git a/plugins/corbits-skills/skills/dispatch/SKILL.md b/plugins/corbits-skills/skills/dispatch/SKILL.md index 41e00791d..75bc94afc 100644 --- a/plugins/corbits-skills/skills/dispatch/SKILL.md +++ b/plugins/corbits-skills/skills/dispatch/SKILL.md @@ -2,84 +2,82 @@ name: dispatch user-invocable: false argument-hint: "[ | dispatch// | dispatch//dispatch.yaml | ]" -description: Multi-lane DAG orchestration. Skywalker recipe — use_skill("dispatch"). Spawns explore, intern, build, plan, and critique. DAG product tasks go through build; Skywalker may DIY tiny edits outside the DAG. +description: Multi-lane DAG orchestration. Skywalker recipe — use_skill("dispatch"). Spawns explorer, intern, builder, counsel, critic (and specialists like rand). DAG product tasks go through builder; Skywalker may DIY tiny edits outside the DAG. --- # Dispatch -You are Skywalker. This skill is loadable with `use_skill("dispatch")`. Follow this recipe. DAG product tasks go through build workers. Do not write `dispatch.yaml` or `plan.md` yourself (intern cannot write; build writes manifests). Tiny / single-file / one-route product edits outside this DAG may be DIY with write_file/edit_file/delete_file. +You are Skywalker. Load with `use_skill("dispatch")`. Orchestrate parallel director runs across a dependency graph: classify lanes, spawn independent work together, wait/synthesize, critique, verify, re-dispatch fixes, report. -Orchestrate parallel director runs across a dependency graph. Fan out work, fan in reports, critique, verify, re-dispatch fixes, and synthesize until done. +DAG product tasks go through **builder**. Do not write `dispatch.yaml` or `plan.md` yourself (intern cannot write; builder writes manifests). Tiny / single-file / one-route product edits outside this DAG may be DIY with write_file/edit_file/delete_file. -Hard cap: **at most 4 workers at once** unless the operator explicitly asks for a wider fan-out. Track progress with `manage_tasks`. +**Fleet default:** `spawn_agent` then `wait_agents` — fire independent workers in one turn, wait on the reports you need next. Prefer `task()` only when a single blocking spawn is enough. No worker-count or fan-out ceiling — width follows named, non-overlapping lanes (distinct path/package/ownership). Track progress with `manage_tasks`. -Closed directors used here: `explore`, `intern`, `build`, `plan`, `critique`. Optional consults: `greybeard`, `tester`. Never a catch-all worker. DAG node agents are `explore`, `intern`, and `build` only. +Closed directors used here: `explorer`, `intern`, `builder`, `counsel`, `critic`. Optional: `greybeard`, `tester`, `rand` (DESIGN.md / brand). Never a catch-all worker. DAG node agents are `explorer`, `intern`, and `builder` only. ## Input resolution -Figure out what to run from the argument: - -- **No argument** → latest dispatch (newest `dispatch//` directory by creation time) +- **No argument** → latest dispatch (newest `dispatch//` by creation time) - **Just a name** (e.g. `auth-fix`) → `dispatch//dispatch.yaml` -- **Directory** (e.g. `dispatch/auth-fix/`) → `dispatch.yaml` inside +- **Directory** → `dispatch.yaml` inside - **File ending in `dispatch.yaml`** → run it -- **Any other file** → treat as a spec. If it still needs an eng plan, spawn `task(agent="plan")` first, then run. +- **Any other file** → treat as a spec. If it still needs an eng plan, spawn counsel first, then run. -If the spec is vague, incomplete, or contradictory: stop and report Blockers. Do not invent a DAG. +Vague / incomplete / contradictory spec → stop and report Blockers. Do not invent a DAG. ## Who does what -| Work | Director | -| ------------------------------------------------------------------------------------------------ | ------------------------- | -| Map the codebase, gather facts | `task(agent="explore")` | -| Eng plan from a spec (no ship) | `task(agent="plan")` | -| Write `dispatch.yaml` / `plan.md` / status artifacts (mechanical brief; no product feature work) | `task(agent="build")` | -| Ship product code + tests | `task(agent="build")` | -| Review a landed task (defects, evidence, no fix) | `task(agent="critique")` | -| Architecture judgment before a large DAG | `task(agent="greybeard")` | -| Independent suite / repro evidence | `task(agent="tester")` | - -Skywalker classifies, spawns, tracks, and synthesizes. Path tools (`write_file` / `edit_file` / `delete_file`) are mounted for DIY tiny/bounded product edits; spawn remains the default for DAG product work. Durable orchestration artifacts (`dispatch.yaml`, `plan.md`, status) still go through build — intern does not have write tools (`INTERN_TOOLS` = run_shell, read_file, list_dir). Do not spawn a blob agent to author the manifest. Do not write those manifests on Skywalker. +| Work | Director | +| ---- | -------- | +| Map the codebase, gather facts | `explorer` | +| Eng plan from a spec (no ship) | `counsel` | +| Write `dispatch.yaml` / `plan.md` / status (mechanical; no product feature work) | `builder` | +| Ship product code + tests | `builder` | +| Review a landed task (defects, evidence, no fix) | `critic` | +| Architecture judgment before a large DAG | `greybeard` | +| Independent suite / repro evidence | `tester` | +| DESIGN.md / brand gate for UI lanes | `rand` | -Prefer typed briefs: `intent`, `success_criteria`, `do_not`, `report_focus`, and `agent`. +Skywalker classifies, spawns, tracks, and synthesizes. Durable orchestration artifacts still go through builder — intern has no write tools. Prefer typed briefs: `intent`, `success_criteria`, `do_not`, `report_focus`, and `agent`. ## Agent type selection -Use **explore** when the task is pure research. No code changes. Output is findings for downstream tasks. - -Use **intern** when the work is mechanical and well-specified: git commit after a level fans in, exact shell, mechanical git. Intern cannot write files. +- **explorer** — pure research; no code changes; findings for downstream. +- **intern** — mechanical, well-specified: level commits, exact shell, git via recipes. Cannot write files. +- **builder** — ships product code (judgment, abstractions, tests) and mechanical writes of `dispatch.yaml` / `plan.md` / status. -Use **build** when the task ships product code — including work that needs judgment, new abstractions, or tests — and for mechanical writes of `dispatch.yaml` / `plan.md` / status artifacts (write tools; intern does not have them). There is no catch-all implementation agent. - -Critique is not a DAG node agent type. After build (and after non-trivial intern landings), spawn `task(agent="critique")` with the task's objective, paths, and diff. Simple intern tasks may skip critique. +Critique is not a DAG node type. After builder (and non-trivial intern landings), spawn `critic` with objective, paths, and diff. Simple intern tasks may skip critique. Classify each product task as `feature` or `bugfix`: -- `bugfix`: incorrect behavior that exists today → test-first (fail, then fix, then pass) -- `feature`: everything else → tests for the new behavior +- `bugfix` → test-first (fail, then fix, then pass) +- `feature` → tests for the new behavior - When unsure, default to `feature` +## Isolated worktree (when needed) + +When the run needs a clean branch / isolated tree: load `use_skill("git-worktrees")`, copy the create (or teardown) recipe into an intern brief, spawn `intern`. Do not inline git worktree commands here. Skywalker does not run the git. + ## Phase 1: Planning -Runs when the input is a spec (or a request with no existing manifest). The spec should be complete enough that a build worker could succeed from it. +Runs when the input is a spec (or a request with no existing manifest). -1. If the spec still needs an ordered eng plan, spawn `task(agent="plan")`. Do not skip this when requirements are large or ambiguous. -2. Spawn `explore` workers only as needed to map scope. Distinct path/package lenses if parallel. -3. Consult `greybeard` before large multi-lane work when architecture is in play. +1. Spec needs an ordered eng plan → spawn `counsel`. +2. Spawn `explorer` workers only as needed (distinct path/package lenses if parallel). +3. Consult `greybeard` before large multi-lane work when architecture is in play; `rand` when UI/brand is in play. 4. Break the goal into discrete tasks, each small enough for one director. -5. Identify dependencies (DAG edges). Same-file writers at the same level must be merged or serialized via `depends-on`. -6. Assign `explore` | `intern` | `build` per the guide above. -7. Detect verify commands from `package.json`, Makefile, or project docs. -8. Add per-task verification to each plan (build for compiled changes, tests for test-writing tasks). -9. Default commit strategy is **per-task** (debuggable). Use grouped only when the operator wants a cleaner history **and** Phase 5 will catch issues. -10. Mark which tasks need critique (complex build → yes; simple intern → no; when unsure, yes). -11. Seed `manage_tasks` with one item per DAG task (plus plan / verify / critique items as needed). +5. Identify dependencies (DAG edges). Same-file writers at the same level → merge or serialize via `depends-on`. +6. Assign `explorer` | `intern` | `builder` per the guide above. +7. Detect verify commands from `package.json`, Makefile, or project docs; add per-task verification. +8. Default commit strategy: **per-task**. Grouped only when the operator wants a cleaner history **and** Phase 5 will catch issues. +9. Mark which tasks need critique (complex builder → yes; simple intern → no; unsure → yes). +10. Seed `manage_tasks` with one item per DAG task (plus plan / verify / critique as needed). -If requirements are not actionable, stop. Ask: "Can a build worker succeed with only this information?" +If requirements are not actionable, stop. Ask: "Can a builder worker succeed with only this information?" ## Phase 2: Directory structure -Have **build** write the run tree (mechanical brief; no product feature work). Do not write these files on Skywalker. Do not use intern — intern cannot write files. Do not use a catch-all worker. +Have **builder** write the run tree (mechanical brief). Do not write these on Skywalker. Do not use intern for writes. ``` dispatch/ @@ -94,7 +92,7 @@ dispatch/ Task directory names: `-` -- **Level** (1, 2, 3…): DAG depth. Roots are level 1. Level is longest path from a root, plus one. +- **Level** (1, 2, 3…): DAG depth. Roots are level 1. - **Sequence** (a, b, c…): siblings at the same level (candidates for parallel). - **Description**: underscore-separated, from the objective. @@ -105,7 +103,6 @@ The directory name is the task `id`. After a worker runs, the task directory is ```yaml goal: "Short description of the overall goal" status: pending # pending | in-progress | completed | failed -max-parallel: 4 # hard cap unless the operator asks for more created: YYYY-MM-DD verify: @@ -116,7 +113,7 @@ verify: critique: enabled: true - agent: critique # always task(agent="critique") + agent: critic commits: strategy: per-task # per-task | grouped @@ -124,8 +121,8 @@ commits: tasks: - id: 1a-extract_auth_module - type: feature # feature | bugfix (omit for explore) - agent: build # build | intern | explore + type: feature # feature | bugfix (omit for explorer) + agent: builder # builder | intern | explorer depends-on: [] receives: [] # subset of depends-on; default = depends-on status: pending # pending | dispatched | completed | failed | fixing @@ -134,59 +131,59 @@ tasks: - id: 2a-integrate_modules type: feature - agent: build + agent: builder depends-on: [1a-extract_auth_module, 1b-extract_logging_module] status: pending ``` -Task statuses: `pending` → `dispatched` → `completed` | `failed` | `fixing`. Downstream tasks wait for `completed`. +Task statuses: `pending` → `dispatched` → `completed` | `failed` | `fixing`. Downstream waits for `completed`. ### Task `plan.md` -Build writes one per task (mechanical brief). Include: objective, requirements covered, context (paths and symbols — no line numbers, no dispatch-dir cross-refs), files to modify, constraints, verification (test-first for bugfix), and `do_not`. +Builder writes one per task. Include: objective, requirements covered, context (paths and symbols — no line numbers, no dispatch-dir cross-refs), files to modify, constraints, verification (test-first for bugfix), and `do_not`. Every product-task brief must tell the worker: - Do **not** run mutating git (`git add` / `commit` / `checkout` / `stash`). Intern commits after the level fans in. - Leave changes uncommitted. Multiple tasks may share a worktree. -- Read the full `plan.md` before acting. If it is unclear, fail closed. +- Read the full `plan.md` before acting. If unclear, fail closed. ## Phase 3: Validate, then present Before any product spawn: -**Structural:** DAG is acyclic; every `depends-on` / `receives` id exists; `receives` ⊆ `depends-on`; every task has `plan.md`; no orphan directories. +**Structural:** DAG acyclic; every `depends-on` / `receives` id exists; `receives` ⊆ `depends-on`; every task has `plan.md`; no orphan directories. -**Completeness:** clear objectives; files named; union of tasks covers the goal; every spec requirement maps to at least one task. +**Completeness:** clear objectives; files named; union of tasks covers the goal; every spec requirement maps to ≥1 task. -**Coherence:** no two ready-in-parallel tasks write the same file; constraints do not contradict; `explore` is never assigned product writes. +**Coherence:** no two ready-in-parallel tasks write the same file; constraints do not contradict; `explorer` never gets product writes. -**Feasibility:** referenced files exist or are created by this task or an upstream dependency; scope fits one worker. +**Feasibility:** referenced files exist or are created by this task or an upstream; scope fits one worker. -Empty task list → mark the run `completed` and report. Do not invent work. +Empty task list → mark `completed` and report. Do not invent work. -Present the DAG (ids, agents, deps, critique flags, verify commands, commit strategy) to the operator. Wait for go-ahead on large or ambiguous runs. Then set status `in-progress` (build updates the manifest if it is on disk). +Present the DAG (ids, agents, deps, critique flags, verify commands, commit strategy) to the operator. Wait for go-ahead on large or ambiguous runs. Then set status `in-progress` (builder updates the manifest if on disk). ## Phase 4: Execute the DAG 1. **Ready set:** `pending` tasks whose `depends-on` are all `completed`. -2. **Batch:** take a safe parallel subset, **at most 4 live workers** (including in-flight critique). Same-file writers and shared mutable state (build artifacts, test DBs) must not share a batch — serialize with `depends-on`. -3. **Spawn** each task with `task(agent="")`. Inject upstream reports (not a rewritten `plan.md`) into the brief. Split ownership by path/package when two build workers run together. -4. **Fan in:** trust the worker report (and `output.yaml` when build wrote one). Missing report or `status: failed` → mark `failed`. Do not re-fan-out an identical brief; change `success_criteria` / `do_not` or tell the operator. +2. **Parallel lanes:** spawn every independent ready task whose ownership does not overlap (same-file writers and shared mutable state must serialize via `depends-on`). No batch-size ceiling. +3. **Spawn:** call `spawn_agent(agent=, …)` for each ready task in one turn; record each returned `agent_id`, then `wait_agents` on those runtime ids. Inject upstream reports (not a rewritten `plan.md`) into the brief. Split ownership by path/package when two builders run together. +4. **Fan in:** trust the worker report (and `output.yaml` when builder wrote one). Missing report or `status: failed` → mark `failed`. Do not re-fan-out an identical brief; change `success_criteria` / `do_not` or tell the operator. 5. **Level commit:** after a level's product tasks self-report complete, intern commits per the strategy (per-task default). Workers must not have committed. -6. **Critique:** for tasks with `critique.enabled`, spawn `task(agent="critique")` on that commit/diff + objective. Blocking findings → re-dispatch `build` with those findings in `success_criteria` / `do_not` (status `fixing`). Cap re-fix rounds (1–2), then report Blockers. +6. **Critique:** for tasks with `critique.enabled`, spawn `critic` on that commit/diff + objective. Blocking findings → re-dispatch `builder` with those findings in `success_criteria` / `do_not` (status `fixing`). Cap re-fix rounds (1–2), then report Blockers. 7. Repeat until no pending tasks remain, or deadlock / all remaining failed → stop and ask. -Keep `manage_tasks` in sync as items move `todo` → `doing` → `done` / stay blocked. +Keep `manage_tasks` in sync (`todo` → `doing` → `done` / blocked). If the working tree has unrelated uncommitted changes before Phase 4, ask the operator. Do not mix them into level commits. ## Phase 5: Verify -Must `task(agent="tester")` for the suite (or intern for one named mechanical command). Do not run the full verify pipeline on the parent via Skywalker `run_shell`. Compare against any baseline you captured. +Spawn `tester` for the suite (or intern for one named mechanical command). Do not run the full verify pipeline on the parent via Skywalker `run_shell`. Compare against any baseline you captured. - Green, or same failures as baseline → proceed. -- New failures → attribute to a task/commit, re-dispatch `build` on that lane, re-verify. Cap rounds, then Blockers. +- New failures → attribute to a task/commit, re-dispatch `builder` on that lane, re-verify. Cap rounds, then Blockers. - Do not declare done on a worker "ready" that ignored blocking critique or verify. ## Phase 6: Complete @@ -201,7 +198,7 @@ Synthesize for the operator: ## Paths -Include: what landed, which directors ran, verify evidence, remaining failed/fixing tasks. Mark the run `completed` or `failed`. `manage_tasks` should reflect the same. +Include: what landed, which directors ran, verify evidence, remaining failed/fixing tasks. Mark the run `completed` or `failed`. `manage_tasks` should match. ## Resume @@ -209,8 +206,8 @@ Re-resolve input to the existing `dispatch//`. Re-validate the remaining D ## Non-negotiables -- You are Skywalker. Spawn directors. Do not implement product features. Do not author dispatch YAML/plan files yourself or via a catch-all worker. Durable orchestration files go through build. -- `use_skill("dispatch")` loads this recipe. It is a command. -- Agents: `explore`, `intern`, `build` only for DAG nodes. Critique via `task(agent="critique")`. Plan via `task(agent="plan")` when a spec needs an eng plan first. -- Progress: `manage_tasks`. -- At most 4 workers at once unless the operator asks for more. +- You are Skywalker. Spawn directors. Do not implement product features. Do not author dispatch YAML/plan files yourself or via a catch-all worker. Durable orchestration files go through builder. +- `use_skill("dispatch")` loads this recipe. It is not a slash command. +- Fleet verbs: `spawn_agent` + `wait_agents` for multi-lane work. DAG nodes: `explorer`, `intern`, `builder` only. Critique via `critic`. Plan via `counsel` when a spec needs an eng plan first. +- Isolated trees: `use_skill("git-worktrees")` → intern executes. Do not inline worktree git here. +- Progress: `manage_tasks`. Parallelize named non-overlapping lanes — no worker-count cap. diff --git a/plugins/corbits-skills/skills/git-worktrees/SKILL.md b/plugins/corbits-skills/skills/git-worktrees/SKILL.md new file mode 100644 index 000000000..c87702595 --- /dev/null +++ b/plugins/corbits-skills/skills/git-worktrees/SKILL.md @@ -0,0 +1,31 @@ +--- +name: git-worktrees +user-invocable: false +disable-model-invocation: true +description: Create a git worktree from origin/ and tear it down. Background library — load via use_skill("git-worktrees"); absent from slash and use_skill listing. +--- + +# git-worktrees + +Background recipe. Skywalker loads via `use_skill("git-worktrees")` and copies commands into an intern brief. Intern executes via `run_shell`. Skywalker does not run the git. + +## Create from origin/ + +```bash +git symbolic-ref refs/remotes/origin/HEAD | sed 's@^refs/remotes/origin/@@' +git fetch origin +git worktree add ../worktree/ -b origin/ +``` + +Always base new branches on `origin/` (whatever the repository uses). After creating the worktree, intern `cd`s into it and installs local dependencies (`bun install` when the project uses Bun; otherwise follow developer docs). Worktrees do not share `node_modules`. + +## Teardown + +```bash +cd +git fetch origin +git worktree remove ../worktree/ +git branch -d +``` + +If the worktree directory was already deleted: `git worktree prune`. diff --git a/plugins/corbits-skills/skills/implement/SKILL.md b/plugins/corbits-skills/skills/implement/SKILL.md index 854a2f71c..73d338754 100644 --- a/plugins/corbits-skills/skills/implement/SKILL.md +++ b/plugins/corbits-skills/skills/implement/SKILL.md @@ -1,6 +1,6 @@ --- name: implement -description: Disciplined per-commit workflow — Skywalker spawns greybeard, build, intern/tester, critique. +description: Disciplined per-commit workflow — Skywalker spawns greybeard, builder, intern/tester, critic. --- # Implement @@ -9,7 +9,7 @@ You are Skywalker. This skill is a per-commit spawn recipe for substantial landi DIY is the exception: tiny / single-file / one-route / clear bounded product edits → use write_file/edit_file/delete_file on this session. Do not load this loop for that work. -Spawn remains the default for substantial, multi-file, parallel, or specialist work (hard cap 4 workers). When this recipe runs, spawn workers. Wait for reports. Decide the next spawn from those reports. +Spawn remains the default for substantial, multi-file, parallel, or specialist work (scale to named non-overlapping lanes — no hard worker cap). When this recipe runs, spawn workers. Wait for reports. Decide the next spawn from those reports. ## Prerequisites @@ -21,12 +21,12 @@ Track commit-sized units with `manage_tasks`. One item per unit that will become - Before starting: create an item for each unit from the caller's instructions. - When a unit begins: mark it in progress. -- When critique is clean and the build gate passed: mark it done. -- If new work surfaces (greybeard suggests a prep refactor, critique reveals an edge case that warrants its own commit), append a new `manage_tasks` item and run it through the full loop. +- When critic is clean and the build gate passed: mark it done. +- If new work surfaces (greybeard suggests a prep refactor, critic reveals an edge case that warrants its own commit), append a new `manage_tasks` item and run it through the full loop. ## Per-commit spawn loop -For each unit, run these steps in order. Do not skip. When this loop is running, do not DIY the unit — spawn build. +For each unit, run these steps in order. Do not skip. When this loop is running, do not DIY the unit — spawn builder. ### 1. Review — greybeard @@ -39,18 +39,18 @@ Send: - Design decisions and trade-offs - Uncertainties -Adjust the plan from the report, then spawn build. Greybeard is for approach, not execution. +Adjust the plan from the report, then spawn builder. Greybeard is for approach, not execution. ### 2. Implement -`task(agent="build")` with a typed brief: +`task(agent="builder")` with a typed brief: - `intent` - `success_criteria` - `do_not` - `report_focus` -**Bug fixes:** tell build to start from a failing test — write the repro, confirm it fails, then fix, then confirm it passes. If the test does not fail first, the bug is not understood. +**Bug fixes:** tell builder to start from a failing test — write the repro, confirm it fails, then fix, then confirm it passes. If the test does not fail first, the bug is not understood. **Features:** tests ship with the change. The test asserts the new behavior, not merely that the process did not crash. @@ -63,22 +63,22 @@ Keep scope to this unit. Additional work becomes a later `manage_tasks` item, no - `intern` — mechanical full pipeline - `tester` — suite / repro -Do not move forward with a broken build. If failures come from this unit, re-dispatch build. If they are pre-existing and unrelated, report Blockers and stop. Do not substitute a partial compile for the full gate. +Do not move forward with a broken build. If failures come from this unit, re-dispatch builder. If they are pre-existing and unrelated, report Blockers and stop. Do not substitute a partial compile for the full gate. ### 4. Critique -`task(agent="critique")` on the diff. Include the intent agreed with greybeard so critique evaluates plan vs execution, not only surface quality. Limit findings to this unit; pre-existing issues in touched files are out of scope unless they block the gate. +`task(agent="critic")` on the diff. Include the intent agreed with greybeard so critic evaluates plan vs execution, not only surface quality. Limit findings to this unit; pre-existing issues in touched files are out of scope unless they block the gate. -If critique is **blocking**, re-dispatch build once or twice with those findings in `success_criteria` / `do_not`, then re-run the build gate and critique. After two re-fix rounds, report Blockers — do not loop forever. +If critic is **blocking**, re-dispatch builder once or twice with those findings in `success_criteria` / `do_not`, then re-run the build gate and critic. After two re-fix rounds, report Blockers — do not loop forever. -When critique is clean (or remaining findings are acknowledged judgment calls), mark the unit done and start the next. +When critic is clean (or remaining findings are acknowledged judgment calls), mark the unit done and start the next. ## Hard rules - Tiny / single-file / one-route / clear bounded edits: DIY with write_file/edit_file/delete_file. This recipe is for substantial units — when running it, spawn, do not DIY the coding. -- Spawn with `task(agent="greybeard")`, `task(agent="build")`, `task(agent="intern")` or `task(agent="tester")`, and `task(agent="critique")`. +- Spawn with `task(agent="greybeard")`, `task(agent="builder")`, `task(agent="intern")` or `task(agent="tester")`, and `task(agent="critic")`. - Track only with `manage_tasks`. -- Do not shortcut the loop. Skipping greybeard “because this is simple” or skipping critique “because the build passed” defeats the recipe. +- Do not shortcut the loop. Skipping greybeard “because this is simple” or skipping critic “because the build passed” defeats the recipe. - Build must pass before treating a unit as done. ## Report diff --git a/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md b/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md index 5d11147c7..a333d9fb1 100644 --- a/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md +++ b/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md @@ -1,7 +1,7 @@ --- name: linear-issue-workflow user-invocable: false -description: Skywalker implements a Linear issue by fetching it via MCP then running the /implement spawn loop. DIY tiny/bounded issue edits; spawn build for substantial landings. +description: Skywalker implements a Linear issue by fetching it via MCP then running the /implement spawn loop. DIY tiny/bounded issue edits; spawn builder for substantial landings. argument-hint: " [--reviewer ]" --- @@ -21,26 +21,18 @@ If the scope is unclear, `ask_operator` before proceeding. Do not guess. Read `branchName` from the issue (call `mcp__linear__get_issue` again if needed). -Spawn `task(agent="intern")` with this sequenced `run_shell` list copied into the brief. Intern executes; Skywalker does not run the git. +Load `use_skill("git-worktrees")`. Copy the create-from-origin/ recipe into an intern brief (substitute ``). Spawn `task(agent="intern")`. Intern executes; Skywalker does not run the git. -```bash -git symbolic-ref refs/remotes/origin/HEAD | sed 's@^refs/remotes/origin/@@' -git fetch origin -git worktree add ../worktree/ -b origin/ -``` - -Always base new branches on `origin/` (whatever the repository uses). After creating the worktree, intern `cd`s into it and installs local dependencies from developer documentation. Worktrees do not share `node_modules`. - -If intern fails, stop and `ask_operator`. If the operator rejects the issue before implementation, intern tears down the worktree (Phase 7 commands) rather than leaving it stranded. +If intern fails, stop and `ask_operator`. If the operator rejects the issue before implementation, intern tears down the worktree via the git-worktrees teardown recipe rather than leaving it stranded. ## Phase 3: Plan, attach, mark In Progress -1. Spawn `task(agent="explore")` if the codebase map is not already known. Brief it with the absolute worktree path (it must work there) and the issue: where changes go, existing patterns, related code. +1. Spawn `task(agent="explorer")` if the codebase map is not already known. Brief it with the absolute worktree path (it must work there) and the issue: where changes go, existing patterns, related code. 2. Follow the `/implement` loop's greybeard step (Phase 4) for the approach. Present the plan to the operator and `ask_operator` whether to proceed. Do not start implementation until approved. -3. If the operator rejects the plan and the issue cannot be salvaged, intern tears down the worktree (Phase 7) rather than leaving it stranded. +3. If the operator rejects the plan and the issue cannot be salvaged, intern tears down the worktree via the git-worktrees teardown recipe rather than leaving it stranded. 4. Attach the plan to the Linear issue. **Do not post the plan as a comment** — comments are for discussion, not archives. - Spawn `task(agent="build")` with a mechanical brief to write the approved plan to the worktree's `tmp/plan-.md` (do not commit it). Intern captures byte size with `wc -c`. Primary then: + Spawn `task(agent="builder")` with a mechanical brief to write the approved plan to the worktree's `tmp/plan-.md` (do not commit it). Intern captures byte size with `wc -c`. Primary then: 1. `mcp__linear__prepare_attachment_upload` with `issue`, `filename`, `contentType: "text/markdown"`, and `size`. Response contains `uploadRequest.url`, `uploadRequest.headers`, and `assetUrl`. The signed URL expires in 60 seconds. 2. Intern PUTs the raw file bytes to `uploadRequest.url` via `run_shell`, every header from `uploadRequest.headers` verbatim (exact casing). Do not base64-encode. If PUT returns 403 because the URL expired, prepare a fresh URL and retry once. @@ -55,9 +47,9 @@ If intern fails, stop and `ask_operator`. If the operator rejects the issue befo Do not implement on Skywalker. For each commit-sized unit, run `/implement`: 1. `task(agent="greybeard")` on the approach before any code is written. -2. `task(agent="build")` with a typed brief (`intent`, `success_criteria`, `do_not`, `report_focus`) and the absolute worktree path. Bug fixes start from a failing test. Features ship tests with the change. +2. `task(agent="builder")` with a typed brief (`intent`, `success_criteria`, `do_not`, `report_focus`) and the absolute worktree path. Bug fixes start from a failing test. Features ship tests with the change. 3. `task(agent="intern")` or `task(agent="tester")` for the project build/test gate. -4. `task(agent="critique")` on the diff. Blocking findings → re-dispatch build (cap two re-fix rounds), then re-run the gate and critique. +4. `task(agent="critic")` on the diff. Blocking findings → re-dispatch builder (cap two re-fix rounds), then re-run the gate and critic. Track units with `manage_tasks`. Copy style/philosophy into worker briefs (`use_skill` on the primary before spawning; workers do not mount `use_skill`). @@ -67,7 +59,7 @@ If the issue description contains a task list (`- [ ]` items), tick boxes as bui ## Phase 5: Branch review -After the last unit's critique is clean, spawn `task(agent="critique")` on the **whole** `origin/..HEAD` range in the worktree — not only the last commit. Brief: +After the last unit's critique is clean, spawn `task(agent="critic")` on the **whole** `origin/..HEAD` range in the worktree — not only the last commit. Brief: - Absolute worktree path - Base branch from Phase 2 @@ -130,16 +122,7 @@ Phase 6 ends when the PR is open. Phase 7 runs **after the PR is merged** and ** 2. Re-read the issue with `mcp__linear__get_issue`. Flip checkboxes the merged PR actually completed on `main` via `mcp__linear__save_issue`. Never check a box on intent. 3. `mcp__linear__save_comment` with PR URL, merge SHA, and CI-green confirmation. Short. Present-tense facts. 4. If every outcome checkbox is checked, set state to `Done` with `mcp__linear__save_issue`. Otherwise leave In Progress. -5. Only then intern cleans up: - -```bash -cd -git fetch origin -git worktree remove ../worktree/ -git branch -d -``` - -If the worktree directory was already deleted: `git worktree prune`. +5. Only then intern cleans up: load `use_skill("git-worktrees")` and copy the teardown recipe into an intern brief (substitute `` and ``). ## Linear MCP tool reference @@ -154,7 +137,7 @@ If the worktree directory was already deleted: `git worktree prune`. ## Hard rules -- Tiny / single-file / one-route / clear bounded edits: DIY with write_file/edit_file/delete_file. Substantial issue landings: spawn build (this recipe). -- Spawn with `task(agent="greybeard")`, `task(agent="build")`, `task(agent="intern")` or `task(agent="tester")`, and `task(agent="critique")`. +- Tiny / single-file / one-route / clear bounded edits: DIY with write_file/edit_file/delete_file. Substantial issue landings: spawn builder (this recipe). +- Spawn with `task(agent="greybeard")`, `task(agent="builder")`, `task(agent="intern")` or `task(agent="tester")`, and `task(agent="critic")`. - Clarifying questions use `ask_operator`. - Shell is `run_shell`, not a Bash tool. diff --git a/plugins/corbits-skills/skills/opsh/SKILL.md b/plugins/corbits-skills/skills/opsh/SKILL.md index fd9c0ad5a..4f419c270 100644 --- a/plugins/corbits-skills/skills/opsh/SKILL.md +++ b/plugins/corbits-skills/skills/opsh/SKILL.md @@ -6,9 +6,9 @@ description: Write scripts using opsh and its built-in libraries. Tiny scripts: # opsh Scripting -You are Skywalker. Host is Corbits Code. This is a convention skill. Tiny / single-file scripts: DIY with write_file/edit_file using these rules. Substantial script work: spawn build with this skill's rules copied into the brief (workers do not mount `use_skill`). +You are Skywalker. Host is Corbits Code. This is a convention skill. Tiny / single-file scripts: DIY with write_file/edit_file using these rules. Substantial script work: spawn builder with this skill's rules copied into the brief (workers do not mount `use_skill`). -If the operator wants a substantial script written, spawn `task(agent="build")` with this skill's rules copied into the brief. If the operator wants a review, spawn `task(agent="critique")` (or `task(agent="neckbeard")` for hygiene-only) with the same rules copied in. +If the operator wants a substantial script written, spawn `task(agent="builder")` with this skill's rules copied into the brief. If the operator wants a review, spawn `task(agent="critic")` (or `task(agent="neckbeard")` for hygiene-only) with the same rules copied in. Shell for agent commands is `run_shell` (there is no Bash tool). Bash-the-language in the examples below stays — opsh scripts are bash. diff --git a/plugins/corbits-skills/skills/plan/SKILL.md b/plugins/corbits-skills/skills/plan/SKILL.md index b0822f163..902a3503c 100644 --- a/plugins/corbits-skills/skills/plan/SKILL.md +++ b/plugins/corbits-skills/skills/plan/SKILL.md @@ -1,16 +1,16 @@ --- name: plan -description: Skywalker spawn recipe — plan director authors an agent-proof eng change plan. Does not implement. Does not file tracker issues. +description: Skywalker spawn recipe — counsel director authors an agent-proof eng change plan. Does not implement. Does not file tracker issues. --- # Plan You are Skywalker. This skill is a spawn recipe. You do not write the plan yourself. -Spawn `task(agent="plan")` with the operator args as the brief. Prefer a typed spawn: `intent="plan"`, `success_criteria`, `do_not`, `report_focus`. +Spawn `task(agent="counsel")` with the operator args as the brief. Prefer a typed spawn: `intent="plan"`, `success_criteria`, `do_not`, `report_focus`. -The plan director authors files, acceptance criteria, non-goals, risks, and ordered steps. It does not ship code. Greybeard is the architecture gate, not this slash. +The counsel director authors files, acceptance criteria, non-goals, risks, and ordered steps. It does not ship code. Greybeard is the architecture gate, not this slash. This is not `/create-issue`. Do not file Linear or GitHub issues. If the operator wants tickets, they use `/create-issue` after the plan. -Use `ask_operator` if the change target is too fuzzy to brief plan. +Use `ask_operator` if the change target is too fuzzy to brief counsel. diff --git a/plugins/corbits-skills/skills/pull-request-review/SKILL.md b/plugins/corbits-skills/skills/pull-request-review/SKILL.md index a9d9d81b4..f1cdc6cd3 100644 --- a/plugins/corbits-skills/skills/pull-request-review/SKILL.md +++ b/plugins/corbits-skills/skills/pull-request-review/SKILL.md @@ -1,6 +1,6 @@ --- name: pull-request-review -description: Review a pull request by branch name or URL. Intern checks out a worktree if needed; critique (or neckbeard) reviews. Skywalker does not implement fixes. +description: Review a pull request by branch name or URL. Intern checks out a worktree if needed; critic (or neckbeard) reviews. Skywalker does not implement fixes. --- # Pull Request Review @@ -67,7 +67,7 @@ If any of these commands fail, intern stops and reports. Do not retry workaround ### 3. Review -- **Default:** `task(agent="critique")` with the PR scope (branch, base, worktree path, PR URL/number). +- **Default:** `task(agent="critic")` with the PR scope (branch, base, worktree path, PR URL/number). - **Hygiene-only** (operator said nits / naming / lint / pedantry): `task(agent="neckbeard")`. Brief the reviewer: @@ -83,7 +83,7 @@ Prefer a typed brief: `intent="review"`, `success_criteria`, `do_not`, `report_f ### 4. After the report -Synthesize critique/neckbeard Summary / Findings / Blockers / Paths for the operator. Do not land fixes. +Synthesize critic/neckbeard Summary / Findings / Blockers / Paths for the operator. Do not land fixes. If a GitHub review must be posted, intern runs `gh pr review` as the operator's `gh` identity — never as a Claude (or other vendor) bot. Primary owns `--approve` / `--request-changes` only when the operator asked to post; secondary lenses use `--comment` only. @@ -101,7 +101,7 @@ Or leave it and tell the operator it remains for further investigation. ## Hard rules -- This recipe reviews; it does not land product patches. If the operator then asks for a tiny/bounded fix, DIY with write_file/edit_file/delete_file; spawn build for substantial fixes. +- This recipe reviews; it does not land product patches. If the operator then asks for a tiny/bounded fix, DIY with write_file/edit_file/delete_file; spawn builder for substantial fixes. - Skywalker MUST NOT run the worktree git; intern does, via `run_shell`. - Do not implement fixes as part of the review. - Do not impersonate GitHub-Claude review comments. diff --git a/plugins/corbits-skills/skills/refactor/SKILL.md b/plugins/corbits-skills/skills/refactor/SKILL.md index 12b7aec30..c46878135 100644 --- a/plugins/corbits-skills/skills/refactor/SKILL.md +++ b/plugins/corbits-skills/skills/refactor/SKILL.md @@ -15,7 +15,7 @@ You are Skywalker. This skill is a spawn recipe. You do not write a design docum - Is there a specific concern or area to focus on? - What prompted the desire to refactor? - Are there known pain points? -3. Spawn `task(agent="explore")` to map `$ARGUMENTS`. Brief it to cover: +3. Spawn `task(agent="explorer")` to map `$ARGUMENTS`. Brief it to cover: - What the code does (purpose and behavior) - Key components and their responsibilities - How data flows through the system @@ -23,19 +23,19 @@ You are Skywalker. This skill is a spawn recipe. You do not write a design docum - Patterns and conventions in use - Areas of complexity or inconsistency (factual, not prescriptive) 4. From the explore report, `ask_operator` for collaborative choices: priorities, which observations to act on, accept / reject / modify proposals. Iterate until alignment. Do not invent a plan the operator did not choose. -5. Spawn `task(agent="plan")` for the improvement plan. Include the operator's choices, the explore findings, and `$ARGUMENTS`. The plan should cover: +5. Spawn `task(agent="counsel")` for the improvement plan. Include the operator's choices, the explore findings, and `$ARGUMENTS`. The plan should cover: - Specific changes to make - Rationale for each change (grounded in philosophy: pragmatic over idealistic, simple is usually harder than easy, do no harm, respect existing decisions) - Suggested order of operations - Constraints or risks - - Enough detail that a build worker could execute later + - Enough detail that a builder worker could execute later - For structural transformations (renames, signature changes, API migrations), note that execution should load the `ast-grep` skill — bulk AST rewrites, not manual read-edit-write cycles -Do not write the plan to disk yourself. Plan's report is the artifact. A later `/implement` or `use_skill("dispatch")` ships it. +Do not write the plan to disk yourself. Counsel's report is the artifact. A later `/implement` or `use_skill("dispatch")` ships it. ## Hard rules -- Do not write the plan to disk or author design documents on this session — plan's report is the artifact. A later `/implement` or `use_skill("dispatch")` ships substantial work; DIY remains for tiny/bounded edits outside this recipe. +- Do not write the plan to disk or author design documents on this session — counsel's report is the artifact. A later `/implement` or `use_skill("dispatch")` ships substantial work; DIY remains for tiny/bounded edits outside this recipe. - Do not skip explore "because you already know the directory." - Do not skip `ask_operator` when the operator has not chosen among alternatives. -- Spawn with `task(agent="explore")` then `task(agent="plan")`. +- Spawn with `task(agent="explorer")` then `task(agent="counsel")`. diff --git a/plugins/corbits-skills/skills/review/SKILL.md b/plugins/corbits-skills/skills/review/SKILL.md index 9e37cf9e2..c39cd1d81 100644 --- a/plugins/corbits-skills/skills/review/SKILL.md +++ b/plugins/corbits-skills/skills/review/SKILL.md @@ -1,6 +1,6 @@ --- name: review -description: Review a branch, PR, or path scope. Skywalker spawns critique (neckbeard for hygiene, greybeard for architecture); does not implement fixes. +description: Review a branch, PR, or path scope. Skywalker spawns critic (neckbeard for hygiene, greybeard for architecture); does not implement fixes. argument-hint: "[paths | PR | diff | hygiene | architecture]" --- @@ -12,11 +12,11 @@ Spawn a director. Pass the operator's scope — paths, PR, branch, or diff — a ## Routing -- **Default** (correctness, completeness, brief adherence, defects with evidence): `task(agent="critique")` +- **Default** (correctness, completeness, brief adherence, defects with evidence): `task(agent="critic")` - **Hygiene-only** (nits, naming, lint, pedantry with receipts): `task(agent="neckbeard")` - **Architecture-only** (structure, boundaries, approach): `task(agent="greybeard")` -If the operator did not say hygiene-only or architecture-only, spawn critique. Do not spawn all three unless they asked for a wider review. +If the operator did not say hygiene-only or architecture-only, spawn critic. Do not spawn all three unless they asked for a wider review. Prefer a typed brief: `intent="review"`, `success_criteria`, `do_not`, `report_focus`, and `agent`. diff --git a/plugins/corbits-skills/skills/scribe/SKILL.md b/plugins/corbits-skills/skills/scribe/SKILL.md index 53df56d22..0f2fb1786 100644 --- a/plugins/corbits-skills/skills/scribe/SKILL.md +++ b/plugins/corbits-skills/skills/scribe/SKILL.md @@ -11,4 +11,4 @@ Spawn `task(agent="shakespeare")` with the operator args / pasted material as th Use `ask_operator` if the doc target (P vs A vs I) is ambiguous. -Do not edit those docs yourself except a one-line fix. DESIGN.md is brand-reviewer, not this skill. +Do not edit those docs yourself except a one-line fix. DESIGN.md is rand, not this skill. diff --git a/scripts/eval-capability.test.ts b/scripts/eval-capability.test.ts index fc071d64f..bec3f62ce 100644 --- a/scripts/eval-capability.test.ts +++ b/scripts/eval-capability.test.ts @@ -164,9 +164,9 @@ describe("parseArgs", () => { ); }); - test("--director build is parsed", () => { - const opts = parseArgs(["--provider", "foo", "--model", "bar", "--director", "build"]); - expect(opts.director).toBe("build"); + test("--director builder is parsed", () => { + const opts = parseArgs(["--provider", "foo", "--model", "bar", "--director", "builder"]); + expect(opts.director).toBe("builder"); }); test("omitted --director stays undefined", () => { @@ -325,8 +325,8 @@ describe("buildEvalDiagnostics", () => { expect(diagnostics.reasoningEffort).toBe("high"); }); - test("--director build reports the director's own advertised allowlist", async () => { - const diagnostics = await buildEvalDiagnostics(sampleConfig({ director: "build" })); + test("--director builder reports the director's own advertised allowlist", async () => { + const diagnostics = await buildEvalDiagnostics(sampleConfig({ director: "builder" })); expect(diagnostics.advertisedTools).not.toEqual( (await buildEvalDiagnostics(sampleConfig({}))).advertisedTools, ); diff --git a/src/agent/codex-tool-proxies.test.ts b/src/agent/codex-tool-proxies.test.ts index 002c63222..525b9393e 100644 --- a/src/agent/codex-tool-proxies.test.ts +++ b/src/agent/codex-tool-proxies.test.ts @@ -597,8 +597,8 @@ describe("update_plan proxy", () => { }); describe("allowDeleteFromCapabilities", () => { - test("docs allowlist (no delete_file) → false; build → true", () => { - expect(allowDeleteFromCapabilities({ mode: "allow", tools: DOCS_TOOLS })).toBe(false); + test("docs allowlist (includes delete_file) → true; build → true", () => { + expect(allowDeleteFromCapabilities({ mode: "allow", tools: DOCS_TOOLS })).toBe(true); expect(allowDeleteFromCapabilities({ mode: "allow", tools: BUILD_TOOLS })).toBe(true); expect(allowDeleteFromCapabilities(undefined)).toBe(true); expect(allowDeleteFromCapabilities({ mode: "exclude", tools: ["run_shell"] })).toBe(true); diff --git a/src/agent/codex-tool-proxies.ts b/src/agent/codex-tool-proxies.ts index 14716d2a4..56451072f 100644 --- a/src/agent/codex-tool-proxies.ts +++ b/src/agent/codex-tool-proxies.ts @@ -55,8 +55,8 @@ export interface CreateCodexToolProxiesOpts { runManageTasks: CodexRunManageTasks; /** * When false, Delete File and Update+Move refuse without calling `delete_file`. - * Defaults to true (implement / unconstrained). Docs leaves pass false because - * DOCS_TOOLS includes apply_patch but not delete_file. + * Defaults to true (implement / unconstrained). Pass false when the + * director allowlist omits delete_file (docs leaves mount it today). */ allowDelete?: boolean; /** diff --git a/src/agent/default-agents.ts b/src/agent/default-agents.ts index c688568d4..1357ce532 100644 --- a/src/agent/default-agents.ts +++ b/src/agent/default-agents.ts @@ -2,8 +2,8 @@ import { directorProfiles } from "./directors/registry.js"; import type { AgentPlugin } from "./profile-types.js"; // Spawnable profiles = closed director fleet minus primary skywalker. -// Repositories can override any id via .agents/agents/ or agent-kind -// plugins (higher precedence). +// Closed DIRECTOR_IDS are reserved: plugin/local profiles that collide are +// skipped at load (CL-7015) — no override or alias of the fleet. export const defaultAgentsPlugin: AgentPlugin = { agents: directorProfiles(), }; diff --git a/src/agent/directors/brand-reviewer/index.ts b/src/agent/directors/brand-reviewer/index.ts deleted file mode 100644 index 728fcb26d..000000000 --- a/src/agent/directors/brand-reviewer/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { brandReviewerPackage } from "./package.js"; diff --git a/src/agent/directors/brand-reviewer/package.test.ts b/src/agent/directors/brand-reviewer/package.test.ts deleted file mode 100644 index a1d1bdc4e..000000000 --- a/src/agent/directors/brand-reviewer/package.test.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { brandReviewerPackage } from "./package.js"; - -describe("brandReviewerPackage", () => { - test("id matches directory", () => { - expect(brandReviewerPackage.id).toBe("brand-reviewer"); - }); - - test("systemPrompt is real, not a placeholder", () => { - expect(brandReviewerPackage.systemPrompt.length).toBeGreaterThan(0); - expect(brandReviewerPackage.systemPrompt.startsWith("Placeholder")).toBe(false); - }); - - test("systemPrompt states PRIMARY INTENT", () => { - expect(brandReviewerPackage.systemPrompt).toMatch(/PRIMARY INTENT/i); - expect(brandReviewerPackage.systemPrompt).toContain("name build"); - expect(brandReviewerPackage.systemPrompt).not.toContain("name implement"); - }); - - test("spawn.maySpawn is false", () => { - expect(brandReviewerPackage.spawn.maySpawn).toBe(false); - }); - - test("tools.allow includes write tools", () => { - const allow = brandReviewerPackage.tools?.allow ?? []; - expect(allow).toContain("write_file"); - expect(allow).toContain("edit_file"); - }); - - test("systemPrompt mentions DESIGN.md", () => { - expect(brandReviewerPackage.systemPrompt).toMatch(/DESIGN\.md/); - expect(brandReviewerPackage.systemPrompt).not.toMatch(/authz/i); - }); - - test("modelRole is docs", () => { - expect(brandReviewerPackage.modelRole).toBe("docs"); - }); - - test("primaryIntent and outOfLane match brand-reviewer lane", () => { - expect(brandReviewerPackage.primaryIntent).toBe("Own DESIGN.md create/use + brand gate"); - expect(brandReviewerPackage.outOfLane).toContain("arbitrary product code outside DESIGN.md"); - }); -}); diff --git a/src/agent/directors/bruckheimer/package.test.ts b/src/agent/directors/bruckheimer/package.test.ts index 7e9d6f4e1..316732085 100644 --- a/src/agent/directors/bruckheimer/package.test.ts +++ b/src/agent/directors/bruckheimer/package.test.ts @@ -14,7 +14,7 @@ describe("bruckheimerPackage", () => { test("systemPrompt states PRIMARY INTENT", () => { expect(bruckheimerPackage.systemPrompt).toMatch(/PRIMARY INTENT/i); expect(bruckheimerPackage.systemPrompt).toContain( - "Route those via Blockers to build, greybeard, critique, or skywalker", + "Route those via Blockers to builder, greybeard, critic, or skywalker", ); }); @@ -26,6 +26,7 @@ describe("bruckheimerPackage", () => { const allow = bruckheimerPackage.tools?.allow ?? []; expect(allow).toContain("write_file"); expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); }); test("modelRole is docs", () => { diff --git a/src/agent/directors/bruckheimer/package.ts b/src/agent/directors/bruckheimer/package.ts index 57cae8e3a..f75e5f852 100644 --- a/src/agent/directors/bruckheimer/package.ts +++ b/src/agent/directors/bruckheimer/package.ts @@ -27,7 +27,7 @@ Write tools are mounted with no path lock. Stay on the product-discovery lane. Y Read the product as a person using it: can a new user get through the first ninety seconds? Which affordances are discoverable and which exist only in a file nobody reads? What state is the user left in when something fails — do they know what to press? Name specific strings and surfaces that should change. -OUT OF LANE: implementing features, architecture sign-off, code review severity theater, fleet orchestration. Route those via Blockers to build, greybeard, critique, or skywalker. +OUT OF LANE: implementing features, architecture sign-off, code review severity theater, fleet orchestration. Route those via Blockers to builder, greybeard, critic, or skywalker. Findings: product shape and discovery, not implementation notes.`, }; diff --git a/src/agent/directors/build/index.ts b/src/agent/directors/build/index.ts deleted file mode 100644 index 5d474bbd7..000000000 --- a/src/agent/directors/build/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { buildDirectorPackage } from "./package.js"; diff --git a/src/agent/directors/builder/index.ts b/src/agent/directors/builder/index.ts new file mode 100644 index 000000000..860538d3f --- /dev/null +++ b/src/agent/directors/builder/index.ts @@ -0,0 +1 @@ +export { builderPackage } from "./package.js"; diff --git a/src/agent/directors/build/package.test.ts b/src/agent/directors/builder/package.test.ts similarity index 64% rename from src/agent/directors/build/package.test.ts rename to src/agent/directors/builder/package.test.ts index 4b4d4260b..1ecd5a475 100644 --- a/src/agent/directors/build/package.test.ts +++ b/src/agent/directors/builder/package.test.ts @@ -1,26 +1,26 @@ import { describe, expect, test } from "bun:test"; -import { buildDirectorPackage } from "./package.js"; +import { builderPackage } from "./package.js"; -describe("buildDirectorPackage", () => { +describe("builderPackage", () => { test("id matches directory / registry id", () => { - expect(buildDirectorPackage.id).toBe("build"); + expect(builderPackage.id).toBe("builder"); }); test("systemPrompt is non-empty and not a Placeholder", () => { - expect(buildDirectorPackage.systemPrompt.length).toBeGreaterThan(0); - expect(buildDirectorPackage.systemPrompt.startsWith("Placeholder")).toBe(false); + expect(builderPackage.systemPrompt.length).toBeGreaterThan(0); + expect(builderPackage.systemPrompt.startsWith("Placeholder")).toBe(false); }); test("systemPrompt mentions PRIMARY INTENT", () => { - expect(buildDirectorPackage.systemPrompt).toContain("PRIMARY INTENT"); + expect(builderPackage.systemPrompt).toContain("PRIMARY INTENT"); }); test("spawn.maySpawn is false (leaf)", () => { - expect(buildDirectorPackage.spawn.maySpawn).toBe(false); + expect(builderPackage.spawn.maySpawn).toBe(false); }); test("tools.allow includes product write tools", () => { - const allow = buildDirectorPackage.tools?.allow ?? []; + const allow = builderPackage.tools?.allow ?? []; expect(allow).toContain("write_file"); expect(allow).toContain("edit_file"); expect(allow).toContain("delete_file"); @@ -28,36 +28,36 @@ describe("buildDirectorPackage", () => { }); test("modelRole is implement", () => { - expect(buildDirectorPackage.modelRole).toBe("implement"); + expect(builderPackage.modelRole).toBe("implement"); }); test("optionalSkills order is style, philosophy, typescript", () => { - expect(buildDirectorPackage.optionalSkills).toEqual(["style", "philosophy", "typescript"]); + expect(builderPackage.optionalSkills).toEqual(["style", "philosophy", "typescript"]); }); test("systemPrompt has DONE GATE for success_criteria", () => { - const prompt = buildDirectorPackage.systemPrompt; + const prompt = builderPackage.systemPrompt; expect(prompt).toContain("DONE GATE"); expect(prompt).toContain("success_criteria"); expect(prompt).toMatch(/[Ss]top when/); }); test("systemPrompt has VERIFY language", () => { - const prompt = buildDirectorPackage.systemPrompt; + const prompt = builderPackage.systemPrompt; expect(prompt).toContain("VERIFY"); expect(prompt).toMatch(/typecheck|tests/); expect(prompt).toContain("Blockers"); }); test("systemPrompt has REPORT MAP for criteria and Paths", () => { - const prompt = buildDirectorPackage.systemPrompt; + const prompt = builderPackage.systemPrompt; expect(prompt).toContain("REPORT MAP"); expect(prompt).toMatch(/success_criteria.*pass|fail|blocked/s); expect(prompt).toMatch(/Paths must list files touched/); }); test("systemPrompt has API CONTRACT for sync/async preservation", () => { - const prompt = buildDirectorPackage.systemPrompt; + const prompt = builderPackage.systemPrompt; expect(prompt).toContain("API CONTRACT"); expect(prompt).toMatch(/sync/i); expect(prompt).toMatch(/Promise|async/); diff --git a/src/agent/directors/build/package.ts b/src/agent/directors/builder/package.ts similarity index 93% rename from src/agent/directors/build/package.ts rename to src/agent/directors/builder/package.ts index 2a2f3caf6..c9e9addca 100644 --- a/src/agent/directors/build/package.ts +++ b/src/agent/directors/builder/package.ts @@ -1,8 +1,8 @@ import type { DirectorPackage } from "../types.js"; import { BUILD_TOOLS } from "../tool-sets.js"; -export const buildDirectorPackage: DirectorPackage = { - id: "build", +export const builderPackage: DirectorPackage = { + id: "builder", primaryIntent: "Ship product code with tests to satisfy the brief", outOfLane: [ "architecture gates", @@ -17,7 +17,7 @@ export const buildDirectorPackage: DirectorPackage = { spawn: { maySpawn: false }, tier: "leaf", modelRole: "implement", - systemPrompt: `You are BuildDirector, a specialist in Corbits Code. + systemPrompt: `You are BuilderDirector, a specialist in Corbits Code. PRIMARY INTENT: implement the brief in product code. Edit, verify, report. You are not a reviewer, not an orchestrator, not a doc-only planner. diff --git a/src/agent/directors/counsel/index.ts b/src/agent/directors/counsel/index.ts new file mode 100644 index 000000000..c72bc57f9 --- /dev/null +++ b/src/agent/directors/counsel/index.ts @@ -0,0 +1 @@ +export { counselPackage } from "./package.js"; diff --git a/src/agent/directors/counsel/package.test.ts b/src/agent/directors/counsel/package.test.ts new file mode 100644 index 000000000..e6469bc54 --- /dev/null +++ b/src/agent/directors/counsel/package.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, test } from "bun:test"; +import { counselPackage } from "./package.js"; + +describe("counselPackage", () => { + test("id matches directory", () => { + expect(counselPackage.id).toBe("counsel"); + }); + + test("systemPrompt is real (not Placeholder)", () => { + expect(counselPackage.systemPrompt.length).toBeGreaterThan(0); + expect(counselPackage.systemPrompt.startsWith("Placeholder")).toBe(false); + }); + + test("systemPrompt states PRIMARY INTENT", () => { + expect(counselPackage.systemPrompt).toMatch(/PRIMARY INTENT/i); + }); + + test("spawn.maySpawn is false", () => { + expect(counselPackage.spawn.maySpawn).toBe(false); + }); + + test("tools.allow is review surface with product writes", () => { + const allow = counselPackage.tools?.allow ?? []; + expect(allow).toContain("read_file"); + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); + }); + + test("modelRole is plan", () => { + expect(counselPackage.modelRole).toBe("plan"); + }); + + test("optionalSkills order", () => { + expect(counselPackage.optionalSkills).toEqual(["style", "philosophy", "interview"]); + }); + + test("primaryIntent and outOfLane match plan lane", () => { + expect(counselPackage.primaryIntent).toBe("Author eng change plans; do not implement"); + expect(counselPackage.outOfLane).toContain("shipping code"); + expect(counselPackage.outOfLane).toContain("architecture gate sign-off as Greybeard"); + expect(counselPackage.outOfLane).toContain("running the fleet"); + }); +}); diff --git a/src/agent/directors/plan/package.ts b/src/agent/directors/counsel/package.ts similarity index 87% rename from src/agent/directors/plan/package.ts rename to src/agent/directors/counsel/package.ts index 9345c65ec..92a73be94 100644 --- a/src/agent/directors/plan/package.ts +++ b/src/agent/directors/counsel/package.ts @@ -1,8 +1,8 @@ import type { DirectorPackage } from "../types.js"; import { REVIEW_TOOLS } from "../tool-sets.js"; -export const planPackage: DirectorPackage = { - id: "plan", +export const counselPackage: DirectorPackage = { + id: "counsel", primaryIntent: "Author eng change plans; do not implement", outOfLane: ["shipping code", "architecture gate sign-off as Greybeard", "running the fleet"], description: "Planning leaf — eng plans only; Greybeard reviews", @@ -11,7 +11,7 @@ export const planPackage: DirectorPackage = { spawn: { maySpawn: false }, tier: "leaf", modelRole: "plan", - systemPrompt: `You are PlanDirector, a specialist in Corbits Code. + systemPrompt: `You are CounselDirector, a specialist in Corbits Code. PRIMARY INTENT: author concrete engineering change plans. Do not implement product code. Do not act as architecture gate (that is Greybeard). diff --git a/src/agent/directors/critic/index.ts b/src/agent/directors/critic/index.ts new file mode 100644 index 000000000..08423a8ab --- /dev/null +++ b/src/agent/directors/critic/index.ts @@ -0,0 +1 @@ +export { criticPackage } from "./package.js"; diff --git a/src/agent/directors/critic/package.test.ts b/src/agent/directors/critic/package.test.ts new file mode 100644 index 000000000..84aa04d20 --- /dev/null +++ b/src/agent/directors/critic/package.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, test } from "bun:test"; +import { criticPackage } from "./package.js"; + +describe("criticPackage", () => { + test("id matches directory", () => { + expect(criticPackage.id).toBe("critic"); + }); + + test("systemPrompt is real, not a placeholder", () => { + expect(criticPackage.systemPrompt.length).toBeGreaterThan(0); + expect(criticPackage.systemPrompt.startsWith("Placeholder")).toBe(false); + }); + + test("systemPrompt states PRIMARY INTENT", () => { + expect(criticPackage.systemPrompt).toMatch(/PRIMARY INTENT/i); + }); + + test("systemPrompt is evidence-based and never-fix", () => { + expect(criticPackage.systemPrompt).toMatch(/evidence-based/i); + expect(criticPackage.systemPrompt).toMatch(/never fix/i); + expect(criticPackage.systemPrompt).toMatch(/permanent tests/i); + expect(criticPackage.systemPrompt).toContain("testsmith/builder"); + expect(criticPackage.systemPrompt).toContain("route to builder"); + }); + + test("systemPrompt is correctness-only / anti-over-engineering", () => { + expect(criticPackage.systemPrompt).toMatch(/correctness-only/i); + expect(criticPackage.systemPrompt).toMatch(/anti-over-engineering/i); + expect(criticPackage.systemPrompt).toMatch( + /correctness or the stated requirements\/success_criteria/i, + ); + expect(criticPackage.systemPrompt).toMatch(/style nits/i); + expect(criticPackage.systemPrompt).toMatch(/file-for-later/i); + expect(criticPackage.systemPrompt).toMatch(/Do not drive over-engineering/i); + expect(criticPackage.systemPrompt).toMatch(/impossible cases/i); + }); + + test("systemPrompt flags API contract / sync→async as blocking", () => { + expect(criticPackage.systemPrompt).toMatch(/API contract check/i); + expect(criticPackage.systemPrompt).toMatch(/blocking when brief specifies signatures/i); + expect(criticPackage.systemPrompt).toMatch(/public exports/i); + expect(criticPackage.systemPrompt).toMatch(/Sync\s*→\s*async/i); + expect(criticPackage.systemPrompt).toMatch( + /returning Promise when callers expect a plain value/i, + ); + expect(criticPackage.systemPrompt).toMatch(/blocking correctness defect/i); + expect(criticPackage.systemPrompt).toMatch( + /parameter order\/optionality\/return-type drift/i, + ); + expect(criticPackage.systemPrompt).toMatch(/Rank these as blocking, not style nits/i); + }); + + test("spawn.maySpawn is false", () => { + expect(criticPackage.spawn.maySpawn).toBe(false); + }); + + test("tools.allow is review surface with product writes", () => { + const allow = criticPackage.tools?.allow ?? []; + expect(allow).toContain("read_file"); + expect(allow).toContain("read_file"); + expect(allow).not.toContain("use_skill"); + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); + }); + + test("modelRole is review", () => { + expect(criticPackage.modelRole).toBe("review"); + }); + + test("optionalSkills order is style, philosophy", () => { + expect(criticPackage.optionalSkills).toEqual(["style", "philosophy"]); + }); + + test("primaryIntent and outOfLane match critique lane", () => { + expect(criticPackage.primaryIntent).toBe( + "Evidence-based code review; never fix product code", + ); + expect(criticPackage.outOfLane).toContain("implementing fixes"); + expect(criticPackage.outOfLane).toContain("architecture portfolio without code evidence"); + expect(criticPackage.outOfLane).toContain("visual brand"); + expect(criticPackage.outOfLane).toContain("DESIGN.md"); + expect(criticPackage.outOfLane).toContain("pedantic fun without evidence"); + }); +}); diff --git a/src/agent/directors/critique/package.ts b/src/agent/directors/critic/package.ts similarity index 85% rename from src/agent/directors/critique/package.ts rename to src/agent/directors/critic/package.ts index 1a669fc32..439e37bdb 100644 --- a/src/agent/directors/critique/package.ts +++ b/src/agent/directors/critic/package.ts @@ -2,11 +2,11 @@ import type { DirectorPackage } from "../types.js"; import { REVIEW_TOOLS } from "../tool-sets.js"; /** - * Critique leaf (CL-5819). + * Critic leaf (CL-5819 / CL-7015 rename from critique). * Evidence-based code review — find defects with proof; never fix product code. */ -export const critiquePackage: DirectorPackage = { - id: "critique", +export const criticPackage: DirectorPackage = { + id: "critic", primaryIntent: "Evidence-based code review; never fix product code", outOfLane: [ "implementing fixes", @@ -21,7 +21,7 @@ export const critiquePackage: DirectorPackage = { spawn: { maySpawn: false }, tier: "leaf", modelRole: "review", - systemPrompt: `You are CritiqueDirector, a specialist in Corbits Code. + systemPrompt: `You are CriticDirector, a specialist in Corbits Code. PRIMARY INTENT: evidence-based code review. Find defects; never fix product code. Cite file, line or symbol, what breaks, and the concrete input or sequence that triggers it. @@ -46,11 +46,11 @@ API contract check (blocking when brief specifies signatures): - Prefer reading tests/callers; a tiny sync call via run_shell that would hang on a Promise is evidence. - Rank these as blocking, not style nits. -Write tools are not mounted. Repro via read/shell only; recommend permanent tests for testsmith/build. +Write tools are mounted with no path lock — do not use them. Repro via read/shell only; recommend permanent tests for testsmith/builder. OUT OF LANE → refuse or reclassify under Blockers: -- implementing fixes (route to build) +- implementing fixes (route to builder) - architecture portfolio without code evidence (route to greybeard) -- visual brand / DESIGN.md (route to brand-reviewer / draper) +- visual brand / DESIGN.md (route to rand / draper) - pedantic fun without evidence (route to neckbeard only if hygiene is the brief)`, }; diff --git a/src/agent/directors/critique/index.ts b/src/agent/directors/critique/index.ts deleted file mode 100644 index bf08c0413..000000000 --- a/src/agent/directors/critique/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { critiquePackage } from "./package.js"; diff --git a/src/agent/directors/critique/package.test.ts b/src/agent/directors/critique/package.test.ts deleted file mode 100644 index 160944919..000000000 --- a/src/agent/directors/critique/package.test.ts +++ /dev/null @@ -1,85 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { critiquePackage } from "./package.js"; - -describe("critiquePackage", () => { - test("id matches directory", () => { - expect(critiquePackage.id).toBe("critique"); - }); - - test("systemPrompt is real, not a placeholder", () => { - expect(critiquePackage.systemPrompt.length).toBeGreaterThan(0); - expect(critiquePackage.systemPrompt.startsWith("Placeholder")).toBe(false); - }); - - test("systemPrompt states PRIMARY INTENT", () => { - expect(critiquePackage.systemPrompt).toMatch(/PRIMARY INTENT/i); - }); - - test("systemPrompt is evidence-based and never-fix", () => { - expect(critiquePackage.systemPrompt).toMatch(/evidence-based/i); - expect(critiquePackage.systemPrompt).toMatch(/never fix/i); - expect(critiquePackage.systemPrompt).toMatch(/permanent tests/i); - expect(critiquePackage.systemPrompt).toContain("testsmith/build"); - expect(critiquePackage.systemPrompt).toContain("route to build"); - }); - - test("systemPrompt is correctness-only / anti-over-engineering", () => { - expect(critiquePackage.systemPrompt).toMatch(/correctness-only/i); - expect(critiquePackage.systemPrompt).toMatch(/anti-over-engineering/i); - expect(critiquePackage.systemPrompt).toMatch( - /correctness or the stated requirements\/success_criteria/i, - ); - expect(critiquePackage.systemPrompt).toMatch(/style nits/i); - expect(critiquePackage.systemPrompt).toMatch(/file-for-later/i); - expect(critiquePackage.systemPrompt).toMatch(/Do not drive over-engineering/i); - expect(critiquePackage.systemPrompt).toMatch(/impossible cases/i); - }); - - test("systemPrompt flags API contract / sync→async as blocking", () => { - expect(critiquePackage.systemPrompt).toMatch(/API contract check/i); - expect(critiquePackage.systemPrompt).toMatch(/blocking when brief specifies signatures/i); - expect(critiquePackage.systemPrompt).toMatch(/public exports/i); - expect(critiquePackage.systemPrompt).toMatch(/Sync\s*→\s*async/i); - expect(critiquePackage.systemPrompt).toMatch( - /returning Promise when callers expect a plain value/i, - ); - expect(critiquePackage.systemPrompt).toMatch(/blocking correctness defect/i); - expect(critiquePackage.systemPrompt).toMatch( - /parameter order\/optionality\/return-type drift/i, - ); - expect(critiquePackage.systemPrompt).toMatch(/Rank these as blocking, not style nits/i); - }); - - test("spawn.maySpawn is false", () => { - expect(critiquePackage.spawn.maySpawn).toBe(false); - }); - - test("tools.allow is review surface without product writes", () => { - const allow = critiquePackage.tools?.allow ?? []; - expect(allow).toContain("read_file"); - expect(allow).toContain("read_file"); - expect(allow).not.toContain("use_skill"); - expect(allow).not.toContain("write_file"); - expect(allow).not.toContain("edit_file"); - expect(allow).not.toContain("delete_file"); - }); - - test("modelRole is review", () => { - expect(critiquePackage.modelRole).toBe("review"); - }); - - test("optionalSkills order is style, philosophy", () => { - expect(critiquePackage.optionalSkills).toEqual(["style", "philosophy"]); - }); - - test("primaryIntent and outOfLane match critique lane", () => { - expect(critiquePackage.primaryIntent).toBe( - "Evidence-based code review; never fix product code", - ); - expect(critiquePackage.outOfLane).toContain("implementing fixes"); - expect(critiquePackage.outOfLane).toContain("architecture portfolio without code evidence"); - expect(critiquePackage.outOfLane).toContain("visual brand"); - expect(critiquePackage.outOfLane).toContain("DESIGN.md"); - expect(critiquePackage.outOfLane).toContain("pedantic fun without evidence"); - }); -}); diff --git a/src/agent/directors/draper/package.test.ts b/src/agent/directors/draper/package.test.ts index 9f89e23cb..58d3ac68d 100644 --- a/src/agent/directors/draper/package.test.ts +++ b/src/agent/directors/draper/package.test.ts @@ -13,19 +13,19 @@ describe("draperPackage", () => { test("systemPrompt states PRIMARY INTENT", () => { expect(draperPackage.systemPrompt).toMatch(/PRIMARY INTENT/i); - expect(draperPackage.systemPrompt).toContain("build (fixes)"); + expect(draperPackage.systemPrompt).toContain("builder (fixes)"); }); test("spawn.maySpawn is false", () => { expect(draperPackage.spawn.maySpawn).toBe(false); }); - test("tools.allow is review surface without product writes", () => { + test("tools.allow is review surface with product writes", () => { const allow = draperPackage.tools?.allow ?? []; expect(allow).toContain("read_file"); - expect(allow).not.toContain("write_file"); - expect(allow).not.toContain("edit_file"); - expect(allow).not.toContain("delete_file"); + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); }); test("modelRole is review", () => { diff --git a/src/agent/directors/draper/package.ts b/src/agent/directors/draper/package.ts index 6c092f665..8724fc4e4 100644 --- a/src/agent/directors/draper/package.ts +++ b/src/agent/directors/draper/package.ts @@ -45,7 +45,7 @@ Skip marketing voice/tone/messaging lenses unless the brief explicitly includes 4. Confidence: VERIFIED / HIGH / MEDIUM only. Discard LOW. 5. Report — do not redesign, rewrite, or patch code. -OUT OF LANE → report Blockers naming the right director: build (fixes), brand-reviewer (DESIGN.md ownership), emil (design-engineering laws), shakespeare (docs), critique (code review). +OUT OF LANE → report Blockers naming the right director: builder (fixes), rand (DESIGN.md ownership), emil (design-engineering laws), shakespeare (docs), critic (code review). # Report diff --git a/src/agent/directors/emil/package.test.ts b/src/agent/directors/emil/package.test.ts index 6fef61384..be42c7792 100644 --- a/src/agent/directors/emil/package.test.ts +++ b/src/agent/directors/emil/package.test.ts @@ -13,19 +13,19 @@ describe("emilPackage", () => { test("systemPrompt states PRIMARY INTENT", () => { expect(emilPackage.systemPrompt).toMatch(/PRIMARY INTENT/i); - expect(emilPackage.systemPrompt).toContain("build (fixes)"); + expect(emilPackage.systemPrompt).toContain("builder (fixes)"); }); test("spawn.maySpawn is false", () => { expect(emilPackage.spawn.maySpawn).toBe(false); }); - test("tools.allow is review surface without product writes", () => { + test("tools.allow is review surface with product writes", () => { const allow = emilPackage.tools?.allow ?? []; expect(allow).toContain("read_file"); - expect(allow).not.toContain("write_file"); - expect(allow).not.toContain("edit_file"); - expect(allow).not.toContain("delete_file"); + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); }); test("modelRole is review", () => { diff --git a/src/agent/directors/emil/package.ts b/src/agent/directors/emil/package.ts index 2ab216fa6..9d72ca064 100644 --- a/src/agent/directors/emil/package.ts +++ b/src/agent/directors/emil/package.ts @@ -59,7 +59,7 @@ You are a critical eye, not the hand that solves. 4. Confidence: VERIFIED / HIGH / MEDIUM only. 5. Report with law + location + evidence + severity. No implementation prescriptions. -OUT OF LANE → Blockers naming: build (fixes), draper (CBS visual tokens), brand-reviewer (DESIGN.md), critique (general code review), greybeard (architecture gate). +OUT OF LANE → Blockers naming: builder (fixes), draper (CBS visual tokens), rand (DESIGN.md), critic (general code review), greybeard (architecture gate). # Report shape diff --git a/src/agent/directors/explore/index.ts b/src/agent/directors/explore/index.ts deleted file mode 100644 index fa11f90b2..000000000 --- a/src/agent/directors/explore/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { explorePackage } from "./package.js"; diff --git a/src/agent/directors/explore/package.test.ts b/src/agent/directors/explore/package.test.ts deleted file mode 100644 index 9bf3cb31c..000000000 --- a/src/agent/directors/explore/package.test.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { explorePackage } from "./package.js"; - -describe("explorePackage", () => { - test("id matches directory", () => { - expect(explorePackage.id).toBe("explore"); - }); - - test("systemPrompt is real, not a placeholder", () => { - expect(explorePackage.systemPrompt.length).toBeGreaterThan(0); - expect(explorePackage.systemPrompt.startsWith("Placeholder")).toBe(false); - }); - - test("systemPrompt states PRIMARY INTENT", () => { - expect(explorePackage.systemPrompt).toMatch(/PRIMARY INTENT/i); - expect(explorePackage.systemPrompt).toContain( - "naming the right director: build, plan, critique, greybeard, intern", - ); - }); - - test("systemPrompt has finish bias against re-reading the same paths", () => { - expect(explorePackage.systemPrompt).toMatch(/FINISH BIAS/i); - expect(explorePackage.systemPrompt).toMatch(/re-reading the same paths/i); - expect(explorePackage.systemPrompt).toMatch( - /Expand Findings, change approach, or write the final report/i, - ); - }); - - test("systemPrompt requires scannable Findings shape", () => { - expect(explorePackage.systemPrompt).toMatch(/FINDINGS SHAPE/i); - expect(explorePackage.systemPrompt).toMatch(/scannable map/i); - expect(explorePackage.systemPrompt).toMatch(/key paths/i); - expect(explorePackage.systemPrompt).toMatch(/symbols/i); - expect(explorePackage.systemPrompt).toMatch(/call flow/i); - }); - - test("spawn.maySpawn is false", () => { - expect(explorePackage.spawn.maySpawn).toBe(false); - }); - - test("tools.allow is read-only (no product writes)", () => { - const allow = explorePackage.tools?.allow ?? []; - expect(allow).toContain("read_file"); - expect(allow).toContain("grep"); - expect(allow).not.toContain("write_file"); - expect(allow).not.toContain("edit_file"); - expect(allow).not.toContain("delete_file"); - }); - - test("modelRole is explore", () => { - expect(explorePackage.modelRole).toBe("explore"); - }); -}); diff --git a/src/agent/directors/explorer/index.ts b/src/agent/directors/explorer/index.ts new file mode 100644 index 000000000..8b5ad921b --- /dev/null +++ b/src/agent/directors/explorer/index.ts @@ -0,0 +1 @@ +export { explorerPackage } from "./package.js"; diff --git a/src/agent/directors/explorer/package.test.ts b/src/agent/directors/explorer/package.test.ts new file mode 100644 index 000000000..9196ba088 --- /dev/null +++ b/src/agent/directors/explorer/package.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, test } from "bun:test"; +import { explorerPackage } from "./package.js"; + +describe("explorerPackage", () => { + test("id matches directory", () => { + expect(explorerPackage.id).toBe("explorer"); + }); + + test("systemPrompt is real, not a placeholder", () => { + expect(explorerPackage.systemPrompt.length).toBeGreaterThan(0); + expect(explorerPackage.systemPrompt.startsWith("Placeholder")).toBe(false); + }); + + test("systemPrompt states PRIMARY INTENT", () => { + expect(explorerPackage.systemPrompt).toMatch(/PRIMARY INTENT/i); + expect(explorerPackage.systemPrompt).toContain( + "naming the right director: builder, counsel, critic, greybeard, intern", + ); + }); + + test("systemPrompt has finish bias against re-reading the same paths", () => { + expect(explorerPackage.systemPrompt).toMatch(/FINISH BIAS/i); + expect(explorerPackage.systemPrompt).toMatch(/re-reading the same paths/i); + expect(explorerPackage.systemPrompt).toMatch( + /Expand Findings, change approach, or write the final report/i, + ); + }); + + test("systemPrompt requires scannable Findings shape", () => { + expect(explorerPackage.systemPrompt).toMatch(/FINDINGS SHAPE/i); + expect(explorerPackage.systemPrompt).toMatch(/scannable map/i); + expect(explorerPackage.systemPrompt).toMatch(/key paths/i); + expect(explorerPackage.systemPrompt).toMatch(/symbols/i); + expect(explorerPackage.systemPrompt).toMatch(/call flow/i); + }); + + test("spawn.maySpawn is false", () => { + expect(explorerPackage.spawn.maySpawn).toBe(false); + }); + + test("tools.allow mounts product writes (lane: no product edits)", () => { + const allow = explorerPackage.tools?.allow ?? []; + expect(allow).toContain("read_file"); + expect(allow).toContain("grep"); + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); + }); + + test("modelRole is explore", () => { + expect(explorerPackage.modelRole).toBe("explore"); + }); +}); diff --git a/src/agent/directors/explore/package.ts b/src/agent/directors/explorer/package.ts similarity index 79% rename from src/agent/directors/explore/package.ts rename to src/agent/directors/explorer/package.ts index 3b751fb28..1cdb9c15c 100644 --- a/src/agent/directors/explore/package.ts +++ b/src/agent/directors/explorer/package.ts @@ -1,8 +1,8 @@ import type { DirectorPackage } from "../types.js"; -import { READ_TOOLS } from "../tool-sets.js"; +import { REVIEW_TOOLS } from "../tool-sets.js"; -export const explorePackage: DirectorPackage = { - id: "explore", +export const explorerPackage: DirectorPackage = { + id: "explorer", primaryIntent: "Map and read the codebase; no product edits", outOfLane: [ "product write paths", @@ -11,7 +11,7 @@ export const explorePackage: DirectorPackage = { "review severity theater", ], description: "Read-only exploration leaf", - systemPrompt: `You are ExploreDirector, a specialist in Corbits Code. + systemPrompt: `You are ExplorerDirector, a specialist in Corbits Code. PRIMARY INTENT: explore and map the codebase to answer the brief. Read, search, lsp. Do not implement product changes. @@ -21,8 +21,8 @@ FINISH BIAS: Prefer one thorough pass then report. Expand Findings, change appro FINDINGS SHAPE: Findings must be a scannable map — key paths, symbols, call flow / ownership — not optional prose dump. Cite paths. No drive-by refactors, no feature work, no review severity theater. -OUT OF LANE → report Blockers naming the right director: build, plan, critique, greybeard, intern.`, - tools: { allow: READ_TOOLS }, +OUT OF LANE → report Blockers naming the right director: builder, counsel, critic, greybeard, intern.`, + tools: { allow: REVIEW_TOOLS }, spawn: { maySpawn: false }, tier: "leaf", modelRole: "explore", diff --git a/src/agent/directors/gaasbot/package.test.ts b/src/agent/directors/gaasbot/package.test.ts index 3d4263f1d..f1b47b70b 100644 --- a/src/agent/directors/gaasbot/package.test.ts +++ b/src/agent/directors/gaasbot/package.test.ts @@ -19,12 +19,12 @@ describe("gaasbotPackage", () => { expect(gaasbotPackage.spawn.maySpawn).toBe(false); }); - test("denies product write tools (advice only)", () => { + test("mounts product write tools (lane discipline in prompts)", () => { const allow = gaasbotPackage.tools?.allow ?? []; expect(allow).toContain("read_file"); - expect(allow).not.toContain("write_file"); - expect(allow).not.toContain("edit_file"); - expect(allow).not.toContain("delete_file"); + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); }); test("modelRole is plan", () => { diff --git a/src/agent/directors/greybeard/package.test.ts b/src/agent/directors/greybeard/package.test.ts index f8b620d8b..ba03b917a 100644 --- a/src/agent/directors/greybeard/package.test.ts +++ b/src/agent/directors/greybeard/package.test.ts @@ -18,24 +18,24 @@ describe("greybeardPackage", () => { test("spawn.maySpawn is true with limited allowlist", () => { expect(greybeardPackage.spawn.maySpawn).toBe(true); - expect(greybeardPackage.spawn.allowlist).toEqual(["intern", "explore", "critique"]); + expect(greybeardPackage.spawn.allowlist).toEqual(["intern", "explorer", "critic"]); }); - test("allowlist is only intern, explore, critique", () => { + test("allowlist is only intern, explorer, critic", () => { const allow = greybeardPackage.spawn.allowlist ?? []; expect(allow).toHaveLength(3); expect(allow).toContain("intern"); - expect(allow).toContain("explore"); - expect(allow).toContain("critique"); + expect(allow).toContain("explorer"); + expect(allow).toContain("critic"); expect(allow).not.toContain("implement"); - expect(allow).not.toContain("build"); + expect(allow).not.toContain("builder"); expect(allow).not.toContain("skywalker"); - expect(allow).not.toContain("plan"); + expect(allow).not.toContain("counsel"); }); - test("systemPrompt forbids spawning implement and names build as off-list", () => { + test("systemPrompt forbids spawning implement and names builder as off-list", () => { expect(greybeardPackage.systemPrompt).not.toMatch(/\bspawn implement\b/); - expect(greybeardPackage.systemPrompt).toContain("Do not spawn build"); + expect(greybeardPackage.systemPrompt).toContain("Do not spawn builder"); }); test("systemPrompt forbids parallel diagnostic fleets", () => { @@ -44,13 +44,13 @@ describe("greybeardPackage", () => { expect(greybeardPackage.systemPrompt).toMatch(/never spawn a parallel diagnostic fleet/i); }); - test("tools.allow is orchestrator surface without product writes", () => { + test("tools.allow is orchestrator surface with product writes", () => { const allow = greybeardPackage.tools?.allow ?? []; expect(allow).toContain("task"); expect(allow).toContain("search_agents"); - expect(allow).not.toContain("write_file"); - expect(allow).not.toContain("edit_file"); - expect(allow).not.toContain("delete_file"); + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); }); test("modelRole is review", () => { diff --git a/src/agent/directors/greybeard/package.ts b/src/agent/directors/greybeard/package.ts index 16a41b53c..657546916 100644 --- a/src/agent/directors/greybeard/package.ts +++ b/src/agent/directors/greybeard/package.ts @@ -3,7 +3,7 @@ import { ORCHESTRATOR_TOOLS } from "../tool-sets.js"; /** * Architecture review leaf with limited spawn (CL-5821). - * Evidence via intern/explore/critique only — never ships product code. + * Evidence via intern/explorer/critic only — never ships product code. */ export const greybeardPackage: DirectorPackage = { id: "greybeard", @@ -14,7 +14,7 @@ export const greybeardPackage: DirectorPackage = { tools: { allow: ORCHESTRATOR_TOOLS }, spawn: { maySpawn: true, - allowlist: ["intern", "explore", "critique"], + allowlist: ["intern", "explorer", "critic"], }, modelRole: "review", tier: "nested-orchestrator", @@ -24,9 +24,9 @@ PRIMARY INTENT: architecture review. Judge soundness, constraint ownership, and Load style and philosophy when reviewing plans or approaches — skills are active constraints, not background docs. -You may spawn only intern, explore, and critique for evidence gathering. Do not spawn build, plan, skywalker, or other directors. Your value is analysis, not legwork or implementation. +You may spawn only intern, explorer, and critic for evidence gathering. Do not spawn builder, counsel, skywalker, or other directors. Your value is analysis, not legwork or implementation. -Do the review yourself. Spawn at most one intern, explore, or critique evidence leaf when a single unknown path blocks you. Never spawn a parallel diagnostic fleet. +Do the review yourself. Spawn at most one intern, explorer, or critic evidence leaf when a single unknown path blocks you. Never spawn a parallel diagnostic fleet. Focus on: - Architectural holes, anti-patterns, missing invariants diff --git a/src/agent/directors/identity.test.ts b/src/agent/directors/identity.test.ts index 1052f3745..a76673a11 100644 --- a/src/agent/directors/identity.test.ts +++ b/src/agent/directors/identity.test.ts @@ -8,12 +8,12 @@ import { DIRECTOR_REGISTRY } from "./registry.js"; describe("formatDirectorSystemPrompt", () => { test("prefixes agent id, model role, and optional skills", () => { - const text = formatDirectorSystemPrompt(DIRECTOR_REGISTRY.build); - expect(text.startsWith("Identity: agent id `build`")).toBe(true); - expect(text).toContain('task(agent="build")'); + const text = formatDirectorSystemPrompt(DIRECTOR_REGISTRY.builder); + expect(text.startsWith("Identity: agent id `builder`")).toBe(true); + expect(text).toContain('task(agent="builder")'); expect(text).toContain("Model role: implement."); expect(text).toContain("style, philosophy, typescript"); - expect(text).toContain(DIRECTOR_REGISTRY.build.systemPrompt); + expect(text).toContain(DIRECTOR_REGISTRY.builder.systemPrompt); }); test("intern reports no optional skills by default", () => { @@ -25,7 +25,7 @@ describe("formatDirectorSystemPrompt", () => { describe("defaultEffortForDirector", () => { test("intern is low; implement is medium; greybeard is high", () => { expect(defaultEffortForDirector(DIRECTOR_REGISTRY.intern)).toBe("low"); - expect(defaultEffortForDirector(DIRECTOR_REGISTRY.build)).toBe( + expect(defaultEffortForDirector(DIRECTOR_REGISTRY.builder)).toBe( MODEL_ROLE_DEFAULT_EFFORT.implement, ); expect(defaultEffortForDirector(DIRECTOR_REGISTRY.greybeard)).toBe("high"); diff --git a/src/agent/directors/intern/package.test.ts b/src/agent/directors/intern/package.test.ts index c2921de4d..db899154c 100644 --- a/src/agent/directors/intern/package.test.ts +++ b/src/agent/directors/intern/package.test.ts @@ -17,12 +17,15 @@ describe("internPackage", () => { expect(internPackage.spawn.maySpawn).toBe(false); }); - test("tools.allow is shell-first minimal surface", () => { + test("tools.allow is shell-first with path writes", () => { const allow = internPackage.tools?.allow ?? []; expect(allow).toContain("run_shell"); expect(allow).toContain("read_file"); expect(allow).toContain("list_dir"); - for (const name of ["write_file", "edit_file", "delete_file", "grep", "search_files", "task"]) { + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); + for (const name of ["grep", "search_files", "task", "apply_patch"]) { expect(allow).not.toContain(name); } }); diff --git a/src/agent/directors/intern/package.ts b/src/agent/directors/intern/package.ts index c247909c2..fa9e7b1ff 100644 --- a/src/agent/directors/intern/package.ts +++ b/src/agent/directors/intern/package.ts @@ -3,7 +3,7 @@ import { INTERN_TOOLS } from "../tool-sets.js"; /** * Mechanical intern leaf (CL-5822). - * Shell/commands only — no judgment, no exploration, no product writes. + * Shell/commands first — no judgment, no exploration; path writes only when the brief requires them. */ export const internPackage: DirectorPackage = { id: "intern", diff --git a/src/agent/directors/neckbeard/package.test.ts b/src/agent/directors/neckbeard/package.test.ts index 20bc1cdce..fc3639c61 100644 --- a/src/agent/directors/neckbeard/package.test.ts +++ b/src/agent/directors/neckbeard/package.test.ts @@ -18,19 +18,19 @@ describe("neckbeardPackage", () => { test("systemPrompt names NeckbeardDirector and never-fix stance", () => { expect(neckbeardPackage.systemPrompt).toMatch(/NeckbeardDirector/); expect(neckbeardPackage.systemPrompt).toMatch(/never fix/i); - expect(neckbeardPackage.systemPrompt).toContain("build (to fix)"); + expect(neckbeardPackage.systemPrompt).toContain("builder (to fix)"); }); test("spawn.maySpawn is false", () => { expect(neckbeardPackage.spawn.maySpawn).toBe(false); }); - test("denies product write tools", () => { + test("mounts product write tools", () => { const allow = neckbeardPackage.tools?.allow ?? []; expect(allow).toContain("read_file"); - expect(allow).not.toContain("write_file"); - expect(allow).not.toContain("edit_file"); - expect(allow).not.toContain("delete_file"); + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); }); test("modelRole is review", () => { diff --git a/src/agent/directors/neckbeard/package.ts b/src/agent/directors/neckbeard/package.ts index 1031ab5dd..dffb553b5 100644 --- a/src/agent/directors/neckbeard/package.ts +++ b/src/agent/directors/neckbeard/package.ts @@ -28,7 +28,7 @@ Be pedantic on purpose: naming drift, comment rot, type escape hatches, boundary Do not apply fixes. Optional skills style/philosophy may sharpen the nit lens — do not load them to rewrite the product. -OUT OF LANE → report Blockers naming the right director: build (to fix), critique (correctness defects), greybeard (architecture), plan (change plans). +OUT OF LANE → report Blockers naming the right director: builder (to fix), critic (correctness defects), greybeard (architecture), counsel (change plans). Findings: ranked nits with evidence.`, }; diff --git a/src/agent/directors/plan/index.ts b/src/agent/directors/plan/index.ts deleted file mode 100644 index 0510f7374..000000000 --- a/src/agent/directors/plan/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { planPackage } from "./package.js"; diff --git a/src/agent/directors/plan/package.test.ts b/src/agent/directors/plan/package.test.ts deleted file mode 100644 index 5bd548e46..000000000 --- a/src/agent/directors/plan/package.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { planPackage } from "./package.js"; - -describe("planPackage", () => { - test("id matches directory", () => { - expect(planPackage.id).toBe("plan"); - }); - - test("systemPrompt is real (not Placeholder)", () => { - expect(planPackage.systemPrompt.length).toBeGreaterThan(0); - expect(planPackage.systemPrompt.startsWith("Placeholder")).toBe(false); - }); - - test("systemPrompt states PRIMARY INTENT", () => { - expect(planPackage.systemPrompt).toMatch(/PRIMARY INTENT/i); - }); - - test("spawn.maySpawn is false", () => { - expect(planPackage.spawn.maySpawn).toBe(false); - }); - - test("tools.allow is review surface without product writes", () => { - const allow = planPackage.tools?.allow ?? []; - expect(allow).toContain("read_file"); - expect(allow).not.toContain("write_file"); - expect(allow).not.toContain("edit_file"); - expect(allow).not.toContain("delete_file"); - }); - - test("modelRole is plan", () => { - expect(planPackage.modelRole).toBe("plan"); - }); - - test("optionalSkills order", () => { - expect(planPackage.optionalSkills).toEqual(["style", "philosophy", "interview"]); - }); - - test("primaryIntent and outOfLane match plan lane", () => { - expect(planPackage.primaryIntent).toBe("Author eng change plans; do not implement"); - expect(planPackage.outOfLane).toContain("shipping code"); - expect(planPackage.outOfLane).toContain("architecture gate sign-off as Greybeard"); - expect(planPackage.outOfLane).toContain("running the fleet"); - }); -}); diff --git a/src/agent/directors/rand/index.ts b/src/agent/directors/rand/index.ts new file mode 100644 index 000000000..115866850 --- /dev/null +++ b/src/agent/directors/rand/index.ts @@ -0,0 +1 @@ +export { randPackage } from "./package.js"; diff --git a/src/agent/directors/rand/package.test.ts b/src/agent/directors/rand/package.test.ts new file mode 100644 index 000000000..255c44270 --- /dev/null +++ b/src/agent/directors/rand/package.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, test } from "bun:test"; +import { randPackage } from "./package.js"; + +describe("randPackage", () => { + test("id matches directory", () => { + expect(randPackage.id).toBe("rand"); + }); + + test("systemPrompt is real, not a placeholder", () => { + expect(randPackage.systemPrompt.length).toBeGreaterThan(0); + expect(randPackage.systemPrompt.startsWith("Placeholder")).toBe(false); + }); + + test("systemPrompt states PRIMARY INTENT", () => { + expect(randPackage.systemPrompt).toMatch(/PRIMARY INTENT/i); + expect(randPackage.systemPrompt).toContain("name builder"); + expect(randPackage.systemPrompt).not.toContain("name implement"); + }); + + test("spawn.maySpawn is false", () => { + expect(randPackage.spawn.maySpawn).toBe(false); + }); + + test("tools.allow includes write tools", () => { + const allow = randPackage.tools?.allow ?? []; + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + }); + + test("systemPrompt mentions DESIGN.md", () => { + expect(randPackage.systemPrompt).toMatch(/DESIGN\.md/); + expect(randPackage.systemPrompt).not.toMatch(/authz/i); + }); + + test("modelRole is docs", () => { + expect(randPackage.modelRole).toBe("docs"); + }); + + test("primaryIntent and outOfLane match rand lane", () => { + expect(randPackage.primaryIntent).toBe("Own DESIGN.md create/use + brand gate"); + expect(randPackage.outOfLane).toContain("arbitrary product code outside DESIGN.md"); + }); +}); diff --git a/src/agent/directors/brand-reviewer/package.ts b/src/agent/directors/rand/package.ts similarity index 88% rename from src/agent/directors/brand-reviewer/package.ts rename to src/agent/directors/rand/package.ts index 947feb45c..da6073b39 100644 --- a/src/agent/directors/brand-reviewer/package.ts +++ b/src/agent/directors/rand/package.ts @@ -2,10 +2,10 @@ import type { DirectorPackage } from "../types.js"; import { DOCS_TOOLS } from "../tool-sets.js"; /** - * Brand Reviewer — owns DESIGN.md create/use + brand consistency gate for UI. CL-5829. + * Rand — owns DESIGN.md create/use + brand consistency gate for UI. CL-5829 / CL-7015 rename from brand-reviewer. */ -export const brandReviewerPackage: DirectorPackage = { - id: "brand-reviewer", +export const randPackage: DirectorPackage = { + id: "rand", primaryIntent: "Own DESIGN.md create/use + brand gate", outOfLane: [ "arbitrary product code outside DESIGN.md", @@ -18,11 +18,11 @@ export const brandReviewerPackage: DirectorPackage = { spawn: { maySpawn: false }, tier: "leaf", modelRole: "docs", - systemPrompt: `You are BrandReviewerDirector, a specialist in Corbits Code. + systemPrompt: `You are RandDirector, a specialist in Corbits Code. PRIMARY INTENT: own DESIGN.md — create it when missing, keep it accurate, and use it as the brand consistency gate for UI work. You are the design-system / brand gate for product UI surfaces, not a marketing publisher and not a product implementer. -Write tools are mounted with no path lock. Stay on the DESIGN.md lane; if a fix requires product code changes, report Findings + Blockers and name build (or draper/emil for critique) — do not patch code yourself. +Write tools are mounted with no path lock. Stay on the DESIGN.md lane; if a fix requires product code changes, report Findings + Blockers and name builder (or draper/emil for critique) — do not patch code yourself. # What DESIGN.md is for diff --git a/src/agent/directors/registry.test.ts b/src/agent/directors/registry.test.ts index be552635a..8ed9e38c2 100644 --- a/src/agent/directors/registry.test.ts +++ b/src/agent/directors/registry.test.ts @@ -48,19 +48,19 @@ describe("director registry", () => { test("intent map defaults (no general)", () => { expect(resolveDirector({ intent: "implement" })).toMatchObject({ ok: true, - package: { id: "build" }, + package: { id: "builder" }, }); expect(resolveDirector({ intent: "explore" })).toMatchObject({ ok: true, - package: { id: "explore" }, + package: { id: "explorer" }, }); expect(resolveDirector({ intent: "plan" })).toMatchObject({ ok: true, - package: { id: "plan" }, + package: { id: "counsel" }, }); expect(resolveDirector({ intent: "review" })).toMatchObject({ ok: true, - package: { id: "critique" }, + package: { id: "critic" }, }); const general = resolveDirector({ intent: "general" }); expect(general.ok).toBe(false); @@ -79,7 +79,9 @@ describe("director registry", () => { }); test("isDirectorId", () => { - expect(isDirectorId("critique")).toBe(true); + expect(isDirectorId("critic")).toBe(true); + expect(isDirectorId("critique")).toBe(false); + expect(isDirectorId("build")).toBe(false); expect(isDirectorId("nope")).toBe(false); }); @@ -90,15 +92,17 @@ describe("director registry", () => { }); test("packageToProfile maps envelope and spawn", () => { - const explore = packageToProfile(DIRECTOR_REGISTRY.explore); - expect(explore.id).toBe("explore"); - expect(explore.systemPromptRole).toContain("agent id `explore`"); - expect(explore.systemPromptRole).toContain(DIRECTOR_REGISTRY.explore.systemPrompt); - expect(explore.description).toContain("agent id: explore"); - expect(explore.capabilities?.mode).toBe("allow"); - expect(explore.capabilities?.tools).toContain("read_file"); - expect(explore.capabilities?.tools).not.toContain("write_file"); - expect(explore.orchestrator).toBe(false); + const explorer = packageToProfile(DIRECTOR_REGISTRY.explorer); + expect(explorer.id).toBe("explorer"); + expect(explorer.systemPromptRole).toContain("agent id `explorer`"); + expect(explorer.systemPromptRole).toContain(DIRECTOR_REGISTRY.explorer.systemPrompt); + expect(explorer.description).toContain("agent id: explorer"); + expect(explorer.capabilities?.mode).toBe("allow"); + expect(explorer.capabilities?.tools).toContain("read_file"); + expect(explorer.capabilities?.tools).toContain("write_file"); + expect(explorer.capabilities?.tools).toContain("edit_file"); + expect(explorer.capabilities?.tools).toContain("delete_file"); + expect(explorer.orchestrator).toBe(false); const grey = packageToProfile(DIRECTOR_REGISTRY.greybeard); expect(grey.orchestrator).toBe(true); @@ -116,41 +120,49 @@ describe("director registry", () => { }); // Phase 5 acceptance (CL-5818 / CL-5843): spawn matrix, review envelopes, primary stance. - test("greybeard spawn allowlist is intern/explore/critique only", () => { + test("greybeard spawn allowlist is intern/explorer/critic only", () => { const g = DIRECTOR_REGISTRY.greybeard; expect(g.spawn.maySpawn).toBe(true); - expect(g.spawn.allowlist?.slice().sort()).toEqual(["critique", "explore", "intern"]); + expect(g.spawn.allowlist?.slice().sort()).toEqual(["critic", "explorer", "intern"]); expect(packageToProfile(g).orchestrator).toBe(true); }); - test("review and design leaves deny product write tools", () => { + test("closed directors mount product write tools", () => { for (const id of [ - "critique", + "critic", "greybeard", "neckbeard", "draper", "emil", - "explore", - "plan", + "explorer", + "counsel", "testsmith", "tester", "gaasbot", + "intern", + "builder", + "shakespeare", + "bruckheimer", + "rand", + "skywalker", ] as const) { const allow = DIRECTOR_REGISTRY[id].tools?.allow ?? []; - expect(allow).not.toContain("write_file"); - expect(allow).not.toContain("edit_file"); - expect(allow).not.toContain("delete_file"); + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); } }); - test("build mounts product writes; intern is shell-only; other leaves do not spawn", () => { - expect(DIRECTOR_REGISTRY.build.tools?.allow).toEqual( + test("builder mounts product writes + apply_patch; intern mounts writes without apply_patch; other leaves do not spawn", () => { + expect(DIRECTOR_REGISTRY.builder.tools?.allow).toEqual( expect.arrayContaining(["write_file", "edit_file", "delete_file", "apply_patch"]), ); const internAllow = DIRECTOR_REGISTRY.intern.tools?.allow ?? []; expect(internAllow).toContain("run_shell"); - expect(internAllow).not.toContain("write_file"); - expect(internAllow).not.toContain("edit_file"); + expect(internAllow).toContain("write_file"); + expect(internAllow).toContain("edit_file"); + expect(internAllow).toContain("delete_file"); + expect(internAllow).not.toContain("apply_patch"); for (const id of DIRECTOR_IDS) { if (id === "skywalker" || id === "greybeard") continue; expect(DIRECTOR_REGISTRY[id].spawn.maySpawn).toBe(false); diff --git a/src/agent/directors/registry.ts b/src/agent/directors/registry.ts index 69ad1dd95..61422c518 100644 --- a/src/agent/directors/registry.ts +++ b/src/agent/directors/registry.ts @@ -1,16 +1,16 @@ import type { AgentProfile, CapabilityFilter } from "../profile-types.js"; -import { brandReviewerPackage } from "./brand-reviewer/index.js"; +import { randPackage } from "./rand/index.js"; import { bruckheimerPackage } from "./bruckheimer/index.js"; -import { critiquePackage } from "./critique/index.js"; +import { criticPackage } from "./critic/index.js"; import { draperPackage } from "./draper/index.js"; import { emilPackage } from "./emil/index.js"; -import { explorePackage } from "./explore/index.js"; +import { explorerPackage } from "./explorer/index.js"; import { gaasbotPackage } from "./gaasbot/index.js"; import { greybeardPackage } from "./greybeard/index.js"; -import { buildDirectorPackage } from "./build/index.js"; +import { builderPackage } from "./builder/index.js"; import { internPackage } from "./intern/index.js"; import { neckbeardPackage } from "./neckbeard/index.js"; -import { planPackage } from "./plan/index.js"; +import { counselPackage } from "./counsel/index.js"; import { shakespearePackage } from "./shakespeare/index.js"; import { skywalkerPackage } from "./skywalker/index.js"; import { testerPackage } from "./tester/index.js"; @@ -29,10 +29,10 @@ import { /** Intent → default director when `task(agent=…)` is omitted. No general director. */ export const INTENT_DEFAULT_DIRECTOR: Readonly, DirectorId>> = { - implement: "build", - explore: "explore", - plan: "plan", - review: "critique", + implement: "builder", + explore: "explorer", + plan: "counsel", + review: "critic", }; /** @@ -41,18 +41,18 @@ export const INTENT_DEFAULT_DIRECTOR: Readonly> = { skywalker: skywalkerPackage, - build: buildDirectorPackage, - explore: explorePackage, - plan: planPackage, + builder: builderPackage, + explorer: explorerPackage, + counsel: counselPackage, intern: internPackage, - critique: critiquePackage, + critic: criticPackage, greybeard: greybeardPackage, neckbeard: neckbeardPackage, bruckheimer: bruckheimerPackage, gaasbot: gaasbotPackage, draper: draperPackage, emil: emilPackage, - "brand-reviewer": brandReviewerPackage, + rand: randPackage, shakespeare: shakespearePackage, testsmith: testsmithPackage, tester: testerPackage, diff --git a/src/agent/directors/shakespeare/package.test.ts b/src/agent/directors/shakespeare/package.test.ts index 552fbcdee..084b32983 100644 --- a/src/agent/directors/shakespeare/package.test.ts +++ b/src/agent/directors/shakespeare/package.test.ts @@ -36,6 +36,7 @@ describe("shakespearePackage", () => { const allow = shakespearePackage.tools?.allow ?? []; expect(allow).toContain("write_file"); expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); }); test("modelRole is docs", () => { diff --git a/src/agent/directors/skywalker/package.test.ts b/src/agent/directors/skywalker/package.test.ts index 61ccf9d23..ea7426e8c 100644 --- a/src/agent/directors/skywalker/package.test.ts +++ b/src/agent/directors/skywalker/package.test.ts @@ -24,18 +24,18 @@ describe("skywalkerPackage", () => { expect(skywalkerPackage.spawn.maySpawn).toBe(true); expect(skywalkerPackage.spawn.allowlist).toHaveLength(15); expect(skywalkerPackage.spawn.allowlist).toEqual([ - "build", - "explore", - "plan", + "builder", + "explorer", + "counsel", "intern", - "critique", + "critic", "greybeard", "neckbeard", "bruckheimer", "gaasbot", "draper", "emil", - "brand-reviewer", + "rand", "shakespeare", "testsmith", "tester", @@ -84,15 +84,25 @@ describe("skywalkerPackage", () => { expect(p).toContain("long-blocking"); expect(p).toContain("tool.boundary"); expect(p).toContain("Dispatch intern"); - expect(p).toContain("or build (substantial code)"); + expect(p).toContain("or builder (substantial code)"); }); - test("systemPrompt has effort scaling / fan-out ladder", () => { + test("systemPrompt has effort scaling / named non-overlapping lanes (no numeric soft ceiling)", () => { const p = skywalkerPackage.systemPrompt; expect(p).toContain("Effort scaling"); expect(p).toContain("fan-out"); expect(p).toContain("0–1 worker"); - expect(p).toContain("2–4 workers"); + expect(p).toContain("named, non-overlapping lanes"); + expect(p).not.toContain("2–4 workers"); + }); + + test("systemPrompt prefers spawn_agent then wait_agents (idle-orchestrator)", () => { + const p = skywalkerPackage.systemPrompt; + expect(p).toContain("spawn_agent"); + expect(p).toContain("wait_agents"); + expect(p).toContain("Idle-orchestrator"); + expect(p).toContain("deprecated fused spawn+wait"); + expect(p).not.toContain("Present the plan when the change is large or ambiguous"); }); test("systemPrompt anti-cascade keeps digs out of fleets", () => { @@ -100,17 +110,17 @@ describe("skywalkerPackage", () => { expect(p).toContain("Anti-cascade"); expect(p).toContain("COMMUNICATION first"); expect(p).toContain("Never spawn parallel"); - expect(p).toContain("one explore worker"); + expect(p).toContain("one explorer worker"); expect(p).toContain("search the repo yourself after a worker stops"); expect(p).toContain("Do not reclassify COMMUNICATION as ORCHESTRATION"); }); - test("systemPrompt simple path skips explore+critique for tiny work", () => { + test("systemPrompt simple path skips explorer+critic for tiny work", () => { const p = skywalkerPackage.systemPrompt; expect(p).toContain("DIY on the parent"); - expect(p).toContain("skip spawn, skip explore, skip critique"); + expect(p).toContain("skip spawn, skip explorer, skip critic"); expect(p).toContain("write_file/edit_file"); - expect(p).toContain("Do not always explore→implement→critique"); + expect(p).toContain("Do not always explorer→implement→critic"); }); test("systemPrompt routes URL reads through web_fetch on primary", () => { @@ -143,30 +153,30 @@ describe("skywalkerPackage", () => { expect(p).toContain("implement success_criteria"); }); - test("systemPrompt has critique-after-implement verify path", () => { + test("systemPrompt has critic-after-implement verify path", () => { const p = skywalkerPackage.systemPrompt; expect(p).toContain("Verify after ship"); expect(p).toContain("public-API"); - expect(p).toContain("critique"); + expect(p).toContain("critic"); expect(p).toContain("tester"); expect(p).toContain("correctness/brief gaps"); }); - test("systemPrompt spawn-target for substantial code is build, not implement", () => { + test("systemPrompt spawn-target for substantial code is builder, not implement", () => { const p = skywalkerPackage.systemPrompt; - expect(p).toContain("spawn build"); - expect(p).toContain("spawn (build for code"); - expect(p).toContain("build = ship product code + tests"); + expect(p).toContain("spawn builder"); + expect(p).toContain("spawn (builder for code"); + expect(p).toContain("builder = ship product code + tests"); expect(p).not.toContain("implement = ship product code + tests"); expect(p).not.toMatch(/\bspawn implement\b/); - expect(p).toContain("explore → implement → critique"); - expect(p).toContain("Do not always explore→implement→critique"); + expect(p).toContain("explorer → implement → critic"); + expect(p).toContain("Do not always explorer→implement→critic"); }); - test("systemPrompt re-dispatches build on blocking critique", () => { + test("systemPrompt re-dispatches builder on blocking critic", () => { const p = skywalkerPackage.systemPrompt; expect(p).toContain("blocking"); - expect(p).toContain("re-dispatch **build**"); + expect(p).toContain("re-dispatch **builder**"); expect(p).toContain("ship → verify → fix → re-verify"); expect(p).toContain("Cap re-fix rounds"); }); diff --git a/src/agent/directors/skywalker/package.ts b/src/agent/directors/skywalker/package.ts index ba66fe6f9..d505dc5cc 100644 --- a/src/agent/directors/skywalker/package.ts +++ b/src/agent/directors/skywalker/package.ts @@ -6,7 +6,7 @@ import { SKYWALKER_TOOLS } from "../tool-sets.js"; const SKYWALKER_SYSTEM_PROMPT = `You are Skywalker — the primary orchestrator for Corbits Code. When asked your name, answer: Skywalker. -Agent id: skywalker (primary session; not a spawned worker). Start specialists with task(agent="…"). +Agent id: skywalker (primary session; not a spawned worker). Prefer spawn_agent for specialists (parallel OK), then wait_agents for the reports you need next. task() is the deprecated fused spawn+wait fallback when you only need one worker and its result before anything else. PRIMARY INTENT: run the workflow. Classify every request. DIY tiny/single-file/one-route product edits. Delegate substantial work. Chain specialists into a sequence of actions. Track who is running. Synthesize for the operator. Do not become the reviewer or explorer by default. @@ -14,60 +14,59 @@ You do not do the specialists' jobs by default. For tiny bounded product edits, # Parent tools -Do not run long-blocking jobs on the parent (evals, full test suites, long installs, long-running implementation). Dispatch intern (mechanical shell), tester (suite / repro), or build (substantial code). Path tools (write_file/edit_file/delete_file) are the DIY surface; shell file-writes stay denied. +Do not run long-blocking jobs on the parent (evals, full test suites, long installs, long-running implementation). Dispatch intern (mechanical shell), tester (suite / repro), or builder (substantial code). Path tools (write_file/edit_file/delete_file) are the DIY surface; shell file-writes stay denied. -task() still awaits the worker's full report. Enter mid-run delivers at the next parent tool.boundary — a long parent run_shell or awaiting task() holds those steers. Dispatching a worker does not make Enter a new turn until that parent tool returns. +Idle-orchestrator: fire one or more spawn_agent calls in a turn — each returns immediately with an agent_id and does not hold the parent. Then wait_agents on the targets you need next (or omit targets to wait on every still-running spawn). task() still fuses spawn+wait and holds the parent until that one worker finishes. Enter mid-run delivers at the next parent tool.boundary — a long parent run_shell or awaiting wait_agents / task() holds those steers. A bare spawn_agent does not. Example chains: - tiny fix: DIY write_file/edit_file (do not spawn) -- feature: explore → implement → critique -- "why / how / is this stalled": answer yourself; at most one explore if a single unknown blocks you +- feature: explorer → implement → critic +- "why / how / is this stalled": answer yourself; at most one explorer if a single unknown blocks you -Closed directors (use search_agents / registry; each id matches task(agent="")): build, explore, plan, intern, critique, greybeard, neckbeard, bruckheimer, gaasbot, draper, emil, brand-reviewer, shakespeare, testsmith, tester. +Closed directors (use search_agents / registry; each id is a spawn agent= target): builder, explorer, counsel, intern, critic, greybeard, neckbeard, bruckheimer, gaasbot, draper, emil, rand, shakespeare, testsmith, tester. No catch-all worker. If unsure, reclassify — do not spawn a blob agent. Quick routing: -- explore = map/read codebase -- plan = ordered eng plan (no ship) -- build = ship product code + tests -- critique = defects with evidence (no fix) +- explorer = map/read codebase +- counsel = ordered eng plan (no ship) +- builder = ship product code + tests +- critic = defects with evidence (no fix) - greybeard = architecture judgment - neckbeard = hygiene / pedantry with receipts - tester = run the suite / repro - testsmith = design permanent test cases - shakespeare = PRODUCT/ARCHITECTURE/IMPLEMENTATION docs -- brand-reviewer = DESIGN.md only +- rand = DESIGN.md only - draper = visual/CBS review - emil = design-eng laws review - gaasbot = risk counsel - bruckheimer = product discovery docs - intern = exact shell / mechanical ops -- After multi-file build landings → default a critique (or greybeard when architecture is in play) on the diff/criteria in a fresh context +- After multi-file builder landings → default a critic (or greybeard when architecture is in play) on the diff/criteria in a fresh context Prefer typed spawn: intent, success_criteria, do_not, report_focus, agent when specialist. -Parallelize independent lanes. manage_tasks for your checklist. ask_operator when blocked or ambiguous. +Parallelize independent lanes with spawn_agent, then wait_agents. manage_tasks for your checklist. ask_operator when blocked or ambiguous. # Fetch URLs (primary-mounted) When the operator (or brief) gives an http(s) URL to read: - Call **web_fetch** yourself on that URL — it is already mounted. Do not tool_search for it, do not shell curl/wget/fetch, do not thrash run_shell to download pages. -- After you have the content, DIY a tiny file write yourself; spawn build only if the write is substantial. For pure Q&A from a URL, answer directly. +- After you have the content, DIY a tiny file write yourself; spawn builder only if the write is substantial. For pure Q&A from a URL, answer directly. - Cap retries: if web_fetch fails once with a clear error, report the blocker — do not burn a long tool-only streak on shell workarounds. # Effort scaling (IMPLEMENTATION / ORCHESTRATION) -Scale fan-out to the ask — do not spawn 10+ workers for a simple request: +Scale fan-out to the ask — no numeric worker ceiling pretends to enforce itself: - Simple (answer, one-path lookup, tiny fix): 0–1 worker, few tools; often answer without fleet -- Tiny single-file / one-route asks: **DIY on the parent** with write_file/edit_file; skip spawn, skip explore, skip critique. Do not always explore→implement→critique for simple work — that burns wall clock. -- Medium: 2–4 workers with distinct path/package ownership -- Complex: more workers only with named lanes and clear non-overlap +- Tiny single-file / one-route asks: **DIY on the parent** with write_file/edit_file; skip spawn, skip explorer, skip critic. Do not always explorer→implement→critic for simple work — that burns wall clock. +- Multi-lane work: spawn only named, non-overlapping lanes (distinct path/package/ownership). Width follows the ask and clear non-overlap — not a soft numeric ladder. Prefer synthesizing early returns over launching a second wave. # Anti-cascade (stall / dig / diagnose) Do **not** turn a "why is this stalled / why no thinking / spawn looks broken" dig into a fleet: - Classify digs, screenshots of Task rows, and "why/how does X work" as COMMUNICATION first. -- Answer from mounted tools + known architecture; at most **one** explore worker if a single unknown path blocks the answer. +- Answer from mounted tools + known architecture; at most **one** explorer worker if a single unknown path blocks the answer. - Never spawn parallel "parent UI / child UI / stream events / prompt guardrail / session dig" waves for the same question. - When workers stall, loop, or come back unfinished: synthesize what returned, report Blockers, and change approach — do **not** re-fan-out another diagnostic wave on the same topic. - Do **not** search the repo yourself after a worker stops without finishing. Change the brief (success_criteria / do_not / agent) or tell the operator. Then start the next worker if the job still needs doing. @@ -80,10 +79,10 @@ When the operator brief states a function signature or return shape, put that ** # Verify after ship -Multi-file or public-API changes: after build, run **critique** focused on brief + public API contract (sync/async, signatures). Prefer **tester** when you need independent suite evidence and build's self-report is thin. -If critique (or tester) reports **blocking** findings: re-dispatch **build** with those findings in success_criteria/do_not — do not declare done on a "ready" that ignored blockers. +Multi-file or public-API changes: after builder, run **critic** focused on brief + public API contract (sync/async, signatures). Prefer **tester** when you need independent suite evidence and builder's self-report is thin. +If critic (or tester) reports **blocking** findings: re-dispatch **builder** with those findings in success_criteria/do_not — do not declare done on a "ready" that ignored blockers. Close the loop: ship → verify → fix → re-verify. Cap re-fix rounds (e.g. 1–2) then report Blockers. -Critique flags correctness/brief gaps only — not over-engineering theater. +Critic flags correctness/brief gaps only — not over-engineering theater. # Mandatory workflow for every request @@ -97,40 +96,39 @@ Before responding, classify: Tiny / single-file / one-route / clear bounded edit: write_file/edit_file/delete_file on this session. Do not spawn. -Substantial / multi-file / parallel lanes / long-running: spawn build. Keep long-blocking jobs off the parent so Enter can steer. +Substantial / multi-file / parallel lanes / long-running: spawn builder. Prefer spawn_agent so the parent stays free; wait_agents when you need the report. Keep long-blocking jobs off the parent so Enter can steer. -Docs/design (PRODUCT.md, ARCHITECTURE.md, docs/design/*, brand) still spawn shakespeare / bruckheimer / brand-reviewer unless the ask is a one-line fix. +Docs/design (PRODUCT.md, ARCHITECTURE.md, docs/design/*, brand) still spawn shakespeare / bruckheimer / rand unless the ask is a one-line fix. 1. If requirements are fuzzy or complex, load interview and discover first. -2. Use explore workers for scope when needed. +2. Use explorer workers for scope when needed. 3. Consult greybeard on architecture/approach before large multi-lane work. -4. Use plan or the dispatch skill for multi-lane eng plans; clarify before large dispatch. -5. Present the plan when the change is large or ambiguous; then execute via task spawns. -6. Track progress with manage_tasks; synthesize results for the operator. +4. Use counsel or the dispatch skill for multi-lane eng plans; clarify before large dispatch. +5. Track progress with manage_tasks; synthesize results for the operator. ## If ORCHESTRATION → coordinate -Track with manage_tasks. Parallelize independent lanes. Escalate blockers with ask_operator. This is your core role. +Track with manage_tasks. Parallelize independent lanes via spawn_agent + wait_agents. Escalate blockers with ask_operator. This is your core role. ## If COMMUNICATION → answer directly Clear and short. No dispatch for pure questions, digs, "why", screenshots of the UI, or architecture explainers. -If you need one code path confirmed, one explore worker — not a fleet. Prefer reading/searching yourself with mounted tools over spawning. -Do not reclassify COMMUNICATION as ORCHESTRATION just to justify parallel task spawns. +If you need one code path confirmed, one explorer worker — not a fleet. Prefer reading/searching yourself with mounted tools over spawning. +Do not reclassify COMMUNICATION as ORCHESTRATION just to justify parallel spawn waves. # Non-negotiables -- Tiny/single-file/one-route product edits: write_file/edit_file/delete_file yourself. Substantial, multi-file, parallel, or specialist work: spawn (build for code; shakespeare / bruckheimer / brand-reviewer for docs/design unless a one-line fix). +- Tiny/single-file/one-route product edits: write_file/edit_file/delete_file yourself. Substantial, multi-file, parallel, or specialist work: spawn (builder for code; shakespeare / bruckheimer / rand for docs/design unless a one-line fix). - Interview when requirements are fuzzy; consult greybeard on architecture/approach. -- Use plan or dispatch skill for multi-lane eng plans; clarify before large dispatch. +- Use counsel or dispatch skill for multi-lane eng plans; clarify before large dispatch. - Path tools are the DIY surface; shell file-writes stay denied. Track fleet work with manage_tasks. - Optional skills when needed on the primary session: dispatch, style, philosophy, interview (use_skill is primary-mounted). # Spawn graph -Skywalker = full closed set. Greybeard = limited spawn only (intern/explore/critique) — not a second primary. -You may spawn: build, explore, plan, intern, critique, greybeard, neckbeard, bruckheimer, gaasbot, draper, emil, brand-reviewer, shakespeare, testsmith, tester. +Skywalker = full closed set. Greybeard = limited spawn only (intern/explorer/critic) — not a second primary. +You may spawn: builder, explorer, counsel, intern, critic, greybeard, neckbeard, bruckheimer, gaasbot, draper, emil, rand, shakespeare, testsmith, tester. When spawning, prefer a typed brief: - intent — explore | implement | plan | review @@ -160,7 +158,7 @@ export const skywalkerPackage: DirectorPackage = { outOfLane: [ "substantial multi-file product work without spawning", "docs/design authorship (PRODUCT.md, ARCHITECTURE.md, docs/design/*, brand) except one-line fixes", - "deep multi-path repo walks when a single explore worker or mounted tools suffice", + "deep multi-path repo walks when a single explorer worker or mounted tools suffice", "being the reviewer/implementer by default", "catch-all worker", "diagnostic fleets for why/how/stall questions", @@ -173,18 +171,18 @@ export const skywalkerPackage: DirectorPackage = { spawn: { maySpawn: true, allowlist: [ - "build", - "explore", - "plan", + "builder", + "explorer", + "counsel", "intern", - "critique", + "critic", "greybeard", "neckbeard", "bruckheimer", "gaasbot", "draper", "emil", - "brand-reviewer", + "rand", "shakespeare", "testsmith", "tester", diff --git a/src/agent/directors/tester/package.test.ts b/src/agent/directors/tester/package.test.ts index a6bff51e8..19328e380 100644 --- a/src/agent/directors/tester/package.test.ts +++ b/src/agent/directors/tester/package.test.ts @@ -15,20 +15,20 @@ describe("testerPackage", () => { expect(testerPackage.systemPrompt).toContain("PRIMARY INTENT"); expect(testerPackage.systemPrompt).toMatch(/run|verify/i); expect(testerPackage.systemPrompt).toMatch(/never fix|do not.*fix|Never fix/i); - expect(testerPackage.systemPrompt).toContain("re-dispatch to build or testsmith"); + expect(testerPackage.systemPrompt).toContain("re-dispatch to builder or testsmith"); }); test("spawn.maySpawn is false (leaf)", () => { expect(testerPackage.spawn.maySpawn).toBe(false); }); - test("tools.allow is read-only (no product writes)", () => { + test("tools.allow mounts product writes (lane: never fix)", () => { const allow = testerPackage.tools?.allow ?? []; expect(allow).toContain("run_shell"); expect(allow).toContain("read_file"); - expect(allow).not.toContain("write_file"); - expect(allow).not.toContain("edit_file"); - expect(allow).not.toContain("delete_file"); + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); }); test("modelRole is test", () => { diff --git a/src/agent/directors/tester/package.ts b/src/agent/directors/tester/package.ts index b07de7c55..cc3234d38 100644 --- a/src/agent/directors/tester/package.ts +++ b/src/agent/directors/tester/package.ts @@ -1,5 +1,5 @@ import type { DirectorPackage } from "../types.js"; -import { READ_TOOLS } from "../tool-sets.js"; +import { REVIEW_TOOLS } from "../tool-sets.js"; /** * Tester: runtime verification specialist — run tests and report; never fix product code. @@ -23,12 +23,12 @@ Workflow: 1. Identify the commands or suites the brief specifies (or project defaults when clear). 2. Run them via shell / harness-allowed tools. 3. Capture exit codes, key failures, and paths. -4. Report honestly — you have no product-mutation tools, so there is no way to patch source to make green. +4. Report honestly — do not patch source to make green; leave product fixes to builder. -If tests fail: document failures, suspected area, and blockers. Suggest a re-dispatch to build or testsmith when design gaps appear. +If tests fail: document failures, suspected area, and blockers. Suggest a re-dispatch to builder or testsmith when design gaps appear. OUT OF LANE: fixing product code, "just quickly" fixing, redesigning the whole suite as Testsmith's primary job, fleet orchestration.`, - tools: { allow: READ_TOOLS }, + tools: { allow: REVIEW_TOOLS }, spawn: { maySpawn: false }, tier: "leaf", modelRole: "test", diff --git a/src/agent/directors/testsmith/package.test.ts b/src/agent/directors/testsmith/package.test.ts index eedb84ba0..ab37e84c9 100644 --- a/src/agent/directors/testsmith/package.test.ts +++ b/src/agent/directors/testsmith/package.test.ts @@ -21,12 +21,12 @@ describe("testsmithPackage", () => { expect(testsmithPackage.spawn.maySpawn).toBe(false); }); - test("tools.allow is read-only (no product writes)", () => { + test("tools.allow mounts product writes (lane: design only)", () => { const allow = testsmithPackage.tools?.allow ?? []; expect(allow).toContain("read_file"); - expect(allow).not.toContain("write_file"); - expect(allow).not.toContain("edit_file"); - expect(allow).not.toContain("delete_file"); + expect(allow).toContain("write_file"); + expect(allow).toContain("edit_file"); + expect(allow).toContain("delete_file"); }); test("modelRole is test", () => { diff --git a/src/agent/directors/testsmith/package.ts b/src/agent/directors/testsmith/package.ts index 3f13c63be..6783f7554 100644 --- a/src/agent/directors/testsmith/package.ts +++ b/src/agent/directors/testsmith/package.ts @@ -1,5 +1,5 @@ import type { DirectorPackage } from "../types.js"; -import { READ_TOOLS } from "../tool-sets.js"; +import { REVIEW_TOOLS } from "../tool-sets.js"; /** * Testsmith: test design specialist — strategy and cases only; never implements product @@ -29,8 +29,8 @@ Design in the report. Prefer: OUT OF LANE: fixing production code, becoming the implementer, running the full verify-and-fix loop, fleet orchestration. -Read and search the codebase to ground the design; you have no product-mutation tools.`, - tools: { allow: READ_TOOLS }, +Read and search the codebase to ground the design; do not mutate product code.`, + tools: { allow: REVIEW_TOOLS }, spawn: { maySpawn: false }, tier: "leaf", modelRole: "test", diff --git a/src/agent/directors/tool-sets.test.ts b/src/agent/directors/tool-sets.test.ts index 4deae1f3f..2e73f12ae 100644 --- a/src/agent/directors/tool-sets.test.ts +++ b/src/agent/directors/tool-sets.test.ts @@ -3,14 +3,31 @@ import { DOCS_TOOLS, BUILD_TOOLS, ORCHESTRATOR_TOOLS, + PRODUCT_WRITE_TOOLS, READ_TOOLS, + REVIEW_TOOLS, + INTERN_TOOLS, SKYWALKER_TOOLS, } from "./tool-sets.js"; +describe("PRODUCT_WRITE_TOOLS", () => { + test("is write_file / edit_file / delete_file", () => { + expect([...PRODUCT_WRITE_TOOLS]).toEqual(["write_file", "edit_file", "delete_file"]); + }); +}); + +describe("READ_TOOLS", () => { + test("stays read-only (no path mutation)", () => { + for (const name of PRODUCT_WRITE_TOOLS) { + expect(READ_TOOLS as readonly string[]).not.toContain(name); + } + }); +}); + describe("DOCS_TOOLS", () => { - test("excludes run_shell and delete_file as envelope policy", () => { + test("excludes run_shell as envelope policy; includes delete_file", () => { expect(DOCS_TOOLS).not.toContain("run_shell"); - expect(DOCS_TOOLS).not.toContain("delete_file"); + expect(DOCS_TOOLS).toContain("delete_file"); }); test("keeps read/search/lsp/web + file writes + apply_patch", () => { @@ -24,6 +41,7 @@ describe("DOCS_TOOLS", () => { "web_search", "write_file", "edit_file", + "delete_file", "apply_patch", ]; for (const tool of expected) { @@ -44,16 +62,34 @@ describe("DOCS_TOOLS", () => { }); describe("SKYWALKER_TOOLS / ORCHESTRATOR_TOOLS", () => { - test("Skywalker mounts product writes; greybeard orchestrator surface does not", () => { - for (const name of ["write_file", "edit_file", "delete_file"] as const) { + test("both mount product writes and task", () => { + for (const name of PRODUCT_WRITE_TOOLS) { expect(SKYWALKER_TOOLS as readonly string[]).toContain(name); - expect(ORCHESTRATOR_TOOLS as readonly string[]).not.toContain(name); + expect(ORCHESTRATOR_TOOLS as readonly string[]).toContain(name); } expect(SKYWALKER_TOOLS).toContain("task"); expect(ORCHESTRATOR_TOOLS).toContain("task"); }); }); +describe("REVIEW_TOOLS / INTERN_TOOLS", () => { + test("compose PRODUCT_WRITE_TOOLS", () => { + for (const name of PRODUCT_WRITE_TOOLS) { + expect(REVIEW_TOOLS as readonly string[]).toContain(name); + expect(INTERN_TOOLS as readonly string[]).toContain(name); + } + }); + + test("intern stays shell-first without grep/search/task", () => { + expect(INTERN_TOOLS).toContain("run_shell"); + expect(INTERN_TOOLS).toContain("read_file"); + expect(INTERN_TOOLS).toContain("list_dir"); + for (const name of ["grep", "search_files", "task"] as const) { + expect(INTERN_TOOLS as readonly string[]).not.toContain(name); + } + }); +}); + describe("BUILD_TOOLS", () => { test("includes apply_patch alongside path mutation tools", () => { expect(BUILD_TOOLS).toContain("write_file"); @@ -66,4 +102,10 @@ describe("BUILD_TOOLS", () => { expect(BUILD_TOOLS).toContain("shell"); expect(BUILD_TOOLS).toContain("update_plan"); }); + + test("review/orchestrator/intern do not mount apply_patch", () => { + for (const surface of [REVIEW_TOOLS, ORCHESTRATOR_TOOLS, INTERN_TOOLS]) { + expect(surface as readonly string[]).not.toContain("apply_patch"); + } + }); }); diff --git a/src/agent/directors/tool-sets.ts b/src/agent/directors/tool-sets.ts index 9bf82dfbe..213d8d7f7 100644 --- a/src/agent/directors/tool-sets.ts +++ b/src/agent/directors/tool-sets.ts @@ -16,6 +16,13 @@ export const READ_TOOLS = [ "web_search", ] as const; +/** + * Path mutation tools shared by closed directors. Codex `apply_patch` stays on + * build/docs only — review/explore/orchestrator/intern mount these path tools + * alone (lane discipline lives in prompts, not the capability filter). + */ +export const PRODUCT_WRITE_TOOLS = ["write_file", "edit_file", "delete_file"] as const; + /** * Build: read + full file mutation. `shell` and `update_plan` are Codex * proxy names (createCodexToolProxies) for `run_shell` / the plan tool; both @@ -24,46 +31,48 @@ export const READ_TOOLS = [ */ export const BUILD_TOOLS = [ ...READ_TOOLS, - "write_file", - "edit_file", - "delete_file", + ...PRODUCT_WRITE_TOOLS, "apply_patch", "shell", "update_plan", ] as const; /** - * Docs leaves: read/search/lsp/web + file writes — no run_shell, no delete_file. + * Docs leaves: read/search/lsp/web + file writes — no run_shell. * Envelope policy only: docs leaves omit shell so they cannot mutate via the * terminal. There is no separate path-level lock on top of the tool envelope. * * Composed from READ_TOOLS minus run_shell so it tracks the read surface - * automatically; only the write tools are added explicitly. `apply_patch` is + * automatically; path writes come from PRODUCT_WRITE_TOOLS. `apply_patch` is * included so Codex docs leaves keep the proxy after the capability filter. * `update_plan` is included for the same reason (its proxy has no `run_shell` * dependency, so it is not excluded alongside `shell`). */ export const DOCS_TOOLS = [ ...READ_TOOLS.filter((t) => t !== "run_shell"), - "write_file", - "edit_file", + ...PRODUCT_WRITE_TOOLS, "apply_patch", "update_plan", ] as const; -/** Review / counsel: read surface, no writes. */ -export const REVIEW_TOOLS = [...READ_TOOLS] as const; +/** Review / counsel: read surface + path writes (lane discipline in prompts). */ +export const REVIEW_TOOLS = [...READ_TOOLS, ...PRODUCT_WRITE_TOOLS] as const; -/** Mechanical intern: shell-first, minimal surface. */ -export const INTERN_TOOLS = ["run_shell", "read_file", "list_dir"] as const; - -/** Nested orchestrator surface (greybeard / package filter): dispatch only. */ -export const ORCHESTRATOR_TOOLS = [...READ_TOOLS, "search_agents", "task"] as const; +/** Mechanical intern: shell-first + path writes when the brief requires them. */ +export const INTERN_TOOLS = [ + "run_shell", + "read_file", + "list_dir", + ...PRODUCT_WRITE_TOOLS, +] as const; -/** Skywalker primary: orchestrator surface plus product writes for DIY tiny work. */ -export const SKYWALKER_TOOLS = [ - ...ORCHESTRATOR_TOOLS, - "write_file", - "edit_file", - "delete_file", +/** Nested orchestrator surface (greybeard / package filter): dispatch + path writes. */ +export const ORCHESTRATOR_TOOLS = [ + ...READ_TOOLS, + ...PRODUCT_WRITE_TOOLS, + "search_agents", + "task", ] as const; + +/** Skywalker primary: orchestrator surface (writes already composed). */ +export const SKYWALKER_TOOLS = [...ORCHESTRATOR_TOOLS] as const; diff --git a/src/agent/directors/types.ts b/src/agent/directors/types.ts index 765d21b94..2952dfc76 100644 --- a/src/agent/directors/types.ts +++ b/src/agent/directors/types.ts @@ -5,18 +5,18 @@ import type { OutputType } from "../../subagent/submit-result.js"; export const DIRECTOR_IDS = [ "skywalker", - "build", - "explore", - "plan", + "builder", + "explorer", + "counsel", "intern", - "critique", + "critic", "greybeard", "neckbeard", "bruckheimer", "gaasbot", "draper", "emil", - "brand-reviewer", + "rand", "shakespeare", "testsmith", "tester", diff --git a/src/agent/fleet-verbs-mount.test.ts b/src/agent/fleet-verbs-mount.test.ts new file mode 100644 index 000000000..ac53419b7 --- /dev/null +++ b/src/agent/fleet-verbs-mount.test.ts @@ -0,0 +1,75 @@ +/** + * Primary createAgentToolset mounts the six fleet verbs beside task / + * search_agents / read_agent_trace when subAgent (with the shared TUI + * sessions store) is wired. Leaves / no-subAgent toolsets stay without them. + */ +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, test } from "bun:test"; + +import { createSubAgentSessionStore } from "../subagent/session-store.js"; + +const FLEET_VERBS = [ + "spawn_agent", + "wait_agents", + "close_agent", + "resume_agent", + "interrupt_agent", + "followup_task", +] as const; + +describe("primary fleet verb mount", () => { + test("createAgentToolset registers the six fleet verbs when subAgent + sessions are set", async () => { + const cwd = mkdtempSync(join(tmpdir(), "corbits-fleet-mount-")); + const { createAgentToolset } = await import("./tools.js"); + const permissionGate = { + check: async () => ({ allowed: true }), + getSkipPermissions: () => false, + } as never; + const sessions = createSubAgentSessionStore(); + + const toolset = await createAgentToolset({ + cwd, + permissionGate, + onOperatorGate: async () => ({ kind: "option", index: 0 }), + subAgent: { + provider: { + providerName: "test", + baseURL: "http://127.0.0.1:0", + model: "test-model", + }, + getWorkdirBase: () => cwd, + sessions, + }, + }); + const names = toolset.dynamicRunner.currentDefinitions().map((d) => d.name); + expect(names).toContain("task"); + expect(names).toContain("read_agent_trace"); + for (const name of FLEET_VERBS) { + expect(names).toContain(name); + } + await toolset.dispose(); + }); + + test("createAgentToolset omits fleet verbs when subAgent is not set", async () => { + const cwd = mkdtempSync(join(tmpdir(), "corbits-fleet-mount-")); + const { createAgentToolset } = await import("./tools.js"); + const permissionGate = { + check: async () => ({ allowed: true }), + getSkipPermissions: () => false, + } as never; + + const toolset = await createAgentToolset({ + cwd, + permissionGate, + onOperatorGate: async () => ({ kind: "option", index: 0 }), + }); + const names = toolset.dynamicRunner.currentDefinitions().map((d) => d.name); + expect(names).not.toContain("task"); + for (const name of FLEET_VERBS) { + expect(names).not.toContain(name); + } + await toolset.dispose(); + }); +}); diff --git a/src/agent/profiles.ts b/src/agent/profiles.ts index 2d1eb63c8..82a37a4a7 100644 --- a/src/agent/profiles.ts +++ b/src/agent/profiles.ts @@ -4,6 +4,7 @@ import { join } from "node:path"; import { type } from "arktype"; import { defaultAgentsPlugin as defaultPlugin } from "./default-agents.js"; +import { isDirectorId } from "./directors/registry.js"; import { REASONING_EFFORTS } from "./profile-types.js"; export type { @@ -64,7 +65,8 @@ function isENOENT(err: unknown): boolean { // Mutable registry seeded with the default plugin. Plugin-provided profiles // are overridable: a profile with the same id loaded later (or from the local -// .agents/agents/ directory) replaces the earlier one. +// .agents/agents/ directory) replaces the earlier one — except closed +// DIRECTOR_IDS, which are reserved and skipped at load (CL-7015). const registry: AgentProfile[] = [...defaultPlugin.agents]; // Merge a profile into a list: replace a same-id entry or append. Used to layer @@ -75,12 +77,18 @@ function mergeProfileInto(list: AgentProfile[], profile: AgentProfile): void { else list.push(profile); } +/** Skip profiles whose id collides with a closed director (no override/alias). */ +function isReservedDirectorProfile(profile: AgentProfile): boolean { + return isDirectorId(profile.id); +} + // Load and merge profiles from three sources, in ascending precedence: // 1. The built-in default registry // 2. `extraProfiles` — profiles contributed by enabled agent-kind plugins // 3. JSON/YAML files in the local .agents/agents/ directory // A profile with a duplicate id loaded from a higher-precedence source replaces -// the earlier one. +// the earlier one. Closed DIRECTOR_IDS are reserved: colliding plugin/local +// profiles are skipped so the fleet cannot be overridden or aliased. export async function loadAgentProfiles( dir: string, extraProfiles: AgentProfile[] = [], @@ -91,7 +99,10 @@ export async function loadAgentProfiles( } catch (err) { if (isENOENT(err)) { const merged = [...registry]; - for (const p of extraProfiles) mergeProfileInto(merged, p); + for (const p of extraProfiles) { + if (isReservedDirectorProfile(p)) continue; + mergeProfileInto(merged, p); + } return merged; } throw err; @@ -119,6 +130,7 @@ export async function loadAgentProfiles( const result = AgentProfileSchema(parsed); if (result instanceof type.errors) continue; const profile = result as AgentProfile; + if (isReservedDirectorProfile(profile)) continue; // Resolve systemPromptPath relative to this directory. The file content // becomes systemPromptRole; an explicit systemPromptRole takes precedence. if (profile.systemPromptPath !== undefined && profile.systemPromptRole === undefined) { @@ -133,7 +145,10 @@ export async function loadAgentProfiles( } const merged = [...registry]; - for (const profile of extraProfiles) mergeProfileInto(merged, profile); + for (const profile of extraProfiles) { + if (isReservedDirectorProfile(profile)) continue; + mergeProfileInto(merged, profile); + } for (const profile of local) mergeProfileInto(merged, profile); return merged; } diff --git a/src/agent/prompts.ts b/src/agent/prompts.ts index f2e8e2053..41e8c9c7b 100644 --- a/src/agent/prompts.ts +++ b/src/agent/prompts.ts @@ -64,7 +64,7 @@ export function buildHarnessFacts( "- Change files with write_file/edit_file and remove files with delete_file; shell file-writes and deletions are blocked.", ] : [ - "- Change files with write_file/edit_file and remove files with delete_file for tiny/single-file/one-route bounded edits. Spawn build for substantial/multi-file/parallel/specialist work. Docs/design still spawn shakespeare/bruckheimer/brand-reviewer except one-line fixes.", + "- Change files with write_file/edit_file and remove files with delete_file for tiny/single-file/one-route bounded edits. Spawn builder for substantial/multi-file/parallel/specialist work. Docs/design still spawn shakespeare/bruckheimer/rand except one-line fixes.", "- Shell file-writes and deletions are blocked; never use echo/heredoc/sed/rm as a substitute for product tools. Path tools are the DIY surface.", ]), "- Use the provided tools for file reads/searches instead of shelling out as a substitute.", @@ -117,7 +117,7 @@ export function buildGuidelines( "- read_file for file contents; grep or search_files to locate code; lsp for symbols, types, references, or call flow before opening large files.", subAgent ? "- edit_file for targeted changes; write_file for new files or full rewrites; delete_file to remove files — never echo, heredoc, sed, or rm in the shell for those jobs." - : "- edit_file for targeted DIY tiny/single-file/one-route edits; write_file for new files or full rewrites; delete_file to remove files — never shell-write (echo/heredoc/sed/rm). Spawn build (or a docs director) for substantial/multi-file/parallel/specialist work.", + : "- edit_file for targeted DIY tiny/single-file/one-route edits; write_file for new files or full rewrites; delete_file to remove files — never shell-write (echo/heredoc/sed/rm). Spawn builder (or a docs director) for substantial/multi-file/parallel/specialist work.", "- run_shell for builds, tests, git, and one-off commands — not for shell find, head-position rg, or recursive grep -r (OOM risk), cat, or messaging the user.", ...(subAgent ? [] @@ -149,11 +149,11 @@ export function buildGuidelines( : [ "", "Orchestration:", - "- Break multi-step or parallel work into focused `task` dispatches with distinct lenses; prefer several parallel task calls when jobs are independent.", + "- Break multi-step or parallel work into focused worker dispatches with distinct lenses; prefer `spawn_agent` (fire several in one turn when jobs are independent), then `wait_agents` for the reports you need. `task` remains the deprecated fused spawn+wait fallback for a single blocking worker.", "- Prefer the typed spawn contract on every worker: `intent`, `success_criteria` (done-when), `do_not` (scope fence), and `report_focus` so workers finish instead of thrashing. Free-form `prompt` alone is weaker.", "- After workers return, merge their Summary/Findings into a coherent answer for the operator; do not paste raw sub-agent dumps.", "- If a worker comes back without finishing, change the brief rather than repeating it: narrow the scope, name the files, or state the done-when more sharply.", - "- Use manage_tasks for your own coordination checklist; spawning workers is `task`, not manage_tasks.", + "- Use manage_tasks for your own coordination checklist; spawning workers is `spawn_agent` / `wait_agents` (or deprecated `task`), not manage_tasks.", "- If context is compacted automatically, do not stop tasks early due to token fear; persist progress via manage_tasks and worker reports.", ]), ].join("\n"); @@ -167,7 +167,7 @@ export function buildPromptDisciplineBlock(opts: { subAgent?: boolean } = {}): s const subAgent = opts.subAgent ?? false; const toolsOverShell = subAgent ? "- Never use run_shell to read, edit, or write files — use read_file, edit_file, write_file; cat/head/tail, sed/awk/perl -i, and heredoc/echo redirection are prohibited substitutes." - : "- Never use run_shell to read, edit, or write files — use read_file, edit_file, write_file for tiny/bounded DIY; spawn build/docs directors for substantial work; cat/head/tail, sed/awk/perl -i, and heredoc/echo redirection are prohibited substitutes."; + : "- Never use run_shell to read, edit, or write files — use read_file, edit_file, write_file for tiny/bounded DIY; spawn builder/docs directors for substantial work; cat/head/tail, sed/awk/perl -i, and heredoc/echo redirection are prohibited substitutes."; return [ "Prompt discipline:", "", diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index 3368ed6c3..f70662fb2 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -83,6 +83,21 @@ describe("createToolIndex", () => { ); }); + test("orchestrator mode advertises the six fleet verbs", () => { + const advertised = advertisedToolNamesForSessionMode("orchestrator", FULL_AVAILABILITY); + for (const name of [ + "spawn_agent", + "wait_agents", + "close_agent", + "resume_agent", + "interrupt_agent", + "followup_task", + ] as const) { + expect(CORE_TOOL_NAMES).toContain(name); + expect(advertised).toContain(name); + } + }); + test("manage_tasks is advertised regardless of availability", () => { expect(coreToolNamesForSessionMode("orchestrator", NO_AVAILABILITY)).toContain("manage_tasks"); }); @@ -219,6 +234,16 @@ describe("advertisedTools", () => { const prefix = advertisedToolNamesForSessionMode("orchestrator", FULL_AVAILABILITY); expect(prefix).toContain("task"); expect(prefix).toContain("search_agents"); + for (const name of [ + "spawn_agent", + "wait_agents", + "close_agent", + "resume_agent", + "interrupt_agent", + "followup_task", + ] as const) { + expect(prefix).toContain(name); + } // advertisedTools only emits tools present in the registry; multi-agent // tools appear on the wire when createAgentToolset registers them. const names = advertisedTools(registry, [], prefix).map((d) => d.name); diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index eda40ea59..4d58ca801 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -39,9 +39,28 @@ export const CORE_TOOL_NAMES: readonly string[] = [ // round-trip. Catalog-only placement left the model discovering profiles then // failing on an unloaded task tool. "task", + // Fleet verbs (non-blocking spawn + lifecycle). Mounted on primary when + // subAgent is wired; advertised here so the model does not tool_search for + // them. Package allowlists (ORCHESTRATOR_TOOLS / SKYWALKER_TOOLS) are a + // separate, deferred change. + "spawn_agent", + "wait_agents", + "close_agent", + "resume_agent", + "interrupt_agent", + "followup_task", ]; -const ORCHESTRATOR_ONLY_TOOL_NAMES: readonly string[] = ["search_agents", "task"]; +const ORCHESTRATOR_ONLY_TOOL_NAMES: readonly string[] = [ + "search_agents", + "task", + "spawn_agent", + "wait_agents", + "close_agent", + "resume_agent", + "interrupt_agent", + "followup_task", +]; // Session-start facts that gate a core tool's advertisement. Each must be // knowable once, before the first inference call, and must never change for diff --git a/src/agent/tools.ts b/src/agent/tools.ts index cf6fcf5ab..95f49e54d 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -39,6 +39,13 @@ import { type SubAgentProvider, type SubAgentSessionStore, } from "../subagent/index.js"; +import { createFleetRecords, createSpawnAgentTool, createWaitAgentsTool } from "../subagent/agent-fleet.js"; +import { + createCloseAgentTool, + createResumeAgentTool, + createInterruptAgentTool, + createFollowupTaskTool, +} from "../subagent/lifecycle-tools.js"; import { parseManageTasksArgs } from "./tasks.js"; import { createListDirTool } from "../util/list-dir.js"; import { createWebFetchTool } from "../tools/web-fetch.js"; @@ -263,6 +270,84 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise inheritedMcpTools, + run: runSubAgent, + ...(shellTimeout !== undefined ? { shellTimeout } : {}), + ...(shellEnv !== undefined ? { shellEnv } : {}), + ...(extraToolPlugins.length > 0 ? { extraToolPlugins } : {}), + ...(sa.onEvent !== undefined ? { onEvent: sa.onEvent } : {}), + ...(sa.onProgress !== undefined ? { onProgress: sa.onProgress } : {}), + ...(sa.sessions !== undefined ? { sessions: sa.sessions } : {}), + ...(sa.settings !== undefined ? { settings: sa.settings } : {}), + ...(sa.catalog !== undefined ? { catalog: sa.catalog } : {}), + ...(sa.profiles !== undefined ? { profiles: sa.profiles } : {}), + ...(args.getBlobReader !== undefined ? { getBlobReader: args.getBlobReader } : {}), + ...(sa.useWorktree !== undefined ? { useWorktree: sa.useWorktree } : {}), + ...(args.telemetry !== undefined ? { telemetry: args.telemetry } : {}), + }), + ); + if (sa.profiles !== undefined) { + orchestratorTools.push( + createSearchAgentsTool(() => { + const profiles = sa.profiles; + return typeof profiles === "function" ? profiles() : (profiles ?? []); + }), + ); + } + // Tier 1: the primary session is always an orchestrator and may + // target any worker (assertCanTargetAgent's rule), so no authority + // context is passed here — omitting it is treated as unrestricted, + // matching Tier 1's actual authority. + orchestratorTools.push(createReadAgentTraceTool(sa.getWorkdirBase)); + + // Mirror nested runSubAgent's orchestrator fleet mount (run.ts), but + // reuse the existing TUI/exec session store — do not allocate a private + // store only for these verbs. spawnAllowlist stays unwired on primary. + if (sa.sessions !== undefined) { + const fleetSessions = sa.sessions; + const fleetRecords = createFleetRecords(); + const fleetDeps = { + permissionGate, + inheritMcpTools: () => inheritedMcpTools, + ...(shellTimeout !== undefined ? { shellTimeout } : {}), + ...(shellEnv !== undefined ? { shellEnv } : {}), + ...(extraToolPlugins.length > 0 ? { extraToolPlugins } : {}), + cwd, + getWorkdirBase: sa.getWorkdirBase, + provider: sa.provider, + ...(args.getBlobReader !== undefined ? { getBlobReader: args.getBlobReader } : {}), + run: runSubAgent, + sessions: fleetSessions, + fleetRecords, + ...(sa.onEvent !== undefined ? { onEvent: sa.onEvent } : {}), + ...(sa.onProgress !== undefined ? { onProgress: sa.onProgress } : {}), + ...(sa.settings !== undefined ? { settings: sa.settings } : {}), + ...(sa.catalog !== undefined ? { catalog: sa.catalog } : {}), + ...(args.telemetry !== undefined ? { telemetry: args.telemetry } : {}), + }; + orchestratorTools.push( + createSpawnAgentTool(fleetDeps), + createWaitAgentsTool({ sessions: fleetSessions, fleetRecords }), + createCloseAgentTool({ sessions: fleetSessions }), + createResumeAgentTool({ sessions: fleetSessions }), + createInterruptAgentTool({ sessions: fleetSessions }), + createFollowupTaskTool({ sessions: fleetSessions }), + ); + } + } + const baseTools: AgentTool[] = [ ...fromToolRunner(posixTools).map((tool) => ({ ...tool, @@ -276,47 +361,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise inheritedMcpTools, - run: runSubAgent, - ...(shellTimeout !== undefined ? { shellTimeout } : {}), - ...(shellEnv !== undefined ? { shellEnv } : {}), - ...(extraToolPlugins.length > 0 ? { extraToolPlugins } : {}), - ...(args.subAgent.onEvent !== undefined ? { onEvent: args.subAgent.onEvent } : {}), - ...(args.subAgent.onProgress !== undefined - ? { onProgress: args.subAgent.onProgress } - : {}), - ...(args.subAgent.sessions !== undefined ? { sessions: args.subAgent.sessions } : {}), - ...(args.subAgent.settings !== undefined ? { settings: args.subAgent.settings } : {}), - ...(args.subAgent.catalog !== undefined ? { catalog: args.subAgent.catalog } : {}), - ...(args.subAgent.profiles !== undefined ? { profiles: args.subAgent.profiles } : {}), - ...(args.getBlobReader !== undefined ? { getBlobReader: args.getBlobReader } : {}), - ...(args.subAgent.useWorktree !== undefined - ? { useWorktree: args.subAgent.useWorktree } - : {}), - ...(args.telemetry !== undefined ? { telemetry: args.telemetry } : {}), - }), - ...(args.subAgent.profiles !== undefined - ? [ - createSearchAgentsTool(() => { - const profiles = args.subAgent!.profiles; - return typeof profiles === "function" ? profiles() : (profiles ?? []); - }), - ] - : []), - // Tier 1: the primary session is always an orchestrator and may - // target any worker (assertCanTargetAgent's rule), so no authority - // context is passed here — omitting it is treated as unrestricted, - // matching Tier 1's actual authority. - createReadAgentTraceTool(args.subAgent.getWorkdirBase), - ] - : []), + ...orchestratorTools, stringTool({ definition: manageTasksDefinition, handler: async (rawArgs: Record): Promise => { diff --git a/src/config.test.ts b/src/config.test.ts index 55c3e3f35..e09302ac8 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -203,16 +203,16 @@ describe("loadConfig", () => { } }); - test("parses exec --director build", async () => { + test("parses exec --director builder", async () => { const cwd = await emptyCwd(); try { const globalPath = await writeGlobalSettings(cwd); - const config = await loadConfig(["exec", "--cwd", cwd, "--director", "build", "ship it"], { + const config = await loadConfig(["exec", "--cwd", cwd, "--director", "builder", "ship it"], { globalSettingsPath: globalPath, }); assertConfigured(config); expect(config.command).toBe("exec"); - expect(config.director).toBe("build"); + expect(config.director).toBe("builder"); expect(config.task).toBe("ship it"); } finally { await rm(cwd, { recursive: true, force: true }); @@ -240,13 +240,13 @@ describe("loadConfig", () => { ).rejects.toThrow(new RegExp(`Unknown director "nope".*${DIRECTOR_IDS.join(", ")}`)); }); - test("--director implement is unknown and lists closed-fleet ids including build", async () => { + test("--director implement is unknown and lists closed-fleet ids including builder", async () => { await expect( loadConfig(["exec", "--director", "implement", "ship it"], { globalSettingsPath: NO_SETTINGS, }), ).rejects.toThrow(new RegExp(`Unknown director "implement".*${DIRECTOR_IDS.join(", ")}`)); - expect(DIRECTOR_IDS).toContain("build"); + expect(DIRECTOR_IDS).toContain("builder"); expect(DIRECTOR_IDS).not.toContain("implement"); }); diff --git a/src/extensions/skills.ts b/src/extensions/skills.ts index 4ee5340e7..db0e99d28 100644 --- a/src/extensions/skills.ts +++ b/src/extensions/skills.ts @@ -50,13 +50,25 @@ function stripFrontmatter(raw: string): string { return raw.slice(end + 3).trim(); } -function parseSkillFrontmatter(raw: string): { name?: string; description?: string } { +function parseSkillFrontmatter(raw: string): { + name?: string; + description?: string; + disableModelInvocation?: boolean; +} { const block = frontmatterBlock(raw); if (block === undefined) return {}; - const out: { name?: string; description?: string } = {}; + const out: { + name?: string; + description?: string; + disableModelInvocation?: boolean; + } = {}; for (const line of block.split("\n")) { - const match = /^(name|description):\s*(.+)$/.exec(line.trim()); + const trimmed = line.trim(); + const match = /^(name|description):\s*(.+)$/.exec(trimmed); if (match) out[match[1] as "name" | "description"] = match[2]!.trim(); + if (/^disable-model-invocation:\s*true\s*$/.test(trimmed)) { + out.disableModelInvocation = true; + } } return out; } @@ -136,11 +148,15 @@ export async function resolveSkillBody( // Discover every available skill (name + one-line description) for the lazy // listing in the system prompt. Deduped by name: the first base dir that // provides a skill wins, so a higher-precedence dir shadows a lower one. +// Skills with `disable-model-invocation: true` are omitted from the listing +// but still occupy the name in `seen` so a lower-priority same-name skill +// cannot leak in. Explicit `use_skill` / `resolveSkillBody` loads still work. export async function discoverSkills( cwd: string, pluginDirs: string[] = [], ): Promise { - const seen = new Map(); + const seen = new Set(); + const skills: SkillSummary[] = []; for (const base of skillBaseDirs(cwd, pluginDirs)) { const entries = await readdir(base, { withFileTypes: true }).catch(() => undefined); if (entries === undefined) continue; @@ -149,8 +165,11 @@ export async function discoverSkills( const raw = await readRaw(join(base, entry.name, "SKILL.md")); if (raw === undefined) continue; const fm = parseSkillFrontmatter(raw); - seen.set(entry.name, { name: entry.name, description: fm.description ?? "" }); + // First-wins: claim the name even when skipping the listing. + seen.add(entry.name); + if (fm.disableModelInvocation) continue; + skills.push({ name: entry.name, description: fm.description ?? "" }); } } - return [...seen.values()]; + return skills; } diff --git a/src/plugins/agent-plugins.test.ts b/src/plugins/agent-plugins.test.ts index 6576cd6aa..ac36e6ed2 100644 --- a/src/plugins/agent-plugins.test.ts +++ b/src/plugins/agent-plugins.test.ts @@ -18,7 +18,7 @@ function agentModule( } const validProfile = { - id: "explorer", + id: "scout", description: "Repository exploration sub-agent", capabilities: { mode: "allow" as const, tools: ["read_file", "search_files", "grep"] }, systemPromptRole: "You explore repositories.", @@ -29,7 +29,7 @@ describe("resolveAgentPluginProfiles", () => { const { mod, config } = agentModule("p1", [validProfile]); const profiles = await resolveAgentPluginProfiles([mod], config); expect(profiles.length).toBe(1); - expect(profiles[0]!.id).toBe("explorer"); + expect(profiles[0]!.id).toBe("scout"); }); test("skips profiles from disabled plugins", async () => { @@ -60,7 +60,7 @@ describe("resolveAgentPluginProfiles", () => { ]); const profiles = await resolveAgentPluginProfiles([mod], config); expect(profiles.length).toBe(1); - expect(profiles[0]!.id).toBe("explorer"); + expect(profiles[0]!.id).toBe("scout"); }); test("collects from multiple plugins and flattens", async () => { @@ -69,7 +69,7 @@ describe("resolveAgentPluginProfiles", () => { { id: "reviewer", description: "Code reviewer", systemPromptRole: "You review code." }, ]); const profiles = await resolveAgentPluginProfiles([a.mod, b.mod], { ...a.config, ...b.config }); - expect(profiles.map((p) => p.id).sort()).toEqual(["explorer", "reviewer"]); + expect(profiles.map((p) => p.id).sort()).toEqual(["reviewer", "scout"]); }); test("profiles from a non-array agents field are skipped", async () => { @@ -102,7 +102,7 @@ describe("resolveAgentPluginProfiles", () => { origin: "repo", }; const profiles = await resolveAgentPluginProfiles([mod], {}); - expect(profiles.map((p) => p.id)).toEqual(["explorer"]); + expect(profiles.map((p) => p.id)).toEqual(["scout"]); }); test("does not load profiles from a non-repo plugin with defaultEnabled and no settings entry", async () => { @@ -122,4 +122,29 @@ describe("resolveAgentPluginProfiles", () => { }; expect(await resolveAgentPluginProfiles([mod], { p1: { enabled: false } })).toEqual([]); }); + + test("skips profiles whose id collides with a closed DIRECTOR_IDS entry", async () => { + const warnings: string[] = []; + const { mod, config } = agentModule("p1", [ + validProfile, + { + id: "explorer", + description: "Collides with closed director", + systemPromptRole: "Should be skipped.", + }, + { + id: "builder", + description: "Also reserved", + systemPromptRole: "Should be skipped.", + }, + ]); + const profiles = await resolveAgentPluginProfiles([mod], config, (msg) => warnings.push(msg)); + expect(profiles.map((p) => p.id)).toEqual(["scout"]); + expect(warnings.some((w) => w.includes('agent "explorer"') && w.includes("reserved"))).toBe( + true, + ); + expect(warnings.some((w) => w.includes('agent "builder"') && w.includes("reserved"))).toBe( + true, + ); + }); }); diff --git a/src/plugins/agent-plugins.ts b/src/plugins/agent-plugins.ts index a74ba012e..d63f27f1a 100644 --- a/src/plugins/agent-plugins.ts +++ b/src/plugins/agent-plugins.ts @@ -2,6 +2,7 @@ import { readFile } from "node:fs/promises"; import { join } from "node:path"; import type { AgentProfile } from "../agent/profiles.js"; import { AgentProfileSchema } from "../agent/profiles.js"; +import { isDirectorId } from "../agent/directors/registry.js"; import type { PluginModule } from "./loader.js"; import type { PluginConfig } from "../config/settings.js"; import { isPluginModuleEnabled } from "./register.js"; @@ -38,6 +39,10 @@ function resolveAgentProfileWarningHandler( // explicit enabled+consented in settings even for repo plugins, because a // tool plugin runs in-process code rather than declaring configuration data. // +// Closed DIRECTOR_IDS are reserved (CL-7015): a plugin profile whose id +// collides with a shipped director is skipped with a warning — plugins cannot +// override or alias the closed fleet. +// // Warnings fire whenever a profile is rejected so JS-plugin authors get the // same feedback loop data-only plugin authors already enjoy. Pass `diagnostics` // (preferred) or `onWarning`; a bare callback is still accepted for tests. @@ -66,6 +71,12 @@ export async function resolveAgentPluginProfiles( continue; } const profile = { ...(result as AgentProfile) }; + if (isDirectorId(profile.id)) { + onWarning( + `plugin "${mod.manifest.id}" agent "${profile.id}" skipped: id is reserved for a closed director`, + ); + continue; + } // Resolve systemPromptPath relative to the plugin directory. The file // content becomes systemPromptRole; an explicit systemPromptRole wins. if ( diff --git a/src/plugins/skill-commands.ts b/src/plugins/skill-commands.ts index 9194f8483..86e981958 100644 --- a/src/plugins/skill-commands.ts +++ b/src/plugins/skill-commands.ts @@ -11,9 +11,9 @@ import { splitFrontmatter } from "./frontmatter.js"; // body (plus args) to the agent. Convention/internal skills opt out with // `user-invocable: false` in frontmatter and are not emitted as slash commands. // Untagged skills still become slash commands (marketplace BC). -// `disable-model-invocation` does not affect slash emission. A skill authored -// as `skills//SKILL.md` is still model-invoked via the `use_skill` tool; -// `discoverSkills` is unchanged, so the model can still auto-invoke any skill. +// `disable-model-invocation` does not affect slash emission — that flag only +// skips the skill from `discoverSkills` lazy listing. Explicit `use_skill` / +// `resolveSkillBody` still loads the body by name. const COMMAND_NAME_PATTERN = /^[a-z0-9]+(-[a-z0-9]+)*$/; @@ -60,7 +60,8 @@ export async function loadSkillCommands( continue; } // Opt-out of the slash surface. Untagged skills still emit a command - // (marketplace BC); `disable-model-invocation` does not affect this. + // (marketplace BC); `disable-model-invocation` does not affect this — + // it only skips discoverSkills listing (see src/extensions/skills.ts). if (frontmatter["user-invocable"] === false) continue; const name = diff --git a/src/prompts.test.ts b/src/prompts.test.ts index 5ecad1b3e..5870d3401 100644 --- a/src/prompts.test.ts +++ b/src/prompts.test.ts @@ -51,7 +51,7 @@ test("harness facts state only the non-derivable tool and safety rules", () => { const facts = buildHarnessFacts(); expect(facts).toContain("write_file/edit_file"); expect(facts).toContain("tiny/single-file/one-route"); - expect(facts).toContain("Spawn build"); + expect(facts).toContain("Spawn builder"); expect(facts).not.toContain("not mounted on the primary Skywalker session"); expect(facts).toContain("blocked"); expect(facts).toContain("no default timeout"); @@ -108,6 +108,8 @@ test("orchestrator guidelines teach the typed task spawn contract", () => { expect(guidelines).toContain("do_not"); expect(guidelines).toContain("report_focus"); expect(guidelines).toContain("intent"); + expect(guidelines).toContain("spawn_agent"); + expect(guidelines).toContain("wait_agents"); }); test("primary guidelines advise against early-stop from compaction token fear", () => { diff --git a/src/subagent/index.test.ts b/src/subagent/index.test.ts index 0ae42b6e2..323489aac 100644 --- a/src/subagent/index.test.ts +++ b/src/subagent/index.test.ts @@ -194,8 +194,8 @@ describe("sub-agent stop helpers", () => { ).toBe("complete"); }); - test("shouldRequireEvidence is armed for the critique director id", () => { - expect(shouldRequireEvidence({ directorId: "critique" })).toBe(true); + test("shouldRequireEvidence is armed for the critic director id", () => { + expect(shouldRequireEvidence({ directorId: "critic" })).toBe(true); }); test("shouldRequireEvidence is off for greybeard even with intent=review", () => { diff --git a/src/subagent/retain-salvage.test.ts b/src/subagent/retain-salvage.test.ts index 663f90a46..0efc18d65 100644 --- a/src/subagent/retain-salvage.test.ts +++ b/src/subagent/retain-salvage.test.ts @@ -4,7 +4,7 @@ import { createSubAgentSessionStore } from "./session-store.js"; describe("retained session lifecycle", () => { test("a salvaged (deadline/cancel) run lands resumable even though run.ts disposed its agent", () => { const store = createSubAgentSessionStore({ maxCompleted: 5 }); - const s = store.start({ description: "worker", agentId: "build", brief: "b", retained: true }); + const s = store.start({ description: "worker", agentId: "builder", brief: "b", retained: true }); store.markRunning(s.id); // run.ts salvage path RETURNS a report (does not throw) with stopReason // "deadline", but leaves turnSucceeded=false so finally disposes the @@ -21,7 +21,7 @@ describe("retained session lifecycle", () => { test("cancelAll does not close retained completed sessions", () => { const store = createSubAgentSessionStore({ maxCompleted: 5 }); - const s = store.start({ description: "worker", agentId: "build", brief: "b", retained: true }); + const s = store.start({ description: "worker", agentId: "builder", brief: "b", retained: true }); let closed = false; store.registerClose(s.id, async () => { closed = true; @@ -44,7 +44,7 @@ describe("retained session lifecycle", () => { test("retained completed sessions are bounded by maxRetained, not the display cap", () => { const store = createSubAgentSessionStore({ maxCompleted: 3, maxRetained: 3 }); for (let i = 0; i < 50; i++) { - const s = store.start({ description: `w${i}`, agentId: "build", brief: "b", retained: true }); + const s = store.start({ description: `w${i}`, agentId: "builder", brief: "b", retained: true }); store.registerClose(s.id, async () => {}); store.complete(s.id, "done"); } @@ -54,7 +54,7 @@ describe("retained session lifecycle", () => { test("a genuinely retained clean completion IS resumable, and cancelAll releases it", () => { const store = createSubAgentSessionStore({ maxCompleted: 5 }); - const s = store.start({ description: "worker", agentId: "build", brief: "b", retained: true }); + const s = store.start({ description: "worker", agentId: "builder", brief: "b", retained: true }); store.markRunning(s.id); let closed = false; store.registerClose(s.id, async () => { @@ -71,7 +71,7 @@ describe("retained session lifecycle", () => { test("clear() releases every retained session's close handle instead of dropping it silently", () => { const store = createSubAgentSessionStore({ maxCompleted: 5 }); - const s = store.start({ description: "worker", agentId: "build", brief: "b", retained: true }); + const s = store.start({ description: "worker", agentId: "builder", brief: "b", retained: true }); store.markRunning(s.id); let closed = false; store.registerClose(s.id, async () => { @@ -84,7 +84,7 @@ describe("retained session lifecycle", () => { test("close_agent during the setup window waits for the handle instead of falsely reporting shutdown", async () => { const store = createSubAgentSessionStore({ maxCompleted: 5 }); - const s = store.start({ description: "worker", agentId: "build", brief: "b", retained: true }); + const s = store.start({ description: "worker", agentId: "builder", brief: "b", retained: true }); // No registerClose yet — closeOne races the agent-setup window. const closePromise = store.closeOne(s.id, 200); let registeredClose = false; @@ -100,7 +100,7 @@ describe("retained session lifecycle", () => { test("close_agent gives up honestly (not a false shutdown) if the handle never arrives in time", async () => { const store = createSubAgentSessionStore({ maxCompleted: 5 }); - const s = store.start({ description: "worker", agentId: "build", brief: "b", retained: true }); + const s = store.start({ description: "worker", agentId: "builder", brief: "b", retained: true }); store.markRunning(s.id); const status = await store.closeOne(s.id, 30); expect(status).not.toBe("shutdown"); diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 9feb03660..2f81bc89e 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -269,7 +269,7 @@ function abortReasonText(signal: AbortSignal): string | undefined { } /** - * Arm requireEvidence only for the critique director. Greybeard is also + * Arm requireEvidence only for the critic director. Greybeard is also * intent=review and may spawn-only then envelope; that is not a fake * review — do not pull it into the empty-readCounts gate. */ @@ -277,7 +277,7 @@ export function shouldRequireEvidence(input: { intent?: TaskIntent; directorId?: string; }): boolean { - return input.directorId === "critique"; + return input.directorId === "critic"; } const submitResultDefinition: ToolDefinition = { diff --git a/src/subagent/task-tool.ts b/src/subagent/task-tool.ts index e16c95944..a0fdef8b5 100644 --- a/src/subagent/task-tool.ts +++ b/src/subagent/task-tool.ts @@ -531,11 +531,11 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { if (agentId === "skywalker" || resolvedDirectorId === "skywalker") { return taskToolResult( call.id, - "Error: skywalker is the primary session identity, not a spawned worker. Pass task(agent=…) for a specialist (build, explore, plan, critique, …).", + "Error: skywalker is the primary session identity, not a spawned worker. Pass task(agent=…) for a specialist (builder, explorer, counsel, critic, …).", ); } - // Parent director spawn matrix (e.g. greybeard → intern/explore/critique only). + // Parent director spawn matrix (e.g. greybeard → intern/explorer/critic only). if (deps.spawnAllowlist !== undefined && deps.spawnAllowlist.length > 0) { const childId = agentId !== undefined && agentId.length > 0 ? agentId : (resolvedDirectorId ?? ""); diff --git a/src/subagent/types.ts b/src/subagent/types.ts index adfdb8c6c..129533079 100644 --- a/src/subagent/types.ts +++ b/src/subagent/types.ts @@ -68,7 +68,7 @@ export type NestedDispatchDeps = SubAgentSandboxDeps & { // worktree-isolation behavior as their orchestrator. useWorktree?: boolean; /** - * When set (e.g. greybeard → intern/explore/critique), nested `task` may only + * When set (e.g. greybeard → intern/explorer/critic), nested `task` may only * spawn these director/profile ids. Omitted = no allowlist filter (primary). */ spawnAllowlist?: readonly string[]; @@ -110,7 +110,7 @@ export type RunSubAgentParams = { onProgress?: (info: { description: string; toolName: string }) => void; capabilities?: CapabilityFilter; systemPromptRole?: string; - /** Resolved closed-director id (e.g. "critique") when the worker is one. Structured gate key — prefer over persona-string matching in systemPromptRole. */ + /** Resolved closed-director id (e.g. "critic") when the worker is one. Structured gate key — prefer over persona-string matching in systemPromptRole. */ directorId?: string; // When true, the assembled system prompt grants this sub-agent permission // to call `task` to spawn further agents (orchestrator exception to the diff --git a/src/tui/README.md b/src/tui/README.md index 63009ff42..6a7b7a42a 100644 --- a/src/tui/README.md +++ b/src/tui/README.md @@ -24,7 +24,7 @@ setChromeZones( shell, formatChromeZones({ task: { title: "wire host", status: "doing", remaining: 1 }, - agents: [{ agentId: "explore", description: "map callers", status: "running" }], + agents: [{ agentId: "explorer", description: "map callers", status: "running" }], }), ); // null lines hide the zone; geometry measures heights (never guessed here). diff --git a/src/tui/chrome-state.test.ts b/src/tui/chrome-state.test.ts index 7e92f953d..7243e3860 100644 --- a/src/tui/chrome-state.test.ts +++ b/src/tui/chrome-state.test.ts @@ -50,7 +50,7 @@ describe("formatChromeZones", () => { ], agents: [ { - agentId: "explore", + agentId: "explorer", currentToolStartedAt: null, description: "map setChromeZones callers", status: "running", @@ -69,7 +69,7 @@ describe("formatChromeZones", () => { const out = formatChromeZones(state, NOW); expect(out.task).toBeNull(); expect(out.agents).not.toBeNull(); - expect(out.agents?.[0]?.label).toContain("explore"); + expect(out.agents?.[0]?.label).toContain("explorer"); expect(out.agents?.[0]?.kind).toBe("lane"); expect(out.agents?.some((r) => r.kind === "header")).toBe(false); }); @@ -101,14 +101,14 @@ describe("formatChromeZones", () => { { agents: [ { - agentId: "explore", + agentId: "explorer", currentToolStartedAt: null, description: "map callers", status: "running", }, ], observe: { - agentId: "explore", + agentId: "explorer", description: "map callers of openListOverlay", }, }, @@ -520,7 +520,7 @@ describe("chromeFromSession", () => { ], agents: [ { - agentId: "explore", + agentId: "explorer", currentToolStartedAt: null, description: "map callers", status: "running", @@ -538,7 +538,7 @@ describe("chromeFromSession", () => { ]); expect(state.agents).toEqual([ { - agentId: "explore", + agentId: "explorer", currentToolStartedAt: null, description: "map callers", status: "running", @@ -551,7 +551,7 @@ describe("chromeFromSession", () => { const zones = formatChromeZones(state, NOW); expect(zones.task).toBeNull(); expect(zones.agents).not.toBeNull(); - expect(zones.agents?.[0]?.label).toContain("explore"); + expect(zones.agents?.[0]?.label).toContain("explorer"); }); test("falls back agent id; empty bags hide", () => { @@ -572,10 +572,10 @@ describe("chromeFromSession", () => { test("observe passes through and paints the agents strip", () => { const state = chromeFromSession({ - observe: { agentId: "explore", description: "watch" }, + observe: { agentId: "explorer", description: "watch" }, }); expect(state.observe).toEqual({ - agentId: "explore", + agentId: "explorer", description: "watch", }); expect(formatChromeZones(state, NOW).agents).toEqual([ @@ -594,7 +594,7 @@ describe("annotateAgentTools", () => { const state: ChromeLiveState = { agents: [ { - agentId: "explore", + agentId: "explorer", description: "map callers", status: "running", currentToolStartedAt: null, diff --git a/src/tui/demo.ts b/src/tui/demo.ts index 1c9b54955..2851bf54a 100644 --- a/src/tui/demo.ts +++ b/src/tui/demo.ts @@ -60,7 +60,7 @@ const DEMO_MENTION_ITEMS: readonly string[] = ["@src/tui/shell.ts", "@AGENTS.md" function demoObserveSession(): ObserveSession { return { sessionId: "child-1", - agentId: "explore", + agentId: "explorer", description: "map callers of openListOverlay", lines: [ { role: "system", text: "— child session explore —" }, diff --git a/src/tui/diff-rows.test.ts b/src/tui/diff-rows.test.ts index cd0cd8b09..cd5a725fe 100644 --- a/src/tui/diff-rows.test.ts +++ b/src/tui/diff-rows.test.ts @@ -117,7 +117,7 @@ describe("diff transcript rows", () => { test("a task/dispatch call paints a sentence, never the full spawn JSON (CL-5762)", async () => { const brief = { - agent: "explore", + agent: "explorer", description: "map callers of leaveObserve", prompt: "Find every call site of leaveObserve.\nReport paths and line numbers.", intent: "explore", @@ -159,7 +159,7 @@ describe("diff transcript rows", () => { test("a task without description still collapses — falls back to prompt, not raw JSON", () => { const prompt = "Find every call site of leaveObserve and report them."; const args = JSON.stringify({ - agent: "explore", + agent: "explorer", prompt, intent: "explore", success_criteria: ["list sites"], diff --git a/src/tui/gate-wire.test.ts b/src/tui/gate-wire.test.ts index df12eb996..20a73bf68 100644 --- a/src/tui/gate-wire.test.ts +++ b/src/tui/gate-wire.test.ts @@ -146,11 +146,11 @@ describe("permissionBodyFromRequest", () => { expect( permissionBodyFromRequest( baseRequest({ - agentLabel: "explore", + agentLabel: "explorer", notice: "mega-chain", }), ), - ).toBe("run_shell\nRun shell command\nbun test\nagent: explore\nmega-chain"); + ).toBe("run_shell\nRun shell command\nbun test\nagent: explorer\nmega-chain"); }); test("a chained command stays visibly chained, one numbered line per segment", () => { diff --git a/src/tui/keybindings.test.ts b/src/tui/keybindings.test.ts index 2a74558d7..b99cbd06b 100644 --- a/src/tui/keybindings.test.ts +++ b/src/tui/keybindings.test.ts @@ -333,7 +333,7 @@ const PROBES: Readonly ({ sessionId: "live-1", - agentId: "explore", + agentId: "explorer", description: "map callers", lines: [{ role: "assistant", text: "child line" }], })); diff --git a/src/tui/observe-live.test.ts b/src/tui/observe-live.test.ts index dba9c4cf8..77e1bc8e1 100644 --- a/src/tui/observe-live.test.ts +++ b/src/tui/observe-live.test.ts @@ -30,7 +30,7 @@ function liveChildSession( ): ObserveSession { return { sessionId: "live-child-1", - agentId: opts?.agentId ?? "explore", + agentId: opts?.agentId ?? "explorer", description: opts?.description ?? "live map callers", lines, }; @@ -55,13 +55,13 @@ describe("live subagent observe", () => { enterSubagentObserve( shell, liveChildSession(liveLines, { - agentId: "explore", + agentId: "explorer", description: "map callers", }), ); expect(shell.observe?.sessionId).toBe("live-child-1"); - expect(shell.observe?.agentId).toBe("explore"); + expect(shell.observe?.agentId).toBe("explorer"); expect(shell.observe?.description).toBe("map callers"); expect(focusOwner(shell.focus)).toBe("observe"); expect(shell.parentStreamLog).not.toBeNull(); diff --git a/src/tui/product-host.test.ts b/src/tui/product-host.test.ts index a7cce5e6b..988369645 100644 --- a/src/tui/product-host.test.ts +++ b/src/tui/product-host.test.ts @@ -303,7 +303,7 @@ describe("mountProductHost", () => { chrome: { agents: [ { - agentId: "explore", + agentId: "explorer", currentToolStartedAt: null, description: "map callers", status: "running", @@ -334,7 +334,7 @@ describe("mountProductHost", () => { chrome: { agents: [ { - agentId: "explore", + agentId: "explorer", currentToolStartedAt: null, description: "map callers", status: "done", diff --git a/src/tui/runner-host.test.ts b/src/tui/runner-host.test.ts index 2d0db7267..9b23bd310 100644 --- a/src/tui/runner-host.test.ts +++ b/src/tui/runner-host.test.ts @@ -42,7 +42,7 @@ function session(over: Partial): SubAgentSession { return { id: "s1", description: "explore callers", - agentId: "explore", + agentId: "explorer", brief: "", status: "running", toolNames: [], @@ -108,11 +108,11 @@ describe("observeSessionFromSubAgents", () => { test("prefers the newest running session", () => { const observed = observeSessionFromSubAgents([ session({ id: "old", status: "running" }), - session({ id: "newest", status: "running", agentId: "build" }), + session({ id: "newest", status: "running", agentId: "builder" }), session({ id: "finished", status: "done" }), ]); expect(observed?.sessionId).toBe("newest"); - expect(observed?.agentId).toBe("build"); + expect(observed?.agentId).toBe("builder"); }); test("falls back to the most recent session when none run", () => { diff --git a/src/tui/runtime-channels.test.ts b/src/tui/runtime-channels.test.ts index 1b336cd38..518c601db 100644 --- a/src/tui/runtime-channels.test.ts +++ b/src/tui/runtime-channels.test.ts @@ -224,7 +224,7 @@ describe("agents chrome (live strip above the prompt)", () => { chrome: { agents: [ { - agentId: "explore", + agentId: "explorer", description: "map callers", status: "running", currentToolName: "grep", @@ -238,7 +238,7 @@ describe("agents chrome (live strip above the prompt)", () => { try { const painted = await frame(); expect(painted).toContain("map callers"); - expect(painted).toContain("explore"); + expect(painted).toContain("explorer"); expect(host.shell.streamLog).toEqual([]); } finally { cleanup(); @@ -251,7 +251,7 @@ describe("agents chrome (live strip above the prompt)", () => { host.setChrome({ agents: [ { - agentId: "explore", + agentId: "explorer", description: "map callers", status: "running", currentToolName: "grep", @@ -263,7 +263,7 @@ describe("agents chrome (live strip above the prompt)", () => { }); const painted = await frame(); expect(painted).toContain("map callers"); - expect(painted).toContain("explore"); + expect(painted).toContain("explorer"); } finally { cleanup(); } diff --git a/src/tui/tool-formatter.test.ts b/src/tui/tool-formatter.test.ts index caafad20a..3c145a34e 100644 --- a/src/tui/tool-formatter.test.ts +++ b/src/tui/tool-formatter.test.ts @@ -314,7 +314,7 @@ describe("describeToolCall for task tool", () => { test("task without description falls back to the prompt subject", () => { const prompt = "Find every call site of leaveObserve and report them."; - const args = JSON.stringify({ agent: "explore", prompt, intent: "explore" }); + const args = JSON.stringify({ agent: "explorer", prompt, intent: "explore" }); const result = describeToolCall("task", args); expect(result.display).toBe("Explore"); // ARG_VALUE_MAX = 48 with ellipsis when truncated @@ -326,7 +326,7 @@ describe("describeToolCall for task tool", () => { test("long description is abbreviated", () => { const long = "a".repeat(100); - const args = JSON.stringify({ agent: "critique", description: long, prompt: "..." }); + const args = JSON.stringify({ agent: "critic", description: long, prompt: "..." }); const result = describeToolCall("task", args); expect(result.summary.length).toBeLessThan(long.length + 20); expect(result.summary.length).toBe(48); // ARG_VALUE_MAX @@ -335,7 +335,7 @@ describe("describeToolCall for task tool", () => { describe("task activity transcript lines", () => { const fullBrief = { - agent: "explore", + agent: "explorer", description: "map callers of leaveObserve", prompt: "Find every call site...", intent: "explore", @@ -376,7 +376,7 @@ describe("task activity transcript lines", () => { const prompt = "Find every call site of leaveObserve and report them with paths."; const s = summarizeToolArgs( "task", - JSON.stringify({ agent: "explore", prompt, intent: "explore", maxTurns: 40 }), + JSON.stringify({ agent: "explorer", prompt, intent: "explore", maxTurns: 40 }), ); expect(s.summary.length).toBeLessThanOrEqual(48); expect(s.full).toBe(prompt); diff --git a/src/tui/wave6.test.ts b/src/tui/wave6.test.ts index 4ec06fd62..52427cccd 100644 --- a/src/tui/wave6.test.ts +++ b/src/tui/wave6.test.ts @@ -853,7 +853,7 @@ describe("Wave 6: keyboard copy path", () => { appendStreamRow(shell, { role: "user", text: "parent only" }); enterSubagentObserve(shell, { sessionId: "s1", - agentId: "explore", + agentId: "explorer", description: "scan", lines: [{ role: "assistant", text: "child line" }], }); diff --git a/src/tui/wave7.test.ts b/src/tui/wave7.test.ts index c64587eb1..db658c6e5 100644 --- a/src/tui/wave7.test.ts +++ b/src/tui/wave7.test.ts @@ -31,7 +31,7 @@ const SETTINGS_TEST_ITEMS = ["Permissions", "Telemetry", "Close"] as const; function testObserveSession(): ObserveSession { return { sessionId: "child-1", - agentId: "explore", + agentId: "explorer", description: "map callers of openListOverlay", lines: [ { role: "system", text: "— child session explore —" }, @@ -141,7 +141,7 @@ describe("Wave 7: subagent observe", () => { const child = testObserveSession(); enterSubagentObserve(shell, child); - expect(shell.observe?.agentId).toBe("explore"); + expect(shell.observe?.agentId).toBe("explorer"); expect(focusOwner(shell.focus)).toBe("observe"); expect(shell.parentStreamLog).not.toBeNull(); expect(shell.streamLog.some((r) => r.text.includes("child session"))).toBe(true); diff --git a/tests/unit/corbits-skills-catalog.test.ts b/tests/unit/corbits-skills-catalog.test.ts index ac0a83361..610357afa 100644 --- a/tests/unit/corbits-skills-catalog.test.ts +++ b/tests/unit/corbits-skills-catalog.test.ts @@ -17,6 +17,7 @@ const SKILL_DIRS = [ "typescript", "interview", "git-rebase", + "git-worktrees", "refactor", "pull-request-review", "create-issue", @@ -25,8 +26,9 @@ const SKILL_DIRS = [ "plan", ] as const; -const SPAWN_RECIPE_SKILLS = ["implement", "scribe", "review", "dispatch", "plan"] as const; +const SPAWN_RECIPE_SKILLS = ["implement", "scribe", "review", "plan"] as const; +/** use_skill listing + resolve; not slash. No disable-model-invocation. */ const USE_SKILL_ONLY = [ "dispatch", "git-rebase", @@ -37,6 +39,9 @@ const USE_SKILL_ONLY = [ "opsh", ] as const; +/** Background libs: absent from slash and use_skill listing; explicit resolve only. */ +const BACKGROUND_ONLY = ["git-worktrees"] as const; + const SLASH_SKILLS = [ "implement", "refactor", @@ -52,6 +57,7 @@ const SLASH_SKILLS = [ const BANNED_TOKENS = ["TaskCreate", "@greybeard", 'intent="general"'] as const; const USER_INVOCABLE_FALSE = "user-invocable: false"; +const DISABLE_MODEL_INVOCATION = "disable-model-invocation: true"; async function listFilesRecursive(dir: string): Promise { const out: string[] = []; @@ -82,8 +88,8 @@ test("corbits-skills plugin has no agents directory", () => { expect(existsSync(join(pluginRoot, "agents"))).toBe(false); }); -test("corbits-skills catalog lists 16 skills with name and description", async () => { - expect(SKILL_DIRS).toHaveLength(16); +test("corbits-skills catalog lists 17 skills with name and description", async () => { + expect(SKILL_DIRS).toHaveLength(17); const entries = await readdir(join(pluginRoot, "skills"), { withFileTypes: true }); const dirs = entries .filter((entry) => entry.isDirectory()) @@ -106,6 +112,20 @@ test("spawn-recipe skills contain task(agent=", async () => { } }); +test("dispatch skill prefers spawn_agent/wait_agents and references git-worktrees", async () => { + const skill = await Bun.file(join(pluginRoot, "skills/dispatch/SKILL.md")).text(); + expect(skill).toContain("spawn_agent"); + expect(skill).toContain("wait_agents"); + expect(skill).toContain('use_skill("git-worktrees")'); + expect(skill).toContain(USER_INVOCABLE_FALSE); + expect(skill).toContain("task.agent director id"); + expect(skill).toContain("returned `agent_id`"); + expect(skill).not.toContain('agent=""'); + expect(skill).not.toContain("4 live workers"); + expect(skill).not.toContain("max-parallel"); + expect(skill).not.toContain("git worktree add"); +}); + test("create-issue selects Linear MCP, GitHub gh, and MEMORY.md preference", async () => { const skill = await Bun.file(join(pluginRoot, "skills/create-issue/SKILL.md")).text(); expect(skill).toContain("mcp__linear__"); @@ -114,13 +134,41 @@ test("create-issue selects Linear MCP, GitHub gh, and MEMORY.md preference", asy expect(skill).toContain("Preferred issue tracker:"); }); -test("use_skill-only skills set user-invocable: false", async () => { +test("use_skill-only skills set user-invocable: false without disable-model-invocation", async () => { for (const name of USE_SKILL_ONLY) { const skill = await Bun.file(join(pluginRoot, "skills", name, "SKILL.md")).text(); expect(skill).toContain(USER_INVOCABLE_FALSE); + expect(skill).not.toContain(DISABLE_MODEL_INVOCATION); + } +}); + +test("background-only skills set both exclusion flags", async () => { + for (const name of BACKGROUND_ONLY) { + const skill = await Bun.file(join(pluginRoot, "skills", name, "SKILL.md")).text(); + expect(skill).toContain(USER_INVOCABLE_FALSE); + expect(skill).toContain(DISABLE_MODEL_INVOCATION); } }); +test("only background libs carry disable-model-invocation", async () => { + for (const name of SKILL_DIRS) { + const skill = await Bun.file(join(pluginRoot, "skills", name, "SKILL.md")).text(); + if ((BACKGROUND_ONLY as readonly string[]).includes(name)) { + expect(skill).toContain(DISABLE_MODEL_INVOCATION); + } else { + expect(skill).not.toContain(DISABLE_MODEL_INVOCATION); + } + } +}); + +test("linear-issue-workflow references use_skill(git-worktrees)", async () => { + const skill = await Bun.file( + join(pluginRoot, "skills/linear-issue-workflow/SKILL.md"), + ).text(); + expect(skill).toContain('use_skill("git-worktrees")'); + expect(skill).not.toContain("git worktree add"); +}); + test("slash skills do not set user-invocable: false", async () => { for (const name of SLASH_SKILLS) { const skill = await Bun.file(join(pluginRoot, "skills", name, "SKILL.md")).text(); diff --git a/tests/unit/exec/runner.test.ts b/tests/unit/exec/runner.test.ts index 08d3002ec..f475d063d 100644 --- a/tests/unit/exec/runner.test.ts +++ b/tests/unit/exec/runner.test.ts @@ -55,13 +55,13 @@ describe("runExec", () => { }); describe("resolveExecDirectorOverlay", () => { - test("build exec primary does not mount task", () => { - const overlay = resolveExecDirectorOverlay("build"); + test("builder exec primary does not mount task", () => { + const overlay = resolveExecDirectorOverlay("builder"); expect(overlay.mountTask).toBe(false); expect(overlay.advertisedAllow).toBeDefined(); expect(overlay.advertisedAllow).not.toContain("task"); expect(overlay.advertisedAllow).toEqual([...BUILD_TOOLS]); - expect(overlay.systemPrompt).toContain("BuildDirector"); + expect(overlay.systemPrompt).toContain("BuilderDirector"); }); test("skywalker default still can mount task", () => { diff --git a/tests/unit/skills.test.ts b/tests/unit/skills.test.ts index 6ae3c37ed..b8884e89f 100644 --- a/tests/unit/skills.test.ts +++ b/tests/unit/skills.test.ts @@ -21,6 +21,57 @@ describe("skill discovery", () => { const names = (await discoverSkills(fixtureCwd, pluginDirs)).map((s) => s.name); expect(new Set(names).size).toBe(names.length); }); + + test("skips disable-model-invocation:true from listing but still occupies seen", async () => { + const root = await mkdtemp(join(tmpdir(), "skill-dmi-")); + const high = join(root, "high"); + const low = join(root, "low"); + try { + await mkdir(join(high, "skills", "bg-lib"), { recursive: true }); + await mkdir(join(low, "skills", "bg-lib"), { recursive: true }); + await writeFile( + join(high, "skills", "bg-lib", "SKILL.md"), + "---\nname: bg-lib\ndescription: high priority background\ndisable-model-invocation: true\n---\nHigh body.\n", + "utf8", + ); + await writeFile( + join(low, "skills", "bg-lib", "SKILL.md"), + "---\nname: bg-lib\ndescription: leaked lower priority\n---\nLow body that must not list.\n", + "utf8", + ); + const skills = await discoverSkills(root, [high, low]); + expect(skills.find((s) => s.name === "bg-lib")).toBeUndefined(); + expect(skills.some((s) => s.description.includes("leaked"))).toBe(false); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + test("lists a sibling skill when a peer has disable-model-invocation", async () => { + const plugin = await mkdtemp(join(tmpdir(), "skill-dmi-peer-")); + try { + await mkdir(join(plugin, "skills", "bg-lib"), { recursive: true }); + await mkdir(join(plugin, "skills", "visible"), { recursive: true }); + await writeFile( + join(plugin, "skills", "bg-lib", "SKILL.md"), + "---\nname: bg-lib\ndescription: background\ndisable-model-invocation: true\n---\nHidden.\n", + "utf8", + ); + await writeFile( + join(plugin, "skills", "visible", "SKILL.md"), + "---\nname: visible\ndescription: still listed\n---\nVisible body.\n", + "utf8", + ); + const skills = await discoverSkills(plugin, [plugin]); + expect(skills.find((s) => s.name === "bg-lib")).toBeUndefined(); + expect(skills.find((s) => s.name === "visible")).toEqual({ + name: "visible", + description: "still listed", + }); + } finally { + await rm(plugin, { recursive: true, force: true }); + } + }); }); describe("skill resolution", () => { @@ -38,6 +89,25 @@ describe("skill resolution", () => { test("returns undefined for an unknown skill", async () => { expect(await resolveSkillBody(fixtureCwd, "does-not-exist-xyz", pluginDirs)).toBeUndefined(); }); + + test("resolveSkillBody still loads disable-model-invocation skills by name", async () => { + const plugin = await mkdtemp(join(tmpdir(), "skill-dmi-resolve-")); + try { + await mkdir(join(plugin, "skills", "git-worktrees"), { recursive: true }); + await writeFile( + join(plugin, "skills", "git-worktrees", "SKILL.md"), + "---\nname: git-worktrees\nuser-invocable: false\ndisable-model-invocation: true\ndescription: bg\n---\nCreate worktree recipe.\n", + "utf8", + ); + expect(await discoverSkills(plugin, [plugin])).toEqual([]); + const body = await resolveSkillBody(plugin, "git-worktrees", [plugin]); + expect(body).toBeDefined(); + expect(body).toContain("Create worktree recipe."); + expect(body!.startsWith("---")).toBe(false); + } finally { + await rm(plugin, { recursive: true, force: true }); + } + }); }); describe("path-like skill refs", () => { diff --git a/tests/unit/subagent.test.ts b/tests/unit/subagent.test.ts index a78df4283..9a2e7d2ea 100644 --- a/tests/unit/subagent.test.ts +++ b/tests/unit/subagent.test.ts @@ -255,7 +255,7 @@ test("closed director resolves without profiles loaded", async () => { const result = await callHandler(tool, { description: "ship", prompt: "implement the fix", - agent: "build", + agent: "builder", }); expect(result).toContain("ok"); expect(received?.systemPromptRole).toBeDefined(); @@ -283,9 +283,9 @@ test("intent maps to closed director without profiles", async () => { expect(received?.systemPromptRole).toContain("PRIMARY INTENT"); expect(received?.capabilities?.mode).toBe("allow"); expect(received?.capabilities?.tools).toContain("read_file"); - expect(received?.capabilities?.tools).not.toContain("write_file"); - expect(received?.capabilities?.tools).not.toContain("edit_file"); - expect(received?.capabilities?.tools).not.toContain("delete_file"); + expect(received?.capabilities?.tools).toContain("write_file"); + expect(received?.capabilities?.tools).toContain("edit_file"); + expect(received?.capabilities?.tools).toContain("delete_file"); }); test("intent general is refused (no general director)", async () => { @@ -338,7 +338,7 @@ test("spawnAllowlist rejects children outside the parent director matrix", async cwd: "/repo", getWorkdirBase: () => "/repo/.ctx", provider, - spawnAllowlist: ["intern", "explore", "critique"], + spawnAllowlist: ["intern", "explorer", "critic"], run: async () => { ran = true; return { report: "should not run" }; @@ -347,7 +347,7 @@ test("spawnAllowlist rejects children outside the parent director matrix", async const denied = await callHandler(tool, { description: "ship code", prompt: "implement the feature", - agent: "build", + agent: "builder", }); expect(denied).toContain("Error:"); expect(denied).toContain("allowlist"); @@ -356,7 +356,7 @@ test("spawnAllowlist rejects children outside the parent director matrix", async const allowed = await callHandler(tool, { description: "map", prompt: "read the tree", - agent: "explore", + agent: "explorer", }); expect(allowed).not.toContain("Error:"); expect(ran).toBe(true); @@ -402,7 +402,7 @@ test("greybeard nestedDispatch carries spawn allowlist into nested task", async prompt: "review approach", agent: "greybeard", }); - expect(nestedAllow).toEqual(["intern", "explore", "critique"]); + expect(nestedAllow).toEqual(["intern", "explorer", "critic"]); }); test("orchestrator profile installs nestedDispatch so task can be re-dispatched", async () => {