deliver

You are running the deliver skill. Goal: take whatever is on the current branch and get it merged with as few CI round-trips as possible.

CI is slow and every avoidable push is a real cost — front-load everything locally before pushing. That includes the review: Phase 3 hands the change to the review-loop skill, which reviews and fixes it with fresh-context reviewers before the first push, so the diff that reaches GitHub is already hardened. A reviewer on the far side of the network — a bot, a person — is something this skill can put on the record (Phase 6b) but never waits on as a first pass.

No-merge mode: when invoked with --no-merge (how the kickoff skill calls this) — or with --review-rounds 0, which implies it — everything through Phase 6b runs unchanged — checks, confidentiality gate, review, PR, CI loop, the tracker's move to in review — but Phase 7's merge condition is forced false: leave the PR ready for review, skip Phase 7b, and report. The merge decision stays with the human.

Phase 7's REVIEW_REQUIRED branch does not fire in this mode. It exists to unblock a merge, and there is no merge to unblock; a human is going to open this PR anyway, which is the whole point of handing it back. Requesting one from here would page a colleague per item on an unattended overnight run — for a PR whose author has not yet looked at it.

Stacked mode: when invoked with --base <branch>, that branch — not the repo's default branch — is what this PR targets and what every diff in this run is computed against. Its purpose is stacking: the parent branch is usually itself an open PR, so this PR's diff shows only the increment on top of it instead of replaying the parent's changes. Phase 0 resolves it once into BASE_REF; nothing downstream re-derives it. Phase 7's stacked-base check then does exactly what it always did — a base that is an open PR blocks auto-merge — which under --base is the expected outcome, not a surprise: land the parent first.

Arguments

• --no-merge — stop at ready-for-review; Phase 7 assesses but never merges. How kickoff calls this.
• --base <branch> — target <branch> instead of the repo default, and compute every diff in the run against it. For stacking.
• --bot-review / --no-bot-review — force Phase 6b's on-record bot review on or off, overriding the reviewer profile knob for this run.
• --review-rounds N — cap Phase 3's local review at N rounds. 0–10, default 3. Raise it for a change whose review keeps finding real defects. Don't lower it to 1 or 2 to save time — Phase 3 fixes what round 1 raises, and with --quiet-rounds 1 a confirming round then has to fit inside the budget. (That confirming round is cheap: one reviewer on the fix commits, and it only has to come back free of blockers and majors, not free of nits. It runs only when round 1 fixed a blocker or major; a round 1 that confirmed nothing worse than a minor ends the loop.) At N=1 any finding at all guarantees budget-exhausted, which Phase 7 reports as blocked, and the loop's own rules forbid buying more rounds by re-invoking. If you have already reviewed the work, pass 0, not 1. 0 skips the review entirely and forces --no-merge — see Phase 3. A value outside 0–10 is a stop, not a clamp: silently reviewing three times when someone asked for thirty is worse than refusing.

Project specifics — read these first

This skill is repo-agnostic. The concrete commands, reviewer, and merge policy come from the repository you're running in. Before Phase 1, gather:

• Check commands per package — how to lint / typecheck / test / run codegen for each workspace. Derive from the repo's AGENTS.md / CLAUDE.md, per-package docs, and package.json (scripts) / Makefile / justfile. If the repo has a ## Skill profile section in its root AGENTS.md, use that — it's the curated source. The CI workflow outranks docs and script names, but not a ## Skill profile (curated on purpose, and what upstream-pr and review-loop read first): when docs or scripts disagree with the run: steps of the lint / format / typecheck jobs in .github/workflows/*, run what CI runs, flags included (a --check the package.json script lacks, a type-aware lint mode). Docs and scripts drift; CI is what turns the PR red.
• Repo identity — one call covers slug, default branch, and the audience the Phase 2b gate keys on:

  gh repo view --json nameWithOwner,defaultBranchRef,visibility,owner

• PR base — where this PR is meant to land. The GitHub default branch is the last resort, not the first: plenty of repos merge into develop, next, or a release line while defaultBranchRef still says main. First hit wins:

  1. the --base argument,
  2. the repo's agent config baseBranch (e.g. .ai/agentic.config.json),
  3. the ## Skill profile key baseBranch in the root AGENTS.md / CLAUDE.md,
  4. what the PR template or CONTRIBUTING says ("Open PRs against develop"),
  5. the default branch from the gh repo view above.

  Same order upstream-pr uses, so the two skills cannot disagree about where a repo's work lands. Ignore a literal "auto" at tiers 2–3 — it means "detect", not a branch called auto. Phase 0 resolves it once into BASE_REF; every later phase reads that variable rather than re-deriving a base of its own, and the report states which tier the value came from whenever it was not the default branch.

• Review on the record — the ## Skill profile key reviewer (singular): a PR review bot's login (copilot-pull-request-reviewer is the usual one), set only by repos that want a bot review attached to the PR. Its presence is what switches Phase 6b on; --bot-review / --no-bot-review override it for one run. It is not the reviewer that hardens the change — that is Phase 3, and it runs whether or not this key exists.
• Human reviewers — the ## Skill profile key reviewers (plural), a distinct knob: who to request when the PR cannot merge without an approving review (Phase 7). Unset, a candidate is derived from recent merged PRs.
• ownerCanSelfMerge — from the ## Skill profile; gates whether gh pr merge --admin is acceptable (see Phase 7). Default: false (don't bypass required reviews).
• Dev-server / port convention — if the repo documents one (e.g. a worktree port rule), follow it whenever you need to start a service for a local test.
• Tracker + its status vocabulary — from the ## Skill profile (tracker). If the repo documents which states mean in progress, in review and done, this skill moves the task along with the PR (Phases 5b and 7b). If it documents a tracker but no vocabulary, don't guess at state names — report the task's current state in Phase 8 instead.

If a needed value isn't documented and you can't infer it, ask the user rather than guessing.

Phases

0. Anchor

Before doing anything, record the starting SHA so the "not much changed since invocation" check at the end is meaningful:

mkdir -p .context/deliver
git rev-parse HEAD > .context/deliver/start-sha
git rev-parse --abbrev-ref HEAD > .context/deliver/branch
export SLUG=$(gh repo view --json nameWithOwner --jq .nameWithOwner)   # Phases 6 and 6b interpolate this

Resolve the review budget here too, and stop rather than clamp on a value outside 0–10 — silently reviewing three times when someone asked for thirty is worse than refusing:

export REVIEW_ROUNDS="${ARG_REVIEW_ROUNDS:-3}"
case "$REVIEW_ROUNDS" in
  ''|*[!0-9]*) echo "--review-rounds must be an integer 0-10"; exit 1 ;;
esac
[ "$REVIEW_ROUNDS" -le 10 ] || { echo "--review-rounds must be 0-10"; exit 1; }
echo "$REVIEW_ROUNDS" > .context/deliver/review-rounds

SLUG is exported here because the gh api calls downstream are merge-gating reads: an unset variable interpolates to "", repos//… 404s, and the run can neither tell a skipped check from a failed one nor find a single review comment — while looking like it asked.

Resolve the PR base once, here, and export it — Phases 1, 2b and 5 all read it, and a base re-derived per phase is how a run ends up checking one range and publishing another:

DEFAULT=$(gh repo view --json defaultBranchRef --jq .defaultBranchRef.name)
CONFIG_BASE=$(jq -r '.baseBranch // empty' .ai/agentic.config.json 2>/dev/null | grep -v '^auto$')
# --base → agent config → profile → PR template/CONTRIBUTING (read, not scripted) → default
export BASE_REF="${ARG_BASE:-${CONFIG_BASE:-${PROFILE_BASE_BRANCH:-$DEFAULT}}}"
git rev-parse --verify "$BASE_REF" >/dev/null || git fetch origin "$BASE_REF"
echo "$BASE_REF" > .context/deliver/base

A --base that doesn't resolve is a stop, not a fallback. Silently dropping back to the default branch would open a PR whose diff replays its parent's commits — reviewable-looking and wrong. If the branch exists on neither the local repo nor the remote, say so and stop.

Refuse to run on the default branch, and refuse when BASE_REF is the branch you're on — a PR cannot target itself. If the working tree has uncommitted changes, surface them and ask the user before continuing — don't silently git add -A.

1. Scope detection

Look at git diff "$BASE_REF"...HEAD --name-only and bucket the touched files by package/workspace (top-level dir, monorepo workspace, or whatever the repo's structure is). Only run the checks for packages that actually changed — don't run one package's tooling for a branch that didn't touch it.

On a stacked run that range is deliberately narrow: the parent's files were already checked on the parent's PR. Don't widen it back to the default branch to be safe — that re-runs the parent's whole check surface on every child in the stack, which is the cost stacking exists to avoid.

2. Local checks (front-load CI)

Run the touched packages' checks IN PARALLEL where independent. These are the same gates CI runs — if any fail, fix locally before pushing. For each touched package, run its documented:

• Codegen (e.g. GraphQL/types) — only if the relevant source changed; never hand-edit generated files.
• Typecheck.
• Lint with autofix — run the real linter, not just a formatter. A formatter (prettier and friends) does NOT catch what the linter (ESLint and friends) catches; run both if the repo separates them.
• i18n extraction / translation — only if user-facing strings changed.
• Targeted tests — the unit/integration tests closest to the diff. Never run a full suite as a pre-push gate — it's far too slow; CI owns full suites. Pick the test files closest to the change.
• e2e — run ONLY the spec(s) directly related to the change. The full e2e suite is too slow for a pre-push gate. If this is a user-facing feature change and there's no e2e spec for it, the change isn't ready — write one (don't ship user-facing behavior without an e2e, and don't mark a planned e2e "optional").
• Infra (if touched) — format + validate the changed config (e.g. tofu fmt/tofu validate). Never apply from this skill.

If any check fails: fix it, re-run, then commit. Keep history clean — squash fixups into the commit they belong to where reasonable.

Phase 3's --gate scoped resolves to this same set, from this same repo config — nothing is handed over, both derive it independently. So getting the repo's documented commands right is what keeps the loop from running a check the repo doesn't have, or the full suite hard rule 3 forbids.

2b. Confidentiality gate — only if this repo is not the sole audience

The audience came from the Phase 0 gh repo view — no extra call. The gate fires when the repo is public, or when its owner is not the client whose material the branch draws on (an FSH-internal repo, another client's repo, a shared library). It does not fire for a private repo owned by the same client the work is for — there, their own details are in their own house.

When it fires: no client's non-public details may land in the push. Names, staff, repo names, local paths (~/src/<client>/…), internal spec/ticket IDs, name-carrying identifiers (module and table prefixes, env-var prefixes, service names), infrastructure (hostnames, endpoints, account IDs), and their data (fixtures, seed data, screenshots, logs). This holds even when the mention is flattering — crediting where a pattern was proven ("ported from client X's field-tested module") is the most common way a name reaches a public diff. State the engineering claim, drop the address. The only exception is a detail already public in the client's own material, verified rather than assumed.

Scan the diff, the commit messages, and the PR body before they are published:

TERMS='acme|acmecorp|acme_|ACME-'                                  # from what the branch drew on
git diff "$BASE_REF"...HEAD | grep -inE "$TERMS"
git log "$BASE_REF"..HEAD --format='%B' | grep -inE "$TERMS"

Grep is the floor — also read the prose the branch adds (specs, READMEs, comments). On a hit before pushing: rewrite (amend/rebase is fine, nothing is published yet). On a hit in something already pushed or public: stop and tell the user — never force-push to hide it, and never decide alone whether to rewrite published history.

The same gate applies to anything Phase 6b posts — see that phase.

3. Review — before anything is pushed

This is the review. Not a warm-up for one: the findings that cost the most rounds live in what the hunks touch — the consumer of a changed function, the environment a config key lands in, the script whose output another script parses — and all of that is readable locally, now, for the price of some tokens instead of a ten-minute wait and a CI run per lap.

First, the skip gate. If Phase 0 resolved REVIEW_ROUNDS to 0, do not invoke review-loop at all — go straight to Phase 4, in no-merge mode for the rest of the run. Never pass --max-rounds 0: that skill runs at least one round per invocation by design, so 0 there means "review once, then immediately exceed the cap", which reports budget-exhausted and lands the run on blocked — the opposite of what was asked for, after doing the work that was meant to be skipped.

That coupling to no-merge is the whole reason 0 is allowed. It lets someone who has already reviewed the work skip a redundant pass without also handing them a way to auto-merge code no reviewer read. Everything else still runs: the checks, the confidentiality gate, the PR, the CI loop. The run stops at ready-for-review and the merge belongs to whoever did the reviewing.

Otherwise invoke the review-loop skill, passing the base and the budget Phase 0 resolved:

review-loop --source local --base "$BASE_REF" --quiet-rounds 1 --max-rounds "$REVIEW_ROUNDS" --gate scoped

--base is not optional. Phase 0 promised nothing downstream re-derives a base; that skill resolves its own from repo config when nobody passes one, and on a --base run it would then review the parent's entire diff instead of this branch's increment — burning the whole budget on code the parent's own PR already reviewed.

If review-loop is not installed — this repo supports symlinking a single skill — don't skip the review and don't improvise a rubric. Run one round of it inline: a fresh reviewer subagent on git diff "$BASE_REF"...HEAD, told nothing of the branch's intent, asked for whole files and callers and a severity, a concrete failure scenario and a fix per finding; verify each finding before acting on it; fix what holds; hand up the judgement calls and anything outside the diff. There will be no state.json, so Phase 7's first condition reads differently on this path, and you must say which path you took. The fallback satisfies it when its single round left nothing actionable open and the HEAD its reviewer read is still HEAD — the same question the field answers, asked by hand. It is one round, so it can never be converged; don't write that word, and don't invent a state.json to hold it. Report it as inline fallback, one round in Phase 8, so a reader can tell a looped review from a single pass.

Those arguments are deliberate and differ from that skill's standalone defaults:

• --quiet-rounds 1 — one round raising nothing blocking is enough here, because this is a front-load rather than a hardening run and the change still has CI and a human ahead of it. The minors and nits it lists travel in the PR body for the reviewer to pick up; spending fix-rounds on them is how a scoped change acquires defects it did not have. Pass --quiet-rounds 2 when the branch warrants it (a migration, an auth path, a public contract); it adds a second single-reviewer follow-up round and is worth it there.
• --max-rounds "$REVIEW_ROUNDS" — default 3; past three, the answer is a human, not another lap. That skill caps a prose-only diff at 2 regardless. --review-rounds N overrides exactly this number and nothing else. The quiet-round threshold and the gate stay put, because they define what "reviewed" means rather than how long to keep at it.
• --gate scoped — the checks from Phase 2, for the packages this diff touches. Not the repo's full gate: a full suite pre-push is slower than the CI it exists to front-load (hard rule 3), and CI owns the full one.

The reviewer prompts are that skill's, verbatim — do not improvise the emphasis lanes. In particular never ask a reviewer what else in the repo states a fact this diff corrects, or whether the change is "complete" repo-wide. That question always returns something, none of it caused by this branch, and acting on it is how a scoped PR turns into a tour of the repository. Pre-existing drift is a follow-up; the loop's hard rule 6 hands it up, but only if the prompt didn't solicit it.

Both artifacts matter: state.json (the machine-readable result) and report.md (the curve, the rubric, the fixed/refuted counts) — both under the run directory the loop names. Phase 5's PR body needs the second; Phase 7 needs the first:

The loop's handed-up findings are not yours to decide. They are the judgement calls it refused to make alone — a design disagreement, a scope question, a deprecation policy. Carry them into the PR body (Phase 5) and the report; don't quietly implement one, and don't quietly drop it.

Findings outside the diff come back handed up too. Report them; don't fix them here. Fixing them turns a reviewable branch into a tour of the repo.

4. Commit & push

Re-run the Phase 2b scan first, if that gate fired. It ran before Phase 3, and Phase 3 has since committed up to three rounds of code it wrote itself — a comment explaining where a pattern came from, a fixture named after a client system. Nothing is published yet, so a rewrite here is still free; after the push it is the one failure hard rule 6 says a later commit can't undo.

git diff "$BASE_REF"...HEAD | grep -inE "$TERMS"
git log "$BASE_REF"..HEAD --format='%B' | grep -inE "$TERMS"

Commit any work made during the checks and the review loop under the same authorship as the branch's existing commits. Use Conventional Commits.

The loop already committed its own rounds, one per round — leave those as they are rather than squashing them into a single "review fixes" commit; the per-round shape is what makes the curve legible in git log.

Keep the loop's run directory out of the commits. If the repo tracks paths.runs, exclude it explicitly; never git add -A.

If the branch isn't pushed yet, push with -u. If it is, just push.

5. PR open or update

gh pr view --json number,url,reviewDecision,reviews,headRefOid 2>/dev/null

• No PR → gh pr create --base "$BASE_REF". Title in Conventional Commits style stating the plain-language outcome. Write the body in the house format, after the context/task line: a short paragraph with no headings for a small PR; otherwise general-to-specific sections (🎯 problem → 🔧 fix → 🧭 impact → 🧩 how the pieces fit → 🔍 details → ✅ testing → ⚠️ worth knowing), dropping empty ones, whose heading words state this PR's actual problem, fix or impact, never ### 🎯 The problem. A repo's own template or documented format wins.
• PR exists → the push updated the code, but check the title/description still tell the truth: if the work drifted since they were written (rebase, review rework, scope change, a referenced PR merged), run the pr-polish skill — a stale description misleads whoever reads it next.

The body carries the review evidence. Moving the review off the PR removes the only public record that one happened, and a human arriving at this PR has no way to tell a reviewed branch from an unreviewed one. Under ✅ Testing, state it plainly (a small PR without headings says it in a sentence or two): the rubric the loop used, how many rounds it ran, how many findings it fixed and refuted, and what gate passed. Then, under ⚠️ Worth knowing, an Open questions list — one line per handed up entry in open[], naming the decision rather than describing a problem. Key it on those specifically: open[] also carries deliberately-left nits and, after a --no-fix run, proposed ones, so a section triggered by "non-empty" would open with a heading and nothing under it. Put the rest under ⚠️ as follow-ups — including every out-of-diff finding, which belongs there and never under Open questions: it is not a decision about this change, and listing it as one invites the next reader to ask why the PR didn't fix it.

On the Phase 3 inline-fallback path there is no open[]: say "inline fallback, one round" and list what that round handed up directly.

Under --review-rounds 0 there is no run directory at all — no state.json, no report.md, nothing to read. Don't hunt for one, and don't leave the section out: say plainly that no local review ran, that the caller asserted they had reviewed the work themselves, and that the merge is therefore left to a human. That sentence is the most useful thing in the body on such a run, because it is the only signal a later reader gets that this diff was never independently read. That list is the most useful thing in the body for the person who reviews next.

Everything in that section is published text and passes the Phase 2b gate first: quote no client identifier into a public PR body just because a reviewer's finding mentioned one.

Save the PR number to .context/deliver/pr-number.

An existing PR's base is not yours to change. If its baseRefName differs from BASE_REF — a stack re-run after the parent landed and the forge retargeted the child, or a --base that disagrees with what is on record — report the mismatch and continue against the base the PR already has. Retargeting silently changes what every approval and every review comment on that PR applied to; only a human may decide that.

5b. Tracker — the work is now up for review

Skip entirely if the repo documents no tracker vocabulary (see Project specifics).

Resolve the task from the PR body's task line — the repo's own convention (Closes X / Part of X / Relates to X). Then, in one fetch of that task:

• Assigned to someone other than the user you're working for → touch nothing. Report it in Phase 8. You don't know what that person is doing with it.
• Still in a not started state → move it to in progress, then to in review. A pushed branch with a PR is unambiguously both; there's no point recording only the later one.
• Already in in review or a terminal state → leave it.

Move the task named by Closes. A Part of / Relates to task belongs to work wider than this PR — leave those alone at every phase.

6. Handle CI failures

gh pr checks <N> --watch

Confirm a "failed" entry against the head commit before believing it. gh pr checks renders a job's skipped conclusion in the same bucket as a failure, so a workflow's if: failure() notification job — which is skipped on every successful run, by design — prints as [FAIL] on a perfectly green PR:

gh api --paginate \
  "repos/$SLUG/commits/$(gh pr view <N> --json headRefOid --jq .headRefOid)/check-runs?per_page=100" \
  --jq '.check_runs[] | "\(.name)\t\(.status)\t\(.conclusion)"'

Read that list both ways: a skipped alert job is not a failure, and a required gate that was filtered out of this run (a path filter, a paths-ignore) is not a pass — it's a green banner over a job that never ran. Neither is a reason to stop; both change what you do next.

If a check genuinely fails, fix it and keep going — don't stop and hand back to the user. Workflow:

1. Pull the failing job's logs: gh run view <run-id> --log-failed (use gh pr checks <N> --json to find the run id).
2. Diagnose the root cause. Common buckets:
   • Lint / typecheck / format — a Phase 2 check that should have been caught locally. Run it locally now, fix, push. Then ask why Phase 2 missed it (skipped a package? formatted but didn't lint?) so it doesn't recur this run.
   • Unit / integration tests — reproduce locally with the exact same command CI ran, fix the underlying code or test, push.
   • e2e flake — re-run the failed job once (gh run rerun <run-id> --failed). If it fails a second time on the same spec with the same fingerprint, it's not a flake — diagnose properly (a flake-hunt skill exists for this; invoke it if the failure looks genuinely race-y). Never paper over with retries/skip/timeout.
   • Migration / DB — run the repo's migration check; if a manual migration is missing a snapshot/state update, fix per the repo's docs.
   • Infra — fix the config, re-run format + validate locally.
3. Push the fix. CI restarts; loop back to monitoring.

Under --review-rounds 0 there is no re-review — Phase 3 was skipped, so there is no invocation to repeat and no reviewed_oid to invalidate. Fix CI as normal, and name the fix in Phase 8 as code no reviewer has read. The rest of this paragraph applies only when Phase 3 actually ran.

A CI fix that changes shipped behaviour goes back through Phase 3. Green checks are not a review: such a fix carries code no reviewer has read, on a reviewed_oid for the old HEAD, and Phase 7 will refuse it. Re-run Phase 3's invocation verbatim — --base and --gate scoped included; a bare review-loop picks up that skill's standalone defaults, which means the repo's whole check set (hard rule 3) and a base re-derived from config (wrong on a stacked run). It resumes its ledger rather than starting over, so the pass is cheap and dedupes against everything already raised. A test-only, snapshot or workflow fix that changes no shipped behaviour keeps the direct path.

Hard stop conditions (escalate to user, don't keep grinding):

• Same failure recurs after 3 fix attempts on the same job — your hypothesis is wrong; stop and ask.
• The fix would require changes outside this branch's scope (e.g. updating a shared package, infra credentials).
• A test failure points at a real bug in code outside the diff (this branch surfaced it but didn't cause it).

Do not merge while any required check is failing or pending. --admin bypasses required reviews, not failing CI (see Hard rules).

6b. Reviews on the PR

Phase 3 hardened the change. This phase is about the PR's own review surface — what is already on it, and whether this repo wants a bot's review added to it.

Always: answer what is already there.

gh pr view <N> --json reviews --jq '.reviews[] | "\(.author.login)\t\(.state)\t\(.commit.oid)"'
gh api "repos/$SLUG/pulls/<N>/comments" --paginate

Read the review body as well as the inline comments. A bot review folds findings into a <details> "Suppressed comments" block, and a body reading "Comments generated: 0" routinely sits above three to five of them — so the two API calls above are not the whole round. This matters most on exactly the path that skips the rest of this phase: plenty of repos have a bot reviewing automatically on open, with no request from anyone, so a deliver run that never sets reviewer still finds a review waiting here. Reading only the inline comments calls such a review clean and merges over it.

The verdict header is not a finding either. 🟡 counts unresolved threads, so a PR whose fixes are all pushed stays yellow; 🔵 on a broad change means a human should read it, and no code fix turns it green. Judge by the findings.

Someone who took the time to comment gets an answer, and an unresolved actionable thread blocks the merge regardless of who left it or whether anyone requested them. Run each finding through the same discipline Phase 3's loop uses — verify it before touching code, fix what holds, reply on the thread, resolve it; reply with the reason and resolve when it doesn't hold; leave it open and hand it up when it's a judgement call. This runs on every invocation, including the ones where the rest of this phase is skipped.

If review-loop is not installed — the case Phase 3 handles — neither part of this phase's loop is available. The always branch above still runs; do it by hand, it is only the two reads plus the reply-and-resolve discipline. Skip the bot round and record it as not run; Phase 7 treats a missing on-record review as non-blocking. At Phase 7's REVIEW_REQUIRED branch, request the human directly: take the login from the reviewers knob, or the most frequent reviewer of recent merged PRs excluding the author, then gh pr edit <N> --add-reviewer.

Conditionally: put a bot's review on the record. Run this when the ## Skill profile sets reviewer, or when the run carries --bot-review. Skip it when the profile sets nothing, or the run carries --no-bot-review. Skipping is the common case and is not a gap: Phase 3's review is recorded in the PR body, and Phase 7 requires it to have covered HEAD.

review-loop --source bot --pr <N> --base "$BASE_REF" --gate scoped

--gate scoped for hard rule 3's reason — that skill's default is the repo's whole check set, which is the full suite CI is already running on this PR.

If Phase 5 reported a base mismatch, pass the PR's own baseRefName here instead of BASE_REF. Phase 5 continues against the base the PR already has; a review diffed against a different ref would reply to threads on a diff nobody is looking at — and after a stack's parent lands, BASE_REF may name a branch the forge has deleted.

It requests the bot, polls for a review of this HEAD, reads the inline comments and the ones the review body folds away, verifies each finding before spending a code change, fixes, replies, resolves, and caps itself at 3 rounds (5 while blocking findings keep arriving). It updates the same state.json, so Phase 7 reads one file whichever sources ran.

Two things belong to this skill, not that one:

• The confidentiality gate applies to every reply and every commit posted here — by the loop or by you. A thread reply is published text on a possibly-public PR, and the bot source pushes its own fix commits for up to five rounds. If the Phase 2b gate fired, re-run its $TERMS scan over what this phase added before it goes out, for the reason Phase 4 gives. (Only if it fired — TERMS is set inside that branch, and an unset one makes grep -inE "$TERMS" match every line of the diff.)
• A bot that never answers is not a blocker on its own. sources.bot.exit: awaiting-review means the record is missing, not that the change is unreviewed — Phase 3's review still stands, and Phase 7's conditions decide what that's worth.

7. Auto-merge decision

In no-merge mode the merge is already decided against — the condition is false by definition, and Phase 7b never runs. The assessment still happens. --no-merge withholds the merge, not the truth about the PR: evaluate the conditions below anyway and report what they say — ready-for-review only when they all hold bar the merge itself, otherwise awaiting-CI or blocked, naming what blocks. Reporting ready-for-review unconditionally is how an exhausted review budget with open findings, or a red pipeline, reaches a human as "done".

If not much has changed since the skill started, just merge. "Not much" means the work since Phase 0's start-sha is mostly review fixups, not new functionality.

START=$(cat .context/deliver/start-sha)
git diff --shortstat "$START"...HEAD     # lines changed since invocation
git log --oneline "$START"..HEAD          # commits since invocation

Merge condition (ALL must hold):

• A review ran at all. --review-rounds 0 skipped Phase 3, so this condition fails by construction — the merge cannot happen. Keep evaluating the rest, exactly as no-merge mode requires: an unresolved thread, a red pipeline or an open parent PR still decide whether this reports blocked, awaiting-CI or blocked-on-parent-PR rather than ready-for-review, and the report must name the skipped review either way. Nothing substitutes for it: green CI is not a review, and neither is the author's word that they read it themselves. AND

• The review covers what is on the branch now. sources.local.exit is converged — that is the review this skill always runs, so it is the one that must have finished, not merely "some source did". And sources.local.reviewed_oid equals HEAD, or every commit since it is one this run made that review-loop's equivalence rule covers: a fix made in response to a finding, or a CI fix that changes no shipped behaviour (a lint autofix, a snapshot update, a workflow tweak). Inside a run you know which commits are which; a re-invocation does not, so commits after it that this run didn't make are unreviewed code and report blocked.

  budget-exhausted on sources.local reports blocked — that is the review this skill always runs, and its cap ending without convergence never lowers the merge bar. On bot it is reported, not blocking: the loop stopped asking a bot, which says nothing about the change that Phase 3's review and the open-threads condition below don't already answer. awaiting-review never blocks on its own either (Phase 6b says why). Judge the change by what is open, not by which source ran out of patience. AND

• No blocker or major sits open in state.json's open[], and no actionable review thread on the PR is unresolved — whoever left it. AND

• Nothing is stacked on top of this PR's base — that is the precise question, and "the base is the default branch" only approximates it: plenty of repos ship through a release or integration branch, and such a repo could never auto-merge under that rule.

  BASE=$(gh pr view <N> --json baseRefName --jq .baseRefName)
  DEFAULT=$(gh repo view --json defaultBranchRef --jq .defaultBranchRef.name)
  # A PR onto the default branch cannot be stacked — skip the query entirely.
  [ "$BASE" = "$DEFAULT" ] || gh pr list --state open --head "$BASE" --json number,url

  The short-circuit is load-bearing, not an optimisation: --head matches on branch name and includes cross-repository PRs, so one open fork PR whose head branch is called main would otherwise mark every PR in the repo as stacked and disable auto-merge outright.

  A hit means the base is itself an open PR waiting to land. Merging into it folds this work into that PR, enlarging a diff someone is mid-review on, and ships nothing. Stop and tell the user to land the parent first. Never retarget the base yourself. On a --base run this hit is the designed outcome, not an accident: a stack merges bottom-up. Report it as blocked-on-parent-PR with the parent's number — a healthy stack waiting its turn, not a failure. AND

• ≤ ~100 lines changed since start-sha, AND
• No new files outside what was already touched at start-sha, AND
• All CI checks on the PR are green (gh pr checks <N> — wait for them, and resolve any [FAIL] against the head commit's check-runs per Phase 6 before calling it red), AND

• The remote carries what you reviewed and fixed:

  [ "$(git rev-parse HEAD)" = "$(gh pr view <N> --json headRefOid --jq .headRefOid)" ]

  A squash merge ships the branch GitHub holds, and --delete-branch then deletes the local one. An unpushed fix is not merged, it is destroyed — and because CI and the reviewer both read the remote, every other condition above would have gone green on the code without it. Push, let CI settle, then re-evaluate.

A forge that requires an approving review is a stop, not a condition to work around.

gh pr view <N> --json mergeStateStatus,reviewDecision

reviewDecision: REVIEW_REQUIRED means branch protection wants a human's approval, and nothing this skill does locally can produce one. Request the reviewers — review-loop --source human --pr <N> --base "$BASE_REF" --gate none --no-fix, which uses the reviewers profile knob or derives a candidate — and report awaiting-review.

--no-fix is what makes --gate none safe here, and the pair has to travel together. This phase runs after CI has been driven green and after the head check above; a source that fixed and pushed from here would leave the PR's newest commit ungated, unreviewed and unwatched, under a report calling it ready. There is nothing to fix in any case — Phase 6b already answered every thread on the PR. This call requests a person and does nothing else. Don't wait on them: a human review arrives on human time. This is the only place this skill requests a person.

If the condition holds → merge:

gh pr merge <N> --squash --delete-branch        # add --admin only if ownerCanSelfMerge

Use --admin only when the ## Skill profile says ownerCanSelfMerge: true (the user owns the repo and doesn't need peer approval). Otherwise merge normally and let required reviews apply.

A non-zero exit is not proof the merge failed — check the PR, never retry blind. --delete-branch also deletes the local branch, and that step fails when another worktree has the base checked out (fatal: 'main' is already used by worktree at …), long after the merge itself succeeded server-side:

gh pr view <N> --json state,mergedAt,mergeCommit

MERGED means done — finish Phase 7b and report it. Re-running gh pr merge on a merged PR instead pushes the deleted branch back and can leave a stray merge commit on the base, which is published history you must not clean up alone. The same read settles a merge that times out or drops its connection.

If the condition doesn't hold → don't merge. Surface the state to the user and stop.

7b. Tracker — close the task

Only once the merge has actually succeeded, and only for the task named by Closes: move it to the done state. Same guard as 5b — never a task assigned to someone else.

If the PR only says Part of / Relates to, or the merge didn't happen, leave the task where 5b put it and say so in Phase 8. Never set an abandoned / cancelled / won't do state from this skill; that's a human judgement, not a consequence of a merge.

Acceptance criteria the merge can't prove (something observable only in a deployed environment) are yours to check before moving the task, not to assume. If you can't check them, leave the task in review and say what's outstanding.

8. Report

Final message to the user must include: PR URL; the base it targets whenever that isn't the default branch (name the parent PR it stacks on); merge status (merged / awaiting-CI / awaiting-review / blocked-on-parent-PR / blocked); the review loop's result — rubric, rounds, fixed, refuted, and its exit, or "no local review ran (--review-rounds 0)" when it was skipped; what it handed up, in full, since those are decisions waiting on the user; whether Phase 6b ran and what came back; the tracker task and the state you left it in (or why you didn't move it); and any thread you left unresolved, with why.

The handed-up findings are the part a reader most needs and most easily loses. List them as decisions, not as a summary of a summary.

Hard rules

1. Never --no-verify, never --no-gpg-sign. If a pre-commit hook fails, fix the root cause.
2. Never push to the default branch. This skill operates on a feature branch only.
3. Never run a full test suite locally — not full e2e, not full unit/integration. Targeted runs only; CI owns full suites. This is why Phase 3 passes --gate scoped.
4. Never merge without CI green. Even with --admin, wait for gh pr checks to be green. Bypassing required reviews is one thing; bypassing failing CI is not.
5. Never merge on a review that didn't cover HEAD. "A review happened" and "a review of this code happened" are different facts, and only the second may gate a merge. sources.local.reviewed_oid is the one that counts — the fresh-context review's own coverage, not the newest HEAD some source happened to look at.
6. Never publish a client's non-public details into a public repo or one owned by anyone but that client — not in the diff, the commit messages, the PR body, or a review reply. See the Phase 2b gate. It's the one failure here a later commit can't undo.
7. Don't expand scope under cover of review feedback. If a finding asks for a refactor beyond the PR's purpose, it comes back handed up — carry it to the user, don't build it.
8. Re-invoke review-loop only for a reason a phase names — Phase 6's behaviour-changing CI fix is the one. Never re-invoke a source that came back budget-exhausted hoping for a different answer: that count resumes by design, and past the cap the answer is a human. A source that converged gets its budget back, because what brings you here is new commits and they deserve the same scrutiny the first pass got. The cap never relaxes Phase 7's conditions either way.
9. A verdict header is not a finding; a folded one is. Read the review body's suppressed comments alongside the inline ones, ignore the colour, verify before fixing, and never re-request a bot to turn 🔵 into 🟢. This applies whenever a PR carries a review — including the common case where a repo's bot reviews on open and nobody requested it.
10. Follow the repo's dev-server/port convention when you start a service for a local test. Don't auto-launch a whole-stack dev script.
11. Never move a tracker task that belongs to someone else, and never move one to done on anything but a successful merge of a PR that says it closes it. A wrong status is worse than a stale one — it's read as a fact by people who weren't in this session.