review-fix-signoff-loop logo

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 status is not enough; run a tiny codex 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-reconcile gate that checks required files, expected API/UI/runtime surfaces, migrations, generated artifacts, and untracked files with git 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, or not-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, and npm 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.md only 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 a file_exists verification for its durable report. Do not rely on interactive PTY idle detection or /exit for loop progress.
  • Report final signoff.
  • Write a final SIGNOFF.md that 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_FOUND without 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.