diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 0798d7f..bed3419 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -5,7 +5,7 @@ }, "metadata": { "description": "Pure-Rust Clean Architecture workflow. Six commands (start, fix, plan, ship, review, install-ci) for axum + sqlx + Dioxus 0.7+ + tokio. Always-latest deps, CI audit gate, anti-slop enforced.", - "version": "4.0.0" + "version": "4.1.0" }, "plugins": [ { diff --git a/CHANGELOG.md b/CHANGELOG.md index f35815b..01ed5d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,49 @@ All notable changes to the code-et plugin will be documented in this file. +## [4.1.0] - 2026-05-07 + +### Added — per-task review subagent in `/code:ship` + +Code review now happens twice in the feature lane: once per task before merge (new), and once across the full feature branch at `/code:review` (pre-PR gate). Per-task review catches logic bugs at the smallest possible diff so task 1's bug never pollutes task 2's foundation. + +After each implementer subagent commits in its worktree, the orchestrator: + +1. Captures the diff against the parent feature branch. +2. Dispatches a reviewer **fork** (`Agent(model: "sonnet")`, no `subagent_type`, no isolation) with the diff payload + rationale + expected_outcome + layer. The reviewer calls `Skill("code-review")` (engineering plugin) and falls back to the same 5-area inline checklist `/code:review` uses when the plugin isn't installed. Returns CRITICAL/HIGH findings as strict JSON. +3. On findings, dispatches **one** review fix-pass (`Agent(subagent_type: "general-purpose", model: "sonnet")`, no isolation, operating via `git -C ` and explicit paths inside the worktree). Same retry budget as the post-merge audit — one cycle max. +4. Merges into the parent feature branch only after the review (and any fix-pass) passes. + +If the fix-pass cannot resolve findings, the orchestrator halts that task and surfaces — leaves the worktree in place for inspection. + +### Added — explicit model assignments per role + +`Agent` calls in `/code:plan` and `/code:ship` now specify the `model` parameter (one of `opus` | `sonnet` | `haiku`) instead of inheriting the orchestrator's model. The split: + +| Role | Model | Why | +|---|---|---| +| Orchestrator (`/code:plan`, `/code:ship`) | inherits (Opus 4.7) | Multi-step coordination + judgment. | +| Per-task implementer | `sonnet` (4.6) | Routine vertical-slice coding from a complete brief. | +| Per-task reviewer | `sonnet` (4.6) | Mechanical diff review with `engineering:code-review` skill. | +| Per-task review fix-pass | `sonnet` (4.6) | Apply review findings; no scope expansion. | +| Post-merge audit fix-pass | `opus` (4.7) | Audit-gate findings often require judgment — layer slips, dep advisories, real test failures vs flakes. | +| Explore (breadth searches in `/code:plan`) | `haiku` (4.5) | Cheap parallel discovery. | + +`code-et-implementer/CLAUDE.md` documents the convention so it's discoverable in any project that installs the plugin. + +### Changed — `/code:fix` Task Brief now states Goal + Verification + +The `/code:fix` Task Brief template (`code-et-implementer/commands/fix.md`) gains two mandatory lines after `Description`: + +- **Goal** — observable success criterion ("what's true after the fix that wasn't before"). +- **Verification** — runnable cmd + expected outcome (or manual repro steps for visual fixes). + +`/code:plan` already encoded this through `metadata.expected_outcome` + `metadata.verification`, which `/code:ship` injects into every dispatched subagent and enforces in the per-subagent contract. `/code:fix` hands the Brief straight to a human implementer — no orchestrator, no `SubagentStop` hook — so the goal-and-test pair belongs in the Brief itself. With the addition both lanes (bug + feature) close the "did we achieve it" loop symmetrically. + +The Rules section also gains: "If you can't state Verification, the bug isn't scoped tightly enough — ask another clarifying question." + +No changes to the Clean Architecture rules — fix.md already references `code-et-implementer/docs/architecture.md` and the Layer column in the Files-to-touch table already enforces the Dependency Rule by inspection. + ## [4.0.0] - 2026-05-06 **Breaking restructure** — the plugin is now pure-Rust only and condenses to six commands. Existing PRDs and tasks under `plans/` and `.claude/.json` continue to work, but the entry-point command names have changed. See the migration table below. diff --git a/code-et-implementer/.claude-plugin/plugin.json b/code-et-implementer/.claude-plugin/plugin.json index 08f680e..c97b0a5 100644 --- a/code-et-implementer/.claude-plugin/plugin.json +++ b/code-et-implementer/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "code", - "version": "4.0.0", + "version": "4.1.0", "description": "Pure-Rust Clean Architecture workflow. Six commands: start, fix, plan, ship, review, install-ci. Always-latest deps, CI audit gate, anti-slop enforced.", "author": { "name": "Kennet Kusk" diff --git a/code-et-implementer/CLAUDE.md b/code-et-implementer/CLAUDE.md index 94008c9..1993468 100644 --- a/code-et-implementer/CLAUDE.md +++ b/code-et-implementer/CLAUDE.md @@ -47,6 +47,21 @@ metadata: { `layer` is mandatory. Each *file* declares its layer; vertical slices may span layers. +## Model Assignments + +Different roles in the workflow run on different models. The `Agent` tool's `model` param accepts `opus` | `sonnet` | `haiku` — each resolves to the latest of that family. + +| Role | Model | Where | +|---|---|---| +| Orchestrator (`/code:plan`, `/code:ship`) | `opus` (4.7) | Inherited; multi-step coordination + judgment. | +| Per-task implementer | `sonnet` (4.6) | `/code:ship` — routine vertical-slice coding. | +| Per-task reviewer | `sonnet` (4.6) | `/code:ship` — diff review via engineering plugin's `code-review` skill (falls back to inline 5-area checklist if the plugin isn't installed). | +| Per-task review fix-pass | `sonnet` (4.6) | `/code:ship` — apply review findings, no scope expansion. | +| Post-merge audit fix-pass | `opus` (4.7) | `/code:ship` — judgment on layer slips, dep advisories, test failures. | +| Explore (breadth searches) | `haiku` (4.5) | `/code:plan`, `/code:fix` — cheap parallel discovery. | + +The principle: heavy lifting (planning, judgment) on Opus; routine coding + diff review on Sonnet; breadth gathering on Haiku. + ## Code Standards - Rust 2024 edition; `cargo clippy --all-targets -- -D warnings`; `cargo fmt --check`. diff --git a/code-et-implementer/commands/fix.md b/code-et-implementer/commands/fix.md index 104ecb7..f4b0681 100644 --- a/code-et-implementer/commands/fix.md +++ b/code-et-implementer/commands/fix.md @@ -107,6 +107,8 @@ Skip whatever the user already answered. **App:** [app name from Apps Overview] **Area:** [crate or module] **Description:** [1-2 sentence summary] +**Goal:** [observable success criterion — what's true after the fix that wasn't before] +**Verification:** `` — [expected outcome on green; for visual fixes: manual repro steps] ### Files to touch | File | Layer | Why | @@ -129,4 +131,5 @@ The `Layer` column is mandatory. Use file paths discovered via `Glob`/`Grep`. Re - If the user already gave enough context, skip straight to the Task Brief. - Reference concrete paths (from Glob) so the user can point and say "that one". - Description ≤2 sentences using fragments. File "Why" column ≤6 words. No hedging or filler. +- **Goal + Verification are mandatory.** Goal is the testable outcome (one sentence, observable). Verification is the cmd that proves it (`cargo nextest run -p ` for unit, `cargo clippy --all-targets -- -D warnings` for lint regressions, manual repro steps for visual/UI). If you can't state Verification, the bug isn't scoped tightly enough — ask another clarifying question. - **Context budget**: FILE-REFERENCE = constraints + orientation. Glob/Grep = file discovery. LSP = scalpel for named symbols. Never read whole files in `/code:fix` — that's `/code:plan`'s job. diff --git a/code-et-implementer/commands/plan.md b/code-et-implementer/commands/plan.md index c010e29..2f9e0ef 100644 --- a/code-et-implementer/commands/plan.md +++ b/code-et-implementer/commands/plan.md @@ -130,7 +130,7 @@ Read the PRD (it is the authoritative spec). **Replace, don't accumulate.** When a slice supersedes existing logic, the task scope **includes deletion of the superseded code**. State the `path:line` being replaced in `metadata.rationale`. No parallel utilities, no `// TODO: remove old X`. -**LSP for symbols.** Use `documentSymbol` / `findReferences` / `definition` to anchor each US/AC to `file:line`. Grep/Glob for discovery; LSP for precision. Never use LSP to enumerate the project. For 3+ independent areas, spawn parallel `Agent(subagent_type: "Explore")` queries in a single message. +**LSP for symbols.** Use `documentSymbol` / `findReferences` / `definition` to anchor each US/AC to `file:line`. Grep/Glob for discovery; LSP for precision. Never use LSP to enumerate the project. For 3+ independent areas, spawn parallel `Agent(subagent_type: "Explore", model: "haiku")` queries in a single message — Haiku 4.5 is the right tier for breadth scans. ### Anti-slop self-critique (before TaskCreate) diff --git a/code-et-implementer/commands/ship.md b/code-et-implementer/commands/ship.md index 3ea0317..8790443 100644 --- a/code-et-implementer/commands/ship.md +++ b/code-et-implementer/commands/ship.md @@ -14,7 +14,18 @@ If the current branch is `main` or `master`, create `feature/: + +## Diff (against parent feature branch) +<diff content — full payload, ≤1500 lines> + +## Rationale (why this task exists) +<metadata.rationale> + +## Expected outcome +<metadata.expected_outcome> + +## Layer +<metadata.layer> + +Review the diff against the rationale + expected outcome. + +**Step A — Try the engineering plugin's code-review skill:** +``` +Skill("code-review") +``` +If it returns findings, use them. If the skill is not installed (the call errors with "skill not found"), fall back to Step B. + +**Step B — Inline 5-area review** (mirror of `/code:review` Step 2): +1. **Layer compliance** — does any new file violate the inward dependency rule? (`code-et-implementer/docs/architecture.md` §"The Dependency Rule") +2. **Anti-slop** — Rule of Three duplicates, mirror tests, defensive validation, dead re-exports. (`code-et-implementer/docs/anti-slop.md` §"Hard rules") +3. **Test coverage** — every acceptance criterion has a corresponding test. (`code-et-implementer/docs/testing.md` §"Per-layer test matrix") +4. **Security** — secrets in `secrecy::Secret<T>`; auth at every interface entry point. (`code-et-implementer/docs/architecture.md` §"Rust security checklist") +5. **Slice integrity** — coherent vertical slice; superseded code deleted in same commit; no `// TODO: remove old X`. + +**Output format — strict.** Output ONLY the JSON array as your final message. No preamble, no code fences, no explanation. If no CRITICAL or HIGH findings, output exactly: `[]` + +Schema: +[{"severity": "CRITICAL|HIGH", "file": "path:line", "issue": "<one sentence>"}] + +Drop MEDIUM and LOW findings — those are for `/code:review` to catch later. Do not modify code. You are a reviewer, not a fixer. +``` + +### Step 3 — On CRITICAL/HIGH findings, dispatch ONE review fix-pass + +Spawn `Agent(subagent_type: "general-purpose", model: "sonnet")` with no isolation. Prompt directs it to operate via `git -C <worktree_path>` and explicit file paths inside `<worktree_path>`: + +``` +# Review fix-pass for <task-id> + +The per-task reviewer flagged the following CRITICAL/HIGH findings on the diff in worktree <worktree_path>: + +<findings JSON> + +Fix each finding. Use absolute paths or `git -C <worktree_path>` for git operations. After fixing, re-run `<metadata.verification>` from inside `<worktree_path>` (must exit 0). Commit the fix-up with subject "fix-up: <task tag>". Return the new HEAD SHA. + +Constraints: same as the implementer (Brevity, Context Hygiene, Clean Architecture rules). No scope expansion — fix the findings only. +``` + +After the review fix-pass returns, **do not re-review**. One cycle max — same retry budget as the post-merge audit. If the fix-pass returns without a new commit or `verification` fails, halt that task and surface findings to the user (leave the worktree in place for inspection). + ## Orchestrator (this skill) After each `Agent` call returns: 1. Read the returned worktree path and branch from the tool result. -2. From the parent feature branch: `git merge --no-ff <subagent-branch>`. -3. `git worktree remove <path>` (the harness auto-cleans empty worktrees, but populated ones need explicit removal). -4. Mark the task completed via `TaskUpdate` only after the merge lands. +2. Run **Per-task review** (above). On CRITICAL/HIGH, dispatch one review fix-pass and continue. +3. From the parent feature branch: `git merge --no-ff <subagent-branch>`. +4. `git worktree remove <path>` (the harness auto-cleans empty worktrees, but populated ones need explicit removal). +5. Mark the task completed via `TaskUpdate` only after the merge lands. -If a subagent reports failure, leave the worktree in place for inspection — do not auto-discard. +If a subagent reports failure, or the review fix-pass cannot resolve findings, leave the worktree in place for inspection — do not auto-discard. ## After all tasks land — audit @@ -105,7 +195,7 @@ The audit mirrors the v4.0 CI gate: `cargo fmt --check`, `cargo clippy -D warnin If audit exits non-zero with CRITICAL or HIGH findings (the typical: layer violation, dependency advisory, clippy lint, test failure): 1. Read `.claude/audit-<UTC>.md` — extract the highest-severity finding's `path:line` + message. -2. Dispatch **one** fix-pass subagent via `Agent` (no worktree isolation — work directly on the feature branch since the task swarm already merged): +2. Dispatch **one** fix-pass subagent via `Agent(subagent_type: "general-purpose", model: "opus")` (no worktree isolation — work directly on the feature branch since the task swarm already merged; Opus 4.7 here because audit-gate findings often require judgment — layer slips, dependency advisories, real test failures vs flakes): ``` # Audit fix-pass The post-implement audit returned <severity> at <path:line>: <message>.