--- name: agentworkforce/review-fix-signoff-loop source: https://app.decimal.ai/s/agentworkforce-review-fix-signoff-loop@2/SKILL.md source_sha256: ee5062278341 --- # Review Fix Signoff Loop ## 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 1. 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 ` prompt and fail early with a clear re-login instruction if it cannot return the expected token. - Write preflight evidence to `.workflow-artifacts//iteration-N/preflight.md`. 2. 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. 3. 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 -- `. - 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 version`, package manifests, lockfile resolved tarballs/integrities, and `npm ls ` 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. 4. 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. 5. 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. 6. 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. 7. 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: ```text 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 ``` A deterministic dual-signoff gate should require: - both review files contain `VERDICT: COMPREHENSIVELY_SATISFIED` - neither review file contains `VERDICT: FINDINGS`, `VERDICT: BLOCKED`, or an open `finding_id` - both review files include the required pass-rationale fields - for cross-repo/provider/package work, both review files include `scope_matrix_verified:` and cite the matrix artifact - the latest final acceptance artifact is green ## 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. Recommended columns: ```text repo | branch | PR | package/provider/surface | expected change | producer version | consumer version | files expected | gates required | status | evidence | owner ``` Use the matrix to prevent "last-mile" omissions: - A package published from one repo must have a matching consumer row in every repo that should install it. - A provider touched in adapter code must have Cloud ingest/write/digest/writeback/dependency rows, or an explicit not-applicable rationale. - A route or API surface with both runtime tests and coverage metadata must list both gates separately. - Untracked generated artifacts must be classified as expected workflow artifacts, ignored local output, or unexpected drift before commit. Example package-consumption evidence: ```bash npm view @scope/pkg version rg -n '"@scope/pkg"' package.json package-lock.json packages -g package.json -g package-lock.json npm ls @scope/pkg --workspace node -e 'const lock=require("./package-lock.json"); console.log(lock.packages["node_modules/@scope/pkg"].version)' ``` ## Fresh Context Implementation Prefer an outer loop that starts a new Agent Relay workflow run per iteration: ```typescript for (let iteration = 1; ; iteration += 1) { await runIteration(iteration, runStamp); // new workflow name, channel, and agent names clearStartFromAfterResumedIteration(); if (hasDualSignoff(iteration)) { writeAndPostSignoffReport(iteration); break; } } ``` Within `runIteration`, suffix workflow name, channel, and agent names with `runStamp-iteration`: ```typescript const suffix = `${runStamp}-${iteration}`; workflow(`my-feature-completion-${suffix}`) .channel(`wf-my-feature-${suffix}`) .agent(`claude-reviewer-${suffix}`, { cli: 'claude', preset: 'reviewer', role: 'Fresh signoff reviewer' }) .agent(`codex-reviewer-${suffix}`, { cli: 'codex', preset: 'reviewer', role: 'Fresh signoff reviewer' }); ``` This prevents reviewer memory from a previous loop from becoming the reason the loop exits. If the outer loop supports `--start-from`, consume it for only the resumed iteration. `START_FROM=fix-review-findings` must not leak into the next fresh-context iteration, or the next iteration will skip review/validation and fail on missing artifacts. After a completed resumed iteration, delete `process.env.START_FROM` and `process.env.PREVIOUS_RUN_ID` before continuing the loop. ## Codex Fixer Reliability For review-fix loop steps, prefer this shape: ```typescript .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` }, }) ``` Use interactive PTY Codex only when the step genuinely needs live channel coordination. For bounded artifact-producing fix/review steps, `preset: 'worker'` exits through the subprocess lifecycle, and `file_exists` proves the required artifact exists. ## PR Signoff Comment Final signoff should be both a durable artifact and a PR comment. Resolution order: 1. explicit env var such as `SIGNOFF_PR_NUMBER`, `PR_NUMBER`, or `GITHUB_PR_NUMBER` 2. `gh pr view --json number --jq .number` 3. if no PR is available, write `PR_COMMENT_FAILED.md` and fail unless the workflow has an explicit skip env var Use a deterministic shell or Node step: ```bash gh pr comment "$PR_NUMBER" --body-file .workflow-artifacts/my-workflow/pr-comment.md ``` The comment body should include: - iteration count - final validation status and command evidence - Claude signoff rationale - Codex signoff rationale - remaining risks or explicit "none" - artifact path for full logs ## Blocked State Do not spin forever when progress is impossible. If agents identify a true external blocker, write: ```text .workflow-artifacts//iteration-N/BLOCKED_NO_COMMIT.md ``` Include exact evidence, missing credentials or services, commands that failed, and the safest retry command. Do not commit, push, or post a success comment from a blocked run. ## 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.