agentleFS
ultimatileultimatile/development-skills/skills/review-pipeline/SKILL.md

review-pipeline

Skill0 starsChanged 33 days agoMITView on GitHub

Full review pipeline from local changes through PR, Copilot review, postmortem elevation, and umbrella drift join. Pauses at the user-controlled merge gate between Phase 4a and 4b.

2 other projects use a version of this.

Adopted by

Most recently taken up by

The file

Show its rules and text

We split this skill into the rules it gives an agent

19 rules. About 0 are common, 7 are distinctive, and 12 are very specific to this project.

  • ≈0

    Common · estimate

    Many files like this have a rule of this kind.

  • ≈7

    Distinctive · estimate

    This author's own choices. Worth a look.

  • 12

    Very specific

    Tied to their project's names, files or tools. Probably skip.

  • Common
  • Distinctive
  • Very specific
  • could be automatic: a setting could enforce it
  1. Very specific:

    Run /done-check against the current diff (committed + staged + unstaged + untracked), passing the root stated at pipeline entry.

    line 23
  2. Distinctive:

    If done-check's step 5 gate is not yet satisfied over everything it returned:

    line 25
  3. Very specific:

    Run /done-check again (fresh, full audit — do not bias the next pass with the previous concerns list)

    line 27
  4. Very specific:

    Run /code-review-gate against the current diff, passing the root stated at pipeline entry, the absolute path of the working tree step 1 ran …

    line 37
  5. Very specific:

    Ensure the diff Phase 0.5 reviewed is committed.

    line 47
  6. Very specific:

    Run /codex-review to review the branch diff, passing the root stated at pipeline entry

    line 48
  7. Distinctive:

    After Phase 1 and 2 complete, before the user merges, fold review findings into durable artifacts:

    line 62
  8. Distinctive:

    When it hands back, continue.

    line 70
  9. Distinctive:

    Find the parent reference: Read the sub-issue body:

    line 84
  10. Distinctive:

    No match → skip Phase 4a and 4b entirely.

    line 90
  11. Very specific:

    Write the final body to a temp file and invoke gh-post pr edit <N> --repo <owner>/<repo> --body-file /tmp/<descriptive-name>.md.

    line 109
  12. Very specific:

    Stop here. The user merges the PR via the GitHub UI or gh pr merge.

    line 113
  13. Distinctive:

    After the user confirms the merge has landed, continue to Phase 4b.

    line 115
  14. Very specific:

    Run /done-check in delta mode, against the previous audit's rows and concerns, with the root stated at pipeline entry.

    line 138
  15. Very specific:

    Never skip done-check, including in fix loops: Every fix commit is itself a diff that can introduce new drift — especially …

    line 144
  16. Very specific:

    Every commit goes through /stage-commit-push: Do not manually run git add/commit/push during the pipeline.

    line 159
  17. Very specific:

    Pre-commit branch gate: Before each /stage-commit-push, and at pipeline entry on the terms Pipeline entry states, verify the current branch …

    line 161
  18. Very specific:

    Triage is mandatory: Never present raw review output to the user.

    line 171
  19. Distinctive:

    Escalation order: Before presenting the fix-direction question (panic vs allow vs convert vs ...), FIRST ask whether the original plan …

    line 177

≈ “Common” is an estimate: at least 1 in 5 of the 350,495 skill files we read have a rule of the same kind, not the same words. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.

Keep what fits and drop the rest. Your version, ready for your agents.
Files
SKILL.md

Review Pipeline

Orchestrate the full flow from local changes through PR review, user merge, postmortem elevation, and umbrella drift join. This skill ties together several sub-skills — invoke each by name.

The pipeline crosses a user-controlled merge gate (Phase 4a → 4b): the user, not Claude, merges the PR. Phases before the gate run on the PR branch; phases after run on main and tracking issues. Claude pauses at the ## ← user merges PR ← section.

Pipeline entry

This pipeline takes two inputs, stated before the first phase the run executes. A run entering at Phase 4a or 4b takes neither.

  • Root — the branch this change merges into, as a bare branch name, supplied by the caller on diff-root's terms. Every gate here is a coverage invocation in that skill's sense, so all of them take this one value: every /done-check invocation (Phase 0 and the fix-loop substep), Phase 0.5's /code-review-gate, Phase 1's /codex-review, and — when the run reads fix commits rather than working from review findings alone — Phase 3's /bug-to-contract and /finding-to-audit. Phase 2 opens the PR against that same branch. This pipeline resolves no root: with none supplied, ask.
  • Branch guard — run the default-branch check in Rules.

Phase 0: Done-check loop

  1. Run /stage-commit-push.
  2. Run /done-check against the current diff (committed + staged + unstaged + untracked), passing the root stated at pipeline entry.
  3. Triage what it returned — both the ⚠ rows of the audit table and the cross-cutting concerns reported under it.
  4. If done-check's step 5 gate is not yet satisfied over everything it returned:
    • Fix the code
    • Run /done-check again (fresh, full audit — do not bias the next pass with the previous concerns list)
    • Re-triage
    • Run /stage-commit-push
  5. Repeat until done-check's step 5 gate is satisfied over both domains — its rows and its cross-cutting concerns.

What binds this phase is that the audit closing it saw the diff that proceeds — step 4 secures that by re-running the audit after every fix.

Phase 0.5: Claude code-review gate

  1. Run /stage-commit-push.
  2. Run /code-review-gate against the current diff, passing the root stated at pipeline entry, the absolute path of the working tree step 1 ran in as the review tree, and an effort chosen on the gate's terms.
  3. Triage the output — classify each finding under the finding-triage SSOT dispositions.
  4. Apply the fix-loop substeps (see Rules), re-running /code-review-gate with the inputs step 2 passed.

Step 4 gives this gate the same property Phase 0 has: the review closing it saw the diff that proceeds.

When that diff received a valid gate review (i.e. the gate was not waived), attribute each Phase 1 and Phase 2 reviewer finding to one of two provenances, and note it in the triage presentation. A finding already present in the diff this gate reviewed is by construction a penetration of it. A finding introduced by a Phase 1 or Phase 2 fix is not: those fix loops run the audit, commit, and re-run the current reviewer, never this gate, so the defect did not exist when this gate ran.

Phase 1: Codex review loop

  1. Ensure the diff Phase 0.5 reviewed is committed. Run /stage-commit-push.
  2. Run /codex-review to review the branch diff, passing the root stated at pipeline entry
  3. Triage the output — classify each finding under the finding-triage SSOT dispositions
  4. Apply the fix-loop substeps (see Rules).

Phase 2: Copilot review

  1. Run /file-pullreq in gate mode — drafts the PR title + body following gh-body-conventions and the standard body skeleton, discharges its evidence claims, runs the laundering pass, and gets the user's approval. The skill stops at approval and emits the approved title + body for the next step. It does NOT create the PR itself.
  2. Run /copilot-review in its normal mode, passing the approved title + body and the root as this pipeline's --base — this creates the PR with --reviewer @copilot and polls until the review arrives. The root is a branch name, which is what that flag takes; /codex-review's --base one phase earlier is a different value, a merge base derived from the same root.
  3. Triage the review — filter to the latest review's comments only (by pull_request_review_id)
  4. Apply the fix-loop substeps (see Rules), replacing the re-review step with two actions in this order: reply per copilot-review § Respond to review, then run ${CLAUDE_SKILL_DIR}/../copilot-review/scripts/pr-with-copilot-review.sh --re-review <PR_URL>. Triage only new comments.
  5. Reply per copilot-review § Respond to review.

Phase 3: Postmortem elevation (pre-merge)

After Phase 1 and 2 complete, before the user merges, fold review findings into durable artifacts:

  1. /bug-to-contract — for each actionable finding from Phase 1 and 2 (not just fix commits), ask whether an implicit contract was violated and whether it is now tested.
  2. /stage-commit-push.
  3. /codex-contract-test-review — for each contract test added in step 1, run a narrow Codex pass: does the test express the claimed contract, and would it fail on the original buggy implementation?

    When it hands back, continue. When it escalates to the user, the pipeline waits there and continues as the user directs.

  4. /finding-to-audit — for findings whose detection would have been diff-inspectable (import direction, pub widening, missing standard trait impl, debug artifacts, hardcoded values, FFI output dropped, etc.), promote to a pre-commit audit rule in the host finding-to-audit selects — a rule-set SSOT such as quality-list or authoritative-text-rules, or a domain-specific audit skill. By default this files an issue against the host skill's repository proposing the rule, reviewed there on its own timeline — so the elevation neither blocks this project's merge gate nor lands as an unreviewed edit to a shared audit surface.
  5. /stage-commit-push.

A finding can map to either bug-to-contract, finding-to-audit, both, or neither. Use both when both apply.

/gate-miss-to-issue (process-postmortem) — when a defect surfaced only at Phase 1/2 review or by user pushback that an earlier gate should have caught, file the gate gap against development-skills (independent of the project merge gate, like /finding-to-audit). Skip when every defect was caught at its earliest gate.

Phase 4a: PR description delta (pre-merge)

Skip when the work is not tied to an umbrella tracking issue. Trigger only when the merged-bound PR or its Closes #N references a sub-issue with a Parent: #<umbrella> line.

  1. Find the parent reference. Read the sub-issue body:

    gh issue view <leaf#> --json body -q .body | rg '^Parent:' | head -1

    No match → skip Phase 4a and 4b entirely.

  2. Derive the plan-vs-actual delta. Compare the sub-issue's Scope / Out of scope / Acceptance against the merged-bound PR's actual diff and behavior. Cover:

    • Scope additions (work that landed but was not in the original Scope) — was it justified, or scope creep?
    • Scope subtractions (Scope items that were deferred or dropped) — were they punted to a follow-up issue?
    • Out-of-scope churn (deferrals that became in-scope, or new deferrals discovered during implementation)
    • Acceptance criteria that were tightened, loosened, or reworded during review

    A "no delta" outcome (everything matched) is a valid answer — record it explicitly.

  3. Edit the PR description. Append a ## Plan-vs-actual delta section to the existing body — full delta with evidence and links to the relevant review iterations.

    Apply gh-body-conventions to the appended section.

    Discharge the appended section's evidence claims per gh-body-conventions § Evidence claims before running the check below, and re-check any completeness claim already in the body — work done since the body was last discharged can have falsified one without its text changing.

    Before invoking gh-post pr edit, run /gh-body-audit against the final body (existing PR body concatenated with the appended delta section). Paragraph boundaries and reference patterns can cross the section seam, so auditing only the appended section would miss them. Pass artifact kind pr. Any ⚠ blocks gh-post pr edit on gh-body-audit step 4's terms.

    Write the final body to a temp file and invoke gh-post pr edit <N> --repo <owner>/<repo> --body-file /tmp/<descriptive-name>.md.

← user merges PR ←

Stop here. The user merges the PR via the GitHub UI or gh pr merge. Do not attempt the merge from Claude unless the user explicitly asks.

After the user confirms the merge has landed, continue to Phase 4b.

Phase 4b: Umbrella drift join (post-merge)

Runs only after the user has merged.

  1. Sub-issue closing comment. Post a compressed delta (≈ 5–10 lines) plus the merged PR link via gh-post issue comment <leaf#> --repo <owner>/<repo> --body-file /tmp/<descriptive-name>.md, then close the sub-issue with gh issue close <leaf#>.
  2. Umbrella body update. Two independent axes:

    • Progress reflection (default action). If the umbrella tracks sub-items by status annotation (- [x] foo checkbox, _[Promoted to #N.]_ / _[Done in #M.]_ inline tag, Phases | Status table column, etc.), update the item the leaf was promoted from. The skill that filed the leaf already wrote the "promoted" annotation; the merge step closes the loop by switching it to "done". Skip only if the umbrella truly has no per-item status convention.
    • Design-assumption change. Edit additionally when the delta changes a parent-level design assumption — a new deferral that affects another phase, a scope shift that invalidates the Phases table, a decision that contradicts the umbrella's "Decisions captured" section.

    A clean implementation with no parent-level implications still gets the progress-reflection edit; the design-assumption axis can be skipped.

  3. Do not edit the sub-issue body.

Rules

  • Fix-loop substeps: while any finding of the latest triage is not closed, as finding-triage defines closed, run these in order:

    1. Oscillation check. Compare current actionable topics against the preserved topics of the previous iteration in the current phase and against the topics settled under Oscillation detection in this run. Take any conceptual topic that recurs through Oscillation detection before fixing it; where that rule escalates, halt and follow the escalation order below — do NOT fix or done-check.
    2. Take each finding that is not closed as its disposition directs. If every finding of the latest triage is now closed and this substep changed no file, the loop ends here; this takes precedence over the rules below against skipping done-check and codex review.
    3. Run /done-check in delta mode, against the previous audit's rows and concerns, with the root stated at pipeline entry.
    4. Run /stage-commit-push.
    5. Re-run the review (fresh, full review — no bias from previous iteration).
    6. Preserve actionable topic classifications for the next iteration's oscillation check.
    7. Re-triage.
  • Never skip done-check, including in fix loops. Every fix commit is itself a diff that can introduce new drift — especially completion-hygiene and paired-artifact-drift.
  • A halted done-check stops the pipeline. This binds every /done-check invocation here. A halted run emits no table. The step that invoked it does not complete: do not fix, do not commit, do not re-review. It has surfaced something to the user; that is where the pipeline waits.
  • Done-check delta mode. done-check defines the mode, what a delta report carries, and what its step 5 gate binds. Satisfy that gate before the subsequent /stage-commit-push rather than only the ⚠ the delta printed. Pay special attention to:

    • paired-artifact-drift: every comment / docstring / PR-body sentence touched by or referring to the fixed code must still be accurate. Accuracy is not the whole test: a fix that answers a reviewer by qualifying a documented behavior — admitting a case the doc did not — strands every unqualified statement of that behavior elsewhere, which an accuracy check over the fix's own text never looks at. Run the item's Qualification completeness sweep on those fixes. Fix loops generate this class, which the first-pass audit cannot pre-empt: the caveat does not exist yet for the earlier text to contradict.
    • completion-hygiene: pre-commit hooks catch lint / fmt / line count, but the fix may have added stray dbg! / println! / scratch test code.
    • behavior-coverage: a fix that edits a docstring / module doc stating a behavioral guarantee can silently widen it to sibling symbols, creating a per-symbol coverage obligation the previous audit never saw. Run the delta pass as a fresh done-check (subagent) invocation rather than informal doc-truth reasoning — confirming the broadened claim is true does not confirm each newly-covered symbol is tested.
    • The PR description: if a fix invalidates a claim in the description (e.g., "previously-missed mutant is now caught" became "now excluded"), update the description in the same iteration. That edit is text changed after its claims were discharged, so re-discharge the description in main context per gh-body-conventions § Evidence claims and re-run /gh-body-audit against the result before posting it.
  • Never skip codex review. Even for small fixes, run the full loop.
  • Never inject previous review comments into the next review prompt.
  • Every commit goes through /stage-commit-push. Do not manually run git add/commit/push during the pipeline.
  • Pre-commit branch gate. Before each /stage-commit-push, and at pipeline entry on the terms Pipeline entry states, verify the current branch is not the repo's default branch:

    default=$(git symbolic-ref --short refs/remotes/origin/HEAD | sed 's@^origin/@@')
    test -n "$default" || { echo "remote HEAD unset — run: git remote set-head origin -a"; exit 1; }
    test "$(git symbolic-ref --short HEAD)" != "$default"

    The first two lines are diff-root's default-branch read, spelled out because this check runs it; that skill states why each is load-bearing. Halt and surface to the user if the branches are equal, or if the default branch does not resolve.

  • Triage is mandatory. Never present raw review output to the user. Classify every finding under the finding-triage SSOT dispositions and lead with actionable items.
  • Select the response before fixing. Select the edit per finding-triage's Response selection (actionable findings); a finding fitting invariant-premise-check or opens-a-question re-triages per those dispositions. For a premise check, if unsure, ask codex a single targeted question via codex exec "<fix proposal + one specific question about the premise>" -o /tmp/fix-check.md. The commit is owned by whichever /stage-commit-push step the current phase runs.
  • Oscillation detection. A conceptual topic is not the same literal comment but the same underlying question — e.g., "is this input valid?", "does this property hold?", "should this parameter accept both values?". When a topic settled earlier in the run, by this rule or by the user's answer to an escalation under it, recurs in any later review iteration, stop fixing and escalate to the user, presenting the earlier settlement. Otherwise, when a topic appears across 2+ consecutive review iterations, first derive the contract of the artifact the topic concerns: list the states that reach it, and evaluate each fix option in contention for truth, usefulness and harm in each state. If some option is at least as good as every other on each of the three in every state, fix toward one such option, and state the option and the derivation in the next message to the user; the topic is then settled. If none is, stop fixing and escalate to the user.

    Escalation order. Before presenting the fix-direction question (panic vs allow vs convert vs ...), FIRST ask whether the original plan scope is correct. Ask in this order:

    1. Is this question even single-actionable inside the current plan? Does the disagreement among reviewers concern an upstream undecided design question (consumer semantics, system invariant, layout authority, etc.) that the plan implicitly assumed?
    2. If yes upstream: rescope. Close the current PR, refile the upstream design question as a separate issue, and let the current API decision fall out of that resolution. This is the right move even when the current fix is technically correct in isolation.
    3. If no upstream issue: present what is known, what is uncertain, and ask the user to choose among the surviving fix options.

    The "no-clarifying-questions" mode does NOT override this rule.

Download now (ZIP)

Does it work?

Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.

No reviews yet

People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.

  • It worked
  • Partly
  • It didn't
What did you use it for? e.g. Reviewing pull requests in a Rails app

Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.

Your trail

Files you open will collect here, so you can find them again.

Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.

Now do this for your own.

Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.