diff --git a/skills/review-loop/SKILL.md b/skills/review-loop/SKILL.md new file mode 100644 index 0000000..140b452 --- /dev/null +++ b/skills/review-loop/SKILL.md @@ -0,0 +1,152 @@ +--- +name: review-loop +description: > + Supervised delivery loop for planner-backed projects: verify the work item is still + real, then plan, spec, tests, code, integrate, deploy, with MiniMax reviewing every + artefact against best-practices plus gotchas until no CRITICAL/HIGH, Sonnet writing + the spec-ahead tests, MiniMax implementing in its own git worktree, and the supervisor + 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 *) +--- + +review-loop + +# /review-loop Skill + +Drive a planner work item (or a small batch) from "is this still a real issue?" +to "deployed and verified live" with an independent MiniMax review of every +artefact. The loop is what shipped WIA-17c..f and PDIV on 2026-09-13 (program +item `824222cb` in agent-runtimes holds every round as a worked example). + +## Pre-gathered context + +### Current date +!`date +%Y-%m-%d` + +### Tooling (check via tool calls before step 0) +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"`. + +### Planner + repo +!`git rev-parse --abbrev-ref HEAD` +!`git status --short | grep -v uv.lock | head -5` + +## Arguments + +`$ARGUMENTS` = one or more work-item UUIDs, optionally followed by `--from ` +(`discover|plan|spec|tests|code|integrate|deploy`). Without `--from`, the skill +runs discovery first and decides the entry stage from what exists. + +## Roles + +| 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 | +| 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/ | + +Each work item gets its own worktree and branch (`mm-` / `mm/`); +nothing is developed on main. (The tooling uses git *worktrees*; a git subtree +is a different mechanism and is not used.) + +## The loop + +Every artefact goes: draft → MiniMax review → adjudicate (accept / reject with a +source-cited reason) → apply → re-review, until a round has zero CRITICAL and +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. + +### 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. +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 +requirement ID checked against the spec files for collisions) and a +decision record `{options, choice, rationale, rejected, follow_ups}`. + +### 2. Spec +Amend the owning `spec/*.md` in place (amend, don't add, when reversing prior +text; add `Revokes prior text:` notes). Every requirement gets: normative +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. + +### 3. Tests (Sonnet) +One subagent per independent area. Brief: read the spec block + every cited +source file; create exactly the named handles in the named files; every new test +`@pytest.mark.xfail(strict=True, reason=" not implemented")`; tests that +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 +the same Sonnet agent with `SendMessage`. Commit `test: … — xfail strict`. + +### 4. Code (MiniMax in a worktree) +Create a child task item under the program item; attach `brief.md` (spec IDs, +the exact test files, the hard rules). Launch detached (max 2 concurrent — the +host runs CI builders): +``` +mm-worktree-task --docs all --timeout 3600 \ + --focus "Make the xfail(strict) tests in brief.md pass; never modify tests/ or spec/; run the named files with --runxfail and the whole test directory before reporting" +``` +Collect with `mm-worktree-task --collect`. +**Gates, in order:** +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. + +### 5. Integrate +``` +git worktree add -f /tmp/claude-1000/test-merge -b test-merge main +git -C /tmp/claude-1000/test-merge merge --no-edit mm/ # each branch +``` +Strip the xfail markers there (per-test decorators, `_XFAIL` constants, +module-level `pytestmark`, `pytest.param(..., marks=...)` rows), run the FULL +suite, fast-forward main (`git merge --ff-only test-merge`), push, remove the +worktree. Regenerate `openapi/` + SDKs only if a route/schema changed (the local +generator produces version-noise churn otherwise — revert it). + +### 6. Deploy and verify live +Empty commit `[CI BUILD] [DEPLOY] deploy: …`; watch with +`GITEA_TOKEN_FILE=~/dev/claude/secrets/gitea/ai_admin/token gac tail --until run-complete` +(detached; a later push cancels an in-flight run — re-trigger). Then +`kubectl rollout status`, confirm the image tag, and **reproduce the original +bug's exact user-visible action** plus a smoke of unrelated reads. Only then +`planner items patch --state completed --description ""`. +If reads look wrong: roll back first (deploy-repo `newTag` + ArgoCD hard +refresh), diagnose second, file an incident item with an RCA. + +## Bookkeeping +- Every review result → `documents put -review-r.md`. +- Follow-ups discovered in review → new `future`/`task` items under the program item, never left in prose. +- 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