diff --git a/CHANGELOG.md b/CHANGELOG.md index 72b9ae9ff..5c0d515ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,12 @@ 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. +- The first-party dispatch skill is gone. Skywalker orchestrates natively. + ### 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..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 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 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 eaae83aa6..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 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. 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/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 deleted file mode 100644 index c3134defd..000000000 --- a/plugins/corbits-skills/skills/dispatch/SKILL.md +++ /dev/null @@ -1,216 +0,0 @@ ---- -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. ---- - -# 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. - -Orchestrate parallel director runs across a dependency graph. Fan out work, fan in reports, critique, verify, re-dispatch fixes, and synthesize until done. - -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")` | - -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. - -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 Skywalker. 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 Skywalker `run_shell`. Compare against any baseline you captured. - -- Green, or same failures as baseline → proceed. -- New failures → attribute to a task/commit, re-dispatch `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 - -- You are Skywalker. Spawn directors. Do not implement product features. Do not author dispatch YAML/plan files yourself or via a catch-all worker. Durable orchestration files go through builder. -- `use_skill("dispatch")` loads this recipe. It is 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/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..834f3560f 100644 --- a/plugins/corbits-skills/skills/implement/SKILL.md +++ b/plugins/corbits-skills/skills/implement/SKILL.md @@ -1,84 +1,176 @@ --- name: implement -description: Disciplined per-commit workflow. Skywalker spawn recipe — greybeard, builder, intern/tester, critic. +description: Disciplined per-commit workflow with Greybeard review, build gates, and Critique loops --- # 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. +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. -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`. +## Prerequisites -Closed directors used here: `greybeard`, `builder`, `intern`, `tester`, `critic`. Never a catch-all worker. +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. 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. + +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. -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`). +### Step 3: Build Gate -## Tracking +Update `activeForm` to "Running build gate: {subject}". -Track commit-sized units with `manage_tasks`. One item per unit that will become a commit. +Run `make` (or the project's equivalent full pipeline: format, lint, build, test). -- 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. +- 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 -## Per-commit spawn loop +### Step 4: Commit -For each unit, run these steps in order. Do not skip. When this loop is running, do not DIY the unit — spawn builder. +Update `activeForm` to "Committing: {subject}". -### 1. Review — greybeard +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. -`task(agent="greybeard")` on the approach before any code is written. +### Step 5: Critic loop -Send: what will change and why, files expected, design decisions and trade-offs, uncertainties. +Update `activeForm` to "Running critic loop: {subject}". -Adjust the plan from the report, then spawn builder. Greybeard is for approach, not execution. +Ask critic to review the committed change. -### 2. Implement — builder +**How to run:** -`task(agent="builder")` with a typed brief: `intent`, `success_criteria`, `do_not`, `report_focus`. +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: -- **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. -- **Features:** tests ship with the change. Assert the new behavior, not merely that the process did not crash. + ``` + 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 + ``` -Keep scope to this unit. Additional work becomes a later `manage_tasks` item. + 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. -### 3. Build gate — intern or tester +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 -`task(agent="intern")` or `task(agent="tester")` for the project build/test gate (`make`, or the project's full pipeline: format, lint, build, test). +**When to stop looping:** -- `intern` — mechanical full pipeline -- `tester` — suite / repro +- 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 -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. +### Step 6: Next -### 4. Critic +Mark the current `manage_tasks` item done. Move to the next unit of work and return to Step 1. -`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. +## Guidelines -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. +**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. -- 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. -- No invented worker-count or fan-out ceiling. +**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), synthesize for the operator: +**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 a333d9fb1..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: 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 feature or fix based on a Linear issue argument-hint: " [--reviewer ]" +user-invocable: false --- # 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. +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; Skywalker does not run the git. +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 Skywalker. 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 Skywalker'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/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..747b4c9ca 100644 --- a/plugins/corbits-skills/skills/plan/SKILL.md +++ b/plugins/corbits-skills/skills/plan/SKILL.md @@ -1,16 +1,26 @@ --- 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. Does not implement. Does not file tracker issues. --- # Plan -You are Skywalker. This skill is a spawn recipe. You do not write the plan yourself. +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`. +If the change target is too fuzzy to plan, `ask_operator` first. -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. +## What the plan must contain -This is not `/create-issue`. Do not file Linear or GitHub issues. 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 -Use `ask_operator` if the change target is too fuzzy to brief counsel. +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 f1cdc6cd3..db3a96897 100644 --- a/plugins/corbits-skills/skills/pull-request-review/SKILL.md +++ b/plugins/corbits-skills/skills/pull-request-review/SKILL.md @@ -1,107 +1,177 @@ --- 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, using a git worktree --- # 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. +Use this skill to review a pull request given a branch name or URL. Do not implement fixes. -## Input +## Input Formats -Accepts either: +This skill 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`) +- 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 +## Workflow -### 1. Parse input +### Step 1: Parse Input -If given a URL, intern extracts the branch via `run_shell` (there is no Bash tool): +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. Worktree checkout if needed +### Step 2: Determine Repository Root -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. +Find the root of the current git repository: ```bash git rev-parse --show-toplevel +``` + +### 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, intern reports the error. Stop. Do not proceed with worktree creation 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 + +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/" +``` + +The worktree path follows the pattern `../worktree/` relative to the repository root. + +### Step 5: Change to Worktree and Checkout Branch + +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 ``` -Worktree path is `../worktree/` relative to the repository root. +Confirm the output matches the expected branch name before proceeding. + +### Step 6: Set Up the Repository + +Before reviewing, the repository must be properly set up. Look for developer documentation that describes how to install dependencies and prepare the codebase: + +1. Search for setup instructions in these common locations: + - `README.md` + - `CONTRIBUTING.md` + - `docs/` directory + - `DEVELOPMENT.md` + - `SETUP.md` + +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. -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. +4. If no setup documentation exists, `ask_operator` how to set up the repository before proceeding. -Base branch: +### Step 7: Determine Base Branch + +Identify the base branch for comparison: ```bash +# For GitHub PRs 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. +# For GitLab MRs +glab mr view --output json | jq -r '.target_branch' -### 3. Review +# If working from branch name only, assume main or master +git branch -r | grep -E 'origin/(main|master)$' | head -1 | sed 's/.*origin\///' +``` -- **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")`. +### Step 8: Load the review skill -Brief the reviewer: +Load and follow the `review` skill to perform the actual review. The `review` skill provides guidance on: -- 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 +- 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) -Prefer a typed brief: `intent="review"`, `success_criteria`, `do_not`, `report_focus`. +### Step 9: Post the Review on GitHub -### 4. After the report +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. -Synthesize critic/neckbeard Summary / Findings / Blockers / Paths for the operator. Do not land fixes. +Follow **Post the Review 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 Claude (or other vendor) bot. Primary owns `--approve` / `--request-changes` only when the operator asked to post; secondary lenses use `--comment` only. +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. -If the operator then wants repairs, that is a later `/implement` or `use_skill("dispatch")` — not this skill. +Do not skip the post because the chat already summarized the findings. ## Cleanup -After the review, intern may remove the worktree: +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: -## Hard rules +- Branch does not exist remotely +- Worktree creation fails +- Checkout fails +- Setup commands fail +- CLI tools (gh, glab) are not available or not authenticated -- This recipe reviews; it does not land product patches. If the operator then asks for a tiny/bounded fix, DIY with write_file/edit_file/delete_file; spawn builder for substantial fixes. -- Skywalker MUST NOT run the worktree git; intern does, via `run_shell`. -- Do not implement fixes as part of the review. -- Do not impersonate GitHub-Claude review comments. +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 c46878135..f99871227 100644 --- a/plugins/corbits-skills/skills/refactor/SKILL.md +++ b/plugins/corbits-skills/skills/refactor/SKILL.md @@ -1,41 +1,88 @@ --- name: refactor argument-hint: -description: Skywalker maps a directory then plans improvements. Explore, then plan. Does not ship product code. +description: Examine code, document its design, and collaboratively plan improvements --- # 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. - -## Recipe - -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. -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 yourself. Counsel's report is the artifact. A later `/implement` or `use_skill("dispatch")` ships it. - -## Hard rules - -- Do not write the plan to disk or author design documents on this session — counsel's report is the artifact. A later `/implement` or `use_skill("dispatch")` ships substantial work; DIY remains for tiny/bounded edits outside this recipe. -- Do not skip explore "because you already know the directory." -- Do not skip `ask_operator` when the operator has not chosen among alternatives. -- Spawn with `task(agent="explorer")` then `task(agent="counsel")`. +Use this skill to analyze existing code, produce a structured design document, and collaboratively plan improvements for future implementation. + +## Initialization + +Before doing anything else, load the `philosophy` skill. The principles in that skill guide how you evaluate design decisions. + +## Workflow + +### Step 1: Understand the Scope + +The user has specified a directory to analyze: `$ARGUMENTS` + +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? + +### Step 2: Examine the Code + +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 + +### 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 + +**Data Flow** - How data moves through the system + +**Dependencies** - What it relies on + +**Patterns** - Design patterns and conventions observed + +**Observations** - Complexity, inconsistencies, or potential concerns (factual, not prescriptive) + +### Step 4: Collaborative Improvement Discussion + +After documenting the current state: + +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 +- 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 + +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 72391d52a..8cc786aeb 100644 --- a/plugins/corbits-skills/skills/review/SKILL.md +++ b/plugins/corbits-skills/skills/review/SKILL.md @@ -1,40 +1,396 @@ --- name: review -description: Review a branch, PR, or path scope. Skywalker spawns critic (neckbeard for hygiene, greybeard for architecture); does not implement fixes. argument-hint: "[paths | PR | diff | hygiene | architecture]" +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 -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. +How to review a branch, pull request, or path. Findings only — do not implement fixes. -Classify the lens, spawn the matching director(s), wait for reports, synthesize. Findings only — never land fixes in this recipe. +Use this skill when performing code reviews or pull request reviews. -## Routing +## Base Branch Determination -- **Default** (correctness, completeness, brief adherence, defects with evidence): `critic` -- **Hygiene-only** (nits, naming, lint, pedantry with receipts): `neckbeard` -- **Architecture-only** (structure, boundaries, approach): `greybeard` +Before reviewing, you must determine the correct base branch. Use these methods +in order of reliability: -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. +### Method 1: Check for Associated PR/MR -## Fleet +If the branch has an open pull request or merge request, use the PR's target: -- **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 +# GitHub +gh pr view --json baseRefName --jq '.baseRefName' 2>/dev/null -Prefer a typed brief: `intent="review"`, `success_criteria`, `do_not`, `report_focus`, and `agent`. +# GitLab +glab mr view --output json 2>/dev/null | jq -r '.target_branch' +``` -## Brief to the worker +### Method 2: Check Upstream Tracking -Include whatever the operator gave you, plus enough for a scoped review: +If the branch tracks a remote branch that itself branched from somewhere: -- Paths, PR number/URL, or branch to review -- 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 -- 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 +git config --get branch..merge +``` -## After the report +### Method 3: Find Merge Base with Default Branches -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. +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 + merge_base=$(git merge-base "origin/$base" HEAD) + echo "$base: $merge_base" + fi +done +``` + +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 Determination + +Focus only on commits that are contained within the branch being reviewed. +Use `git diff ...HEAD` to determine what is in scope. + +```bash +# 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 + +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. + +Only evaluate the changes introduced by the branch. + +## 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. + +If reviewing TypeScript code, consider loading the `typescript` +skill for detailed guidance on type patterns, naming, and idioms. + +## Delegating to Sub-agents + +When delegating file review to sub-agents, provide the output of +`git diff ...HEAD -- ` rather than the full file contents. + +Sub-agents that receive full files cannot distinguish branch changes from +pre-existing code and will flag out-of-scope issues. + +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 Philosophy + +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 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 + +A review should help the author, not burden them. Do not raise issues that are +not important to the code being reviewed. Specifically: + +- 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. + +If something should be fixed, say so directly. If it's not worth fixing, don't +bring it up. + +## Comment Tone + +Review comments are posted under your name to a human author. Write them +as you would speak 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. + +## 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. + +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 and removed the old fixed-delay path." +Good: "The HTTP client retries transient failures with exponential backoff." + +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. + +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. + +**Subject-line audits.** Most checks scan the output of: + +```bash +git log ..HEAD --format='%s' +``` + +Scan for: + +- **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." + +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' +``` + +Empty output is clean. Any line returned is an over-72-character subject violation. + +**Body audits.** Most checks scan the output of: + +```bash +git log ..HEAD --format='%b' +``` + +Scan for: + +- **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. + +For body-line length, use the same length-aware filter: + +```bash +git log ..HEAD --format='%b' | awk 'length > 72' +``` + +Empty output is clean. Any line returned is an over-72-character body line. + +Affirmative claims about these audits must cite the specific command whose output proved them — see **Cite the Check**. + +## Review Checklist + +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. + +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") + +## Post the Review on GitHub + +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. + +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. + +### When to post + +Post when any of these is true: + +- 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 + +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 +# 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 +)" +``` + +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 +## · + + + +### Findings + +- `path/to/file.ts:12` — +- `path/to/other.ts:40` — <…> + +### Notes + +- +``` + +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 + +**Findings rules:** + +- 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 posting + +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 0f2fb1786..35bcc637e 100644 --- a/plugins/corbits-skills/skills/scribe/SKILL.md +++ b/plugins/corbits-skills/skills/scribe/SKILL.md @@ -1,14 +1,397 @@ --- name: scribe -description: Skywalker spawn recipe — shakespeare writes PRODUCT.md, ARCHITECTURE.md, and IMPLEMENTATION.md. +description: Maintain product, architecture, and implementation docs — routes input, detects gaps, and interviews for completeness --- # 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). +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. -Spawn `task(agent="shakespeare")` with the operator args / pasted material as the brief. Shakespeare owns PRODUCT.md, ARCHITECTURE.md, and IMPLEMENTATION.md. +## Document Discovery -Use `ask_operator` if the doc target (P vs A vs I) is ambiguous. +Before processing input, locate the documentation files: -Do not edit those docs yourself except a one-line fix. DESIGN.md is rand, not this skill. +1. Search for existing files matching `PRODUCT.md`, `ARCHITECTURE.md`, and `IMPLEMENTATION.md` (case-insensitive) in: + - Repository root + - `docs/` directory + +2. If documents exist, use their locations. If multiple matches exist for the same type, prefer the repository root. + +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 + +**Product** - Product-level documentation + +- What we're building and why +- User-facing value propositions +- Vision and goals +- Target users and use cases +- Business justification + +**Architecture** - System architecture documentation + +- How the system is structured +- Components and their relationships +- Abstractions and interfaces +- Data flow and control flow +- Design decisions that are technology-agnostic + +**Implementation** - Implementation documentation + +- Specific technology choices +- Protocols and formats +- Libraries and frameworks +- Concrete technical details +- Configuration and deployment specifics + +## Using the Question Tool + +Throughout this skill, you will use `ask_operator` to interact with the user. `ask_operator` presents multiple-choice questions with predefined options. + +**Key mechanics:** + +- 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 + +**When to provide context-aware options:** + +- 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 + +**Example invocation:** + +```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" + } + ] + } + ] +} +``` + +The tool returns the selected options as an array of labels (e.g., `["Both", "Under 5 seconds"]`). + +## Execution Steps + +### Step 0: Document Discovery and Reading + +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. + +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 + +### 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/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/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.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..935d0f8a1 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). # 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 9aee4893f..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,11 +25,10 @@ const SKILL_DIRS = [ "plan", ] as const; -const SPAWN_RECIPE_SKILLS = ["implement", "scribe", "review", "dispatch", "plan"] as const; +const SPAWN_RECIPE_SKILLS = ["implement"] as const; /** use_skill listing + resolve; not slash. No disable-model-invocation. */ const USE_SKILL_ONLY = [ - "dispatch", "git-rebase", "linear-issue-workflow", "style", @@ -88,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()) @@ -128,11 +126,9 @@ 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 per-commit workflow 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")'); expect(skill).toContain("Do not invent a worker-count or fan-out ceiling"); expect(skill).toContain("Close the loop"); @@ -144,6 +140,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); @@ -172,22 +179,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");