diff --git a/skills/review-loop/SKILL.md b/skills/review-loop/SKILL.md index 140b452..323689b 100644 --- a/skills/review-loop/SKILL.md +++ b/skills/review-loop/SKILL.md @@ -8,7 +8,7 @@ description: > gating that tests/ and spec/ are untouched before merging. Pass one or more work-item UUIDs (and optionally the entry stage) as arguments. user_invocable: true -allowed-tools: Read, Write, Edit, Agent, SendMessage, Bash(date *), Bash(test *), Bash(ls *), Bash(cat *), Bash(mkdir *), Bash(chmod *), Bash(setsid *), Bash(pgrep *), Bash(python3 *), Bash(git status*), Bash(git rev-parse *), Bash(git log *), Bash(git diff *), Bash(git add *), Bash(git commit *), Bash(git pull *), Bash(git push), Bash(git merge *), Bash(git worktree *), Bash(git checkout main -- *), Bash(git -C *), Bash(mm-worktree-task *), Bash(ask-minimax-run *), Bash(review-minimax-run *), Bash(gac *), Bash(kubectl *), Bash(ssh-add *) +allowed-tools: Read, Write, Edit, Agent, SendMessage, Bash(date *), Bash(test *), Bash(ls *), Bash(cat *), Bash(mkdir *), Bash(chmod *), Bash(setsid *), Bash(pgrep *), Bash(python3 *), Bash(git status*), Bash(git rev-parse *), Bash(git log *), Bash(git diff *), Bash(git add *), Bash(git commit *), Bash(git pull *), Bash(git push), Bash(git merge *), Bash(git worktree *), Bash(git checkout main -- *), Bash(git -C *), Bash(mm-worktree-task *), Bash(review-loop-run *), Bash(review-minimax-run *), Bash(gac *), Bash(kubectl *), Bash(ssh-add *) --- review-loop @@ -29,7 +29,7 @@ item `824222cb` in agent-runtimes holds every round as a worked example). Run these with the Bash tool and stop if any is MISSING: `test -f "$HOME/.claude-oreillyit-minimax/provider.env"` (MiniMax profile), `test -x "$HOME/sbin/mm-worktree-task"`, and -`test -x "$HOME/dev/claude/projects/small-scripts/scripts/review-minimax-run"`. +`test -x "$HOME/sbin/review-loop-run"` (the review launcher; source in small-scripts). ### Planner + repo !`git rev-parse --abbrev-ref HEAD` @@ -46,7 +46,7 @@ runs discovery first and decides the entry stage from what exists. | Role | Who | Never does | |---|---|---| | Supervisor | this session | write tests or code MiniMax should write; skip a gate | -| Verifier / reviewer | MiniMax M3 via `ask-minimax-run` (detached) | modify repo files | +| Verifier / reviewer | MiniMax M3 via `review-loop-run launch` (detached) | modify repo files | | Test author | Sonnet subagent (`Agent`, model sonnet) | touch spec/, controlplane/, lib/ | | Coder | MiniMax via `mm-worktree-task` in its own git worktree | touch tests/ or spec/ | @@ -62,19 +62,33 @@ zero HIGH. Cap 4 rounds; record residue on the item and move on. Reject up front the "symbol/module does not exist yet" class for spec-ahead artefacts, or it dominates every round. +Every review is launched with one command (prompt template, best-practice and +gotcha reading set, detached `setsid nohup` launch, and severity parsing are all +built in): +``` +review-loop-run launch --kind --name --round \ + --file [--file ...] --context "" +review-loop-run status --name # running / done exit=N + severity counts +review-loop-run counts # exit 4 while CRITICAL+HIGH > 0 = another round +``` +Round N+1 automatically feeds round N's result back with the "do not re-raise +adjudicated findings" instruction. Preview a prompt with `--dryrun`. Reviews take +5-10 minutes: schedule a wakeup instead of polling `status`. + ### 0. Discover (per UUID) 1. `python3 -m lib.cp_cli planner items get ` + `documents list/get` into the scratchpad. -2. MiniMax verification prompt: "is the described defect still present in the - code on main? cite file:line or commit; verdict RESOLVED / STILL OPEN / - PARTIAL". Spot-check one citation with git yourself. +2. `review-loop-run launch --kind verify --name verify- --file [--file ...]` + (verdict RESOLVED / STILL OPEN / PARTIAL with cited evidence). Spot-check one + citation with git yourself. 3. RESOLVED → `planner items patch --state completed --description ""`. PARTIAL / OPEN → attach the verdict as `verification-.md` and continue. ### 1. Plan (only for items that need a decision) -Write the plan to the scratchpad; upload as `plan-v1.md` on the item; MiniMax -review with spec-driven-development, test-driven-development, security- -architecture, api-design, validation, kubernetes + the project's -`memory/gotchas-*.md`. The plan MUST carry an ID-allocation table (every new +Write the plan to the scratchpad; upload as `plan-v1.md` on the item; +`review-loop-run launch --kind plan --name plan- --file ` (the +kind's reading set covers spec-driven-development, test-driven-development, +security-architecture, api-design, validation, kubernetes and the project's +`memory/gotchas-*.md`). The plan MUST carry an ID-allocation table (every new requirement ID checked against the spec files for collisions) and a decision record `{options, choice, rationale, rejected, follow_ups}`. @@ -85,9 +99,9 @@ statement, **Why**, **Test handles** with exact test names and file, and a module-layout row for every new/changed file. Pin cross-module seams to exact signatures and the real attribute names — read the source first; the reviewer will catch invented names but only after a wasted round. -Review prompt inputs: the `git diff spec/...` file, mechanical-test-generation, -spec-driven-development, api-design, security-architecture, gotchas-specs, -gotchas-python. Commit `spec: ()` when clean. +Write `git diff -- spec/.md` to the scratchpad and run +`review-loop-run launch --kind spec --name spec- --file --context "..."`. +Commit `spec: ()` when clean. ### 3. Tests (Sonnet) One subagent per independent area. Brief: read the spec block + every cited @@ -96,11 +110,11 @@ source file; create exactly the named handles in the named files; every new test already pass today stay unmarked and are reported; access not-yet-existing symbols inside the test body (never `from x import New` at top, never `importorskip`); must not modify spec/ or source; run the files and the -`--co` of the whole suite. Gate: 0 xpassed, 0 errors. Then MiniMax reviews the -test files against the spec handles + test-driven-development + -mechanical-test-generation + gotchas-python (mock-target divergence, -`object.__new__` fixtures that skip `__init__`, `AsyncMock` on sync calls, -assertions that pass today for the wrong reason). Send accepted findings back to +`--co` of the whole suite. Gate: 0 xpassed, 0 errors. Then +`review-loop-run launch --kind tests --name tests- --file --file ` +(the template checks mock-target divergence, `object.__new__` fixtures that skip +`__init__`, `AsyncMock` on sync calls, assertions that pass today for the wrong +reason). Send accepted findings back to the same Sonnet agent with `SendMessage`. Commit `test: … — xfail strict`. ### 4. Code (MiniMax in a worktree) @@ -116,7 +130,7 @@ Collect with `mm-worktree-task --collect`. 1. `git diff main...mm/ -- tests/ spec/` is EMPTY. If not: `git checkout main -- tests/ spec/` in the worktree, note the violation, re-run. 2. In the worktree: target files with `--runxfail` all pass; the whole test directory has no new failures (strict XPASS of the new tests is the expected signal). 3. A test that cannot pass is a test or spec bug → fix it on main via the Sonnet agent, commit, `git -C merge main`. -4. MiniMax code review of `git diff main...` (correctness vs spec text, security, gotchas-python, wire-shape changes, no test edits). Small accepted fixes: apply yourself in the worktree and commit; large ones: re-run the coder with a `review-r1-fixes.md` brief. +4. Write `git diff main...mm/` to the scratchpad; `review-loop-run launch --kind code --name code- --file --file --context "worktree: ; tests: "` (correctness vs spec text, security, gotchas-python, wire-shape changes, no test edits). Small accepted fixes: apply yourself in the worktree and commit; large ones: re-run the coder with a `review-r1-fixes.md` brief. ### 5. Integrate ``` @@ -145,7 +159,7 @@ refresh), diagnose second, file an incident item with an RCA. - End: `/log`, CONTEXT.md cache, commit + push both the project and any secondary repo touched (e.g. `agent-runtime-framework`). ## Hard rules and gotchas -- Launch every MiniMax run through a `setsid nohup