review-fix-signoff-loop
When one review pass isn't enough and you need code to meet a spec end to end, this pattern loops fix and review iterations with fresh...
SKILL.md
Full skill instructions
Purpose
Use this pattern for high-stakes implementation workflows where a normal "implement, test, review once" flow is not enough. The workflow must keep repairing and re-reviewing until independent signoff agents agree the spec is fully wired end to end.
Pair this with writing-agent-relay-workflows for SDK syntax and relay-80-100-workflow for deterministic validation gates.
Required Shape
- Run deterministic preflight before agents start.
- Confirm repository root, required specs, declared write scope, credentials needed for PR comments, and whether commit/push/PR creation is in scope.
- For cross-repo or package-release work, write a scope matrix before implementation: repositories, branches, PRs, packages, providers/features touched, published versions, consuming package manifests, lockfiles, and expected downstream bumps.
- Probe the CLIs used by later agent steps. For Codex,
codex login statusis not enough; run a tinycodex exec --ephemeral --json --sandbox read-only -m <supported-model>prompt and fail early with a clear re-login instruction if it cannot return the expected token. - Write preflight evidence to
.workflow-artifacts/<workflow>/iteration-N/preflight.md. - Implement with scoped owners.
- Use Codex workers for code changes unless the codebase has a reason to prefer another CLI.
- Split backend, frontend, desktop, tests, docs, or infrastructure into explicit non-overlapping ownership areas.
- Each worker writes a durable summary artifact with changed files and commands run.
- Reconcile before validation.
- Add a deterministic
implementation-reconcilegate that checks required files, expected API/UI/runtime surfaces, migrations, generated artifacts, and untracked files withgit status --short -- <paths>. - For multi-provider changes, reconcile against the scope matrix: every touched provider/package must be classified as
implemented,dependency-only,intentionally-deferred, ornot-applicable, with proof. Do not let "we only bumped the package I remembered" pass this gate. - For package-release flows, reconcile producer and consumer state:
npm view <pkg> version, package manifests, lockfile resolved tarballs/integrities, andnpm ls <pkg>from every consuming workspace. - For CI failures, map each failing job to its exact local command or documented non-local equivalent. Distinguish similarly named gates (for example handler coverage vs acceptance route coverage) and replay the one that actually failed.
- Use
failOnError: false, then route the captured output to a repair owner. - Run repairable validation.
- Use capture -> fix -> rerun for typecheck, targeted tests, integration or E2E tests, and regression checks.
- Include exact failing CI commands when available before broader "nearby" checks. A nearby green gate is supporting evidence, not proof that the reported CI failure is fixed.
- Red validation output is input for a repair agent, not an immediate workflow failure.
- Write
BLOCKED_NO_COMMIT.mdonly for true external blockers. - Run fresh-context signoff reviews.
- Start a new workflow run, new agent names, or otherwise new agent contexts for each loop iteration.
- Run Claude and Codex signoff reviews independently over the same post-validation repo state.
- Reviewers must read specs, diff, validation logs, artifacts, and actual files.
- Break only on dual signoff.
- The loop may exit only when both reviewers write the exact satisfied verdict and final deterministic acceptance is green.
- If either reviewer finds issues or is blocked, run a Codex fix pass and start a new fresh-context review iteration.
- Make the Codex fix pass a non-interactive one-shot worker (
preset: 'worker') with afile_existsverification for its durable report. Do not rely on interactive PTY idle detection or/exitfor loop progress. - Report final signoff.
- Write a final
SIGNOFF.mdthat includes iteration count, validation evidence, Claude rationale, Codex rationale, remaining risks, and artifact paths. - Include the final scope matrix with every repository/package/provider row signed off, deferred with owner/date, or marked not applicable. For release flows, include published and consumed versions.
- Post the same report to the PR. Resolve the PR from an explicit env var first, then from
gh pr view.
Verdict Contract
Use a strict text contract so deterministic gates can parse the result:
VERDICT: COMPREHENSIVELY_SATISFIED | FINDINGS | BLOCKED
why_passed: required when VERDICT is COMPREHENSIVELY_SATISFIED
end_to_end_wiring_verified: required when VERDICT is COMPREHENSIVELY_SATISFIED
deterministic_evidence: required when VERDICT is COMPREHENSIVELY_SATISFIED
scope_matrix_verified: required when VERDICT is COMPREHENSIVELY_SATISFIED for cross-repo/provider/package work
remaining_risks: required when VERDICT is COMPREHENSIVELY_SATISFIED
finding_id: stable-id when VERDICT is FINDINGS
severity: blocker | high | medium | low
file: path
issue: concrete gap
fix_required: exact change needed
test_required: deterministic proof needed
evidence: commands, files, or spec clause
Scope Matrix
Create a machine-readable and human-readable matrix before the first fix pass for work that spans repositories, packages, providers, or CI gates. Keep it updated every iteration.
repo | branch | PR | package/provider/surface | expected change | producer version | consumer version | files expected | gates required | status | evidence | owner
Fresh Context Implementation
Prefer an outer loop that starts a new Agent Relay workflow run per iteration:
for (let iteration = 1; ; iteration += 1) {
await runIteration(iteration, runStamp); // new workflow name, channel, and agent names
clearStartFromAfterResumedIteration();
if (hasDualSignoff(iteration)) {
writeAndPostSignoffReport(iteration);
break;
}
}
Codex Fixer Reliability
For review-fix loop steps, prefer this shape:
.agent(`codex-review-fixer-${suffix}`, {
cli: 'codex',
model: CodexModels.GPT_5_4,
preset: 'worker',
role: 'Review-finding fixer. Repairs valid findings and hardens tests/proofs.',
retries: 2,
})
.step('fix-review-findings', {
agent: `codex-review-fixer-${suffix}`,
dependsOn: ['dual-signoff-gate'],
task: `Read iteration artifacts. Fix every valid finding, rerun relevant checks, and write ${dir}/review-fix-report.md.`,
verification: { type: 'file_exists', value: `${ROOT}/${dir}/review-fix-report.md` },
})
PR Signoff Comment
Final signoff should be both a durable artifact and a PR comment.
gh pr comment "$PR_NUMBER" --body-file .workflow-artifacts/my-workflow/pr-comment.md
Blocked State
Do not spin forever when progress is impossible. If agents identify a true external blocker, write:
.workflow-artifacts/<workflow>/iteration-N/BLOCKED_NO_COMMIT.md
Common Mistakes
- Reusing the same reviewer context every loop. Start a new run or new reviewer agents for each iteration.
- Letting a reviewer write
NO_ISSUES_FOUNDwithout pass rationale. Require the full verdict contract. - Treating green tests as signoff. Green deterministic gates are required evidence, not a substitute for fresh review.
- Hard-failing the first red validation gate. Capture it, repair it, then rerun.
- Posting a PR comment before both signoff agents agree on the same final state.
- Forgetting to count iterations. The final report must say how many loops it took.
