From 5200a0a6b120f61096ffcc5cd9571cf2a608ed63 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Fri, 4 Sep 2026 16:32:21 -0700 Subject: [PATCH] Require a plan before substantial Builder work Skywalker feature chain is explorer then plan then implement then critic. Builder blocks when the counsel /plan packet is missing. Tiny parent-DIY stays plan-optional. /implement does not steal planning from /plan. --- CHANGELOG.md | 1 + docs/ARCHITECTURE.md | 2 +- docs/PRODUCT.md | 2 +- .../corbits-skills/skills/native-integration/SKILL.md | 2 ++ src/agent/directors/builder/package.test.ts | 10 ++++++++++ src/agent/directors/builder/package.ts | 4 ++++ src/agent/directors/skywalker/package.test.ts | 11 +++++++---- src/agent/directors/skywalker/package.ts | 10 +++++----- tests/unit/corbits-skills-catalog.test.ts | 2 ++ 9 files changed, 33 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d1f3f7b2..3ccf81a0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename ### Changed - Drop unused `@opentui/keymap`, `@opentui/solid`, and `solid-js`. The interactive TUI is imperative `@opentui/core` only. +- Require a counsel / /plan plan before substantial Builder work. Skywalker feature chain is explorer then plan then implement then critic. Tiny parent-DIY stays plan-optional. /implement does not steal planning from /plan. - Align /create-issue phases and # Background / # Outcome format with GaaS linear-create. Slash stays /create-issue. Tracker selection stays (Linear MCP first, else GitHub/GitLab/MEMORY.md). - Restore the /review skill body 1:1 with GaaS code-review. Slash stays /review. GitHub posting, Linear In Review, ask_operator, and findings-only stay on native-integration. - Restore the scribe skill 1:1 with GaaS. ask_operator mapping stays on native-integration. Slash /scribe remains. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index c21983c17..e82f4253c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -415,7 +415,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 are **how-to playbooks** (steps, done-definition) — not director personas and not fleet routers. Identity and who-does-what live on director system prompts. Default slashes: `/implement`, `/plan`, `/refactor`, `/review`, `/pull-request-review`, `/create-issue`, `/scribe`, `/interview`, `/ast-grep`. `/review` is how to review a branch (base, scope, signal); `/pull-request-review` is worktree checkout then the review skill; `/scribe` is how to maintain PRODUCT / ARCHITECTURE / IMPLEMENTATION; `/implement` is the per-commit greybeard → implement → gate → critic loop; `/plan` authors an eng change plan and does not implement or file tickets; `/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`. The less-is-more quality bar (`idiot-proof`) is baked into builder and critic; it is not a slash. There is no first-party dispatch skill — Skywalker orchestrates natively. Draper and emil are closed directors via `spawn_agent(agent=…)`, not slashes. There is no catch-all worker. The operator types the slash; the primary follows the playbook. +Primary is Skywalker. Bundled skill bodies are **how-to playbooks** (steps, done-definition) — not director personas and not fleet routers. Identity and who-does-what live on director system prompts. Default slashes: `/implement`, `/plan`, `/refactor`, `/review`, `/pull-request-review`, `/create-issue`, `/scribe`, `/interview`, `/ast-grep`. `/review` is how to review a branch (base, scope, signal); `/pull-request-review` is worktree checkout then the review skill; `/scribe` is how to maintain PRODUCT / ARCHITECTURE / IMPLEMENTATION; `/implement` is the per-commit greybeard → implement → gate → critic loop and does not steal planning from `/plan`; substantial Builder work consumes a counsel / `/plan` plan first; `/plan` authors an eng change plan and does not implement or file tickets; `/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`. The less-is-more quality bar (`idiot-proof`) is baked into builder and critic; it is not a slash. There is no first-party dispatch skill — Skywalker orchestrates natively. Draper and emil are closed directors via `spawn_agent(agent=…)`, not slashes. There is no catch-all worker. The operator types the slash; the primary follows the playbook. #### Discovery and precedence diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index 85c527da5..7d127e653 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -105,7 +105,7 @@ recovery line instead of dumping the file path and parse details. The TUI has an extensible slash-command framework. Built-ins include `/help` (shortcut + command overlay), `/model` (models-only picker for connected accounts; **Alt+A** or `/connect` adds a provider), `/settings`, `/permissions`, `/plugins`, `/clear`, `/new`, `/mcp` (enable, disable, or remove servers), 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 how-to playbook — the slash sends the skill body to the primary, which follows the steps. Skills do not assign identity or route the fleet; that stays on director system prompts. `/review` is how to review a branch; `/scribe` is how to maintain PRODUCT / ARCHITECTURE / IMPLEMENTATION; `/implement` is the per-commit review/build/critique loop; `/plan` authors an eng change plan (files, AC, non-goals, risks, ordered steps) and does not implement. `/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`). There is no first-party dispatch skill — Skywalker orchestrates natively. `git-rebase`, `linear-issue-workflow`, `style`, `philosophy`, `native-integration`, `typescript`, and `opsh` stay `use_skill` only (`user-invocable: false`). The less-is-more quality bar (`idiot-proof`) is baked into builder and critic — not a slash and not listed for `use_skill`. Draper and emil are not slashes; they remain closed directors via `spawn_agent(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 how-to playbook — the slash sends the skill body to the primary, which follows the steps. Skills do not assign identity or route the fleet; that stays on director system prompts. `/review` is how to review a branch; `/scribe` is how to maintain PRODUCT / ARCHITECTURE / IMPLEMENTATION; `/implement` is the per-commit review/build/critique loop — it does not steal planning from `/plan`. Substantial Builder work consumes a counsel / `/plan` plan first; tiny parent-DIY stays plan-optional. `/plan` authors an eng change plan (files, AC, non-goals, risks, ordered steps) and does not implement. `/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`). There is no first-party dispatch skill — Skywalker orchestrates natively. `git-rebase`, `linear-issue-workflow`, `style`, `philosophy`, `native-integration`, `typescript`, and `opsh` stay `use_skill` only (`user-invocable: false`). The less-is-more quality bar (`idiot-proof`) is baked into builder and critic — not a slash and not listed for `use_skill`. Draper and emil are not slashes; they remain closed directors via `spawn_agent(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** or `/connect` 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, Ollama, 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. diff --git a/plugins/corbits-skills/skills/native-integration/SKILL.md b/plugins/corbits-skills/skills/native-integration/SKILL.md index 7b94a09fc..a1b327ffa 100644 --- a/plugins/corbits-skills/skills/native-integration/SKILL.md +++ b/plugins/corbits-skills/skills/native-integration/SKILL.md @@ -51,6 +51,8 @@ GaaS scribe uses the `question` tool. Corbits extras: `ask_operator` (tool mappi When GaaS implement says you are orchestrated by karen, that is the Corbits primary (Skywalker). Route those disposition decisions through the primary, not a worker. +GaaS implement "Initial Planning" / Greybeard-before-code is not `/plan`. Substantial Builder work consumes a counsel / `/plan` plan (files, acceptance criteria, non-goals, risks, ordered steps) and blocks if that plan is missing. Tiny parent-DIY stays plan-optional. `/plan` and counsel author; they do not ship. `/implement` does not steal planning from `/plan`. Do not fork the GaaS implement body. + ## Linear claim-first When the work tracks a Linear issue and Linear MCP is available: set the issue to In Progress before explore/build thrash. Parallel lanes claim their own IDs. When a PR is ready for review, move the issue to In Review — never Done at PR-open. If Linear MCP is unavailable, report that status could not be updated. diff --git a/src/agent/directors/builder/package.test.ts b/src/agent/directors/builder/package.test.ts index 6d71fa93e..94c3de570 100644 --- a/src/agent/directors/builder/package.test.ts +++ b/src/agent/directors/builder/package.test.ts @@ -50,6 +50,16 @@ describe("builderPackage", () => { expect(p).toMatch(/never silently skip/i); }); + test("systemPrompt requires a counsel / /plan plan for substantial work", () => { + const p = builderPackage.systemPrompt; + expect(p).toContain("## Plan"); + expect(p).toContain("counsel / `/plan` plan"); + expect(p).toContain("If that plan is missing from the brief"); + expect(p).toContain("do not invent one and do not ship"); + expect(p).toContain("Tiny parent-DIY edits are plan-optional"); + expect(p).toContain("`/implement` does not steal planning from `/plan`"); + }); + test("systemPrompt requires style, philosophy, and idiot-proof prerequisites", () => { const p = builderPackage.systemPrompt; expect(p).toContain("Prerequisites"); diff --git a/src/agent/directors/builder/package.ts b/src/agent/directors/builder/package.ts index 066264dc2..ffe913d0d 100644 --- a/src/agent/directors/builder/package.ts +++ b/src/agent/directors/builder/package.ts @@ -32,6 +32,10 @@ You are a disciplined implementer worker (maySpawn:false) — not Critic, not Ex Before substantial repo work: follow style, philosophy, native-integration, and idiot-proof (baked; use_skill is not mounted). Follow AGENTS.md and /docs. Apply typescript when writing TypeScript. +## Plan + +Substantial work consumes a counsel / \`/plan\` plan: files/paths, acceptance criteria, non-goals, risks, and ordered steps. If that plan is missing from the brief, do not invent one and do not ship — report Blockers for the parent. Tiny parent-DIY edits are plan-optional and are not this worker. \`/plan\` and counsel author the plan; they do not ship. \`/implement\` does not steal planning from \`/plan\`. + ## Implement and Test The order of operations depends on whether you're fixing a bug or building a feature. In both cases, follow the repository's existing test conventions — look at how existing tests are structured, where they live, what framework they use, and match that style. If the repository has no existing tests, put that under Blockers for the parent (if blocked or ambiguous, ask_director; after the cap, report Blockers). diff --git a/src/agent/directors/skywalker/package.test.ts b/src/agent/directors/skywalker/package.test.ts index c2f4f86fb..c965cbdf5 100644 --- a/src/agent/directors/skywalker/package.test.ts +++ b/src/agent/directors/skywalker/package.test.ts @@ -142,9 +142,9 @@ describe("skywalkerPackage", () => { 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 explorer, skip critic"); + expect(p).toContain("skip spawn, skip explorer, skip plan, skip critic"); expect(p).toContain("write_file/edit_file"); - expect(p).toContain("Do not always explorer→implement→critic"); + expect(p).toContain("Do not always explorer→plan→implement→critic"); }); test("systemPrompt routes URL reads through web_fetch on primary", () => { @@ -220,8 +220,11 @@ describe("skywalkerPackage", () => { 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("explorer → implement → critic"); - expect(p).toContain("Do not always explorer→implement→critic"); + expect(p).toContain("explorer → plan → implement → critic"); + expect(p).toContain("Do not always explorer→plan→implement→critic"); + expect(p).toContain("Substantial builder work consumes a counsel"); + expect(p).toContain("Tiny parent-DIY edits stay plan-optional"); + expect(p).toContain("`/implement` does not steal planning from `/plan`"); }); test("systemPrompt re-dispatches builder on blocking critic", () => { diff --git a/src/agent/directors/skywalker/package.ts b/src/agent/directors/skywalker/package.ts index 9613b6135..60284e49e 100644 --- a/src/agent/directors/skywalker/package.ts +++ b/src/agent/directors/skywalker/package.ts @@ -29,7 +29,7 @@ You are the chat surface. Workers cannot ask_operator; they ask_director. When w Example chains: - tiny fix: DIY write_file/edit_file (do not spawn) -- feature: explorer → implement → critic +- feature: explorer → plan → 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 is a spawn agent= target): builder, explorer, counsel, intern, critic, greybeard, neckbeard, bruckheimer, gaasbot, draper, emil, rand, shakespeare, testsmith, tester. @@ -67,7 +67,7 @@ When the operator (or brief) gives an http(s) URL to read: Scale fan-out to the ask — the runtime queues excess rather than refusing: - 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 explorer, skip critic. Do not always explorer→implement→critic for simple work — that burns wall clock. +- Tiny single-file / one-route asks: **DIY on the parent** with write_file/edit_file; skip spawn, skip explorer, skip plan, skip critic. Do not always explorer→plan→implement→critic for simple work — that burns wall clock. - Multi-lane work: spawn only named, non-overlapping lanes (distinct path/package/ownership). Width follows independent lanes. Do not invent a numeric cap. # Anti-cascade (stall / dig / diagnose) @@ -107,14 +107,14 @@ Before responding, classify: Tiny / single-file / one-route / clear bounded edit: write_file/edit_file/delete_file on this session. Do not spawn. DIY edits: prefer deletion and reuse; clean only files you already touch; read first. -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. +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. Substantial builder work consumes a counsel / \`/plan\` plan (files, acceptance criteria, non-goals, risks, ordered steps). If that plan is missing, spawn counsel (or wait for \`/plan\`) before builder — put the plan in the builder brief. Builder blocks if the plan is still missing. Tiny parent-DIY edits stay plan-optional. \`/implement\` does not steal planning from \`/plan\`. 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 explorer workers for scope when needed. 3. Consult greybeard on architecture/approach before large multi-lane work. -4. Use counsel for multi-lane eng plans; clarify before a large fan-out. +4. Use counsel / \`/plan\` for the eng plan substantial builder work consumes; they do not ship. \`/implement\` does not steal planning from \`/plan\`. Clarify before a large fan-out. 5. Track progress with manage_tasks; synthesize results for the operator. ## If ORCHESTRATION → coordinate @@ -131,7 +131,7 @@ Do not reclassify COMMUNICATION as ORCHESTRATION just to justify parallel spawn - 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 counsel for multi-lane eng plans; clarify before a large fan-out. +- Use counsel / \`/plan\` for the eng plan substantial builder work consumes; they do not ship. \`/implement\` does not steal planning from \`/plan\`. Clarify before a large fan-out. - Path tools are the DIY surface; shell file-writes stay denied. Track fleet work with manage_tasks. - When claiming Linear work: set the issue to In Progress via Linear MCP as a hard first step before explore/build thrash. Parallel lanes claim their own IDs. When a PR is ready for review, move the issue to In Review — never Done at PR-open. If Linear MCP is unavailable, report that status could not be updated. - Optional skills when needed on the primary session: style, philosophy, native-integration, interview (use_skill is primary-mounted). diff --git a/tests/unit/corbits-skills-catalog.test.ts b/tests/unit/corbits-skills-catalog.test.ts index d1d8e998c..ab049a59e 100644 --- a/tests/unit/corbits-skills-catalog.test.ts +++ b/tests/unit/corbits-skills-catalog.test.ts @@ -451,6 +451,8 @@ test("native-integration maps GaaS tool names and parks Corbits extras", async ( expect(skill).toContain("Do not fork the GaaS ast-grep body"); expect(skill).toContain("Do not fork the GaaS code-review body"); expect(skill).toContain("findings-only"); + expect(skill).toContain("Do not fork the GaaS implement body"); + expect(skill).toContain("`/implement` does not steal planning from `/plan`"); expect(skill).toContain("run `sg` via `run_shell`"); expect(skill).toContain("prove"); });