feat(agents): wire domain reviewers into ship-feature(-lane) review stage
Route Stage 4 / review-phase reviews to ccam:backend-reviewer, ccam:frontend-reviewer, ccam:mcp-reviewer (parallel to the generic Code Reviewer) by which of server/, client/, mcp/ the diff touches. Give each reviewer a parseable verdict line and project memory to match qc-local's rigor, and update docs/LANES.md's per-stage agent description accordingly.
This commit is contained in:
@@ -3,9 +3,10 @@ name: backend-reviewer
|
||||
description: Review backend route and hook logic for regressions, data integrity risks, and missing tests.
|
||||
tools: Read, Grep, Glob, Bash
|
||||
model: opus
|
||||
memory: project
|
||||
---
|
||||
|
||||
You are a backend reviewer for this repository.
|
||||
You are a backend reviewer for this repository. Review ONLY files under `server/` in the diff you are given — ignore `client/`, `mcp/`, or any other path even if it appears in the same diff range; another reviewer covers those.
|
||||
|
||||
Focus on:
|
||||
- Hook event lifecycle correctness.
|
||||
@@ -14,7 +15,13 @@ Focus on:
|
||||
- Transaction and persistence correctness.
|
||||
- Missing or weak verification coverage.
|
||||
|
||||
Output:
|
||||
- Prioritized findings.
|
||||
- File references.
|
||||
- Reproduction or validation notes.
|
||||
## Output (MANDATORY — the caller parses your last line)
|
||||
|
||||
- Findings as `path:line — severity — problem — suggested fix`, most severe first.
|
||||
- End with exactly one of:
|
||||
- `BACKEND-REVIEW: CLEAN`
|
||||
- `BACKEND-REVIEW: FINDINGS — <n>`
|
||||
|
||||
## Agent memory
|
||||
|
||||
Record recurring backend defect patterns and false-positive traps you find here (e.g. hook shapes that look wrong but aren't, common state-machine edge cases). Future backend-reviewer runs read this.
|
||||
|
||||
@@ -3,9 +3,10 @@ name: frontend-reviewer
|
||||
description: Review React UI changes for behavior regressions, state consistency, and UX breakage.
|
||||
tools: Read, Grep, Glob, Bash
|
||||
model: sonnet
|
||||
memory: project
|
||||
---
|
||||
|
||||
You are a frontend reviewer for this repository.
|
||||
You are a frontend reviewer for this repository. Review ONLY files under `client/` in the diff you are given — ignore `server/`, `mcp/`, or any other path even if it appears in the same diff range; another reviewer covers those.
|
||||
|
||||
Focus on:
|
||||
- Routing and navigation consistency.
|
||||
@@ -14,7 +15,13 @@ Focus on:
|
||||
- Breaking visual or interaction regressions.
|
||||
- Missing tests for changed UI behavior.
|
||||
|
||||
Output:
|
||||
- Prioritized findings.
|
||||
- File references.
|
||||
- Suggested verification steps.
|
||||
## Output (MANDATORY — the caller parses your last line)
|
||||
|
||||
- Findings as `path:line — severity — problem — suggested fix`, most severe first.
|
||||
- End with exactly one of:
|
||||
- `FRONTEND-REVIEW: CLEAN`
|
||||
- `FRONTEND-REVIEW: FINDINGS — <n>`
|
||||
|
||||
## Agent memory
|
||||
|
||||
Record recurring frontend defect patterns and false-positive traps you find here (e.g. websocket race conditions that look wrong but aren't, sibling components that should stay in sync). Future frontend-reviewer runs read this.
|
||||
|
||||
+12
-5
@@ -3,9 +3,10 @@ name: mcp-reviewer
|
||||
description: Review MCP server changes for tool safety, schema quality, and host integration correctness.
|
||||
tools: Read, Grep, Glob, Bash
|
||||
model: opus
|
||||
memory: project
|
||||
---
|
||||
|
||||
You are an MCP-focused reviewer for this repository.
|
||||
You are an MCP-focused reviewer for this repository. Review ONLY files under `mcp/` in the diff you are given — ignore `server/`, `client/`, or any other path even if it appears in the same diff range; another reviewer covers those.
|
||||
|
||||
Focus on:
|
||||
- Tool naming and schema strictness.
|
||||
@@ -14,7 +15,13 @@ Focus on:
|
||||
- Stdio protocol safety (stderr-only logs).
|
||||
- Host configuration and runbook documentation accuracy.
|
||||
|
||||
Output:
|
||||
- Prioritized findings.
|
||||
- File references.
|
||||
- Verification commands to run.
|
||||
## Output (MANDATORY — the caller parses your last line)
|
||||
|
||||
- Findings as `path:line — severity — problem — suggested fix`, most severe first.
|
||||
- End with exactly one of:
|
||||
- `MCP-REVIEW: CLEAN`
|
||||
- `MCP-REVIEW: FINDINGS — <n>`
|
||||
|
||||
## Agent memory
|
||||
|
||||
Record recurring MCP defect patterns and false-positive traps you find here (e.g. schema shapes that look loose but are intentional, host quirks). Future mcp-reviewer runs read this.
|
||||
|
||||
+1
-1
@@ -1072,7 +1072,7 @@ The lane card shows a lock badge when the lane **holds** a named lock (its `lane
|
||||
|
||||
The **ship-feature-lane** skill is the ported Shipyard driving skill for autonomous end-to-end feature pipelines in a single CCAM lane. It coordinates the entire development flow: implementation (TDD) → independent unit tests → pre-push CI gates + dev preflight → e2e testing → code review → local QC → senior gate authorization → PR push and publication → CI watch → final report.
|
||||
|
||||
**Every stage whose job is to judge the work runs in a fresh-context subagent, never in the driving session.** The plan debate (Stage 0b), unit tests (Stage 1b), the e2e run and its triage (Stage 3), code review (Stage 4), browser QC (Stage 6), the senior gate (Stage 7), and a re-triage of CI that stayed red after a fix (Stage 10) are each dispatched with the Agent tool to a subagent that starts with an empty context and receives only artifacts it can verify itself — working directory, lane spec, diff range, commands, and the other agents' reports verbatim, never the driving session's paraphrase of why a finding was fine. The stages the driving session keeps are the ones that *do* rather than judge: intake, implementation, the mechanical gates, publishing, the CI/PR watch, and applying every fix the judges ask for. `subagent_type: fork` is explicitly forbidden for these: a fork inherits the implementing session's context, so it inherits the belief that the code is already correct. The driving session implements and applies fixes; it never grades its own work. Stages 1b/3/4 use the generic `general-purpose`/`Code Reviewer` types (nothing to install); Stages 6/7 use the lane-installed `qc-local`/`senior-gate-reviewer` agents.
|
||||
**Every stage whose job is to judge the work runs in a fresh-context subagent, never in the driving session.** The plan debate (Stage 0b), unit tests (Stage 1b), the e2e run and its triage (Stage 3), code review (Stage 4), browser QC (Stage 6), the senior gate (Stage 7), and a re-triage of CI that stayed red after a fix (Stage 10) are each dispatched with the Agent tool to a subagent that starts with an empty context and receives only artifacts it can verify itself — working directory, lane spec, diff range, commands, and the other agents' reports verbatim, never the driving session's paraphrase of why a finding was fine. The stages the driving session keeps are the ones that *do* rather than judge: intake, implementation, the mechanical gates, publishing, the CI/PR watch, and applying every fix the judges ask for. `subagent_type: fork` is explicitly forbidden for these: a fork inherits the implementing session's context, so it inherits the belief that the code is already correct. The driving session implements and applies fixes; it never grades its own work. Stages 1b/3 use the generic `general-purpose` type (nothing to install). Stage 4 uses the generic `Code Reviewer` type as its catch-all, plus — in parallel, routed by which of `server/`, `client/`, `mcp/` the diff touches — the plugin-qualified `ccam:backend-reviewer` / `ccam:frontend-reviewer` / `ccam:mcp-reviewer` domain reviewers (nothing to install; `ccam:` addressing works from any lane). Stages 6/7 use the lane-installed `qc-local`/`senior-gate-reviewer` agents.
|
||||
|
||||
### Getting the skill (nothing to do, if you run the plugin)
|
||||
|
||||
|
||||
@@ -119,9 +119,16 @@ The e2e hook doesn't run migrations itself — it tests the already-running stac
|
||||
- `ccam stage review --status running`, then launch a fresh-context reviewer agent (Agent tool, `subagent_type: Code Reviewer` — FOREGROUND, **never `fork`**; a reviewer who already watched you write the code is not a reviewer). Hand it ONLY: the lane's working directory, the lane spec path (requirement + acceptance criteria), the diff commands, and the brief below. Do NOT summarize your own implementation for it, and do NOT pre-empt findings — let it read the diff cold.
|
||||
- Run the **`code-review` skill at effort `high`** on the feature diff vs `origin/development` — the deterministic gate, not an ad-hoc read. No PR is open yet, so point it at the local diff: `git diff origin/development...feat/<slug>` (and `git log origin/development..feat/<slug>` for commits). ONLY if the `code-review` skill is unavailable in its session, fall back to a manual review of that diff (correctness, security, tests, migration/contract safety, this repo's `CLAUDE.md` + `.claude/rules/` boundaries).
|
||||
- Final line: `REVIEW: CLEAN` or `REVIEW: FINDINGS — <n>`, with the findings listed above it as `path:line — severity — problem — suggested fix`.
|
||||
- Triage its findings with **receiving-code-review** judgment (verify each, neither blind agreement nor blind rejection) and apply what's worth applying on the feature branch. Changed source → re-run **from Stage 1b**; changed only tests → **from Stage 2**. Keep the findings + your resolutions — the senior gate reads them.
|
||||
- **Domain reviewers, in parallel with the above (same message, separate Agent calls).** Check `git diff --name-only origin/development...feat/<slug>` for touched layers and launch every matching one — FOREGROUND, fresh context, **never `fork`**, addressed as `ccam:backend-reviewer` / `ccam:frontend-reviewer` / `ccam:mcp-reviewer` (plugin-qualified — no lane install needed, unlike `qc-local`/`senior-gate-reviewer`):
|
||||
- Any path under `server/` → `ccam:backend-reviewer`.
|
||||
- Any path under `client/` → `ccam:frontend-reviewer`.
|
||||
- Any path under `mcp/` → `ccam:mcp-reviewer`.
|
||||
- None of those three touched (root/`skills/`/`agents/`/`scripts/`/docs-only change) → skip all three; the generic Code Reviewer above is the only gate.
|
||||
- Hand each ONLY the lane's working directory and the diff range `git diff origin/development...feat/<slug>` — each scopes itself to its own domain's files even though the diff range is the same for all.
|
||||
- Each ends with its own parseable line: `BACKEND-REVIEW:` / `FRONTEND-REVIEW:` / `MCP-REVIEW:` — `CLEAN` or `FINDINGS — <n>`.
|
||||
- Triage ALL findings — the generic review plus every domain reviewer that ran — together with **receiving-code-review** judgment (verify each, neither blind agreement nor blind rejection) and apply what's worth applying on the feature branch. Changed source → re-run **from Stage 1b**; changed only tests → **from Stage 2**. Keep the findings + your resolutions — the senior gate reads them.
|
||||
- The Stage-6 `qc-local` report covers the user-flow review for the senior gate.
|
||||
- `ccam stage review --evidence "<findings count> findings, resolved"`
|
||||
- `ccam stage review --evidence "<total findings count across all reviewers that ran> findings, resolved"`
|
||||
|
||||
### 5 — QC plan *(bound the test scope before any browser QC)*
|
||||
- Author a **QC Plan** the browser-QC agents (Stage 6 now, and a future Stage 13 once dev-QC exists) will execute against — so QC covers everything that matters and nothing that doesn't (no missed scenarios, no wandering into unrelated areas). Derive it from the acceptance points (lane spec) + the real change surface (`git diff origin/development...feat/<slug>` and `--stat`). Three parts:
|
||||
|
||||
@@ -97,8 +97,23 @@ you dropped back to.
|
||||
a reviewer. It runs the **code-review** skill on the working diff
|
||||
(see also **superpowers:requesting-code-review** for what to hand over).
|
||||
Final line: `REVIEW: CLEAN` or `REVIEW: FINDINGS — <n>`.
|
||||
- Apply what is worth applying with **superpowers:receiving-code-review**
|
||||
judgment: verify each point, neither blind agreement nor blind rejection.
|
||||
- **Domain reviewers, in parallel with the above (same message, separate
|
||||
Agent calls).** Check `git diff --name-only` (against the base this task
|
||||
started from) for touched layers and launch every matching one —
|
||||
FOREGROUND, fresh context, never `fork`, addressed as `ccam:backend-reviewer`
|
||||
/ `ccam:frontend-reviewer` / `ccam:mcp-reviewer`:
|
||||
- Any path under `server/` → `ccam:backend-reviewer`.
|
||||
- Any path under `client/` → `ccam:frontend-reviewer`.
|
||||
- Any path under `mcp/` → `ccam:mcp-reviewer`.
|
||||
- None of those three touched → skip all three; the generic Code Reviewer
|
||||
above is the only gate.
|
||||
- Hand each ONLY the repo path and the working diff — each scopes itself to
|
||||
its own domain's files. Each ends with its own parseable line:
|
||||
`BACKEND-REVIEW:` / `FRONTEND-REVIEW:` / `MCP-REVIEW:` — `CLEAN` or
|
||||
`FINDINGS — <n>`.
|
||||
- Apply what is worth applying — the generic review plus every domain
|
||||
reviewer that ran — with **superpowers:receiving-code-review** judgment:
|
||||
verify each point, neither blind agreement nor blind rejection.
|
||||
- Changed code in response? Re-run the `tests` phase.
|
||||
|
||||
### gate — evidence before the completion claim
|
||||
|
||||
Reference in New Issue
Block a user