From dff33aa2753ee5f4a06db39e745ddc704c1ab84c Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 25 Aug 2026 07:39:46 -0700 Subject: [PATCH 1/7] Rewrite first-party skills as how-to playbooks Skills are procedures the primary follows, not persona prompts. Identity stays on director system prompts. --- CHANGELOG.md | 5 ++++ docs/ARCHITECTURE.md | 2 +- docs/PRODUCT.md | 2 +- .../corbits-skills/skills/ast-grep/SKILL.md | 4 ---- .../skills/create-issue/SKILL.md | 4 ++-- .../corbits-skills/skills/dispatch/SKILL.md | 14 +++++------ .../corbits-skills/skills/git-rebase/SKILL.md | 23 ++++++++----------- .../skills/git-worktrees/SKILL.md | 2 +- .../corbits-skills/skills/implement/SKILL.md | 12 +++++----- .../skills/linear-issue-workflow/SKILL.md | 10 ++++---- plugins/corbits-skills/skills/opsh/SKILL.md | 6 ++--- plugins/corbits-skills/skills/plan/SKILL.md | 15 ++++++------ .../skills/pull-request-review/SKILL.md | 14 +++++------ .../corbits-skills/skills/refactor/SKILL.md | 14 +++++------ plugins/corbits-skills/skills/review/SKILL.md | 18 +++++++-------- plugins/corbits-skills/skills/scribe/SKILL.md | 12 ++++++---- tests/unit/corbits-skills-catalog.test.ts | 14 +++++++++-- 17 files changed, 89 insertions(+), 82 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72b9ae9ff..2ebc7f293 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename ## [Unreleased] +### Changed + +- First-party skills are how-to playbooks (what to do, in what order, what done looks like), + not director personas. Identity stays on director system prompts. + ### Fixed - One-shot confirmation flashes (copy, mouse toggle, attach results, reasoning effort, stall recovery) now clear themselves after a short TTL. Rate-limit waits no longer park on the bottom notice row; the durable error stays in the transcript. Live stall notice and landing hold still omit a TTL so they stay until replaced. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 20e93e6a0..4abec9f83 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -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 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. +Primary is Skywalker. Bundled skill bodies that are operator slashes are **how-to playbooks** (steps, routing, done-definition) — not director personas. Action skills tell the primary to `task(agent="")` — there is no catch-all worker. Identity lives on director system prompts. 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 diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index eaae83aa6..996e3351b 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 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. +**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 (often `task(agent="")`). Skills do not assign identity; that stays on director system prompts. `/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. diff --git a/plugins/corbits-skills/skills/ast-grep/SKILL.md b/plugins/corbits-skills/skills/ast-grep/SKILL.md index 11b938a01..7972cab9b 100644 --- a/plugins/corbits-skills/skills/ast-grep/SKILL.md +++ b/plugins/corbits-skills/skills/ast-grep/SKILL.md @@ -422,7 +422,3 @@ ast-grep handles the bulk structural transformation. Use manual edits for: - One-off fixups after a bulk rewrite (e.g., adjusting a special case that the pattern caught incorrectly) The ideal workflow for a large refactor: ast-grep for the mechanical bulk, manual edits for the exceptions, build verification to confirm everything holds together. - -## Acknowledgment - -After reviewing this skill, state: "I have reviewed the ast-grep skill." diff --git a/plugins/corbits-skills/skills/create-issue/SKILL.md b/plugins/corbits-skills/skills/create-issue/SKILL.md index acbe31aa7..760c5e781 100644 --- a/plugins/corbits-skills/skills/create-issue/SKILL.md +++ b/plugins/corbits-skills/skills/create-issue/SKILL.md @@ -6,9 +6,9 @@ argument-hint: "[description] [--from-doc]" # Create Issue -You are Skywalker. Host is Corbits. Slash `/create-issue` and `use_skill("create-issue")`. Clarifying questions use `ask_operator`. Do not invent Claude-only tools. Do not invent a Linear REST client. Do not restate MCP tool names or schemas — use the mounted Linear MCP tools as they appear in your toolset. +How to create well-structured issues (and, on Linear, projects / project updates / initiatives when the operator asks). Tracker selection first, then quality phases, then create. -Create well-structured issues (and, on Linear, projects / project updates / initiatives when the operator asks). Tracker selection first, then quality phases, then create. +Clarifying questions use `ask_operator`. Do not invent Claude-only tools. Do not invent a Linear REST client. Do not restate MCP tool names or schemas — use the mounted Linear MCP tools as they appear in the toolset. ## Tracker selection diff --git a/plugins/corbits-skills/skills/dispatch/SKILL.md b/plugins/corbits-skills/skills/dispatch/SKILL.md index c3134defd..f346b1034 100644 --- a/plugins/corbits-skills/skills/dispatch/SKILL.md +++ b/plugins/corbits-skills/skills/dispatch/SKILL.md @@ -2,14 +2,14 @@ name: dispatch user-invocable: false argument-hint: "[ | dispatch// | dispatch//dispatch.yaml | ]" -description: Multi-lane DAG orchestration. Skywalker recipe — use_skill("dispatch"). Spawns explorer, intern, builder, counsel, and critic. DAG product tasks go through builder; Skywalker may DIY tiny edits outside the DAG. +description: Multi-lane DAG orchestration via use_skill("dispatch"). Spawns explorer, intern, builder, counsel, and critic. Use for DAG product work; tiny edits outside the DAG may be DIY. --- # Dispatch -You are Skywalker. This skill is loadable with `use_skill("dispatch")`. Follow this recipe. DAG product tasks go through builder workers. 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. +How to orchestrate parallel director runs across a dependency graph. Fan out work, fan in reports, critique, verify, re-dispatch fixes, and synthesize until done. -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 workers. Do not write `dispatch.yaml` or `plan.md` on the parent (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. Default batch size: **4 live workers** (recipe default — not a hard runtime cap). Widen when lanes are named and non-overlapping, or when the operator asks. Track progress with `manage_tasks`. @@ -39,7 +39,7 @@ If the spec is vague, incomplete, or contradictory: stop and report Blockers. Do | 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 builder — 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. +Classify, spawn, track, and synthesize. 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 builder — 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 the parent. Prefer typed briefs: `intent`, `success_criteria`, `do_not`, `report_focus`, and `agent`. @@ -79,7 +79,7 @@ If requirements are not actionable, stop. Ask: "Can a builder worker succeed wit ## Phase 2: Directory structure -Have **builder** 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; no product feature work). Do not write these files on the parent. Do not use intern — intern cannot write files. Do not use a catch-all worker. ``` dispatch/ @@ -183,7 +183,7 @@ If the working tree has unrelated uncommitted changes before Phase 4, ask the op ## 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. +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 `run_shell`. Compare against any baseline you captured. - Green, or same failures as baseline → proceed. - New failures → attribute to a task/commit, re-dispatch `builder` on that lane, re-verify. Cap rounds, then Blockers. @@ -209,7 +209,7 @@ 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 builder. +- Spawn directors. Do not implement product features. Do not author dispatch YAML/plan files on the parent or via a catch-all worker. Durable orchestration files go through builder. - `use_skill("dispatch")` loads this recipe. It is a command. - Agents: `explorer`, `intern`, `builder` only for DAG nodes. Critique via `task(agent="critic")`. Plan via `task(agent="counsel")` when a spec needs an eng plan first. - Progress: `manage_tasks`. diff --git a/plugins/corbits-skills/skills/git-rebase/SKILL.md b/plugins/corbits-skills/skills/git-rebase/SKILL.md index a5ae54910..1c4dbeddb 100644 --- a/plugins/corbits-skills/skills/git-rebase/SKILL.md +++ b/plugins/corbits-skills/skills/git-rebase/SKILL.md @@ -1,27 +1,24 @@ --- name: git-rebase user-invocable: false -description: Reshape git history with rebase — edit-in-place, squash/fixup, drop, split, reword, or validate every replayed commit. Skywalker plans; intern executes sequenced non-interactive git via run_shell. Load whenever a commit that is not HEAD needs changing, or for branch-history cleanup before push. +description: Reshape git history with rebase — edit-in-place, squash/fixup, drop, split, reword, or validate every replayed commit. Plan the surgery; intern executes sequenced non-interactive git via run_shell. Load whenever a commit that is not HEAD needs changing, or for branch-history cleanup before push. --- # git-rebase -You are Skywalker. Host is Corbits Code. This skill is a spawn recipe. You do not run the rebase. You do not edit files or run git yourself. - -Use this skill when a branch's commit history needs rewriting — squashing fixups, dropping wrong-turn commits, splitting bundled changes, rewording messages — without interactive prompts. +How to reshape a branch's commit history without interactive prompts — squashing fixups, dropping wrong-turn commits, splitting bundled changes, rewording messages. Do not run the rebase or edit files on the parent. `git rebase -i` is normally driven through an interactive editor. The techniques below drive every editor invocation programmatically so the rebase runs to completion without a human at the keyboard. Scripted rebases are reproducible, re-runnable, and self-documenting in a way that vim-driven ones never are. -## Skywalker recipe +## Recipe 1. Read the techniques below. Identify the surgery (drop, squash, split, reword, edit-in-place, validate). 2. If a step needs judgment (what to squash, which commits to drop, how to split, which message), `ask_operator` first. Do not guess. 3. Copy the exact sequenced commands from this skill into an intern brief. -4. Spawn `task(agent="intern")` with that sequenced command list. Intern executes via `run_shell` (there is no Bash tool). Intern drives every editor via `GIT_SEQUENCE_EDITOR` / `-c sequence.editor` / `-c core.editor` inline in the git command — do not tell intern to `write_file` an editor script. Intern runs git and resolves mechanical conflicts as the brief specifies. - +4. Spawn `task(agent="intern")` with that sequenced command list. Intern executes via `run_shell`. Intern drives every editor via `GIT_SEQUENCE_EDITOR` / `-c sequence.editor` / `-c core.editor` inline in the git command — do not tell intern to `write_file` an editor script. Intern runs git and resolves mechanical conflicts as the brief specifies. 5. If intern hits a judgment call mid-rebase, `ask_operator` then re-dispatch intern with the decision. -Skywalker synthesizes. Intern mutates git. +Plan and synthesize. Intern mutates git. ## Techniques (copy into the intern brief) @@ -549,12 +546,12 @@ need to `git add` and continue, but you don't re-do the resolution work. ## Workflow: a rebase session start to finish -Intern executes this sequence via `run_shell`. Skywalker does not run git. -If a step needs judgment, Skywalker `ask_operator`s first, then copies the -decision into the intern brief. +Intern executes this sequence via `run_shell`. Do not run git on the parent. +If a step needs judgment, `ask_operator` first, then copy the decision into +the intern brief. 1. **Identify what needs fixing.** Intern reads `git log --oneline origin/main..HEAD` - and the diffs. Skywalker lists the surgery: drops, rewords, squashes, + and the diffs. List the surgery: drops, rewords, squashes, splits, in-place amends — `ask_operator` when which-commits is a judgment call. 2. **Branch your way back.** Intern: @@ -570,7 +567,7 @@ decision into the intern brief. 4. **For each operation, intern:** - Runs the rebase with `GIT_SEQUENCE_EDITOR` / `-c sequence.editor` inline (if scripting the todo). Do not `write_file` an editor script. - Supplies pre-canned commit messages via `-c core.editor` / `GIT_EDITOR` inline (if rewording), or `git commit --amend -m`. - - Resolves conflicts as they arise (mechanical). Judgment → intern stops; Skywalker `ask_operator`s. + - Resolves conflicts as they arise (mechanical). Judgment → intern stops; `ask_operator` then re-brief intern. - When the rebase finishes, runs `git diff backup--pre-rebase HEAD`. If the intent was to change content, the diff is meaningful and intern reports it. If the intent was only to reshape history, the diff is empty. diff --git a/plugins/corbits-skills/skills/git-worktrees/SKILL.md b/plugins/corbits-skills/skills/git-worktrees/SKILL.md index c87702595..2c95d1acb 100644 --- a/plugins/corbits-skills/skills/git-worktrees/SKILL.md +++ b/plugins/corbits-skills/skills/git-worktrees/SKILL.md @@ -7,7 +7,7 @@ description: Create a git worktree from origin/ and tear it down # 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. +How to create a worktree from origin/ and tear it down. Load via `use_skill("git-worktrees")` and copy commands into an intern brief. Intern executes via `run_shell`. Do not run the git on the parent. ## Create from origin/ diff --git a/plugins/corbits-skills/skills/implement/SKILL.md b/plugins/corbits-skills/skills/implement/SKILL.md index 90ba786aa..31fa1aee6 100644 --- a/plugins/corbits-skills/skills/implement/SKILL.md +++ b/plugins/corbits-skills/skills/implement/SKILL.md @@ -1,15 +1,15 @@ --- name: implement -description: Disciplined per-commit workflow. Skywalker spawn recipe — greybeard, builder, intern/tester, critic. +description: Disciplined per-commit workflow. Sequential spawn loop — greybeard, builder, intern/tester, critic. Use for substantial commit-sized landings, not tiny bounded edits. --- # Implement -You are Skywalker. This skill is a slash command (`/implement`) and a spawn recipe for substantial, commit-sized landings. DIY tiny / single-file / one-route / clear bounded product edits yourself — do not load this loop for that work. +How to land substantial, commit-sized work. Tiny / single-file / one-route / clear bounded product edits: DIY — do not load this loop. -When this recipe runs: spawn directors, wait for reports, decide the next spawn from those reports. The loop is sequential by design (one unit at a time). Do not invent a worker-count or fan-out ceiling. Track units with `manage_tasks`. +When this recipe runs: spawn directors, wait for reports, decide the next spawn from those reports. Sequential by design (one unit at a time). Do not invent a worker-count or fan-out ceiling. Track units with `manage_tasks`. -Closed directors used here: `greybeard`, `builder`, `intern`, `tester`, `critic`. Never a catch-all worker. +Closed directors: `greybeard`, `builder`, `intern`, `tester`, `critic`. Never a catch-all worker. ## Prerequisites @@ -22,7 +22,7 @@ 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 critic is clean and the build gate passed: mark it done. -- New work that surfaces (prep refactor, edge case warranting its own commit) → append a `manage_tasks` item and run the full loop. +- New work that surfaces → append a `manage_tasks` item and run the full loop. ## Per-commit spawn loop @@ -69,7 +69,7 @@ When critic is clean (or remaining findings are acknowledged judgment calls), ma - Track only with `manage_tasks`. - Do not shortcut the loop. Skipping greybeard "because this is simple" or critic "because the build passed" defeats the recipe. - Build must pass before treating a unit as done. -- No invented worker-count or fan-out ceiling. +- Do not invent a worker-count or fan-out ceiling. ## Report diff --git a/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md b/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md index a333d9fb1..9c7786aa5 100644 --- a/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md +++ b/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md @@ -1,13 +1,13 @@ --- 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 builder for substantial landings. +description: Implement 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 ]" --- # Linear Issue Workflow -You are Skywalker. Host is Corbits Code. This skill is a spawn recipe for substantial issue work. Tiny / single-file / one-route / clear bounded product edits: DIY with write_file/edit_file/delete_file. Substantial landings: Linear MCP on the primary, then the `/implement` spawn loop. +How to land a Linear issue. Tiny / single-file / one-route / clear bounded product edits: DIY with write_file/edit_file/delete_file. Substantial landings: Linear MCP on the primary, then the `/implement` spawn loop. If Linear MCP (`mcp__linear__*`) is missing, stop and tell the operator. Do not invent Claude-only tools. @@ -21,7 +21,7 @@ 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). -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. +Load `use_skill("git-worktrees")`. Copy the create-from-origin/ recipe into an intern brief (substitute ``). Spawn `task(agent="intern")`. Intern executes; do not run the git on the parent. 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. @@ -44,7 +44,7 @@ If intern fails, stop and `ask_operator`. If the operator rejects the issue befo ## Phase 4: Implement — spawn loop -Do not implement on Skywalker. For each commit-sized unit, run `/implement`: +Do not implement on the parent. For each commit-sized unit, run `/implement`: 1. `task(agent="greybeard")` on the approach before any code is written. 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. @@ -69,7 +69,7 @@ After the last unit's critique is clean, spawn `task(agent="critic")` on the **w Fix-every-finding: treat surviving findings as a worklist and re-enter Phase 4 for each. Cap three whole-branch re-reviews. If findings remain, `ask_operator` — do not push. -The only path to leaving a finding unfixed is a greybeard waiver: `task(agent="greybeard")` with the finding, proposed disposition, and relevant diff. Accept the ruling by default. Escalate with `ask_operator` only if you disagree or greybeard is unreachable. Never waive on Skywalker's own authority. +The only path to leaving a finding unfixed is a greybeard waiver: `task(agent="greybeard")` with the finding, proposed disposition, and relevant diff. Accept the ruling by default. Escalate with `ask_operator` only if you disagree or greybeard is unreachable. Never waive on the parent's own authority. ## Phase 6: Push and PR — intern, after confirmation diff --git a/plugins/corbits-skills/skills/opsh/SKILL.md b/plugins/corbits-skills/skills/opsh/SKILL.md index 4f419c270..09c6c667c 100644 --- a/plugins/corbits-skills/skills/opsh/SKILL.md +++ b/plugins/corbits-skills/skills/opsh/SKILL.md @@ -6,11 +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 builder with this skill's rules copied into the brief (workers do not mount `use_skill`). +How to write scripts with opsh and its libraries. Tiny / single-file scripts: DIY with write_file/edit_file using these rules. Substantial script work: spawn `task(agent="builder")` with these rules copied into the brief (workers do not mount `use_skill`). For a review, spawn `task(agent="critic")` (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. +Shell for agent commands is `run_shell`. Bash-the-language in the examples below stays — opsh scripts are bash. opsh is a scripting environment for operations use. It is a curated bash environment with sensible defaults and a standard library. Scripts are diff --git a/plugins/corbits-skills/skills/plan/SKILL.md b/plugins/corbits-skills/skills/plan/SKILL.md index 902a3503c..39ae580c0 100644 --- a/plugins/corbits-skills/skills/plan/SKILL.md +++ b/plugins/corbits-skills/skills/plan/SKILL.md @@ -1,16 +1,17 @@ --- name: plan -description: Skywalker spawn recipe — counsel director authors an agent-proof eng change plan. Does not implement. Does not file tracker issues. +description: Author an agent-proof eng change plan via counsel. Use when the operator wants a plan, not code or tracker tickets. --- # Plan -You are Skywalker. This skill is a spawn recipe. You do not write the plan yourself. +How to produce an engineering change plan. Does not implement. Does not file tracker issues. -Spawn `task(agent="counsel")` with the operator args as the brief. Prefer a typed spawn: `intent="plan"`, `success_criteria`, `do_not`, `report_focus`. +## Steps -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. +1. If the change target is too fuzzy to brief, `ask_operator` first. +2. Spawn `task(agent="counsel")` with the operator args as the brief. Prefer a typed spawn: `intent="plan"`, `success_criteria`, `do_not`, `report_focus`. +3. Counsel authors files, acceptance criteria, non-goals, risks, and ordered steps. It does not ship code. +4. Return counsel's report. Greybeard is the architecture gate — not this skill. -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 counsel. +Not `/create-issue`. If the operator wants tickets, they use `/create-issue` after the plan. diff --git a/plugins/corbits-skills/skills/pull-request-review/SKILL.md b/plugins/corbits-skills/skills/pull-request-review/SKILL.md index f1cdc6cd3..e90412f40 100644 --- a/plugins/corbits-skills/skills/pull-request-review/SKILL.md +++ b/plugins/corbits-skills/skills/pull-request-review/SKILL.md @@ -1,11 +1,11 @@ --- name: pull-request-review -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. +description: Review a pull request by branch name or URL. Intern checks out a worktree if needed; critic (or neckbeard) reviews. Does not implement fixes. --- # Pull Request Review -You are Skywalker. Host is Corbits Code. This skill is a spawn recipe. Do not implement fixes as part of the review. Do not impersonate GitHub-Claude (or any other vendor) review comments. +How to review a pull request given a branch name or URL. Do not implement fixes as part of the review. Do not impersonate GitHub-Claude (or any other vendor) review comments. ## Input @@ -14,11 +14,11 @@ Accepts either: - A branch name (e.g. `feature/add-auth`) - A pull request URL from GitHub or GitLab (e.g. `https://github.com/owner/repo/pull/123`) -## Recipe +## Steps ### 1. Parse input -If given a URL, intern extracts the branch via `run_shell` (there is no Bash tool): +If given a URL, intern extracts the branch via `run_shell`: ```bash # GitHub @@ -30,7 +30,7 @@ glab mr view --output json | jq -r '.source_branch' ### 2. Worktree checkout if needed -If the PR branch is not already the current checkout, spawn `task(agent="intern")` with this sequenced `run_shell` list copied into the brief. Intern executes; Skywalker does not run the git. +If the PR branch is not already the current checkout, spawn `task(agent="intern")` with this sequenced `run_shell` list copied into the brief. Intern executes; do not run the git on the parent. ```bash git rev-parse --show-toplevel @@ -85,7 +85,7 @@ Prefer a typed brief: `intent="review"`, `success_criteria`, `do_not`, `report_f 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. +If a GitHub review must be posted, intern runs `gh pr review` as the operator's `gh` identity — never as a vendor bot. Primary owns `--approve` / `--request-changes` only when the operator asked to post; secondary lenses use `--comment` only. If the operator then wants repairs, that is a later `/implement` or `use_skill("dispatch")` — not this skill. @@ -102,6 +102,6 @@ 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 builder for substantial fixes. -- Skywalker MUST NOT run the worktree git; intern does, via `run_shell`. +- Do not run the worktree git on the parent; 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 c46878135..7f2f792cf 100644 --- a/plugins/corbits-skills/skills/refactor/SKILL.md +++ b/plugins/corbits-skills/skills/refactor/SKILL.md @@ -1,16 +1,16 @@ --- name: refactor argument-hint: -description: Skywalker maps a directory then plans improvements. Explore, then plan. Does not ship product code. +description: Map a directory then plan improvements. Explore, then plan. Does not ship product code. --- # Refactor -You are Skywalker. This skill is a spawn recipe. You do not write a design document. `$ARGUMENTS` is the directory to analyze. This recipe maps and plans; it does not ship. Tiny / single-file / one-route product edits outside this recipe may be DIY with write_file/edit_file/delete_file. +How to map a directory and plan improvements. `$ARGUMENTS` is the directory. This recipe maps and plans; it does not ship. Tiny / single-file / one-route product edits outside this recipe may be DIY with write_file/edit_file/delete_file. -## Recipe +## Steps -1. Load philosophy via `use_skill("philosophy")` on the primary **before spawning**. Those principles guide how you evaluate design decisions and what you put in worker briefs. +1. Load philosophy via `use_skill("philosophy")` on the primary **before spawning**. Those principles guide how to evaluate design decisions and what goes in worker briefs. 2. If `$ARGUMENTS` is missing or the directory is broad, `ask_operator` before exploring: - Is there a specific concern or area to focus on? - What prompted the desire to refactor? @@ -31,11 +31,11 @@ You are Skywalker. This skill is a spawn recipe. You do not write a design docum - 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. Counsel's report is the artifact. A later `/implement` or `use_skill("dispatch")` ships it. +Do not write the plan to disk. 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 — 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 write the plan to disk or author design documents on this session — counsel's report is the artifact. +- Do not skip explore "because the directory is already known." - Do not skip `ask_operator` when the operator has not chosen among alternatives. - 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 72391d52a..156645668 100644 --- a/plugins/corbits-skills/skills/review/SKILL.md +++ b/plugins/corbits-skills/skills/review/SKILL.md @@ -1,20 +1,18 @@ --- name: review -description: Review a branch, PR, or path scope. Skywalker spawns critic (neckbeard for hygiene, greybeard for architecture); does not implement fixes. +description: Review a branch, PR, or path scope. Routes critic (default), neckbeard (hygiene), or greybeard (architecture). Does not implement fixes. argument-hint: "[paths | PR | diff | hygiene | architecture]" --- # Review -You are Skywalker. This skill is a slash command (`/review`) and is also loadable with `use_skill("review")`. Do not implement fixes. Do not write product patches to "just quickly" address findings. Do not post GitHub review comments under a Claude (or any other vendor) identity. - -Classify the lens, spawn the matching director(s), wait for reports, synthesize. Findings only — never land fixes in this recipe. +How to review a branch, PR, or path. Findings only — do not implement fixes. Do not write product patches to "just quickly" address findings. Do not post GitHub review comments under a vendor identity. ## Routing -- **Default** (correctness, completeness, brief adherence, defects with evidence): `critic` -- **Hygiene-only** (nits, naming, lint, pedantry with receipts): `neckbeard` -- **Architecture-only** (structure, boundaries, approach): `greybeard` +- **Default** (correctness, completeness, brief adherence, defects with evidence): critic +- **Hygiene-only** (nits, naming, lint, pedantry with receipts): neckbeard +- **Architecture-only** (structure, boundaries, approach): greybeard If the operator did not say hygiene-only or architecture-only, spawn critic alone. Do not spawn all three unless they asked for a wider review. @@ -27,11 +25,11 @@ Prefer a typed brief: `intent="review"`, `success_criteria`, `do_not`, `report_f ## Brief to the worker -Include whatever the operator gave you, plus enough for a scoped review: +Include whatever the operator gave, plus: -- Paths, PR number/URL, or branch to review +- Paths, PR number/URL, or branch - Base for comparison when known (`git diff ...HEAD`); if the base is unclear, ask rather than guessing `main` -- That only the operator's scope is in scope — pre-existing issues outside the diff are out of lane +- Only the operator's scope is in scope — pre-existing issues outside the diff are out of lane - Do not implement fixes; findings only, with evidence (`path:line`) - Signal over noise: skip hypotheticals and style nits that do not affect correctness, readability, or maintainability (neckbeard is the exception when hygiene was requested) diff --git a/plugins/corbits-skills/skills/scribe/SKILL.md b/plugins/corbits-skills/skills/scribe/SKILL.md index 0f2fb1786..01be66bdf 100644 --- a/plugins/corbits-skills/skills/scribe/SKILL.md +++ b/plugins/corbits-skills/skills/scribe/SKILL.md @@ -1,14 +1,16 @@ --- name: scribe -description: Skywalker spawn recipe — shakespeare writes PRODUCT.md, ARCHITECTURE.md, and IMPLEMENTATION.md. +description: Update PRODUCT.md, ARCHITECTURE.md, and IMPLEMENTATION.md via shakespeare. Use when those docs need writing or alignment. --- # Scribe -You are Skywalker. This skill is a spawn recipe. Spawn `task(agent="shakespeare")` for PRODUCT.md, ARCHITECTURE.md, and IMPLEMENTATION.md unless the ask is a one-line fix (DIY with write_file/edit_file). +How to maintain PRODUCT.md, ARCHITECTURE.md, and IMPLEMENTATION.md. -Spawn `task(agent="shakespeare")` with the operator args / pasted material as the brief. Shakespeare owns PRODUCT.md, ARCHITECTURE.md, and IMPLEMENTATION.md. +## Steps -Use `ask_operator` if the doc target (P vs A vs I) is ambiguous. +1. If the doc target (P vs A vs I) is ambiguous, `ask_operator`. +2. One-line fix: edit the file directly with write_file/edit_file. +3. Otherwise spawn `task(agent="shakespeare")` with the operator args / pasted material as the brief. -Do not edit those docs yourself except a one-line fix. DESIGN.md is rand, not this skill. +DESIGN.md is rand, not this skill. diff --git a/tests/unit/corbits-skills-catalog.test.ts b/tests/unit/corbits-skills-catalog.test.ts index 9aee4893f..68577b0a8 100644 --- a/tests/unit/corbits-skills-catalog.test.ts +++ b/tests/unit/corbits-skills-catalog.test.ts @@ -128,9 +128,8 @@ test("typescript skill guides TS quality without fake enforcement", async () => expect(skill).not.toMatch(/^### Don't$/m); }); -test("implement skill is a sequential Skywalker spawn recipe without a false 4-cap", async () => { +test("implement skill is a sequential spawn loop without a false 4-cap", async () => { const skill = await Bun.file(join(pluginRoot, "skills/implement/SKILL.md")).text(); - expect(skill).toContain("You are Skywalker"); expect(skill).toContain('task(agent="greybeard")'); expect(skill).toContain('task(agent="builder")'); expect(skill).toContain('task(agent="critic")'); @@ -144,6 +143,17 @@ test("implement skill is a sequential Skywalker spawn recipe without a false 4-c expect(skill).not.toContain("INTERN_TOOLS"); }); +test("first-party skills are how-to playbooks, not director personas", async () => { + for (const name of SKILL_DIRS) { + const skill = await Bun.file(join(pluginRoot, "skills", name, "SKILL.md")).text(); + expect(skill).not.toContain("You are Skywalker"); + expect(skill).not.toMatch(/You are \w+Director/); + expect(skill).not.toContain("Host is Corbits"); + expect(skill).not.toContain("## Acknowledgment"); + expect(skill).not.toMatch(/I have reviewed the .+ skill/); + } +}); + test("style skill is guidance, not ceremony or tool-contract restatement", async () => { const skill = await Bun.file(join(pluginRoot, "skills/style/SKILL.md")).text(); expect(skill).toContain(USER_INVOCABLE_FALSE); From 1fc5bfa9643aee021e519607281af7e13965f11c Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 25 Aug 2026 07:46:28 -0700 Subject: [PATCH 2/7] Restore action skills to the agents how-to playbooks Review is how to review a branch, not how to route directors. Scribe, refactor, and plan follow the same split. --- docs/ARCHITECTURE.md | 2 +- docs/PRODUCT.md | 2 +- .../corbits-skills/skills/implement/SKILL.md | 57 +++-- plugins/corbits-skills/skills/plan/SKILL.md | 23 +- .../skills/pull-request-review/SKILL.md | 81 +++---- .../corbits-skills/skills/refactor/SKILL.md | 85 +++++--- plugins/corbits-skills/skills/review/SKILL.md | 200 ++++++++++++++++-- plugins/corbits-skills/skills/scribe/SKILL.md | 62 +++++- tests/unit/corbits-skills-catalog.test.ts | 31 +-- 9 files changed, 399 insertions(+), 144 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4abec9f83..312b4dfb8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -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 **how-to playbooks** (steps, routing, done-definition) — not director personas. Action skills tell the primary to `task(agent="")` — there is no catch-all worker. Identity lives on director system prompts. 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. +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`. Dispatch is `use_skill` only, not a default slash. Draper and emil are closed directors via `task(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 996e3351b..486637dee 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 how-to playbook — the slash sends the skill body to the primary, which follows the steps (often `task(agent="")`). Skills do not assign identity; that stays on director system prompts. `/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. +**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`). 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. diff --git a/plugins/corbits-skills/skills/implement/SKILL.md b/plugins/corbits-skills/skills/implement/SKILL.md index 31fa1aee6..e40a7e3b2 100644 --- a/plugins/corbits-skills/skills/implement/SKILL.md +++ b/plugins/corbits-skills/skills/implement/SKILL.md @@ -1,79 +1,78 @@ --- name: implement -description: Disciplined per-commit workflow. Sequential spawn loop — greybeard, builder, intern/tester, critic. Use for substantial commit-sized landings, not tiny bounded edits. +description: Disciplined per-commit workflow with greybeard review, a build gate, and a critic loop. Use when each commit should be reviewed and verified before it lands. --- # Implement -How to land substantial, commit-sized work. Tiny / single-file / one-route / clear bounded product edits: DIY — do not load this loop. +How to produce reviewed, verified commits. Load when each commit should go through architectural review, build verification, and code critique before it lands. -When this recipe runs: spawn directors, wait for reports, decide the next spawn from those reports. Sequential by design (one unit at a time). Do not invent a worker-count or fan-out ceiling. Track units with `manage_tasks`. +This is a standalone skill, not part of dispatch. The caller defines what work to do and where the commit boundaries are. This skill defines _how_ each commit gets produced. -Closed directors: `greybeard`, `builder`, `intern`, `tester`, `critic`. Never a catch-all worker. +Tiny / single-file / one-route / clear bounded edits do not need this loop. ## Prerequisites -Load `style` and `philosophy` via `use_skill` on the primary **before spawning**. Copy those conventions into every worker brief (workers do not mount `use_skill`). +Load `style` and `philosophy` first. Follow their conventions throughout. Workers do not mount `use_skill` — copy those conventions into any worker brief. ## Tracking Track commit-sized units with `manage_tasks`. One item per unit that will become a commit. -- Before starting: create an item for each unit from the caller's instructions. -- When a unit begins: mark it in progress. -- When critic is clean and the build gate passed: mark it done. -- New work that surfaces → append a `manage_tasks` item and run the full loop. +- Before starting: create an item for each unit from the caller's instructions +- When a unit begins: mark it in progress +- When critic is clean and the build gate passed: mark it done +- New work that surfaces → append an item and run the full loop -## Per-commit spawn loop +## Per-commit workflow -For each unit, run these steps in order. Do not skip. When this loop is running, do not DIY the unit — spawn builder. +For each unit, run these steps in order. Do not skip. -### 1. Review — greybeard +### 1. Greybeard — approach `task(agent="greybeard")` on the approach before any code is written. Send: what will change and why, files expected, design decisions and trade-offs, uncertainties. -Adjust the plan from the report, then spawn builder. Greybeard is for approach, not execution. +If greybeard identifies problems, adjust before implementing. A different approach deserves a serious look. Disagreement needs a reason. Greybeard is for approach, not execution. -### 2. Implement — builder +### 2. Implement and test -`task(agent="builder")` with a typed brief: `intent`, `success_criteria`, `do_not`, `report_focus`. - -- **Bug fixes:** start from a failing test — write the repro, confirm it fails, fix, confirm it passes. If the test does not fail first, the bug is not understood. +- **Bug fixes:** write a failing repro first, confirm it fails, fix, confirm it passes. If the test does not fail first, the bug is not understood. - **Features:** tests ship with the change. Assert the new behavior, not merely that the process did not crash. +Follow the repository's existing test conventions. If there are no tests, ask what framework to use before proceeding. + Keep scope to this unit. Additional work becomes a later `manage_tasks` item. -### 3. Build gate — intern or tester +### 3. Build gate + +Run the project's full pipeline (`make`, or format / lint / build / test). `task(agent="intern")` for a mechanical full pipeline, or `task(agent="tester")` for suite / repro evidence. -`task(agent="intern")` or `task(agent="tester")` for the project build/test gate (`make`, or the project's full pipeline: format, lint, build, test). +Do not move forward with a broken build. Failures from this unit → fix and re-run. Pre-existing unrelated failures → Blockers and stop. Do not substitute a partial compile for the full gate. -- `intern` — mechanical full pipeline -- `tester` — suite / repro +### 4. Commit -Do not move forward with a broken build. Failures from this unit → re-dispatch builder. Pre-existing unrelated failures → Blockers and stop. Do not substitute a partial compile for the full gate. +Create the commit. Follow `style`. Tests land in the same commit as the implementation. -### 4. Critic +### 5. Critic loop -`task(agent="critic")` on the diff. Include the intent agreed with greybeard so critic evaluates plan vs execution. Limit findings to this unit; pre-existing issues in touched files are out of scope unless they block the gate. +`task(agent="critic")` on `git show HEAD`. Include the intent agreed with greybeard so critic evaluates plan vs execution. Limit findings to this unit. -Blocking findings → re-dispatch builder with those findings in `success_criteria` / `do_not`, then re-run the gate and critic. Close the loop; if still blocked, report Blockers — do not loop forever. +Blocking findings → fix, re-run the gate, land the fix on the right commit (amend HEAD, or edit-in-place via `git-rebase` for an earlier commit), then re-run critic. Close the loop; if still blocked, report Blockers — do not loop forever. When critic is clean (or remaining findings are acknowledged judgment calls), mark the unit done and start the next. ## Non-negotiables -- Tiny / single-file / one-route / clear bounded edits: DIY. This recipe is for substantial units — when running it, spawn builder; do not DIY the coding. -- Spawn `greybeard` → `builder` → `intern`|`tester` → `critic` via `task(agent=…)`. -- Track only with `manage_tasks`. - Do not shortcut the loop. Skipping greybeard "because this is simple" or critic "because the build passed" defeats the recipe. - Build must pass before treating a unit as done. - Do not invent a worker-count or fan-out ceiling. +- Track only with `manage_tasks`. ## Report -When the requested units are done (or blocked), synthesize for the operator: +When the requested units are done (or blocked): ## Summary diff --git a/plugins/corbits-skills/skills/plan/SKILL.md b/plugins/corbits-skills/skills/plan/SKILL.md index 39ae580c0..747b4c9ca 100644 --- a/plugins/corbits-skills/skills/plan/SKILL.md +++ b/plugins/corbits-skills/skills/plan/SKILL.md @@ -1,17 +1,26 @@ --- name: plan -description: Author an agent-proof eng change plan via counsel. Use when the operator wants a plan, not code or tracker tickets. +description: Author an agent-proof eng change plan. Does not implement. Does not file tracker issues. --- # Plan How to produce an engineering change plan. Does not implement. Does not file tracker issues. -## Steps +If the change target is too fuzzy to plan, `ask_operator` first. -1. If the change target is too fuzzy to brief, `ask_operator` first. -2. Spawn `task(agent="counsel")` with the operator args as the brief. Prefer a typed spawn: `intent="plan"`, `success_criteria`, `do_not`, `report_focus`. -3. Counsel authors files, acceptance criteria, non-goals, risks, and ordered steps. It does not ship code. -4. Return counsel's report. Greybeard is the architecture gate — not this skill. +## What the plan must contain -Not `/create-issue`. If the operator wants tickets, they use `/create-issue` after the plan. +1. Files / paths to touch +2. Acceptance criteria mapped from the ask +3. Non-goals +4. Risks and open questions +5. Ordered steps a later `/implement` can execute without guessing + +When requirements are fuzzy, put open questions under Blockers instead of inventing scope. + +## What this is not + +- Not `/create-issue`. If the operator wants tickets, they use `/create-issue` after the plan. +- Not an architecture gate. Greybeard reviews approach; this skill only authors the plan. +- Not implementation. Do not ship the change. diff --git a/plugins/corbits-skills/skills/pull-request-review/SKILL.md b/plugins/corbits-skills/skills/pull-request-review/SKILL.md index e90412f40..efcd0b397 100644 --- a/plugins/corbits-skills/skills/pull-request-review/SKILL.md +++ b/plugins/corbits-skills/skills/pull-request-review/SKILL.md @@ -1,16 +1,14 @@ --- name: pull-request-review -description: Review a pull request by branch name or URL. Intern checks out a worktree if needed; critic (or neckbeard) reviews. Does not implement fixes. +description: Review a pull request by branch name or URL, using a git worktree. Loads the review skill for the actual read. Does not implement fixes. --- # Pull Request Review -How to review a pull request given a branch name or URL. Do not implement fixes as part of the review. Do not impersonate GitHub-Claude (or any other vendor) review comments. +How to review a pull request given a branch name or URL. Do not implement fixes. Do not impersonate GitHub-Claude (or any other vendor) review comments. ## Input -Accepts either: - - A branch name (e.g. `feature/add-auth`) - A pull request URL from GitHub or GitLab (e.g. `https://github.com/owner/repo/pull/123`) @@ -18,7 +16,7 @@ Accepts either: ### 1. Parse input -If given a URL, intern extracts the branch via `run_shell`: +If given a URL, extract the branch: ```bash # GitHub @@ -28,80 +26,87 @@ gh pr view --json headRefName --jq '.headRefName' glab mr view --output json | jq -r '.source_branch' ``` -### 2. Worktree checkout if needed - -If the PR branch is not already the current checkout, spawn `task(agent="intern")` with this sequenced `run_shell` list copied into the brief. Intern executes; do not run the git on the parent. +### 2. Repository root ```bash git rev-parse --show-toplevel +``` + +### 3. Fetch and verify the branch + +```bash git fetch origin git rev-parse --verify "origin/" ``` -If the branch does not exist, intern reports the error. Stop. Do not proceed with worktree creation or review. +If the branch does not exist, report the error and stop. Do not create a worktree or review. + +### 4. Create worktree ```bash REPO_ROOT=$(git rev-parse --show-toplevel) WORKTREE_PATH="${REPO_ROOT}/../worktree/" mkdir -p "${REPO_ROOT}/../worktree" git worktree add "$WORKTREE_PATH" "origin/" +``` + +Path is `../worktree/` relative to the repository root. + +### 5. Checkout + +```bash cd "$WORKTREE_PATH" git checkout git branch --show-current ``` -Worktree path is `../worktree/` relative to the repository root. +Confirm the branch name before continuing. + +### 6. Set up the repository -Then intern follows documented setup in `README.md`, `CONTRIBUTING.md`, `docs/`, `DEVELOPMENT.md`, or `SETUP.md` — install deps, env, migrations, build — exactly as documented. Do not assume the setup process. If no setup docs exist, `ask_operator` before proceeding. +Follow documented setup exactly — `README.md`, `CONTRIBUTING.md`, `docs/`, `DEVELOPMENT.md`, or `SETUP.md`. Typical: install deps, env, migrations, build. -Base branch: +Do not assume the setup process. If no setup docs exist, `ask_operator` before proceeding. + +### 7. Base branch ```bash gh pr view --json baseRefName --jq '.baseRefName' # or glab mr view --output json | jq -r '.target_branch' -# branch-name only: origin/main or origin/master — ask rather than guessing if both exist ``` -If any of these commands fail, intern stops and reports. Do not retry workarounds. `ask_operator` how to proceed. - -### 3. Review +Branch-name only: do not guess if both `main` and `master` exist — `ask_operator`. -- **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")`. +If any command fails, stop, report the error, and `ask_operator`. Do not retry workarounds. -Brief the reviewer: +### 8. Review -- Paths, PR number/URL, or branch -- Base for comparison (`git diff ...HEAD`); if the base is unclear, `ask_operator` rather than guessing `main` -- Only this PR's diff is in scope — pre-existing issues outside the diff are out of lane -- Do not implement fixes; findings only, with evidence (`path:line`) -- Signal over noise (neckbeard is the exception when hygiene was requested) -- Do not write GitHub review comment prose impersonating Claude or any vendor +Load and follow the `review` skill. That skill covers: -Prefer a typed brief: `intent="review"`, `success_criteria`, `do_not`, `report_focus`. +- Scope via `git diff ...HEAD` +- Pre-existing code +- Convention compliance +- Test-coverage philosophy +- Signal over noise +- Cite the check +- The review checklist +- Posting on GitHub when a PR URL/number is known -### 4. After the report +### 9. Post on GitHub -Synthesize critic/neckbeard Summary / Findings / Blockers / Paths for the operator. Do not land fixes. +When the review targets a GitHub PR, post the finished review on the PR before cleanup. A review that only lives in chat is not done. Follow **Post on GitHub** in the `review` skill. -If a GitHub review must be posted, intern runs `gh pr review` as the operator's `gh` identity — never as a vendor bot. Primary owns `--approve` / `--request-changes` only when the operator asked to post; secondary lenses use `--comment` only. - -If the operator then wants repairs, that is a later `/implement` or `use_skill("dispatch")` — not this skill. +Do not skip the post because chat already summarized the findings. ## Cleanup -After the review, intern may remove the worktree: - ```bash git worktree remove "$WORKTREE_PATH" ``` Or leave it and tell the operator it remains for further investigation. -## Hard rules +## Error handling -- 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. -- Do not run the worktree git on the parent; intern does, via `run_shell`. -- Do not implement fixes as part of the review. -- Do not impersonate GitHub-Claude review comments. +If any command fails, stop and `ask_operator`. Common cases: missing remote branch, worktree/checkout/setup failure, `gh`/`glab` missing or unauthenticated. Present the error output. diff --git a/plugins/corbits-skills/skills/refactor/SKILL.md b/plugins/corbits-skills/skills/refactor/SKILL.md index 7f2f792cf..debf401d3 100644 --- a/plugins/corbits-skills/skills/refactor/SKILL.md +++ b/plugins/corbits-skills/skills/refactor/SKILL.md @@ -1,41 +1,64 @@ --- name: refactor argument-hint: -description: Map a directory then plan improvements. Explore, then plan. Does not ship product code. +description: Examine a directory, document its design, and collaboratively plan improvements. Does not ship product code. --- # Refactor -How to map a directory and plan improvements. `$ARGUMENTS` is the directory. This recipe maps and plans; it does not ship. Tiny / single-file / one-route product edits outside this recipe may be DIY with write_file/edit_file/delete_file. +How to analyze existing code, document its design, and plan improvements. `$ARGUMENTS` is the directory. This skill maps and plans; it does not ship. + +## Initialization + +Load `philosophy` first. Those principles guide how design decisions are evaluated. ## Steps -1. Load philosophy via `use_skill("philosophy")` on the primary **before spawning**. Those principles guide how to evaluate design decisions and what goes in worker briefs. -2. If `$ARGUMENTS` is missing or the directory is broad, `ask_operator` before exploring: - - 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="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 - - Dependencies (internal and external) - - 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="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 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. 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 — counsel's report is the artifact. -- Do not skip explore "because the directory is already known." -- Do not skip `ask_operator` when the operator has not chosen among alternatives. -- Spawn with `task(agent="explorer")` then `task(agent="counsel")`. +### 1. Understand the scope + +If `$ARGUMENTS` is missing or the directory is broad, `ask_operator` before exploring: + +- Is there a specific concern or area to focus on? +- What prompted the desire to refactor? +- Are there known pain points? + +### 2. Examine the code + +Map `$ARGUMENTS`: + +- What the code does (purpose and behavior) +- Key components and their responsibilities +- How data flows through the system +- Dependencies (internal and external) +- Patterns and conventions in use +- Areas of complexity or inconsistency (factual, not prescriptive) + +### 3. Document current design + +Write a structured markdown document in the working directory. Filename should reflect what was analyzed. + +- **Overview** — what this code does and its role +- **Components** — key parts and responsibilities +- **Data flow** — how data moves +- **Dependencies** — what it relies on +- **Patterns** — conventions observed +- **Observations** — complexity or inconsistency (factual, not prescriptive) + +### 4. Collaborative improvement + +Present observations and `ask_operator` about priorities. Propose improvements with rationale grounded in philosophy (pragmatic over idealistic, simple is usually harder than easy, do no harm, respect existing decisions). Let the operator accept, reject, or modify. Iterate until alignment. Do not invent a plan the operator did not choose. + +### 5. Write the plan + +Append an **Improvement Plan**: + +- Specific changes to make +- Rationale for each change +- Suggested order of operations +- Constraints or risks +- Enough detail that a later `/implement` could execute +- For structural transformations (renames, signature changes, API migrations), note that execution should load `ast-grep` — bulk AST rewrites, not manual read-edit-write cycles + +## Output + +One markdown file containing the design analysis and the improvement plan. A later `/implement` ships it. diff --git a/plugins/corbits-skills/skills/review/SKILL.md b/plugins/corbits-skills/skills/review/SKILL.md index 156645668..174c1d338 100644 --- a/plugins/corbits-skills/skills/review/SKILL.md +++ b/plugins/corbits-skills/skills/review/SKILL.md @@ -1,38 +1,200 @@ --- name: review -description: Review a branch, PR, or path scope. Routes critic (default), neckbeard (hygiene), or greybeard (architecture). Does not implement fixes. +description: Review a branch, PR, or path. Determines base and scope, judges only the branch diff, and reports findings with evidence. Does not implement fixes. argument-hint: "[paths | PR | diff | hygiene | architecture]" --- # Review -How to review a branch, PR, or path. Findings only — do not implement fixes. Do not write product patches to "just quickly" address findings. Do not post GitHub review comments under a vendor identity. +How to review a branch, pull request, or path. Findings only — do not implement fixes. -## Routing +Load this skill when performing a code review or pull request review. Hygiene-only asks still follow this skill (nits with receipts). Architecture-only asks still follow this skill (structure and boundaries with evidence). -- **Default** (correctness, completeness, brief adherence, defects with evidence): critic -- **Hygiene-only** (nits, naming, lint, pedantry with receipts): neckbeard -- **Architecture-only** (structure, boundaries, approach): greybeard +## Base branch -If the operator did not say hygiene-only or architecture-only, spawn critic alone. Do not spawn all three unless they asked for a wider review. +Determine the base before reviewing. Use these methods in order: -## Fleet +### 1. Associated PR/MR -- **One lens:** `task(agent="")` — blocking single spawn; prefer this when only one worker is needed. -- **Wider review** (operator asked for more than one lens): `spawn_agent(agent="", …)` once per lens in the same turn; record each returned `agent_id`, then `wait_agents` on those ids. +```bash +gh pr view --json baseRefName --jq '.baseRefName' 2>/dev/null +glab mr view --output json 2>/dev/null | jq -r '.target_branch' +``` -Prefer a typed brief: `intent="review"`, `success_criteria`, `do_not`, `report_focus`, and `agent`. +### 2. Upstream tracking -## Brief to the worker +```bash +git config --get branch..merge +``` -Include whatever the operator gave, plus: +### 3. Merge-base with default branches -- Paths, PR number/URL, or branch -- Base for comparison when known (`git diff ...HEAD`); if the base is unclear, ask rather than guessing `main` -- Only the operator's scope is in scope — pre-existing issues outside the diff are out of lane -- Do not implement fixes; findings only, with evidence (`path:line`) -- Signal over noise: skip hypotheticals and style nits that do not affect correctness, readability, or maintainability (neckbeard is the exception when hygiene was requested) +```bash +for base in main master develop; do + if git rev-parse --verify "origin/$base" >/dev/null 2>&1; then + echo "$base: $(git merge-base "origin/$base" HEAD)" + fi +done +``` + +If none of these is clear, `ask_operator` which branch to use as the base. Do not guess `main`. A wrong base includes out-of-scope changes or misses in-scope ones. + +## Scope + +Only commits on the branch under review are in scope. Use the three-dot diff: + +```bash +git log --oneline ..HEAD +git diff ...HEAD +git diff ...HEAD -- +``` + +## Pre-existing code + +A bug, convention violation, or inconsistent name that existed before the branch is out of scope — including a refactor that keeps a pre-existing type, variable, or pattern. + +Evaluate only what the branch introduced. + +## Convention compliance + +New logic, patterns, and naming must follow the project's conventions. Pre-existing code that appears in the diff because of a refactor is exempt. + +For TypeScript, load the `typescript` skill. + +## Delegating file review + +When delegating a file review, pass `git diff ...HEAD -- `, not the full file. A reviewer who sees the whole file cannot tell branch changes from pre-existing code. + +If a full file is required for context, name the line ranges the branch modified and that only those ranges are in scope. + +## Test coverage + +Do not request tests for behavior a well-maintained library already provides. Ask for coverage on: + +- Business logic and domain-specific validation +- Integration points +- Error paths and edge cases +- Custom algorithms and transformations + +## Signal over noise + +Help the author. Do not burden them. + +- Do not flag hypotheticals the code does not need to handle now +- Do flag architectural choices that constrain future work, even if they work today +- Do not leave style nits that do not affect correctness, readability, or maintainability (hygiene-only reviews are the exception) +- Do not say "this could be cleaner" without a concrete reason it matters + +If it should be fixed, say so. If it is not worth fixing, do not bring it up. + +## Comment tone + +Write as to a colleague: + +- Questions or proposals, not commands — "Could we extract this?" not "Extract this." +- Explain why, not only what +- Describe the consequence, not "wrong" / "broken" / "bad" +- Small fix → suggest the one-liner +- Do not soften feedback until the author cannot tell whether something must change + +## Describe the branch as it stands + +Describe the current diff against the base, not the journey that produced it. Re-derive from `git diff ...HEAD`. Do not trust an existing PR description. + +Bad: "I refactored the retry logic to use exponential backoff." +Good: "The HTTP client retries transient failures with exponential backoff." + +Past-tense journey framing rots as the branch iterates. Present tense of what the code now does stays true. + +## Cite the check + +Every affirmative claim ("tests pass", "messages are clean", "no regressions") needs a concrete check: a command whose output was read, a file/line range examined for a named pattern. "It looked fine" is not a check. + +When a check cannot be cited: run it, strike the claim, or narrow to what was actually examined. + +- "Tests pass." → "`bun run test` exited 0," or strike. +- "No race conditions." → "read `lock.go:42-68`; no acquire-while-holding cycles in those lines. Did not analyze `pool.go`." + +## Reviewer-of-record checks + +The agent whose verdict ships must run these in-session and read the raw output. Do not delegate them. + +- `git diff ...HEAD --stat` — scan for unexpected `Bin` markers and files outside stated scope +- `git log --oneline ..HEAD` — commits must match the issue/ask scope +- The subject and body audits under Commit-message style + +File-by-file behavioral review may be delegated. The checks above may not. + +## Commit-message coherence + +For every commit: read the message, read `git show `, confirm they match. Flag undescribed material changes, unmentioned behavior, or drive-by formatting in an unrelated commit. + +The message is a summary, not a line-by-line narration. Surprise vs the diff is the bar. + +## Commit-message style + +Canonical rules live in the `style` skill. Audit independently of coherence. + +Subjects (`git log ..HEAD --format='%s'`): + +- No `word:`, `[tag]`, or `(scope)` prefixes (`feat:`, ticket IDs, `WIP:`) +- No filenames or paths +- No trailing punctuation +- No vague subjects ("Update code", "Fix bug", "Address review") +- Length: `git log ..HEAD --format='%s' | awk 'length > 72'` — empty is clean + +Bodies (`git log ..HEAD --format='%b'`): + +- No tracker IDs or PR/session references +- No "previous commit" / "next commit" series talk +- Length: `git log ..HEAD --format='%b' | awk 'length > 72'` — empty is clean + +Affirmative claims about these audits must cite the command. See Cite the check. + +## Checklist + +Items marked _(reviewer-of-record)_ stay with the agent whose verdict ships. + +1. Determine the base branch +2. `git log --oneline ..HEAD` _(reviewer-of-record)_ +3. `git diff ...HEAD --stat` _(reviewer-of-record)_ +4. Review each changed file, only lines the branch modified +5. Per commit: message matches diff; subject/body pass the style audit _(reviewer-of-record)_ +6. New code follows project conventions +7. Summarize findings with `path:line`; cite the check behind any affirmative claim +8. If the review targets an open GitHub PR and the operator asked to post, post it (see below) + +## Post on GitHub + +Post only when the operator asked to leave a review on an open PR (or `pull-request-review` / a PR URL made that the job). A local read with no PR is done in chat. + +Use a real review, as the operator's `gh` identity — never a vendor bot: + +```bash +gh pr review --approve|--comment|--request-changes --body "$(cat <<'EOF' + +EOF +)" +``` + +Body shape: + +```markdown +## Review · + + + +### Findings + +- `path/to/file.ts:12` — +``` + +Hard bans: throat-clearing, AI filler, journey narration, emoji, restating the diff with no finding, "LGTM" alone. + +Every finding has `path:line` and a concrete failure mode. If it is not worth the author's time, drop it. + +After posting, paste the review URL. `--request-changes` means the PR is not merge-ready. ## After the report -Synthesize Summary / Findings / Blockers / Paths for the operator. Do not land fixes. If the operator then wants repairs, that is a later `/implement` or `use_skill("dispatch")` — not this skill. +Synthesize Summary / Findings / Blockers / Paths. Do not land fixes. If the operator then wants repairs, that is a later `/implement` — not this skill. diff --git a/plugins/corbits-skills/skills/scribe/SKILL.md b/plugins/corbits-skills/skills/scribe/SKILL.md index 01be66bdf..2951c785c 100644 --- a/plugins/corbits-skills/skills/scribe/SKILL.md +++ b/plugins/corbits-skills/skills/scribe/SKILL.md @@ -1,16 +1,66 @@ --- name: scribe -description: Update PRODUCT.md, ARCHITECTURE.md, and IMPLEMENTATION.md via shakespeare. Use when those docs need writing or alignment. +description: Maintain PRODUCT.md, ARCHITECTURE.md, and IMPLEMENTATION.md. Routes input, detects gaps, and asks for completeness. --- # Scribe -How to maintain PRODUCT.md, ARCHITECTURE.md, and IMPLEMENTATION.md. +How to maintain product, architecture, and implementation docs. Analyze input, route it to the correct document, then check gaps and cross-doc consistency. One-line wording fixes may be edited directly. + +Clarifying questions use `ask_operator`. DESIGN.md is rand, not this skill. + +## Document discovery + +Before classifying input, locate and read existing docs (case-insensitive) in the repo root and `docs/`: + +- PRODUCT.md, ARCHITECTURE.md, IMPLEMENTATION.md +- Prefer root when multiple matches exist +- Defaults when missing: create at repository root after confirming + +Read existing docs first — vocabulary, patterns, constraints, similar features. + +## Document types + +**PRODUCT.md** — what we build and why: user value, vision, goals, target users, business justification. + +**ARCHITECTURE.md** — how the system is structured: components, relationships, abstractions, data/control flow, technology-agnostic design. + +**IMPLEMENTATION.md** — concrete tech: libraries, protocols, formats, configuration, deployment. ## Steps -1. If the doc target (P vs A vs I) is ambiguous, `ask_operator`. -2. One-line fix: edit the file directly with write_file/edit_file. -3. Otherwise spawn `task(agent="shakespeare")` with the operator args / pasted material as the brief. +### 1. Analyze and classify + +Use general heuristics plus project vocabulary from the existing docs (project terms win when clear): + +- **Product:** user needs, value, market, "users can", goals without how +- **Architecture:** components, interactions, abstractions, tech-agnostic design +- **Implementation:** named technologies, wire formats, config, "uses" / "built on" + +### 2. Route and deepen + +If classification is clear, update the right document. + +If ambiguous or multi-category, do not only ask "which document?" Decompose into distinct claims and route each. Prefer context-aware `ask_operator` options from existing docs; fall back to general options when docs are empty. One statement may update multiple docs. + +### 3. Update + +Read the target, place content (extend section / new section / revise), match existing style. Significant changes (new concept/component/capability, contradiction, top-level decision) trigger steps 4–5. Minor clarifications skip to the report. + +### 4. Cross-document consistency (significant only) + +Check sibling docs for implied missing entries (new architecture with no product justification, product capability with no architecture, implementation naming an undescribed component). Surface targeted questions; update from answers. + +### 5. Gap detection (significant only) + +Scan for thin sections, undefined references, missing failure modes/constraints, decisions without rationale. Probe with contextual options. If the operator declines further probing, stop unless they ask. + +### 6. Report + +Confirm what changed and where. Summarize consistency/gap follow-ups. + +## Errors -DESIGN.md is rand, not this skill. +- **Missing doc:** `ask_operator` whether to create it. +- **Conflict with existing content:** `ask_operator` replace / keep both / merge. +- **Unclear scope:** `ask_operator` which document and why. diff --git a/tests/unit/corbits-skills-catalog.test.ts b/tests/unit/corbits-skills-catalog.test.ts index 68577b0a8..e1c3ea41e 100644 --- a/tests/unit/corbits-skills-catalog.test.ts +++ b/tests/unit/corbits-skills-catalog.test.ts @@ -26,7 +26,7 @@ const SKILL_DIRS = [ "plan", ] as const; -const SPAWN_RECIPE_SKILLS = ["implement", "scribe", "review", "dispatch", "plan"] as const; +const SPAWN_RECIPE_SKILLS = ["implement", "dispatch"] as const; /** use_skill listing + resolve; not slash. No disable-model-invocation. */ const USE_SKILL_ONLY = [ @@ -128,10 +128,9 @@ test("typescript skill guides TS quality without fake enforcement", async () => expect(skill).not.toMatch(/^### Don't$/m); }); -test("implement skill is a sequential spawn loop without a false 4-cap", async () => { +test("implement skill is a per-commit workflow without a false 4-cap", async () => { const skill = await Bun.file(join(pluginRoot, "skills/implement/SKILL.md")).text(); expect(skill).toContain('task(agent="greybeard")'); - expect(skill).toContain('task(agent="builder")'); expect(skill).toContain('task(agent="critic")'); expect(skill).toContain("Do not invent a worker-count or fan-out ceiling"); expect(skill).toContain("Close the loop"); @@ -182,22 +181,30 @@ test("philosophy skill is guidance without fake enforcement", async () => { expect(skill).not.toContain("use_skill("); }); -test("review skill routes critic/neckbeard/greybeard via task or spawn_agent/wait_agents", async () => { +test("review skill is a code-review playbook, not a director router", async () => { const skill = await Bun.file(join(pluginRoot, "skills/review/SKILL.md")).text(); - expect(skill).toContain("task(agent="); - expect(skill).toContain("spawn_agent"); - expect(skill).toContain("wait_agents"); - expect(skill).toContain("returned `agent_id`"); - expect(skill).toContain("critic"); - expect(skill).toContain("neckbeard"); - expect(skill).toContain("greybeard"); - expect(skill).toContain("Do not implement fixes"); + expect(skill).toContain("git diff ...HEAD"); + expect(skill).toContain("Cite the check"); + expect(skill).toContain("Signal over noise"); + expect(skill).toContain("Pre-existing code"); + expect(skill).toContain("do not implement fixes"); expect(skill).toContain("Findings only"); + expect(skill).not.toContain("spawn_agent"); + expect(skill).not.toContain("wait_agents"); expect(skill).not.toContain('task(agent="critic")'); expect(skill).not.toContain('task(agent="neckbeard")'); expect(skill).not.toContain('task(agent="greybeard")'); }); +test("pull-request-review checkouts a worktree then loads the review skill", async () => { + const skill = await Bun.file(join(pluginRoot, "skills/pull-request-review/SKILL.md")).text(); + expect(skill).toContain("git worktree add"); + expect(skill).toContain("review` skill"); + expect(skill).toContain("Do not implement fixes"); + expect(skill).not.toContain("spawn_agent"); + expect(skill).not.toContain('task(agent="critic")'); +}); + test("interview skill is an ask_operator utility with no false caps", async () => { const skill = await Bun.file(join(pluginRoot, "skills/interview/SKILL.md")).text(); expect(skill).toContain("ask_operator"); From a0311f8064edca9f177f6152bde19f2d26e3263e Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 25 Aug 2026 07:54:11 -0700 Subject: [PATCH 3/7] Base action skills on the agents playbooks Copy the gaas how-tos and swap only Corbits tool names and flags. --- .../corbits-skills/skills/implement/SKILL.md | 179 ++++++-- .../skills/linear-issue-workflow/SKILL.md | 317 ++++++++++---- .../skills/pull-request-review/SKILL.md | 147 +++++-- .../corbits-skills/skills/refactor/SKILL.md | 76 ++-- plugins/corbits-skills/skills/review/SKILL.md | 388 ++++++++++++----- plugins/corbits-skills/skills/scribe/SKILL.md | 399 ++++++++++++++++-- tests/unit/corbits-skills-catalog.test.ts | 6 +- 7 files changed, 1198 insertions(+), 314 deletions(-) diff --git a/plugins/corbits-skills/skills/implement/SKILL.md b/plugins/corbits-skills/skills/implement/SKILL.md index e40a7e3b2..62fbb6320 100644 --- a/plugins/corbits-skills/skills/implement/SKILL.md +++ b/plugins/corbits-skills/skills/implement/SKILL.md @@ -1,83 +1,176 @@ --- name: implement -description: Disciplined per-commit workflow with greybeard review, a build gate, and a critic loop. Use when each commit should be reviewed and verified before it lands. +description: Disciplined per-commit workflow with Greybeard review, build gates, and Critique loops --- # Implement -How to produce reviewed, verified commits. Load when each commit should go through architectural review, build verification, and code critique before it lands. +A disciplined implementation workflow that produces reviewed, verified commits. Load this skill when you want each commit to go through architectural review, build verification, and code critique before it lands. -This is a standalone skill, not part of dispatch. The caller defines what work to do and where the commit boundaries are. This skill defines _how_ each commit gets produced. +## Prerequisites -Tiny / single-file / one-route / clear bounded edits do not need this loop. +Before using this workflow, load the `style` and `philosophy` skills. Follow their conventions throughout. -## Prerequisites +## When to Use + +This is a standalone skill, loaded on request. It is not part of dispatch. Use it when you want a single agent to work through a series of commits with review discipline. + +The caller defines what work to do and where the commit boundaries are. This skill defines _how_ each commit gets produced. + +## Tracking Progress + +Track progress with `manage_tasks`. One item per commit-sized unit. + +### Initial Planning + +Before starting implementation, create a `manage_tasks` item for each commit-sized unit of work from the caller's instructions: + +- **subject**: Clear imperative description of the unit of work +- **description**: Enough context that you could pick it up cold +- **activeForm**: Present continuous form for the spinner (e.g., "Refactoring HTTP client retry logic") + +### During the Per-Commit Workflow + +When you begin a unit of work, mark its `manage_tasks` item in progress. As you move through the workflow steps, update the task's `activeForm` to reflect which step you're in: + +- **Step 1**: "Reviewing approach with Greybeard: {subject}" +- **Step 2**: "Implementing: {subject}" +- **Step 3**: "Running build gate: {subject}" +- **Step 4**: "Committing: {subject}" +- **Step 5**: "Running critic loop: {subject}" + +When the commit lands and Critique is clean, mark the task `completed`. + +### Discovered Work + +If new work surfaces during implementation (Greybeard suggests a preparatory refactor, Critique reveals a missing edge case that warrants its own commit), append a `manage_tasks` item and work it through the full per-commit workflow. + +## Workflow Per Commit + +For each logical unit of work that results in a commit, follow these steps in order. Do not skip steps. + +### Step 1: Greybeard Review + +Mark the task `in_progress` and set `activeForm` to "Reviewing approach with Greybeard: {subject}". + +Before writing any code, describe your implementation approach to Greybeard and ask for feedback. + +**What to send Greybeard:** + +- What you're about to change and why +- Which files you expect to touch +- Any design decisions or trade-offs you're considering +- Anything you're uncertain about + +**How to handle feedback:** + +- If Greybeard identifies problems with your approach, adjust before proceeding +- If Greybeard suggests a fundamentally different approach, consider it seriously +- You don't need to agree with every suggestion, but you need a reason to disagree +- Once you're aligned on approach, move to Step 2 + +Use `task(agent="greybeard")` for this step. + +### Step 2: Implement and Test + +Update `activeForm` to "Implementing: {subject}". + +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, ask the caller what test framework and conventions to use before proceeding. + +**For bug fixes (test-first):** + +1. Write a test that reproduces the bug. +2. Run the test and verify it **fails**. If it doesn't fail, you don't understand the bug well enough to fix it. Go back and refine the test until it demonstrates the broken behavior. +3. Implement the fix, following the approach reviewed in Step 1. +4. Run the test again and verify it **passes**. If it doesn't pass, your fix is incomplete. + +**For new features:** + +1. Implement the feature, following the approach reviewed in Step 1. +2. Write a test that exercises the new functionality and asserts on the expected behavior. The test should verify that the code works as designed and implemented, not just that it doesn't crash. +3. Run the test and verify it **passes**. + +Keep the test focused on the behavior introduced by this commit. Don't test unrelated functionality. The test is part of the deliverable, not an afterthought. -Load `style` and `philosophy` first. Follow their conventions throughout. Workers do not mount `use_skill` — copy those conventions into any worker brief. +Keep the scope tight to what was discussed. If you discover additional work is needed, finish the current commit's scope first and note the additional work for a future commit. -## Tracking +### Step 3: Build Gate -Track commit-sized units with `manage_tasks`. One item per unit that will become a commit. +Update `activeForm` to "Running build gate: {subject}". -- Before starting: create an item for each unit from the caller's instructions -- When a unit begins: mark it in progress -- When critic is clean and the build gate passed: mark it done -- New work that surfaces → append an item and run the full loop +Run `make` (or the project's equivalent full pipeline: format, lint, build, test). -## Per-commit workflow +- If the build passes, move to Step 4 +- If the build fails due to your changes, fix the failures and re-run until it passes +- If the build fails due to pre-existing issues unrelated to your changes, report the failure to the caller and let them decide how to proceed +- Do not move forward with a broken build +- Do not substitute partial builds (e.g., running only the compiler) for the full pipeline -For each unit, run these steps in order. Do not skip. +### Step 4: Commit -### 1. Greybeard — approach +Update `activeForm` to "Committing: {subject}". -`task(agent="greybeard")` on the approach before any code is written. +Create the commit. Follow the commit message conventions from the `style` skill. Include the test in the same commit as the implementation — they are one logical unit of work. -Send: what will change and why, files expected, design decisions and trade-offs, uncertainties. +### Step 5: Critic loop -If greybeard identifies problems, adjust before implementing. A different approach deserves a serious look. Disagreement needs a reason. Greybeard is for approach, not execution. +Update `activeForm` to "Running critic loop: {subject}". -### 2. Implement and test +Ask critic to review the committed change. -- **Bug fixes:** write a failing repro first, confirm it fails, fix, confirm it passes. If the test does not fail first, the bug is not understood. -- **Features:** tests ship with the change. Assert the new behavior, not merely that the process did not crash. +**How to run:** -Follow the repository's existing test conventions. If there are no tests, ask what framework to use before proceeding. +1. Spawn `task(agent="critic")` and ask it to review the output of `git show HEAD`. Include the intent from Step 1 (what the change is meant to accomplish and the approach agreed with Greybeard) so Critique can evaluate whether the implementation matches the plan, not just surface-level quality. Tell critic to limit its findings to the scope of the current commit -- pre-existing issues in touched files are out of scope. +2. Read its findings +3. For each issue marked VERIFIED or HIGH confidence: fix it +4. Re-run the build gate (Step 3) to verify fixes +5. Land the fixes on the right commit. If the target is HEAD, `git commit --amend`. Otherwise use `git rebase -i` with `edit` on the target commit: -Keep scope to this unit. Additional work becomes a later `manage_tasks` item. + ``` + git rebase -i + # In the editor, change "pick ..." to "edit ..." + # ... make the fix at the stop ... + git add + git commit --amend --no-edit + git rebase --continue + ``` -### 3. Build gate + If the situation calls for more elaborate history surgery, search your available skills for one whose description covers git rebase or branch-history cleanup, and load it. Re-run the build gate after the rebase completes. -Run the project's full pipeline (`make`, or format / lint / build / test). `task(agent="intern")` for a mechanical full pipeline, or `task(agent="tester")` for suite / repro evidence. +6. Ask critic to review `git show HEAD` again. Re-include the original intent from Step 1 and tell it what you fixed since the last pass so it can focus on verifying the fixes and checking for new issues rather than re-reviewing the entire change from scratch. +7. Repeat until Critique comes back clean or all remaining findings are acknowledged and intentional -Do not move forward with a broken build. Failures from this unit → fix and re-run. Pre-existing unrelated failures → Blockers and stop. Do not substitute a partial compile for the full gate. +**When to stop looping:** -### 4. Commit +- critic reports no issues +- Remaining findings are judgment calls you've consciously decided against, not oversights +- The build passes after the last round of fixes -Create the commit. Follow `style`. Tests land in the same commit as the implementation. +### Step 6: Next -### 5. Critic loop +Mark the current `manage_tasks` item done. Move to the next unit of work and return to Step 1. -`task(agent="critic")` on `git show HEAD`. Include the intent agreed with greybeard so critic evaluates plan vs execution. Limit findings to this unit. +## Guidelines -Blocking findings → fix, re-run the gate, land the fix on the right commit (amend HEAD, or edit-in-place via `git-rebase` for an earlier commit), then re-run critic. Close the loop; if still blocked, report Blockers — do not loop forever. +**Close the loop.** Ship → verify → fix → re-verify. -When critic is clean (or remaining findings are acknowledged judgment calls), mark the unit done and start the next. +**Do not invent a worker-count or fan-out ceiling.** -## Non-negotiables +**Don't shortcut the loop.** The value is in the discipline. Skipping Greybeard "because this change is simple" or skipping Critique "because the build passes" defeats the purpose. -- Do not shortcut the loop. Skipping greybeard "because this is simple" or critic "because the build passed" defeats the recipe. -- Build must pass before treating a unit as done. -- Do not invent a worker-count or fan-out ceiling. -- Track only with `manage_tasks`. +**Keep commits focused, but do not drop findings.** When critic surfaces something outside the current commit's scope, every finding must be assigned one of four dispositions: (a) fix in the current commit, (b) commit it separately on this branch, (c) file a new issue with concrete acceptance criteria, or (d) accept it as-is. "Out of scope" is not a disposition. "Note it for later" is not a disposition unless you also say which of (a)–(d) "later" means. -## Report +**Disposition (d) always requires operator approval** — neither you nor greybeard can drop a finding on your own. For (c), the issue must be filed in this session, with its ID or URL in the status update; a promise to file it later is dropping the work. Consult the operator before choosing (c) or (d). -When the requested units are done (or blocked): +**Build must pass before every commit, amend, and rebase stop.** Never commit code that doesn't compile or pass tests. Fix build failures first, then commit, amend, or continue the rebase. -## Summary +**Greybeard is for approach, critic is for execution.** Greybeard reviews your plan before you write code. critic reviews your code after you write it. Don't conflate the two. -## Findings +## When Shipping a PR -## Blockers +This skill produces commits. When the work is tracked in Linear and ends in a pull request, hand off to `linear-issue-workflow` for Phase 6–7. Non-negotiable bar (also encoded in `review` and `pull-request-review`): -## Paths +1. **Post a real GitHub PR review** with `gh pr review` after the PR exists — not a one-line self-review issue comment. +2. **Multi-persona when used:** if critic / greybeard / OSS-quality lenses ran with substance, each posts its own labeled review. Primary owns approve/request-changes; secondary lenses comment only. +3. **No AI slop in review bodies:** lens · verdict, one present-tense line on what the branch does, `path:line` findings. No filler, journey narration, or "LGTM" alone. +4. **Linear checkboxes and Done are post-merge + green CI only.** Flip boxes only when `main` actually has the outcome. Never partial-Done on open PR. diff --git a/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md b/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md index 9c7786aa5..4c550dc7a 100644 --- a/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md +++ b/plugins/corbits-skills/skills/linear-issue-workflow/SKILL.md @@ -1,104 +1,243 @@ --- name: linear-issue-workflow -user-invocable: false -description: Implement a Linear issue by fetching it via MCP then running the /implement spawn loop. DIY tiny/bounded issue edits; spawn builder for substantial landings. +description: Implement a feature or fix based on a Linear issue argument-hint: " [--reviewer ]" +user-invocable: false --- # Linear Issue Workflow -How to land a Linear issue. Tiny / single-file / one-route / clear bounded product edits: DIY with write_file/edit_file/delete_file. Substantial landings: Linear MCP on the primary, then the `/implement` spawn loop. +Use this skill when implementing features or fixes tracked in Linear. + +## Phase 1: Understand the Issue + +Fetch the Linear issue using `mcp__linear__get_issue`. The returned issue includes title, description, status, branch name, and other metadata you will need later. + +Ask the user clarifying questions if the scope is unclear before proceeding. + +## Phase 2: Set Up Worktree + +Read the branch name from the `branchName` field on the issue fetched in Phase 1 (call `mcp__linear__get_issue` again if needed). + +Load `use_skill("git-worktrees")` and follow the create-from-origin/ recipe (substitute ``). Worktrees do not share `node_modules`. + +All subsequent work — exploration, planning, implementation, and review — happens in the worktree directory. + +## Phase 3: Explore and Plan + +1. Use the `explore` subagent to understand the codebase. Brief it with the absolute path to the worktree (it must `cd` there before doing anything else), and ask it to cover: + - Where changes need to be made + - Existing patterns to follow + - Related code that might be affected + +2. Create an implementation plan covering: + - Files to modify + - New functions/types to add + - Tests to write + +3. Present the plan to the user and ask if they would like any changes before proceeding. Do not start implementation until the user approves the plan. + + If the user rejects the plan and the issue cannot be salvaged with a revised plan, tear down the worktree and branch with the Phase 7 commands rather than leaving them stranded. + +4. Attach the plan as a file to the Linear issue. **Do not post the plan as a comment** — comments are for discussion, not archives, and a multi-page plan dumped inline creates noise on the issue. + + The procedure has four steps, three of which are MCP or HTTP calls. The signed upload URL from `prepare_attachment_upload` expires 60 seconds after it is returned, so steps 2 and 3 must happen in immediate succession — do not pause for unrelated work between them. + + 1. Write the plan to a file under the worktree's `tmp/` (e.g., `tmp/plan-.md`). `tmp/` is the project's throwaway directory — do not commit the file (and if the project does not yet have `tmp/` gitignored, do not add to git). Capture its byte size with `wc -c < tmp/plan-.md`. + + 2. Call `mcp__linear__prepare_attachment_upload` with `issue`, `filename`, `contentType: "text/markdown"`, and `size`. The response contains `uploadRequest.url`, `uploadRequest.headers`, and `assetUrl`. **Step 3 must follow within 60 seconds — the signed URL expires.** + + 3. PUT the raw file bytes to `uploadRequest.url`. Pass every header from `uploadRequest.headers` verbatim — exact casing is required; modified or omitted headers return HTTP 403. One `-H` flag per entry: + + ```bash + curl -X PUT --data-binary @tmp/plan-.md \ + -H ": " \ + -H ": " \ + "" + ``` + + Do not base64-encode the file, do not transform it. If the PUT fails with 403 because the URL expired (more than 60 seconds since step 2), **call `prepare_attachment_upload` again for a fresh URL** — retrying the dead URL will keep failing. + + 4. Call `mcp__linear__create_attachment_from_upload` with `issue`, the `assetUrl` from step 2, `title: "Implementation plan"`, and `subtitle: ""` (so the entry stays identifiable if multiple plans accumulate across re-plans). + + Keep the local file through implementation and review — it is the working reference, colocated with the code in the worktree's `tmp/`. Phase 7's `git worktree remove` deletes it along with the rest of the worktree. The Linear attachment is the canonical copy; the local file is a working convenience for the active branch. Re-planning overwrites the local file — Linear retains prior versions as separate attachments via the `` subtitle. + + Exception: plans with no structure — no file-by-file breakdown, no enumerated steps, no headings, no nested lists — can be posted as a comment instead. Structural shape, not length, is the test; a single-sentence plan is fine inline, a 10-line bulleted plan is not. -If Linear MCP (`mcp__linear__*`) is missing, stop and tell the operator. Do not invent Claude-only tools. +5. Mark the issue as "In Progress" using `mcp__linear__save_issue` with the appropriate state. -## Phase 1: Fetch the issue +## Phase 4: Implement -Fetch with `mcp__linear__get_issue`. The returned issue includes title, description, status, branch name, and other metadata. +Carry out the plan from Phase 3 in the worktree using whichever implementation approach fits the work and your role. -If the scope is unclear, `ask_operator` before proceeding. Do not guess. +Break the work into commit-sized units. Each commit should represent one logical change that can be reviewed and understood independently. -## Phase 2: Worktree — intern +### Keep the issue checkboxes current -Read `branchName` from the issue (call `mcp__linear__get_issue` again if needed). +If the issue description contains a task list (`- [ ]` items), tick the boxes as you complete each one. The checkboxes are the issue's at-a-glance progress signal — leaving them stale makes the issue lie about what's done. -Load `use_skill("git-worktrees")`. Copy the create-from-origin/ recipe into an intern brief (substitute ``). Spawn `task(agent="intern")`. Intern executes; do not run the git on the parent. +Update the description with `mcp__linear__save_issue`, passing the full description with the relevant `- [ ]` flipped to `- [x]`. Do not rewrite or reorganize the surrounding text — only flip the box. Update as each item finishes, not in a single batch at the end; partial progress is what the checkboxes exist to show. -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. +If the issue description has no task list, skip this step — do not invent one. -## Phase 3: Plan, attach, mark In Progress +## Phase 5: Self-Review -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 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. +Once implementation is complete, run a **whole-branch code review** before pushing. The review covers the entire diff from the base branch to HEAD — not just the most recent commit — so any finding anywhere in the branch's history is in scope. - 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: +### Reviewer-of-Record Checks (in-session) - 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. - 3. `mcp__linear__create_attachment_from_upload` with `issue`, the `assetUrl`, `title: "Implementation plan"`, and `subtitle: ""`. +The reviewer-of-record is the agent whose verdict ships — in this workflow, you, the orchestrator. Before delegating any part of the review, run the audits whose value depends on the reviewer-of-record's own eyes on raw output. The canonical command list and patterns live in `review`'s _Reviewer-of-Record Checks_ and _Commit-Message Style Audit_ sections; load `review` and follow them in this session. - Exception: plans with no structure — no file-by-file breakdown, no enumerated steps, no headings, no nested lists — can be a comment via `mcp__linear__save_comment` instead. Structural shape, not length, is the test. +Read the raw output. Stop conditions — fix before continuing: -5. Mark the issue "In Progress" with `mcp__linear__save_issue`. +- Any violation surfaced by the commit-message audits. The canonical pattern lists (prefixes, vague subjects, body issues, length limits) live in `style` and `review`'s _Commit-Message Style Audit_; this section does not maintain a copy. +- `Bin` marker on any file you did not expect to be binary (source code, markdown, config). +- Files in the diff outside the issue's scope. -## Phase 4: Implement — spawn loop +After fixing a stop condition, re-run the in-session checks. Repeat until the output is clean. Do not dispatch the subagent until then. -Do not implement on the parent. For each commit-sized unit, run `/implement`: +### Subagent review (deeper read) -1. `task(agent="greybeard")` on the approach before any code is written. -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="critic")` on the diff. Blocking findings → re-dispatch builder (cap two re-fix rounds), then re-run the gate and critic. +Dispatch the `critique` subagent for the file-by-file behavioral read, architectural review, and commit-message coherence check. Running these in a subagent keeps the deeper output out of the main context and gives independent eyes on patterns. -Track units with `manage_tasks`. Copy style/philosophy into worker briefs (`use_skill` on the primary before spawning; workers do not mount `use_skill`). +Brief the subagent with: -### Checkboxes +- The absolute path to the worktree (it must `cd` there before doing anything else). +- The base branch to diff against (the remote default branch resolved in Phase 2, e.g. `origin/main`), so `review` does not dead-end on its "ask the user" path. Make explicit that the review must cover the full `base..HEAD` range, every commit on the branch. +- An instruction to load the `review` skill and follow its checklist against the current branch, _except_ the items marked _(reviewer-of-record)_ — the orchestrator has already run those. +- The Linear issue ID and a one-line summary of the change's _intent_ — what the change is for. "This branch refactors retry logic to use exponential backoff" is fine; "this branch should not introduce blocking calls in the sendPack path" pre-frames findings and is forbidden. +- A request for findings with `file:line` references — not PR-comment prose. +- An instruction that the subagent's final message must list every finding verbatim, with no summarization or omission. This prevents collapse during transmission; it does _not_ prevent the more fundamental loss of signals that do not fit a finding shape at all (binary markers, surprising stat counts). Those belong to the reviewer-of-record checks above. -If the issue description contains a task list (`- [ ]` items), tick boxes as build reports each one complete. Update with `mcp__linear__save_issue`, passing the full description with only the relevant `- [ ]` flipped to `- [x]`. Do not rewrite surrounding text. If there is no task list, skip — do not invent one. +Do **not** include author-supplied "blocking criteria" or "things to look for" in the brief. The skill itself is the criteria; supplementing it narrows the subagent's lens to what you already suspect might be wrong, suppressing unknown-unknowns. The intent summary above is bounded for the same reason — keep it to _what the change is for_, never _what the reviewer should find_. -## Phase 5: Branch review +### Fix every finding -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: +Treat the returned findings as a worklist and **fix every one**. The `review` skill's "Signal Over Noise" guidance has already filtered out pedantic taste-only nitpicks upstream — anything that survived to the final findings is something the reviewer judged worth the author's time. "Nit," "minor," "stylistic," and "suggestion" describe the reviewer's confidence about severity; they are not dispositions and do not authorize skipping. The only path to leaving a finding unfixed is a Greybeard waiver (see below). -- Absolute worktree path -- Base branch from Phase 2 -- Linear issue ID and a one-line intent (what the change is for — never what the reviewer should find) -- Findings with `file:line` — not PR-comment prose -- Do not implement fixes +- Fix issues in additional commits, or via `git rebase -i` with `edit` on the target commit when a fix belongs on an earlier commit (mark the target `edit`, make the fix at the stop, `git commit --amend --no-edit`, `git rebase --continue`). +- After fixing, re-run the reviewer-of-record checks in-session, then re-dispatch the review subagent against the full branch as it now stands. Every re-review is a fresh, self-contained read of `base..HEAD` exactly as it is — no scoping to the delta from a prior pass, no carryover ledger of earlier findings. The new findings replace the previous set; the previous are gone. +- Repeat fix-and-review until the review returns clean. +- Cap at three re-reviews. The cap counts subagent dispatches; in-session check failures the orchestrator fixes between dispatches do not consume a slot. If findings remain after the third, stop and surface the situation to the user directly. Do not soften it: tell the user plainly that the branch has been through three review-and-fix passes and the reviewer is still finding issues, list each outstanding finding with its `file:line`, and state explicitly that the branch is **not ready to push**. Do not proceed to Phase 6, do not propose a waiver, and do not offer to "just push anyway" — wait for the user to decide whether to keep iterating, rescope the issue, or abandon the branch. -Fix-every-finding: treat surviving findings as a worklist and re-enter Phase 4 for each. Cap three whole-branch re-reviews. If findings remain, `ask_operator` — do not push. +### Waivers -The only path to leaving a finding unfixed is a greybeard waiver: `task(agent="greybeard")` with the finding, proposed disposition, and relevant diff. Accept the ruling by default. Escalate with `ask_operator` only if you disagree or greybeard is unreachable. Never waive on the parent's own authority. +The only path to leaving a finding unfixed is a Greybeard waiver. If you believe a finding should not be fixed — because the "fix" would be pure churn (taste-only rewording, unrelated refactor, change the project has explicitly chosen not to make) or because you actively disagree with the reviewer — dispatch the `greybeard` subagent with the finding, your proposed disposition, and the relevant diff. **A Greybeard "waive" ruling is the waiver** — record it in the disposition note and move on; no further user sign-off is needed. Accept Greybeard's call by default. Escalate to the user only if (a) you actively disagree with Greybeard's ruling, or (b) the Greybeard subagent is unreachable or returns an unusable response. In either case, present both positions (or the failure mode) and let the user decide. Do not route routine waiver requests through the user, and never waive a finding on your own authority. -## Phase 6: Push and PR — intern, after confirmation +By the time the gate closes, every finding is fixed or Greybeard-waived; the findings themselves are iteration history and are not preserved as a Phase 6 artifact. Hold onto each Greybeard ruling — Phase 6 names the waivers as present-state exceptions on the branch and cites the ruling that authorized each one. Do not keep the fix SHAs, the fixed findings, or the per-iteration log; none of those describe the branch as it stands. -Once Phase 5 is clean (or every remaining finding is greybeard-waived): +## Phase 6: Push and PR -Intern rebases via `run_shell`: +Once the Phase 5 gate has cleared (the review returned clean, or every remaining finding has a Greybeard waiver), work through the steps below in order. Drafting precedes confirmation so the user authorizes specific artifacts — the title, body, and PR-review body — rather than a promise about what they will say. + +### Rebase against the remote default branch ```bash git fetch origin git rebase origin/ ``` -Then intern re-runs the build gate. Draft PR title, body, and (if posting) review body from `git diff origin/...HEAD` and `git log origin/..HEAD --format='%s'` — present tense, no journey narration. Title: verb-first, no `feat:` prefix, no ticket ID in the subject. Body: +Verify the build still passes after rebasing. + +### Re-derive the PR artifacts from the current diff + +The PR title, body, and GitHub PR-review body are PR-shaped artifacts and are bound by `review`'s _Describe the Branch As It Stands_ rule: present-tense statements of what the code _does_, never past-tense narration of how it got there. + +Draft from the diff, not from memory. The implementation history is the single largest source of journey framing; memory of the work reliably reproduces it. Read the output of both: + +```bash +git diff origin/...HEAD +git log origin/..HEAD --format='%s' +``` + +The full diff matters, not just `--stat`. `--stat` shows filenames and line counts; a present-tense description of behaviour has to come from the diff itself. + +After drafting each artifact, scan it for past-tense verbs (`was`, `were`, `had`, `added`, `fixed`, `refactored`, `introduced`, `removed`, `updated`, `changed`) and rewrite each into present-tense. If the rewrite forces dropping a sentence because it has no current-state content, that sentence was journey narrative and should not have been in the artifact. + +### PR title shape + +A single present-tense statement of what the code does. `style`'s commit-subject rules apply: start with a verb, no leading prefix (no `feat:`, no `[scope]`, no ticket ID), no trailing punctuation, no abbreviations. + +### PR body shape ```markdown ## Summary -<1-3 bullets, present tense> +<1-3 bullets describing what the code does, present tense> ## Verification - +` works end-to-end against ``. +State what is true now, not what was done.> Closes ``` -`ask_operator` to confirm title, body, reviewer, and whether to push. Do not push until confirmed. +`## Summary` and `## Verification` are the only sections in the body. Do not add `## Changes`, `## Context`, `## Notes`, or other sections — the diff is right there, and additional prose either restates the diff (past-tense) or re-narrates the journey (also past-tense). -Intern then: +### PR review on GitHub (not a self-review issue comment) + +After the PR exists, **post a real GitHub PR review** via `gh pr review`. Follow **Post the Review on GitHub** in the `review` skill. + +Do **not** leave a one-line self-review issue comment (`gh pr comment` with "Self-review returned clean"). That is noise. The review body is the record. + +**Clean review (common case).** Primary post: + +```bash +gh pr review --approve --body "$(cat <<'EOF' +## Review · Approve + + + +No findings. +EOF +)" +``` + +If repo policy forbids self-approve, use `--comment` with the same body shape — still a full review body, not a one-liner. + +**Findings remain (should be rare after Phase 5).** Use `--request-changes` when anything blocks merge; use `--comment` for non-blocking residual notes. Body shape: + +```markdown +## Review · Request changes + + + +### Findings + +- `path/to/file.ts:12` — +``` + +**Waivers exist.** Primary post still carries the verdict. Name each waived finding as a present-state exception; cite Greybeard as authorization. Greybeard (if it ran) posts a **separate** labeled `--comment` review that owns the waiver list — do not collapse both into one mushy paragraph. + +```markdown +## Review · Approve + + + +### Notes + +- `file:line` — . Greybeard-authorized: . +``` + +```markdown +## Greybeard · Comment + +Waiver rulings for this branch: + +- `file:line` — authorized on +``` + +**Multi-persona rule.** If Phase 5 (or an explicit user request) ran additional lenses (`critique`, `greybeard`, OSS/quality), each lens with substance posts its own labeled review. Primary owns `--approve` / `--request-changes`. Secondary lenses use `--comment` only. Do not invent personas that did not run. Body shape and hard bans live in `review` → **Post the Review on GitHub** (no AI filler, no journey narration, no "LGTM" alone). + +Do not paste Phase 5 iteration history, fix SHAs, or "for context" preambles. The merged result is what ships. + +### Confirm with the user + +Surface the drafted title, body, and PR-review body (per persona if multi-lens) to the user. The user confirms whether to push and post with those exact artifacts. Do not push until confirmed. + +### Push and post ```bash git push -u origin @@ -107,37 +246,73 @@ gh pr create \ --title "" \ --reviewer \ --body "$(cat <<'EOF' - + EOF )" ``` -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. Paste the PR URL and every review URL to the operator. +Then post the PR review(s) with `gh pr review` as drafted above. Paste the PR URL **and** every review URL to the user. -## Phase 7: After merge — Linear closeout and cleanup +## Phase 7: After Merge — Linear Closeout and Cleanup -Phase 6 ends when the PR is open. Phase 7 runs **after the PR is merged** and **CI is green**. Do not mark the Linear issue Done on open PR alone. +Phase 6 ends when the PR is open and the review is posted. Phase 7 runs **after the PR is merged** and **CI is green** on the merge (or on `main` at the merge commit). Do not mark the Linear issue Done on open PR alone. Do not tick outcome checkboxes on hope. -1. Intern confirms merge and CI via `run_shell` (`gh pr view`, `gh pr checks` / `gh run list`). If CI is red, do not tick Linear outcomes. -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: load `use_skill("git-worktrees")` and copy the teardown recipe into an intern brief (substitute `` and ``). +### 1. Confirm merge -## Linear MCP tool reference +```bash +gh pr view --json state,mergedAt,mergeCommit,url +``` + +`state` must be `MERGED`. Record the merge commit SHA. + +### 2. Confirm CI green + +```bash +gh run list --commit --limit 5 +# or +gh pr checks +``` + +If CI is red, fix forward or reopen — do **not** tick Linear outcomes or mark Done. + +### 3. Update Linear checkboxes + +Re-read the issue with `mcp__linear__get_issue`. For every checklist item the merged PR actually completed on `main`, flip `- [ ]` to `- [x]` via `mcp__linear__save_issue` (full description rewrite or patch). Leave unchecked anything not truly done. Never check a box because the PR _intended_ to cover it — only because `main` now does. + +### 4. Comment merge evidence + +`mcp__linear__save_comment` with PR URL, merge SHA, and CI-green confirmation. Short. Present tense facts only. + +### 5. Mark Done only when complete + +If **every** outcome checkbox is checked and nothing residual remains, set state to `Done` with `mcp__linear__save_issue`. If anything is still open, leave the issue In Progress (or the team's equivalent) with remaining boxes unchecked — never partial-Done theater. + +### 6. Clean up the worktree + +Only after merge + green CI + Linear closeout (or an explicit user decision to abandon). The orchestrator is still `cd`'d into the worktree from Phase 2; leave it first: + +```bash +cd # leave the worktree +git fetch origin +git worktree remove ../worktree/ +git branch -d +``` + +If the worktree directory was already manually deleted, prune stale worktree references: + +```bash +git worktree prune +``` -| Action | Tool | -| -------------------------- | ---------------------------------------------------------------------------------------------------- | -| Fetch issue | `mcp__linear__get_issue` | -| Get branch name | `mcp__linear__get_issue` (`branchName`) | -| Update status / checkboxes | `mcp__linear__save_issue` | -| Add comment | `mcp__linear__save_comment` | -| Attach file | `mcp__linear__prepare_attachment_upload` → intern PUT → `mcp__linear__create_attachment_from_upload` | -| List teams | `mcp__linear__list_teams` | +## Linear MCP Tool Reference -## Hard rules +Common operations: -- 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. +| Action | Tool | +| --------------- | ------------------------------------------------------------------------------------------------------------------ | +| Fetch issue | `mcp__linear__get_issue` | +| Get branch name | `mcp__linear__get_issue` (read `branchName` from result) | +| Update status | `mcp__linear__save_issue` (set the state) | +| Add comment | `mcp__linear__save_comment` | +| Attach file | `mcp__linear__prepare_attachment_upload` → PUT → `mcp__linear__create_attachment_from_upload` (see Phase 3 step 4) | +| List teams | `mcp__linear__list_teams` | diff --git a/plugins/corbits-skills/skills/pull-request-review/SKILL.md b/plugins/corbits-skills/skills/pull-request-review/SKILL.md index efcd0b397..db3a96897 100644 --- a/plugins/corbits-skills/skills/pull-request-review/SKILL.md +++ b/plugins/corbits-skills/skills/pull-request-review/SKILL.md @@ -1,112 +1,177 @@ --- name: pull-request-review -description: Review a pull request by branch name or URL, using a git worktree. Loads the review skill for the actual read. Does not implement fixes. +description: Review a pull request by branch name or URL, using a git worktree --- # Pull Request Review -How to review a pull request given a branch name or URL. Do not implement fixes. Do not impersonate GitHub-Claude (or any other vendor) review comments. +Use this skill to review a pull request given a branch name or URL. Do not implement fixes. -## Input +## Input Formats -- A branch name (e.g. `feature/add-auth`) -- A pull request URL from GitHub or GitLab (e.g. `https://github.com/owner/repo/pull/123`) +This skill accepts either: -## Steps +- A branch name (e.g., `feature/add-auth`) +- A pull request URL from GitHub or GitLab (e.g., `https://github.com/owner/repo/pull/123`) -### 1. Parse input +## Workflow -If given a URL, extract the branch: +### Step 1: Parse Input + +If given a URL, extract the pull request information: ```bash -# GitHub +# GitHub PR URL format: https://github.com/owner/repo/pull/123 +# GitLab MR URL format: https://gitlab.com/owner/repo/-/merge_requests/123 + +# For GitHub, use gh CLI to get branch name gh pr view --json headRefName --jq '.headRefName' -# GitLab +# For GitLab, use glab CLI or parse the MR page glab mr view --output json | jq -r '.source_branch' ``` -### 2. Repository root +### Step 2: Determine Repository Root + +Find the root of the current git repository: ```bash git rev-parse --show-toplevel ``` -### 3. Fetch and verify the branch +### Step 3: Fetch and Verify the Branch + +Fetch the branch from the remote and verify it exists: ```bash +# Fetch the specific branch git fetch origin -git rev-parse --verify "origin/" + +# Verify the branch exists +if ! git rev-parse --verify "origin/" >/dev/null 2>&1; then + echo "Error: Branch '' does not exist on the remote." + exit 1 +fi ``` -If the branch does not exist, report the error and stop. Do not create a worktree or review. +If the branch does not exist, inform the user of the error and stop. Do not proceed with worktree creation or review. + +### Step 4: Create Worktree -### 4. Create worktree +Create a worktree in a sibling directory under `worktree/`: ```bash +# From repository root REPO_ROOT=$(git rev-parse --show-toplevel) WORKTREE_PATH="${REPO_ROOT}/../worktree/" + +# Create the worktree directory if needed mkdir -p "${REPO_ROOT}/../worktree" + +# Add the worktree git worktree add "$WORKTREE_PATH" "origin/" ``` -Path is `../worktree/` relative to the repository root. +The worktree path follows the pattern `../worktree/` relative to the repository root. + +### Step 5: Change to Worktree and Checkout Branch -### 5. Checkout +Change the working directory to the new worktree and ensure the branch is checked out: ```bash cd "$WORKTREE_PATH" + +# Checkout the branch git checkout + +# Verify you are on the correct branch git branch --show-current ``` -Confirm the branch name before continuing. +Confirm the output matches the expected branch name before proceeding. -### 6. Set up the repository +### Step 6: Set Up the Repository -Follow documented setup exactly — `README.md`, `CONTRIBUTING.md`, `docs/`, `DEVELOPMENT.md`, or `SETUP.md`. Typical: install deps, env, migrations, build. +Before reviewing, the repository must be properly set up. Look for developer documentation that describes how to install dependencies and prepare the codebase: -Do not assume the setup process. If no setup docs exist, `ask_operator` before proceeding. +1. Search for setup instructions in these common locations: + - `README.md` + - `CONTRIBUTING.md` + - `docs/` directory + - `DEVELOPMENT.md` + - `SETUP.md` -### 7. Base branch +2. Follow the documented setup steps exactly. Common setup tasks include: + - Installing dependencies (`npm install`, `yarn`, `pip install`, `bundle install`, etc.) + - Setting up environment variables + - Running database migrations + - Building assets or compiling code + +3. Do not assume the setup process. Every repository has its own conventions and requirements. + +4. If no setup documentation exists, `ask_operator` how to set up the repository before proceeding. + +### Step 7: Determine Base Branch + +Identify the base branch for comparison: ```bash +# For GitHub PRs gh pr view --json baseRefName --jq '.baseRefName' -# or + +# For GitLab MRs glab mr view --output json | jq -r '.target_branch' + +# If working from branch name only, assume main or master +git branch -r | grep -E 'origin/(main|master)$' | head -1 | sed 's/.*origin\///' ``` -Branch-name only: do not guess if both `main` and `master` exist — `ask_operator`. +### Step 8: Load the review skill -If any command fails, stop, report the error, and `ask_operator`. Do not retry workarounds. +Load and follow the `review` skill to perform the actual review. The `review` skill provides guidance on: -### 8. Review +- Scope determination using git diff +- Handling pre-existing code +- Convention compliance +- Test coverage philosophy +- Signal over noise (avoiding unactionable findings) +- Review checklist +- **Post the Review on GitHub** (required when a PR URL/number is known) -Load and follow the `review` skill. That skill covers: +### Step 9: Post the Review on GitHub -- Scope via `git diff ...HEAD` -- Pre-existing code -- Convention compliance -- Test-coverage philosophy -- Signal over noise -- Cite the check -- The review checklist -- Posting on GitHub when a PR URL/number is known +When the review targets a GitHub PR (URL, number, or branch with an open PR), **post the finished review on the PR** before cleanup. A review that only lives in chat is not done. -### 9. Post on GitHub +Follow **Post the Review on GitHub** in the `review` skill: -When the review targets a GitHub PR, post the finished review on the PR before cleanup. A review that only lives in chat is not done. Follow **Post on GitHub** in the `review` skill. +1. Map the verdict to a `gh pr review` action: + - Approve → `--approve` + - Comment → `--comment` + - Request changes → `--request-changes` +2. Body: clean multi-line shape — lens label, one present-tense line on what the branch does, findings with `path:line`, no AI filler. Hard bans live in that skill section. +3. If additional personas ran (`critique`, `greybeard`, OSS/quality), each lens with substance posts its own labeled review. Primary owns the merge action; secondary lenses use `--comment` only. +4. Paste the posted review URL(s) into the user-facing summary. -Do not skip the post because chat already summarized the findings. +Do not skip the post because the chat already summarized the findings. ## Cleanup +After the review is complete, the worktree can be removed: + ```bash git worktree remove "$WORKTREE_PATH" ``` -Or leave it and tell the operator it remains for further investigation. +Inform the user that the worktree remains available for further investigation and can be removed manually when no longer needed. + +## Error Handling + +If any command fails during the workflow, do not retry or attempt workarounds. Stop immediately and `ask_operator` how to proceed. Common failure scenarios include: -## Error handling +- Branch does not exist remotely +- Worktree creation fails +- Checkout fails +- Setup commands fail +- CLI tools (gh, glab) are not available or not authenticated -If any command fails, stop and `ask_operator`. Common cases: missing remote branch, worktree/checkout/setup failure, `gh`/`glab` missing or unauthenticated. Present the error output. +Present the error output to the user and ask how they would like to proceed. diff --git a/plugins/corbits-skills/skills/refactor/SKILL.md b/plugins/corbits-skills/skills/refactor/SKILL.md index debf401d3..f99871227 100644 --- a/plugins/corbits-skills/skills/refactor/SKILL.md +++ b/plugins/corbits-skills/skills/refactor/SKILL.md @@ -1,64 +1,88 @@ --- name: refactor argument-hint: -description: Examine a directory, document its design, and collaboratively plan improvements. Does not ship product code. +description: Examine code, document its design, and collaboratively plan improvements --- # Refactor -How to analyze existing code, document its design, and plan improvements. `$ARGUMENTS` is the directory. This skill maps and plans; it does not ship. +Use this skill to analyze existing code, produce a structured design document, and collaboratively plan improvements for future implementation. ## Initialization -Load `philosophy` first. Those principles guide how design decisions are evaluated. +Before doing anything else, load the `philosophy` skill. The principles in that skill guide how you evaluate design decisions. -## Steps +## Workflow -### 1. Understand the scope +### Step 1: Understand the Scope -If `$ARGUMENTS` is missing or the directory is broad, `ask_operator` before exploring: +The user has specified a directory to analyze: `$ARGUMENTS` -- Is there a specific concern or area to focus on? +If the directory is broad, `ask_operator` before exploring: + +- Is there a specific concern or area they want to focus on? - What prompted the desire to refactor? - Are there known pain points? -### 2. Examine the code +### Step 2: Examine the Code -Map `$ARGUMENTS`: +Explore the specified directory to understand: - What the code does (purpose and behavior) - Key components and their responsibilities - How data flows through the system - Dependencies (internal and external) - Patterns and conventions in use -- Areas of complexity or inconsistency (factual, not prescriptive) +- Areas of complexity or inconsistency + +### Step 3: Document Current Design + +Write a structured markdown document to the current working directory. Choose a filename that reflects what was analyzed. + +Document structure: + +**Overview** - What this code does and its role in the larger system + +**Components** - Key parts and their responsibilities -### 3. Document current design +**Data Flow** - How data moves through the system -Write a structured markdown document in the working directory. Filename should reflect what was analyzed. +**Dependencies** - What it relies on -- **Overview** — what this code does and its role -- **Components** — key parts and responsibilities -- **Data flow** — how data moves -- **Dependencies** — what it relies on -- **Patterns** — conventions observed -- **Observations** — complexity or inconsistency (factual, not prescriptive) +**Patterns** - Design patterns and conventions observed -### 4. Collaborative improvement +**Observations** - Complexity, inconsistencies, or potential concerns (factual, not prescriptive) -Present observations and `ask_operator` about priorities. Propose improvements with rationale grounded in philosophy (pragmatic over idealistic, simple is usually harder than easy, do no harm, respect existing decisions). Let the operator accept, reject, or modify. Iterate until alignment. Do not invent a plan the operator did not choose. +### Step 4: Collaborative Improvement Discussion -### 5. Write the plan +After documenting the current state: -Append an **Improvement Plan**: +1. Present your observations and `ask_operator` about priorities +2. Propose specific improvements with rationale grounded in philosophy principles (pragmatic, simple over easy, etc.) +3. Let the user accept, reject, or modify proposals +4. Ask follow-up questions to refine the approach +5. Iterate until alignment is reached + +### Step 5: Write the Plan + +Append an **Improvement Plan** section to the document with: - Specific changes to make - Rationale for each change - Suggested order of operations -- Constraints or risks -- Enough detail that a later `/implement` could execute -- For structural transformations (renames, signature changes, API migrations), note that execution should load `ast-grep` — bulk AST rewrites, not manual read-edit-write cycles +- Any constraints or risks to be aware of +- Enough detail that another agent could execute the plan +- For structural transformations (renames, signature changes, API migrations), note that the `ast-grep` skill should be loaded during execution — it enables bulk AST-based rewrites instead of manual read-edit-write cycles ## Output -One markdown file containing the design analysis and the improvement plan. A later `/implement` ships it. +A single markdown file in the user's current working directory containing both the design analysis and the improvement plan. + +## Guiding Principles + +From the philosophy skill: + +- **Pragmatic over idealistic** - Don't propose changes for theoretical purity +- **Simple is usually harder than easy** - Favor designs that are genuinely simple, not just quick +- **Do no harm** - Consider risks to stability and correctness +- Respect existing decisions; understand why things are the way they are before proposing changes diff --git a/plugins/corbits-skills/skills/review/SKILL.md b/plugins/corbits-skills/skills/review/SKILL.md index 174c1d338..8cc786aeb 100644 --- a/plugins/corbits-skills/skills/review/SKILL.md +++ b/plugins/corbits-skills/skills/review/SKILL.md @@ -1,200 +1,396 @@ --- name: review -description: Review a branch, PR, or path. Determines base and scope, judges only the branch diff, and reports findings with evidence. Does not implement fixes. argument-hint: "[paths | PR | diff | hygiene | architecture]" +description: Review a branch, PR, or path. Determines base and scope, judges only the branch diff, and reports findings with evidence. Does not implement fixes. --- # Review How to review a branch, pull request, or path. Findings only — do not implement fixes. -Load this skill when performing a code review or pull request review. Hygiene-only asks still follow this skill (nits with receipts). Architecture-only asks still follow this skill (structure and boundaries with evidence). +Use this skill when performing code reviews or pull request reviews. + +## Base Branch Determination -## Base branch +Before reviewing, you must determine the correct base branch. Use these methods +in order of reliability: -Determine the base before reviewing. Use these methods in order: +### Method 1: Check for Associated PR/MR -### 1. Associated PR/MR +If the branch has an open pull request or merge request, use the PR's target: ```bash +# GitHub gh pr view --json baseRefName --jq '.baseRefName' 2>/dev/null + +# GitLab glab mr view --output json 2>/dev/null | jq -r '.target_branch' ``` -### 2. Upstream tracking +### Method 2: Check Upstream Tracking + +If the branch tracks a remote branch that itself branched from somewhere: ```bash git config --get branch..merge ``` -### 3. Merge-base with default branches +### Method 3: Find Merge Base with Default Branches + +Attempt to find a common ancestor with likely default branches: ```bash +# Check if main or master exists and find merge-base for base in main master develop; do if git rev-parse --verify "origin/$base" >/dev/null 2>&1; then - echo "$base: $(git merge-base "origin/$base" HEAD)" + merge_base=$(git merge-base "origin/$base" HEAD) + echo "$base: $merge_base" fi done ``` -If none of these is clear, `ask_operator` which branch to use as the base. Do not guess `main`. A wrong base includes out-of-scope changes or misses in-scope ones. +This helps identify which branch HEAD diverged from, but does not guarantee +the intent of the branch author. + +### When Base Cannot Be Determined + +If none of the above methods provide a clear answer, do not assume. `ask_operator`: + +> I cannot determine which branch this was based off. What branch should I use +> as the base for comparison? Common options include `main`, `master`, or +> `develop`. + +Proceeding with an incorrect base will cause the review to include out-of-scope +changes or miss in-scope changes. -## Scope +## Scope Determination -Only commits on the branch under review are in scope. Use the three-dot diff: +Focus only on commits that are contained within the branch being reviewed. +Use `git diff ...HEAD` to determine what is in scope. ```bash -git log --oneline ..HEAD -git diff ...HEAD -git diff ...HEAD -- +# List commits on the branch +git log --oneline ..HEAD + +# Get the full diff for review +git diff ...HEAD + +# Get diff for a specific file +git diff ...HEAD -- ``` -## Pre-existing code +## Pre-existing Code + +If a bug, convention violation, or inconsistent naming existed in code before +the branch, do not treat it as a problem in the review. This includes cases +where refactored code retains a pre-existing type name, variable name, or +pattern that does not match current conventions. -A bug, convention violation, or inconsistent name that existed before the branch is out of scope — including a refactor that keeps a pre-existing type, variable, or pattern. +Only evaluate the changes introduced by the branch. -Evaluate only what the branch introduced. +## Convention Compliance -## Convention compliance +All new logic, patterns, and naming introduced by the branch should follow +the project's established conventions. Pre-existing code that appears in +the diff due to refactoring is exempt from this requirement. -New logic, patterns, and naming must follow the project's conventions. Pre-existing code that appears in the diff because of a refactor is exempt. +If reviewing TypeScript code, consider loading the `typescript` +skill for detailed guidance on type patterns, naming, and idioms. -For TypeScript, load the `typescript` skill. +## Delegating to Sub-agents -## Delegating file review +When delegating file review to sub-agents, provide the output of +`git diff ...HEAD -- ` rather than the full file contents. -When delegating a file review, pass `git diff ...HEAD -- `, not the full file. A reviewer who sees the whole file cannot tell branch changes from pre-existing code. +Sub-agents that receive full files cannot distinguish branch changes from +pre-existing code and will flag out-of-scope issues. -If a full file is required for context, name the line ranges the branch modified and that only those ranges are in scope. +If full files must be provided for context, explicitly instruct the sub-agent +which line ranges were modified by the branch and that only those ranges are +in scope. -## Test coverage +## Test Coverage Philosophy -Do not request tests for behavior a well-maintained library already provides. Ask for coverage on: +Do not request additional test coverage for functionality provided by external +libraries. Focus test coverage requests on: - Business logic and domain-specific validation -- Integration points -- Error paths and edge cases -- Custom algorithms and transformations +- Integration points between components +- Error handling paths and edge cases +- Custom algorithms and data transformations + +Trust well-maintained libraries to do their job. If a library's behavior needs +testing, that suggests reconsidering whether to use that library. + +## Signal Over Noise -## Signal over noise +A review should help the author, not burden them. Do not raise issues that are +not important to the code being reviewed. Specifically: -Help the author. Do not burden them. +- Do not flag hypothetical problems in unlikely scenarios that the code does + not need to handle today or in the near future. +- Do flag architectural choices that would limit or constrain future + implementations, even if they work fine today. +- Do not leave minor stylistic nits that have no impact on correctness, + readability, or maintainability. +- Do not make vague observations like "this could be cleaner" without a + concrete reason it matters. -- Do not flag hypotheticals the code does not need to handle now -- Do flag architectural choices that constrain future work, even if they work today -- Do not leave style nits that do not affect correctness, readability, or maintainability (hygiene-only reviews are the exception) -- Do not say "this could be cleaner" without a concrete reason it matters +If something should be fixed, say so directly. If it's not worth fixing, don't +bring it up. -If it should be fixed, say so. If it is not worth fixing, do not bring it up. +## Comment Tone -## Comment tone +Review comments are posted under your name to a human author. Write them +as you would speak to a colleague: -Write as to a colleague: +- Frame suggestions as questions or proposals, not commands. + "Could we extract this?" not "Extract this." +- Explain the _why_ -- "so that X" or "because Y" -- not just the _what_. +- Avoid words like "wrong", "broken", "bad". Describe the consequence + instead: "this will produce incorrect values when..." not "this is wrong." +- If the fix is small, suggest it concretely. A one-liner suggestion is + more helpful than a paragraph explaining what to change. +- Do not soften feedback to the point of obscuring it. The author should + understand whether something needs to change or is merely a thought. -- Questions or proposals, not commands — "Could we extract this?" not "Extract this." -- Explain why, not only what -- Describe the consequence, not "wrong" / "broken" / "bad" -- Small fix → suggest the one-liner -- Do not soften feedback until the author cannot tell whether something must change +## Describe the Branch As It Stands -## Describe the branch as it stands +Review comments and PR descriptions must describe the branch's _current_ diff against the base, not the journey that produced it. The merged result is what ships; intermediate states, earlier review rounds, and pre-iteration code are not part of the artifact. -Describe the current diff against the base, not the journey that produced it. Re-derive from `git diff ...HEAD`. Do not trust an existing PR description. +Re-derive the description from `git diff ...HEAD` every time. Do not trust the existing PR description — it may have been written before the branch was iterated on, and reviewers reading it now should see what is true now. -Bad: "I refactored the retry logic to use exponential backoff." +Bad: "I refactored the retry logic to use exponential backoff and removed the old fixed-delay path." Good: "The HTTP client retries transient failures with exponential backoff." -Past-tense journey framing rots as the branch iterates. Present tense of what the code now does stays true. +Bad (review comment): "This used to call `foo()` but now calls `bar()` — did you mean to..." +Good (review comment): "`bar()` is being called here without checking its return value." + +Past-tense framing rots: as the branch evolves through review, descriptions of "what was changed" stop matching the diff. Present-tense framing of "what the code now does" stays correct as long as the diff is correct. + +## Cite the Check + +A review report may include affirmative verification claims — "tests pass," "messages are clean," "no regressions," "convention compliance verified." When a review says something is _verified_, the reader takes that as a checked fact. If the check was never run, the report is dishonest. The dishonesty does not surface until a human reviewer finds the issue the agent claimed did not exist. + +**For every affirmative claim in your review, you must be able to cite the specific check that proved it.** A check is a concrete artifact: a command whose output you read, a tool invocation whose results you inspected, a file/line range you examined for a specific pattern. "I considered it" and "it looked fine to me" are not checks. + +When you cannot cite a check, do one of: + +1. **Run the check** and cite it. +2. **Strike the claim** from the report. +3. **Narrow the claim to what you actually examined.** "Did not observe race conditions in `lock.go:42-68`" is honest; "no race conditions in the new locking code" is not. + +Some properties — subtle concurrency bugs, performance pathologies, security gaps — cannot be fully verified by a single check. Honest narrowing is better than a dishonest absolute: say what you looked at and what you looked for, and do not claim absence beyond that boundary. + +**Bad → Good:** + +- "All commit messages are clean." → cite the specific audit command (see _Commit-Message Style Audit_), or strike. +- "Tests pass." → "`npm test` exited 0," or strike. +- "Convention compliance verified." → "diffed naming and error-handling shape in the new handlers against `src/api/user.ts` and `src/api/billing.ts`; same `Result` return pattern, same `assert`-style guards," or strike. +- "No race conditions." → "read `lock.go:42-68`, traced lock acquisition order; no acquire-while-holding cycles in those lines. Did not analyze interactions with `pool.go` or callers outside the diff." + +## Reviewer-of-Record Checks + +"Cite the Check" requires that affirmative claims have backing. This rule narrows it: for the checks below, the backing must be the reviewer-of-record's own eyes on raw output. The reviewer-of-record is the agent whose verdict ships — when a workflow delegates the deeper read to a subagent, the orchestrator remains the reviewer-of-record and these checks stay with them. + +Their value is in catching unknown-unknowns. A delegate asked for "a punch list of findings" returns things that fit the punch-list shape; `Bin 0 -> 8181 bytes` on a `.ts` file does not look like a finding, it looks like stat noise, and gets collapsed away. Only direct inspection preserves the signal. + +**Reviewer-of-record must run, in-session, and read the raw output:** + +- `git diff ...HEAD --stat` — scan for `Bin` markers on any file you did not expect to be binary (source code, markdown, config) and for files outside the branch's stated scope. +- `git log --oneline ..HEAD` — confirm the commits on the branch match the issue's scope; unexpected or off-topic commits are stop conditions. +- The subject and body audits enumerated under _Commit-Message Style Audit_. That section is the canonical command catalog and pattern list; this section's contribution is the delegation rule — those audits are reviewer-of-record, not subagent work. + +Delegating the deeper read (file-by-file behavioral review, architectural analysis, commit-message coherence) to a subagent is fine and often valuable for context isolation. Delegating the checks above is not — their value is the raw output landing in front of the reviewer-of-record's eyes. + +## Commit-Message Coherence + +Each commit's message is a claim about what the commit contains. Verify that +claim. For every commit on the branch, read the message, read the diff +(`git show `), and confirm that the two match. + +If a commit contains material changes that are unrelated to its message — +files touched that have nothing to do with the stated purpose, unmentioned +behavioral changes, or new functionality that the message does not describe — +flag it. Undescribed changes hiding inside a commit undermine the reviewability +and auditability of the history. + +This includes whitespace or formatting changes to code that is not otherwise +being modified. Drive-by cleanup belongs in its own commit — its presence in +an unrelated commit is itself a problem. + +Do not require that every modified line be individually narrated in the message. +The message is a summary. But if someone reading only the message would be +surprised by what the diff actually contains, that is a problem worth raising. + +## Commit-Message Style Audit + +Coherence (above) checks that each message accurately describes its diff. Style audit checks that each message conforms to the project's commit-message rules (see the `style` skill, which is the canonical source for the prefix-family list and other rules). The two are independent; both have to be run. -## Cite the check +Each check below is reviewer-of-record territory (see _Reviewer-of-Record Checks_) — run them in-session and read the raw output yourself. Do not delegate them; a subagent asked to "verify style compliance" will return a generic "looks fine" read with no audit trail. -Every affirmative claim ("tests pass", "messages are clean", "no regressions") needs a concrete check: a command whose output was read, a file/line range examined for a named pattern. "It looked fine" is not a check. +**Subject-line audits.** Most checks scan the output of: -When a check cannot be cited: run it, strike the claim, or narrow to what was actually examined. +```bash +git log ..HEAD --format='%s' +``` + +Scan for: -- "Tests pass." → "`bun run test` exited 0," or strike. -- "No race conditions." → "read `lock.go:42-68`; no acquire-while-holding cycles in those lines. Did not analyze `pool.go`." +- **Prefix violations.** Any subject starting with a `word:`, `[tag]`, or `(scope)` pattern. Includes Conventional Commits (`feat:`, `fix:`), component or scope prefixes (`Anthropic adapter:`, `mm:`, `[X86]`), ticket IDs (`INTR-79:`), and status tags (`WIP:`). Project convention is plain English sentences; any prefix is a violation regardless of how idiomatic it looks in other ecosystems. +- **Filename or path references.** Tokens that look like file paths or extensions (`server.ts`, `INFERENCE.md`, `src/foo/bar.py`). The diff lists what changed; subjects describe the change, not the file. +- **Trailing punctuation.** Subjects ending with `.`, `!`, or `?`. +- **Vague subjects.** "Update code," "Fix bug," "Misc changes," "Address review." -## Reviewer-of-record checks +For the length limit, use a length-aware filter so the check is not eyeball-counting: + +```bash +git log ..HEAD --format='%s' | awk 'length > 72' +``` -The agent whose verdict ships must run these in-session and read the raw output. Do not delegate them. +Empty output is clean. Any line returned is an over-72-character subject violation. -- `git diff ...HEAD --stat` — scan for unexpected `Bin` markers and files outside stated scope -- `git log --oneline ..HEAD` — commits must match the issue/ask scope -- The subject and body audits under Commit-message style +**Body audits.** Most checks scan the output of: -File-by-file behavioral review may be delegated. The checks above may not. +```bash +git log ..HEAD --format='%b' +``` -## Commit-message coherence +Scan for: -For every commit: read the message, read `git show `, confirm they match. Flag undescribed material changes, unmentioned behavior, or drive-by formatting in an unrelated commit. +- **External tracker references.** Linear/Jira/GitHub IDs (`INTR-79`, `JIRA-1234`, `#456`, `Closes XXX-99`). The commit must explain itself. +- **References to other commits in the series.** "as discussed in the previous commit," "the next commit wires this up," "see also abc1234." A commit describes its own state, not the branch's trajectory. +- **References to PR review comments or session conversation.** Ephemeral context the future reader cannot access. -The message is a summary, not a line-by-line narration. Surprise vs the diff is the bar. +For body-line length, use the same length-aware filter: -## Commit-message style +```bash +git log ..HEAD --format='%b' | awk 'length > 72' +``` -Canonical rules live in the `style` skill. Audit independently of coherence. +Empty output is clean. Any line returned is an over-72-character body line. -Subjects (`git log ..HEAD --format='%s'`): +Affirmative claims about these audits must cite the specific command whose output proved them — see **Cite the Check**. -- No `word:`, `[tag]`, or `(scope)` prefixes (`feat:`, ticket IDs, `WIP:`) -- No filenames or paths -- No trailing punctuation -- No vague subjects ("Update code", "Fix bug", "Address review") -- Length: `git log ..HEAD --format='%s' | awk 'length > 72'` — empty is clean +## Review Checklist -Bodies (`git log ..HEAD --format='%b'`): +Items marked _(reviewer-of-record)_ must be run by the agent whose verdict ships — see _Reviewer-of-Record Checks_. Do not delegate them to a subagent. Unmarked items are delegable. -- No tracker IDs or PR/session references -- No "previous commit" / "next commit" series talk -- Length: `git log ..HEAD --format='%b' | awk 'length > 72'` — empty is clean +1. Determine the base branch using the methods in "Base Branch Determination" +2. Run `git log --oneline ..HEAD` to understand the scope _(reviewer-of-record)_ +3. Run `git diff ...HEAD --stat` to see which files changed _(reviewer-of-record)_ +4. Review each changed file, focusing only on lines modified by the branch +5. For every commit on the branch, verify: + - Diff matches the commit message (see "Commit-Message Coherence") + - Subject and body pass the style audit (see "Commit-Message Style Audit") _(reviewer-of-record)_ +6. Check that new code follows project conventions +7. Summarize findings with specific file:line references; cite the check behind any affirmative claim (see "Cite the Check") +8. When the review targets an open GitHub PR, post the review on the PR (see "Post the Review on GitHub") -Affirmative claims about these audits must cite the command. See Cite the check. +## Post the Review on GitHub -## Checklist +When the branch under review has an open GitHub pull request, **post the finished review on the PR**. A review that only lives in the chat session is not done. -Items marked _(reviewer-of-record)_ stay with the agent whose verdict ships. +This step is the delivery of the review, not a second pass of analysis. By the time you post, findings are already decided. Do not reopen the read while drafting the body. -1. Determine the base branch -2. `git log --oneline ..HEAD` _(reviewer-of-record)_ -3. `git diff ...HEAD --stat` _(reviewer-of-record)_ -4. Review each changed file, only lines the branch modified -5. Per commit: message matches diff; subject/body pass the style audit _(reviewer-of-record)_ -6. New code follows project conventions -7. Summarize findings with `path:line`; cite the check behind any affirmative claim -8. If the review targets an open GitHub PR and the operator asked to post, post it (see below) +### When to post -## Post on GitHub +Post when any of these is true: -Post only when the operator asked to leave a review on an open PR (or `pull-request-review` / a PR URL made that the job). A local read with no PR is done in chat. +- The user asked for a PR review (`pull-request-review`, a PR URL, or an explicit "review #N") +- `linear-issue-workflow` Phase 5 self-review has cleared and Phase 6 is opening or updating the PR +- An open PR exists for the branch and the review's purpose is to leave a record on it -Use a real review, as the operator's `gh` identity — never a vendor bot: +Do **not** post when the user only asked for a private/local read with no PR, or when the branch has no open PR and creating one is out of scope. + +### Multi-persona reviews + +When the workflow ran more than one review lens (for example `critique` for behavioral/architecture, `greybeard` for waivers or product judgment, an OSS/quality agent for packaging and public-API bar), each lens that produced a distinct judgment **posts its own review**. Do not collapse independent verdicts into one mushy "team thinks" paragraph. + +| Lens | What it owns | When to post | +| ---------------------- | ------------------------------------------------------------------ | ----------------------------------------------------------------------------------------- | +| Primary / orchestrator | Verdict on the branch as it stands; residual findings; waiver list | Always when posting | +| Critique | Behavioral bugs, missing tests, architecture, commit coherence | When a critique subagent ran | +| Greybeard | Waiver rulings and intentional exceptions | When Greybeard authorized any waiver, or when product/architecture judgment was requested | +| OSS / quality | Public-API, packaging, polish bar for shippable surface | When that lens was explicitly run | + +Same GitHub account is fine. Label each post so a human can tell which lens spoke. Prefer separate `gh pr review` / `gh pr comment` posts over one mega-comment when more than one lens has substance. + +If only the primary review ran, post once. Do not invent extra personas. + +### Command + +Use a real GitHub review, not a floating issue comment, for the primary verdict: ```bash -gh pr review --approve|--comment|--request-changes --body "$(cat <<'EOF' +# Approve — branch is ready to merge as it stands +gh pr review --approve --body "$(cat <<'EOF' + +EOF +)" + +# Comment — findings or notes that do not block merge by themselves +gh pr review --comment --body "$(cat <<'EOF' + +EOF +)" + +# Request changes — at least one finding must be fixed before merge +gh pr review --request-changes --body "$(cat <<'EOF' EOF )" ``` -Body shape: +Secondary persona posts may use `gh pr review --comment` or `gh pr comment`. Prefer `gh pr review --comment` so they appear in the Reviews timeline. Never `--approve` from a secondary lens that did not own the merge verdict. + +### Body shape (no AI slop) + +Write like a senior engineer leaving a review on a busy PR. Short. Specific. Present tense. No journey narration. + +**Required shape:** ```markdown -## Review · +## · - + ### Findings -- `path/to/file.ts:12` — +- `path/to/file.ts:12` — +- `path/to/other.ts:40` — <…> + +### Notes + +- ``` -Hard bans: throat-clearing, AI filler, journey narration, emoji, restating the diff with no finding, "LGTM" alone. +When the review is clean: + +```markdown +## Review · Approve + +Hub list/get/upload under `/api/tenants/:id/artifacts`; Library reads that plane. + +No findings. +``` + +**Hard bans** (delete on sight before posting): + +- Throat-clearing: "Great work", "Thanks for this", "Overall this looks solid", "Happy to approve" +- AI filler: "I'd like to highlight", "It's worth noting", "In conclusion", "Going forward" +- Journey talk: "this PR adds… then fixes… after feedback…" +- Fake balance: praising three things to soften one finding +- Emoji, decorative headers, horizontal rules used as ornament +- Restating the diff file-by-file when there is no finding +- "LGTM" alone with no one-line present-tense description of what the branch does -Every finding has `path:line` and a concrete failure mode. If it is not worth the author's time, drop it. +**Findings rules:** -After posting, paste the review URL. `--request-changes` means the PR is not merge-ready. +- Every finding has a `path:line` (or `path` when line is meaningless) and a concrete failure mode +- Severity is the review action (`--request-changes` vs `--comment`), not adjectives in the body +- No "nit:" / "minor:" / "suggestion:" as a way to smuggle unactionable taste — if it is not worth the author's time, drop it (see Signal Over Noise) -## After the report +### After posting -Synthesize Summary / Findings / Blockers / Paths. Do not land fixes. If the operator then wants repairs, that is a later `/implement` — not this skill. +Paste the review URL(s) back to the user. If `--request-changes`, do not mark the Linear issue Done or tell the user the PR is merge-ready. diff --git a/plugins/corbits-skills/skills/scribe/SKILL.md b/plugins/corbits-skills/skills/scribe/SKILL.md index 2951c785c..35bcc637e 100644 --- a/plugins/corbits-skills/skills/scribe/SKILL.md +++ b/plugins/corbits-skills/skills/scribe/SKILL.md @@ -1,66 +1,397 @@ --- name: scribe -description: Maintain PRODUCT.md, ARCHITECTURE.md, and IMPLEMENTATION.md. Routes input, detects gaps, and asks for completeness. +description: Maintain product, architecture, and implementation docs — routes input, detects gaps, and interviews for completeness --- # Scribe -How to maintain product, architecture, and implementation docs. Analyze input, route it to the correct document, then check gaps and cross-doc consistency. One-line wording fixes may be edited directly. +Use this skill to maintain documentation across multiple documents that represent different levels of abstraction. When the user provides input, analyze it and route it to the correct document. Scribe is not a passive filing system — after recording what the user provides, it actively identifies gaps, checks cross-document consistency, and asks targeted questions to strengthen the documentation. -Clarifying questions use `ask_operator`. DESIGN.md is rand, not this skill. +## Document Discovery -## Document discovery +Before processing input, locate the documentation files: -Before classifying input, locate and read existing docs (case-insensitive) in the repo root and `docs/`: +1. Search for existing files matching `PRODUCT.md`, `ARCHITECTURE.md`, and `IMPLEMENTATION.md` (case-insensitive) in: + - Repository root + - `docs/` directory -- PRODUCT.md, ARCHITECTURE.md, IMPLEMENTATION.md -- Prefer root when multiple matches exist -- Defaults when missing: create at repository root after confirming +2. If documents exist, use their locations. If multiple matches exist for the same type, prefer the repository root. -Read existing docs first — vocabulary, patterns, constraints, similar features. +3. If no documents exist, use these defaults when creating new ones: + - `PRODUCT.md` in repository root + - `ARCHITECTURE.md` in repository root + - `IMPLEMENTATION.md` in repository root -## Document types +## Document Types -**PRODUCT.md** — what we build and why: user value, vision, goals, target users, business justification. +**Product** - Product-level documentation -**ARCHITECTURE.md** — how the system is structured: components, relationships, abstractions, data/control flow, technology-agnostic design. +- What we're building and why +- User-facing value propositions +- Vision and goals +- Target users and use cases +- Business justification -**IMPLEMENTATION.md** — concrete tech: libraries, protocols, formats, configuration, deployment. +**Architecture** - System architecture documentation -## Steps +- How the system is structured +- Components and their relationships +- Abstractions and interfaces +- Data flow and control flow +- Design decisions that are technology-agnostic -### 1. Analyze and classify +**Implementation** - Implementation documentation -Use general heuristics plus project vocabulary from the existing docs (project terms win when clear): +- Specific technology choices +- Protocols and formats +- Libraries and frameworks +- Concrete technical details +- Configuration and deployment specifics -- **Product:** user needs, value, market, "users can", goals without how -- **Architecture:** components, interactions, abstractions, tech-agnostic design -- **Implementation:** named technologies, wire formats, config, "uses" / "built on" +## Using the Question Tool -### 2. Route and deepen +Throughout this skill, you will use `ask_operator` to interact with the user. `ask_operator` presents multiple-choice questions with predefined options. -If classification is clear, update the right document. +**Key mechanics:** -If ambiguous or multi-category, do not only ask "which document?" Decompose into distinct claims and route each. Prefer context-aware `ask_operator` options from existing docs; fall back to general options when docs are empty. One statement may update multiple docs. +- You can present multiple questions in a single tool call (as an array of questions) +- Each question has a header, question text, and multiple options +- Each option has a label and description +- Users can select one or multiple options (if `multiple: true`) +- The tool automatically includes a "Type your own answer" option by default +- Questions are answered together as a batch, but you should make options context-aware based on information you already have -### 3. Update +**When to provide context-aware options:** -Read the target, place content (extend section / new section / revise), match existing style. Significant changes (new concept/component/capability, contradiction, top-level decision) trigger steps 4–5. Minor clarifications skip to the report. +- Reference similar features, patterns, or components already documented +- Suggest options based on answers from previous interactions in the session +- Use project-specific terminology from existing documents +- When no patterns exist (empty/minimal documents), provide general options as fallbacks -### 4. Cross-document consistency (significant only) +**Example invocation:** -Check sibling docs for implied missing entries (new architecture with no product justification, product capability with no architecture, implementation naming an undescribed component). Surface targeted questions; update from answers. +```json +{ + "questions": [ + { + "header": "Document classification", + "question": "Is 'fast and reliable' a user-facing promise or a system design requirement?", + "options": [ + { + "label": "User-facing promise", + "description": "Add to PRODUCT.md like other user benefits" + }, + { + "label": "System design requirement", + "description": "Add to ARCHITECTURE.md with latency targets" + }, + { + "label": "Both", + "description": "It's a user promise AND a technical constraint" + } + ] + }, + { + "header": "Performance target", + "question": "Does 'fast' have a concrete target?", + "options": [ + { + "label": "Under 5 seconds", + "description": "Similar to report generation target" + }, + { + "label": "Different target", + "description": "Specify a different performance goal" + }, + { + "label": "No specific target", + "description": "Keep it qualitative for now" + } + ] + } + ] +} +``` -### 5. Gap detection (significant only) +The tool returns the selected options as an array of labels (e.g., `["Both", "Under 5 seconds"]`). -Scan for thin sections, undefined references, missing failure modes/constraints, decisions without rationale. Probe with contextual options. If the operator declines further probing, stop unless they ask. +## Execution Steps -### 6. Report +### Step 0: Document Discovery and Reading -Confirm what changed and where. Summarize consistency/gap follow-ups. +Before classifying input, locate and read the existing documentation files to learn project-specific vocabulary and patterns. This information will be used throughout all subsequent steps for context-aware questioning. -## Errors +1. Search for existing files matching `PRODUCT.md`, `ARCHITECTURE.md`, and `IMPLEMENTATION.md` (case-insensitive) in repository root and `docs/` directory +2. Read all existing documents to extract: + - Key terms and component names + - Patterns in how features are described + - Existing constraints, limits, or policies + - Similar features that can serve as templates +3. If documents are empty or minimal, note that general options will be needed instead of context-aware ones -- **Missing doc:** `ask_operator` whether to create it. -- **Conflict with existing content:** `ask_operator` replace / keep both / merge. -- **Unclear scope:** `ask_operator` which document and why. +### Step 1: Analyze Input + +Read the user's input and determine which category it falls into. Classification is based on two sources: general heuristics and project-specific signals learned from existing documents (Step 0). + +**General heuristics:** + +_Product signals:_ + +- Describes user needs or problems +- Explains value or benefits +- Discusses market or competitive positioning +- Uses language like "users can", "enables", "provides value" +- Talks about goals without specifying how + +_Architecture signals:_ + +- Describes components or modules +- Explains how parts interact +- Defines abstractions or interfaces +- Discusses system properties without naming specific technologies +- Technology-agnostic design decisions + +_Implementation signals:_ + +- Names specific technologies, protocols, or formats +- Describes wire formats or API specifications +- Specifies configuration details +- Uses language like "uses", "built on", "implemented with" +- Concrete technical choices + +**Project-specific signals:** + +Read the existing documents to learn the project's vocabulary. Extract key terms, component names, and patterns that indicate document ownership. For example: + +- If the architecture document discusses "the kernel" and "agents", mentions of these terms suggest architectural content +- If the implementation document discusses "SMTP" and "IMAP", mentions of email protocols suggest implementation content +- If the product document discusses "wallets" as a user-facing feature, wallet mentions in a value context suggest product content + +Use these learned signals alongside general heuristics. Project-specific vocabulary takes precedence when it provides a clear signal. + +### Step 2: Classify and Deepen + +If the categorization is clear, proceed to update the appropriate document. + +If the input is ambiguous or spans multiple categories, do not simply ask "which document?" Instead, use `ask_operator` to interview the user and decompose the input into distinct claims that can each be routed precisely: + +1. Explain what makes the input ambiguous — identify the product, architecture, and/or implementation aspects you see in it. +2. Use `ask_operator` to ask targeted questions that separate those aspects. Based on the context you have from existing documents (Step 0) and the user's input, provide relevant options that help clarify the intent. + +**If documents have content with patterns to reference:** + +- When user mentions "fast and reliable", reference existing performance promises or design constraints: + - "User-facing promise (add to PRODUCT.md like [similar feature])" + - "System design requirement (add to ARCHITECTURE.md with latency targets)" + - "Both - it's a user promise AND a technical constraint" +- When user mentions a component name, reference similar components: + - "[Component] is user-facing (like [similar component] in PRODUCT.md)" + - "[Component] is an internal abstraction (add to ARCHITECTURE.md)" + +**If documents are empty/minimal (no patterns to reference):** + +- Provide general options without specific references: + - "User-facing promise (add to PRODUCT.md)" + - "System design requirement (add to ARCHITECTURE.md)" + - "Both - it's a user promise AND a technical constraint" + +3. Route each extracted piece to its appropriate document. A single user statement may result in updates to multiple documents. + +### Step 3: Update Document + +Read the target document to understand its current structure and content. + +Determine where in the document the new content belongs: + +- Does it extend an existing section? +- Does it require a new section? +- Does it modify existing content? + +Make the update, maintaining the document's existing style and structure. + +After updating, assess whether the change is **significant**. A change is significant if it: + +- Introduces a new concept, component, or section +- Contradicts or substantially revises existing content +- Adds a top-level capability or design decision + +If the change is minor — extending an existing section with more detail, fixing wording, adding a clarification — skip Steps 4 and 5 and proceed directly to Step 6. + +### Step 4: Cross-Document Consistency + +_Only for significant updates._ + +Read the other two documents and check whether the new content implies entries that should exist in sibling documents but don't. Common patterns to look for: + +- A new architecture component with no corresponding product justification +- A new product capability with no architectural description of how it works +- An implementation detail referencing a component not described in architecture +- A product goal with no implementation approach mentioned + +If gaps are found, use `ask_operator` to present them as a batch of 2-4 questions. Based on the context from existing documents (Step 0) and the change just made, provide specific, relevant options. + +**Example with existing patterns:** + +If you just added an export service to ARCHITECTURE.md, and PRODUCT.md has no mention of exports: + +> I updated ARCHITECTURE.md with the export service. I noticed some potential gaps in other documents. + +Use `ask_operator` with: + +- Question 1: "Should PRODUCT.md describe data export as a user-facing capability?" + - **If PRODUCT.md has similar features**: "Add export as data access capability (like reports feature)" / "Add as part of reporting feature" + - **If PRODUCT.md is minimal**: "Yes, add as new user-facing capability" / "No, exports are internal only" +- Question 2: "How should IMPLEMENTATION.md describe export generation?" + - **If IMPLEMENTATION.md describes other services**: "Similar to [existing service], using [library]" / "Different approach (specify details)" + - **If IMPLEMENTATION.md is minimal**: "Specify library/technology used" / "Defer implementation details for now" + +For each question the user answers, update the corresponding document before proceeding. + +### Step 5: Gap Detection and Completeness + +_Only for significant updates._ + +Scan the updated document for weaknesses: + +- Concepts referenced but not elaborated +- Sections that are thin relative to their importance +- Missing failure modes, edge cases, or constraints +- Decisions stated without rationale + +Use `ask_operator` to present 2-4 probing questions as a batch. Focus on non-obvious gaps — things the user might not think to document unprompted. Based on the content just added, questions already answered in this session, and patterns from existing documentation, provide specific, contextual options. + +**Example with existing patterns:** + +If you just added an export service to ARCHITECTURE.md: + +- Question 1: "What happens when an export fails mid-generation?" + - **If other services have retry logic**: "Automatic retry (like [existing service])" / "User must re-trigger" / "Saved as partial export for resume" + - **If no retry patterns exist**: "Automatic retry" / "User must re-trigger" / "Partial export saved for resume" +- Question 2: "Are there size or rate limits on exports?" + - **If similar features have limits**: "Same limits as [similar feature]" / "Different limits (specify)" / "No hard limits - best effort" + - **If no limits documented**: "10k rows / 100MB max" / "No hard limits - best effort" / "To be determined" +- Question 3: "Who has permission to trigger exports?" + - **If docs mention role-based access**: "Any authenticated user" / "Only admin/owner roles" / "Configurable per workspace" + - **If auth not documented**: "Any authenticated user" / "Role-restricted (specify roles)" / "To be determined" + +Update the document with any answers the user provides. If the user declines to answer, move on without pressing. + +**User fatigue consideration:** If the user has declined 3 or more gap detection questions in this session, skip remaining gap detection steps unless the user explicitly requests them. + +### Step 6: Report + +Briefly confirm what was changed and in which document. If Steps 4 or 5 resulted in additional updates, summarize those as well: + +> Updated ARCHITECTURE.md: added export service component under "Data Pipeline" section. +> Also updated PRODUCT.md: added data export as a user-facing capability (from consistency check). + +## Examples + +These examples demonstrate how classification and the active documentation steps work. The specific terms will vary by project. + +### Classification + +**Input:** "Users can export their data in multiple formats" +**Classification:** Product (describes user-facing capability) + +**Input:** "The export service validates permissions before generating files" +**Classification:** Architecture (describes component responsibility and interaction) + +**Input:** "Exports are generated as CSV using the fast-csv library" +**Classification:** Implementation (names specific format and library) + +### Depth Elicitation (Step 2) + +**Input:** "Data export is fast and reliable" +**Classification:** Ambiguous — has both product and architecture aspects. + +Instead of asking "which document?", use `ask_operator` to decompose. After reading existing docs (Step 0) and seeing that PRODUCT.md already mentions "reports" as a user-facing feature and ARCHITECTURE.md discusses latency targets for other services: + +Use `ask_operator`: + +- Question 1: "Is 'fast and reliable' a promise to users or a system design requirement?" + - Option 1: "User-facing promise (add to PRODUCT.md like other user benefits)" + - Option 2: "System design requirement (add to ARCHITECTURE.md with latency targets)" + - Option 3: "Both - it's a user promise AND a technical constraint" +- Question 2: "Does 'fast' have a concrete target?" + - Option 1: "Yes - under 5 seconds (similar to report generation target)" + - Option 2: "Yes - but different target (specify)" + - Option 3: "No specific target yet" + +If the user selects "Both" and "under 5 seconds", this produces two updates: + +- **PRODUCT.md:** Data export completes in under 5 seconds for typical datasets (up to 10k rows). +- **ARCHITECTURE.md:** The export pipeline must meet a 5-second latency target for datasets up to 10k rows. + +### Cross-Document Consistency (Step 4) + +**Scenario:** User adds "The notification service delivers messages through email, SMS, and push" to ARCHITECTURE.md. + +After updating, scribe reads the other documents and finds that PRODUCT.md has no mention of notifications as a user-facing feature, but does mention "alerts" in a different context. IMPLEMENTATION.md describes other third-party integrations using specific provider names. + +Use `ask_operator`: + +- Question 1: "Should PRODUCT.md describe notifications as a user-facing capability?" + - Option 1: "Yes - add as new notifications feature (users receive updates via email/SMS/push)" + - Option 2: "Yes - integrate with existing 'alerts' feature (notifications are how alerts are delivered)" + - Option 3: "No - notifications are internal only, not user-facing" +- Question 2: "Should IMPLEMENTATION.md specify the notification providers?" + - Option 1: "Yes - using [provider] (similar to how we document other integrations)" + - Option 2: "Yes - but different providers (specify which)" + - Option 3: "Not yet - still evaluating options" + +### Gap Detection (Step 5) + +**Scenario:** User adds a new "Authentication" section to ARCHITECTURE.md describing token-based auth with refresh tokens. + +After updating, scribe scans the section and identifies gaps. From reading ARCHITECTURE.md (Step 0), scribe notices other sections mention security constraints and timeout values. IMPLEMENTATION.md describes storage mechanisms for other sensitive data. + +Use `ask_operator`: + +- Question 1: "What happens when a refresh token is revoked?" + - Option 1: "User signed out immediately (like session invalidation elsewhere in the system)" + - Option 2: "User signed out at next request (deferred enforcement)" + - Option 3: "Configurable per deployment" +- Question 2: "Is there a maximum session duration?" + - Option 1: "Yes - 30 days (similar to other timeout values in the docs)" + - Option 2: "Yes - but different duration (specify)" + - Option 3: "No hard limit - refresh tokens last indefinitely until revoked" +- Question 3: "How are tokens stored on the client side?" + - Option 1: "Same as [other sensitive data] - in secure storage" + - Option 2: "Different approach (specify storage mechanism)" + - Option 3: "Client implementation decision - not specified in architecture" + +## Error Handling + +### Document does not exist + +If the target document does not exist, use `ask_operator` to ask the user if they want to create it, with context about what type of document it is: + +Question: "The [DOCUMENT].md file does not exist. Should I create it?" +Options: + +- "Yes, create [DOCUMENT].md (will contain [brief description based on document type])" +- "No, use a different document instead" + +### Content conflicts + +If the new content contradicts existing content, use `ask_operator` to flag it with specific options: + +Question: "This conflicts with existing content in [DOCUMENT].md: '[existing content]'. How should I resolve this?" +Options based on the nature of the conflict: + +- "Replace old content with new (new information supersedes old)" +- "Keep both with clarification (they represent different aspects/contexts)" +- "Merge the two (combine into comprehensive description)" + +### Unclear scope + +If the input is too broad or vague to place in a specific document, use `ask_operator` to narrow it down: + +Question: "I'm not sure where '[user input]' belongs. Can you help me place it?" +Options based on what aspects you can detect: + +- "PRODUCT.md ([specific user-facing aspect you detected])" +- "ARCHITECTURE.md ([specific structural aspect you detected])" +- "IMPLEMENTATION.md ([specific technical aspect you detected])" +- "Multiple documents (it spans several concerns)" diff --git a/tests/unit/corbits-skills-catalog.test.ts b/tests/unit/corbits-skills-catalog.test.ts index e1c3ea41e..b589410de 100644 --- a/tests/unit/corbits-skills-catalog.test.ts +++ b/tests/unit/corbits-skills-catalog.test.ts @@ -184,9 +184,9 @@ test("philosophy skill is guidance without fake enforcement", async () => { test("review skill is a code-review playbook, not a director router", async () => { const skill = await Bun.file(join(pluginRoot, "skills/review/SKILL.md")).text(); expect(skill).toContain("git diff ...HEAD"); - expect(skill).toContain("Cite the check"); - expect(skill).toContain("Signal over noise"); - expect(skill).toContain("Pre-existing code"); + expect(skill).toContain("Cite the Check"); + expect(skill).toContain("Signal Over Noise"); + expect(skill).toContain("Pre-existing Code"); expect(skill).toContain("do not implement fixes"); expect(skill).toContain("Findings only"); expect(skill).not.toContain("spawn_agent"); From f713be755ede08e4a6ee8cb19a2b3cd341b5dab4 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 25 Aug 2026 08:29:00 -0700 Subject: [PATCH 4/7] Hide dispatch as a Skywalker background library Skywalker already orchestrates. Keep the on-disk DAG protocol behind an explicit use_skill load, not in the listing. --- CHANGELOG.md | 2 ++ docs/ARCHITECTURE.md | 2 +- docs/PRODUCT.md | 2 +- plugins/corbits-skills/skills/dispatch/SKILL.md | 7 +++++-- src/agent/directors/identity.test.ts | 2 +- src/agent/directors/skywalker/package.test.ts | 7 +------ src/agent/directors/skywalker/package.ts | 8 ++++---- tests/unit/corbits-skills-catalog.test.ts | 3 +-- 8 files changed, 16 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ebc7f293..bece1e4d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename - First-party skills are how-to playbooks (what to do, in what order, what done looks like), not director personas. Identity stays on director system prompts. +- Dispatch is a background library for an existing `dispatch/` tree. Skywalker + orchestrates natively and no longer lists dispatch as an optional skill. ### Fixed diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 312b4dfb8..abc9d7bae 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -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 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`. Dispatch is `use_skill` only, not a default slash. Draper and emil are closed directors via `task(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; `/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`. Dispatch is a background library (`disable-model-invocation`) for an existing `dispatch/` tree — Skywalker orchestrates natively without it. Draper and emil are closed directors via `task(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 486637dee..34126ddd6 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 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`). 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 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`). Dispatch is a background library for an existing `dispatch/` tree (hidden from slash and the use_skill listing). `git-rebase`, `linear-issue-workflow`, `style`, `philosophy`, `typescript`, and `opsh` stay `use_skill` only (`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. diff --git a/plugins/corbits-skills/skills/dispatch/SKILL.md b/plugins/corbits-skills/skills/dispatch/SKILL.md index f346b1034..7817d069b 100644 --- a/plugins/corbits-skills/skills/dispatch/SKILL.md +++ b/plugins/corbits-skills/skills/dispatch/SKILL.md @@ -1,13 +1,16 @@ --- name: dispatch user-invocable: false +disable-model-invocation: true argument-hint: "[ | dispatch// | dispatch//dispatch.yaml | ]" -description: Multi-lane DAG orchestration via use_skill("dispatch"). Spawns explorer, intern, builder, counsel, and critic. Use for DAG product work; tiny edits outside the DAG may be DIY. +description: On-disk DAG protocol (dispatch.yaml / plan.md / resume). Background library — load via use_skill("dispatch") when a dispatch/ tree already exists. Absent from slash and use_skill listing. Skywalker orchestrates natively without this skill. --- # Dispatch -How to orchestrate parallel director runs across a dependency graph. Fan out work, fan in reports, critique, verify, re-dispatch fixes, and synthesize until done. +On-disk DAG protocol for an existing `dispatch/` tree. Skywalker orchestrates natively with spawn_agent / wait_agents / manage_tasks — do not load this skill for ordinary multi-lane work. + +How to run a `dispatch.yaml` graph: fan out work, fan in reports, critique, verify, re-dispatch fixes, and synthesize until done. Fan out work, fan in reports, critique, verify, re-dispatch fixes, and synthesize until done. DAG product tasks go through builder workers. Do not write `dispatch.yaml` or `plan.md` on the parent (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. diff --git a/src/agent/directors/identity.test.ts b/src/agent/directors/identity.test.ts index 5ef5347ba..0357f6746 100644 --- a/src/agent/directors/identity.test.ts +++ b/src/agent/directors/identity.test.ts @@ -80,7 +80,7 @@ describe("formatDirectorSystemPrompt", () => { expect(text).not.toContain("use_skill is not mounted on workers"); expect(text).not.toMatch(/guidance is baked/i); expect(text).toContain("use_skill is primary-mounted"); - expect(text).toContain("dispatch, style, philosophy, interview"); + expect(text).toContain("style, philosophy, interview"); }); test("counsel does not bake interview ask_operator guidance (CL-6803)", () => { diff --git a/src/agent/directors/skywalker/package.test.ts b/src/agent/directors/skywalker/package.test.ts index c0a6e271f..62213e7f0 100644 --- a/src/agent/directors/skywalker/package.test.ts +++ b/src/agent/directors/skywalker/package.test.ts @@ -56,12 +56,7 @@ describe("skywalkerPackage", () => { }); test("optionalSkills order", () => { - expect(skywalkerPackage.optionalSkills).toEqual([ - "dispatch", - "style", - "philosophy", - "interview", - ]); + expect(skywalkerPackage.optionalSkills).toEqual(["style", "philosophy", "interview"]); }); test("primaryIntent and outOfLane", () => { diff --git a/src/agent/directors/skywalker/package.ts b/src/agent/directors/skywalker/package.ts index 61acf8443..65079bb84 100644 --- a/src/agent/directors/skywalker/package.ts +++ b/src/agent/directors/skywalker/package.ts @@ -112,7 +112,7 @@ Docs/design (PRODUCT.md, ARCHITECTURE.md, docs/design/*, brand) still spawn shak 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 or the dispatch skill for multi-lane eng plans; clarify before large dispatch. +4. Use counsel for multi-lane eng plans; clarify before a large fan-out. 5. Track progress with manage_tasks; synthesize results for the operator. ## If ORCHESTRATION → coordinate @@ -129,9 +129,9 @@ 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 or dispatch skill for multi-lane eng plans; clarify before large dispatch. +- Use counsel for multi-lane eng plans; clarify before a large fan-out. - 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). +- Optional skills when needed on the primary session: style, philosophy, interview (use_skill is primary-mounted). For an existing dispatch/ tree, load use_skill("dispatch"). # Spawn graph @@ -175,7 +175,7 @@ export const skywalkerPackage: DirectorPackage = { ], description: "Primary orchestration director — chains specialists into a workflow", systemPrompt: SKYWALKER_SYSTEM_PROMPT, - optionalSkills: ["dispatch", "style", "philosophy", "interview"], + optionalSkills: ["style", "philosophy", "interview"], tools: { allow: SKYWALKER_TOOLS }, spawn: { maySpawn: true, diff --git a/tests/unit/corbits-skills-catalog.test.ts b/tests/unit/corbits-skills-catalog.test.ts index b589410de..dcf461ca4 100644 --- a/tests/unit/corbits-skills-catalog.test.ts +++ b/tests/unit/corbits-skills-catalog.test.ts @@ -30,7 +30,6 @@ const SPAWN_RECIPE_SKILLS = ["implement", "dispatch"] as const; /** use_skill listing + resolve; not slash. No disable-model-invocation. */ const USE_SKILL_ONLY = [ - "dispatch", "git-rebase", "linear-issue-workflow", "style", @@ -40,7 +39,7 @@ const USE_SKILL_ONLY = [ ] as const; /** Background libs: absent from slash and use_skill listing; explicit resolve only. */ -const BACKGROUND_ONLY = ["git-worktrees"] as const; +const BACKGROUND_ONLY = ["git-worktrees", "dispatch"] as const; const SLASH_SKILLS = [ "implement", From 909206bdafe5cad27be1206bc184d2164ef13d20 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 25 Aug 2026 08:31:07 -0700 Subject: [PATCH 5/7] Tell Skywalker when native fleet beats the dispatch protocol Load the dispatch skill only for an existing dispatch tree or an operator-named dispatch run, never because the work is large. --- src/agent/directors/skywalker/package.test.ts | 9 +++++++++ src/agent/directors/skywalker/package.ts | 7 ++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/agent/directors/skywalker/package.test.ts b/src/agent/directors/skywalker/package.test.ts index 62213e7f0..2b43ca271 100644 --- a/src/agent/directors/skywalker/package.test.ts +++ b/src/agent/directors/skywalker/package.test.ts @@ -180,6 +180,15 @@ describe("skywalkerPackage", () => { expect(p).toContain("Do not always explorer→implement→critic"); }); + test("systemPrompt defaults to native fleet and loads dispatch only for an existing tree", () => { + const p = skywalkerPackage.systemPrompt; + expect(p).toContain("Native fleet vs dispatch/ protocol"); + expect(p).toContain("Size is not a reason to load dispatch"); + expect(p).toContain('use_skill("dispatch")'); + expect(p).toContain("never because the work is large"); + expect(p).not.toContain("Use counsel or the dispatch skill"); + }); + test("systemPrompt re-dispatches builder on blocking critic", () => { const p = skywalkerPackage.systemPrompt; expect(p).toContain("blocking"); diff --git a/src/agent/directors/skywalker/package.ts b/src/agent/directors/skywalker/package.ts index 65079bb84..856b78247 100644 --- a/src/agent/directors/skywalker/package.ts +++ b/src/agent/directors/skywalker/package.ts @@ -119,6 +119,10 @@ Docs/design (PRODUCT.md, ARCHITECTURE.md, docs/design/*, brand) still spawn shak Track with manage_tasks. Parallelize independent lanes via spawn_agent + wait_agents. After each spawn wave, update the operator before blocking. Escalate blockers with ask_operator. This is your core role. +Native fleet vs dispatch/ protocol — default native: +- **Native (A):** spawn_agent + wait_agents + manage_tasks. Use this for every new multi-lane job, including large ones. Counsel plans; you fan out. Size is not a reason to load dispatch. +- **dispatch/ protocol (B):** load use_skill("dispatch") only when (1) a dispatch// tree or dispatch.yaml already exists and the job is to run/resume it, or (2) the operator named dispatch / a dispatch.yaml / a spec-as-dispatch-input. Do not create a dispatch/ tree to justify loading the skill. + ## If COMMUNICATION → answer directly Clear and short. No dispatch for pure questions, digs, "why", screenshots of the UI, or architecture explainers. @@ -131,7 +135,8 @@ Do not reclassify COMMUNICATION as ORCHESTRATION just to justify parallel spawn - Interview when requirements are fuzzy; consult greybeard on architecture/approach. - Use counsel for multi-lane eng plans; clarify before a large fan-out. - 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: style, philosophy, interview (use_skill is primary-mounted). For an existing dispatch/ tree, load use_skill("dispatch"). +- Optional skills when needed on the primary session: style, philosophy, interview (use_skill is primary-mounted). +- Load use_skill("dispatch") only for an existing dispatch/ tree or an operator-named dispatch run — never because the work is large. # Spawn graph From c38b4e26165b947a311eff8c71a586c9be4a79b7 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 25 Aug 2026 08:34:17 -0700 Subject: [PATCH 6/7] Revert "Tell Skywalker when native fleet beats the dispatch protocol" This reverts commit 909206bdafe5cad27be1206bc184d2164ef13d20. --- src/agent/directors/skywalker/package.test.ts | 9 --------- src/agent/directors/skywalker/package.ts | 7 +------ 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/src/agent/directors/skywalker/package.test.ts b/src/agent/directors/skywalker/package.test.ts index 2b43ca271..62213e7f0 100644 --- a/src/agent/directors/skywalker/package.test.ts +++ b/src/agent/directors/skywalker/package.test.ts @@ -180,15 +180,6 @@ describe("skywalkerPackage", () => { expect(p).toContain("Do not always explorer→implement→critic"); }); - test("systemPrompt defaults to native fleet and loads dispatch only for an existing tree", () => { - const p = skywalkerPackage.systemPrompt; - expect(p).toContain("Native fleet vs dispatch/ protocol"); - expect(p).toContain("Size is not a reason to load dispatch"); - expect(p).toContain('use_skill("dispatch")'); - expect(p).toContain("never because the work is large"); - expect(p).not.toContain("Use counsel or the dispatch skill"); - }); - test("systemPrompt re-dispatches builder on blocking critic", () => { const p = skywalkerPackage.systemPrompt; expect(p).toContain("blocking"); diff --git a/src/agent/directors/skywalker/package.ts b/src/agent/directors/skywalker/package.ts index 856b78247..65079bb84 100644 --- a/src/agent/directors/skywalker/package.ts +++ b/src/agent/directors/skywalker/package.ts @@ -119,10 +119,6 @@ Docs/design (PRODUCT.md, ARCHITECTURE.md, docs/design/*, brand) still spawn shak Track with manage_tasks. Parallelize independent lanes via spawn_agent + wait_agents. After each spawn wave, update the operator before blocking. Escalate blockers with ask_operator. This is your core role. -Native fleet vs dispatch/ protocol — default native: -- **Native (A):** spawn_agent + wait_agents + manage_tasks. Use this for every new multi-lane job, including large ones. Counsel plans; you fan out. Size is not a reason to load dispatch. -- **dispatch/ protocol (B):** load use_skill("dispatch") only when (1) a dispatch// tree or dispatch.yaml already exists and the job is to run/resume it, or (2) the operator named dispatch / a dispatch.yaml / a spec-as-dispatch-input. Do not create a dispatch/ tree to justify loading the skill. - ## If COMMUNICATION → answer directly Clear and short. No dispatch for pure questions, digs, "why", screenshots of the UI, or architecture explainers. @@ -135,8 +131,7 @@ Do not reclassify COMMUNICATION as ORCHESTRATION just to justify parallel spawn - Interview when requirements are fuzzy; consult greybeard on architecture/approach. - Use counsel for multi-lane eng plans; clarify before a large fan-out. - 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: style, philosophy, interview (use_skill is primary-mounted). -- Load use_skill("dispatch") only for an existing dispatch/ tree or an operator-named dispatch run — never because the work is large. +- Optional skills when needed on the primary session: style, philosophy, interview (use_skill is primary-mounted). For an existing dispatch/ tree, load use_skill("dispatch"). # Spawn graph From ede2676cc1d9184c08b62f4f56883dffcd9f38b2 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 25 Aug 2026 08:37:06 -0700 Subject: [PATCH 7/7] Delete the first-party dispatch skill Skywalker already orchestrates. The yaml DAG protocol is gone with the skill; do not load a second orchestrator. --- CHANGELOG.md | 3 +- docs/ARCHITECTURE.md | 2 +- docs/PRODUCT.md | 2 +- .../corbits-skills/skills/dispatch/SKILL.md | 219 ------------------ .../corbits-skills/skills/implement/SKILL.md | 2 +- src/agent/directors/identity.ts | 2 +- src/agent/directors/skywalker/package.ts | 2 +- tests/unit/corbits-skills-catalog.test.ts | 9 +- 8 files changed, 10 insertions(+), 231 deletions(-) delete mode 100644 plugins/corbits-skills/skills/dispatch/SKILL.md diff --git a/CHANGELOG.md b/CHANGELOG.md index bece1e4d7..5c0d515ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,8 +17,7 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename - First-party skills are how-to playbooks (what to do, in what order, what done looks like), not director personas. Identity stays on director system prompts. -- Dispatch is a background library for an existing `dispatch/` tree. Skywalker - orchestrates natively and no longer lists dispatch as an optional skill. +- The first-party dispatch skill is gone. Skywalker orchestrates natively. ### Fixed diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index abc9d7bae..53510c687 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -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 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`. Dispatch is a background library (`disable-model-invocation`) for an existing `dispatch/` tree — Skywalker orchestrates natively without it. Draper and emil are closed directors via `task(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; `/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`. There is no first-party dispatch skill — Skywalker orchestrates natively. Draper and emil are closed directors via `task(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 34126ddd6..0de577422 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 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`). Dispatch is a background library for an existing `dispatch/` tree (hidden from slash and the use_skill listing). `git-rebase`, `linear-issue-workflow`, `style`, `philosophy`, `typescript`, and `opsh` stay `use_skill` only (`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 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`, `typescript`, and `opsh` stay `use_skill` only (`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. diff --git a/plugins/corbits-skills/skills/dispatch/SKILL.md b/plugins/corbits-skills/skills/dispatch/SKILL.md deleted file mode 100644 index 7817d069b..000000000 --- a/plugins/corbits-skills/skills/dispatch/SKILL.md +++ /dev/null @@ -1,219 +0,0 @@ ---- -name: dispatch -user-invocable: false -disable-model-invocation: true -argument-hint: "[ | dispatch// | dispatch//dispatch.yaml | ]" -description: On-disk DAG protocol (dispatch.yaml / plan.md / resume). Background library — load via use_skill("dispatch") when a dispatch/ tree already exists. Absent from slash and use_skill listing. Skywalker orchestrates natively without this skill. ---- - -# Dispatch - -On-disk DAG protocol for an existing `dispatch/` tree. Skywalker orchestrates natively with spawn_agent / wait_agents / manage_tasks — do not load this skill for ordinary multi-lane work. - -How to run a `dispatch.yaml` graph: fan out work, fan in reports, critique, verify, re-dispatch fixes, and synthesize until done. Fan out work, fan in reports, critique, verify, re-dispatch fixes, and synthesize until done. - -DAG product tasks go through builder workers. Do not write `dispatch.yaml` or `plan.md` on the parent (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. - -Default batch size: **4 live workers** (recipe default — not a hard runtime cap). Widen when lanes are named and non-overlapping, or when the operator asks. Track progress with `manage_tasks`. - -Closed directors used here: `explorer`, `intern`, `builder`, `counsel`, `critic`. Optional consults: `greybeard`, `tester`. 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) -- **Just a name** (e.g. `auth-fix`) → `dispatch//dispatch.yaml` -- **Directory** (e.g. `dispatch/auth-fix/`) → `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="counsel")` first, then run. - -If the spec is vague, incomplete, or contradictory: stop and report Blockers. Do not invent a DAG. - -## Who does what - -| Work | Director | -| ------------------------------------------------------------------------------------------------ | ------------------------- | -| Map the codebase, gather facts | `task(agent="explorer")` | -| Eng plan from a spec (no ship) | `task(agent="counsel")` | -| Write `dispatch.yaml` / `plan.md` / status artifacts (mechanical brief; no product feature work) | `task(agent="builder")` | -| Ship product code + tests | `task(agent="builder")` | -| Review a landed task (defects, evidence, no fix) | `task(agent="critic")` | -| Architecture judgment before a large DAG | `task(agent="greybeard")` | -| Independent suite / repro evidence | `task(agent="tester")` | - -Classify, spawn, track, and synthesize. 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 builder — 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 the parent. - -Prefer typed briefs: `intent`, `success_criteria`, `do_not`, `report_focus`, and `agent`. - -## Agent type selection - -Use **explorer** 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. - -Use **builder** 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 builder (and after non-trivial intern landings), spawn `task(agent="critic")` with the task's 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 -- When unsure, default to `feature` - -## 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 builder worker could succeed from it. - -1. If the spec still needs an ordered eng plan, spawn `task(agent="counsel")`. Do not skip this when requirements are large or ambiguous. -2. Spawn `explorer` 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. -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 `explorer` | `intern` | `builder` 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 builder → yes; simple intern → no; when unsure, yes). -11. Seed `manage_tasks` with one item per DAG task (plus plan / verify / critique items as needed). - -If requirements are not actionable, stop. Ask: "Can a builder worker succeed with only this information?" - -## Phase 2: Directory structure - -Have **builder** write the run tree (mechanical brief; no product feature work). Do not write these files on the parent. Do not use intern — intern cannot write files. Do not use a catch-all worker. - -``` -dispatch/ - / - dispatch.yaml - 1a-extract_auth_module/plan.md - 1b-extract_logging_module/plan.md - 2a-integrate_modules/plan.md -``` - -`` is short kebab-case. Prefer `dispatch/` in `.gitignore`. - -Task directory names: `-` - -- **Level** (1, 2, 3…): DAG depth. Roots are level 1. Level is longest path from a root, plus one. -- **Sequence** (a, b, c…): siblings at the same level (candidates for parallel). -- **Description**: underscore-separated, from the objective. - -The directory name is the task `id`. After a worker runs, the task directory is its scratchpad (`plan.md` in, `output.yaml` and logs out). - -### Manifest (`dispatch.yaml`) - -```yaml -goal: "Short description of the overall goal" -status: pending # pending | in-progress | completed | failed -max-parallel: 4 # recipe default batch size (not a hard runtime cap) -created: YYYY-MM-DD - -verify: - workdir: "" # empty = repo root - build: "bun run build" # omit if n/a - test: "bun test" - lint: "bun run lint" - -critique: - enabled: true - agent: critic # always task(agent="critic") - -commits: - strategy: per-task # per-task | grouped - message-source: objective - -tasks: - - id: 1a-extract_auth_module - 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 - critique: - enabled: true - - - id: 2a-integrate_modules - type: feature - 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 `plan.md` - -Builder 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`. - -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. - -## 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. - -**Completeness:** clear objectives; files named; union of tasks covers the goal; every spec requirement maps to at least one task. - -**Coherence:** no two ready-in-parallel tasks write the same file; constraints do not contradict; `explorer` is never assigned product writes. - -**Feasibility:** referenced files exist or are created by this task or an upstream dependency; scope fits one worker. - -Empty task list → mark the run `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` (builder updates the manifest if it is 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; default batch size is **4 live workers** (including in-flight critique) — widen when lanes are named and non-overlapping. 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 builder workers 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="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. - -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 `run_shell`. Compare against any baseline you captured. - -- Green, or same failures as baseline → proceed. -- 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 - -Synthesize for the operator: - -## Summary - -## Findings - -## Blockers - -## 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. - -## Resume - -Re-resolve input to the existing `dispatch//`. Re-validate the remaining DAG. Continue from the ready set. Do not re-plan completed work unless the operator asks. - -## Non-negotiables - -- Spawn directors. Do not implement product features. Do not author dispatch YAML/plan files on the parent or via a catch-all worker. Durable orchestration files go through builder. -- `use_skill("dispatch")` loads this recipe. It is a command. -- Agents: `explorer`, `intern`, `builder` only for DAG nodes. Critique via `task(agent="critic")`. Plan via `task(agent="counsel")` when a spec needs an eng plan first. -- Progress: `manage_tasks`. -- Default batch size 4 live workers (recipe default, not a hard runtime cap); widen for named non-overlapping lanes or when the operator asks. diff --git a/plugins/corbits-skills/skills/implement/SKILL.md b/plugins/corbits-skills/skills/implement/SKILL.md index 62fbb6320..834f3560f 100644 --- a/plugins/corbits-skills/skills/implement/SKILL.md +++ b/plugins/corbits-skills/skills/implement/SKILL.md @@ -13,7 +13,7 @@ Before using this workflow, load the `style` and `philosophy` skills. Follow the ## When to Use -This is a standalone skill, loaded on request. It is not part of dispatch. Use it when you want a single agent to work through a series of commits with review discipline. +This is a standalone skill, loaded on request. Use it when you want a single agent to work through a series of commits with review discipline. The caller defines what work to do and where the commit boundaries are. This skill defines _how_ each commit gets produced. diff --git a/src/agent/directors/identity.ts b/src/agent/directors/identity.ts index 2002b826b..bf134eaa3 100644 --- a/src/agent/directors/identity.ts +++ b/src/agent/directors/identity.ts @@ -11,7 +11,7 @@ import { formatBakedOptionalSkills } from "./bake-skills.js"; * Workers (non-orchestrator): bake first-party optionalSkills bodies (CL-6803) * and only advertise that bake when at least one body resolved. Primary * orchestrator (skywalker): use_skill is mounted — list skill names only; do - * not bake huge dispatch/interview bodies or claim use_skill is unmounted. + * not bake huge interview bodies or claim use_skill is unmounted. */ export function formatDirectorSystemPrompt(pkg: DirectorPackage): string { const names = pkg.optionalSkills; diff --git a/src/agent/directors/skywalker/package.ts b/src/agent/directors/skywalker/package.ts index 65079bb84..935d0f8a1 100644 --- a/src/agent/directors/skywalker/package.ts +++ b/src/agent/directors/skywalker/package.ts @@ -131,7 +131,7 @@ Do not reclassify COMMUNICATION as ORCHESTRATION just to justify parallel spawn - Interview when requirements are fuzzy; consult greybeard on architecture/approach. - Use counsel for multi-lane eng plans; clarify before a large fan-out. - 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: style, philosophy, interview (use_skill is primary-mounted). For an existing dispatch/ tree, load use_skill("dispatch"). +- Optional skills when needed on the primary session: style, philosophy, interview (use_skill is primary-mounted). # Spawn graph diff --git a/tests/unit/corbits-skills-catalog.test.ts b/tests/unit/corbits-skills-catalog.test.ts index dcf461ca4..3c693c283 100644 --- a/tests/unit/corbits-skills-catalog.test.ts +++ b/tests/unit/corbits-skills-catalog.test.ts @@ -8,7 +8,6 @@ const pluginRoot = join(import.meta.dirname, "../../plugins/corbits-skills"); const SKILL_DIRS = [ "implement", - "dispatch", "scribe", "review", "ast-grep", @@ -26,7 +25,7 @@ const SKILL_DIRS = [ "plan", ] as const; -const SPAWN_RECIPE_SKILLS = ["implement", "dispatch"] as const; +const SPAWN_RECIPE_SKILLS = ["implement"] as const; /** use_skill listing + resolve; not slash. No disable-model-invocation. */ const USE_SKILL_ONLY = [ @@ -39,7 +38,7 @@ const USE_SKILL_ONLY = [ ] as const; /** Background libs: absent from slash and use_skill listing; explicit resolve only. */ -const BACKGROUND_ONLY = ["git-worktrees", "dispatch"] as const; +const BACKGROUND_ONLY = ["git-worktrees"] as const; const SLASH_SKILLS = [ "implement", @@ -87,8 +86,8 @@ test("corbits-skills plugin has no agents directory", () => { expect(existsSync(join(pluginRoot, "agents"))).toBe(false); }); -test("corbits-skills catalog lists 17 skills with name and description", async () => { - expect(SKILL_DIRS).toHaveLength(17); +test("corbits-skills catalog lists 16 skills with name and description", async () => { + expect(SKILL_DIRS).toHaveLength(16); const entries = await readdir(join(pluginRoot, "skills"), { withFileTypes: true }); const dirs = entries .filter((entry) => entry.isDirectory())