--- name: PR Delivery Loop slug: pr-delivery-loop category: DevOps description: PR Delivery Loop standardizes how implementation tasks are delivered across the Avibe repositories, including branch and scope rules, review and CI gates, and close-out criteria. Use it for PR work that must follow the Avibe delivery process. github: "https://github.com/avibe-bot/avibe/tree/master/.agents/skills/pr-delivery-loop" language: Python stars: 494 forks: 76 install: "npx degit https://github.com/avibe-bot/avibe/tree/master/.agents/skills/pr-delivery-loop ~/.claude/skills/pr-delivery-loop" installs_to: ~/.claude/skills/pr-delivery-loop source_path: .agents/skills/pr-delivery-loop/SKILL.md collection_size: 5 category_size: 798 collection_url: "https://dirskills.com/collections/avibe-bot/avibe" added: 2026-08-26T05:12:57.632Z last_synced: 2026-08-26T05:12:57.632Z canonical_url: "https://dirskills.com/skills/pr-delivery-loop" --- # PR Delivery Loop PR Delivery Loop standardizes how implementation tasks are delivered across the Avibe repositories, including branch and scope rules, review and CI gates, and close-out criteria. Use it for PR work that must follow the Avibe delivery process. **Install:** ```bash npx degit https://github.com/avibe-bot/avibe/tree/master/.agents/skills/pr-delivery-loop ~/.claude/skills/pr-delivery-loop ``` ## README # PR Delivery Loop — implementation-lane standard ## Distribution - The canonical maintained copy lives in the Avibe repository at `.agents/skills/pr-delivery-loop/SKILL.md`. - Every other applicable repository carries a byte-for-byte copy of this skill file at the same repo-relative path. Do not copy watcher implementations or their tests into this skill. - In the multi-repo workspace, the project-level skill entry is a symlink to the Avibe copy. Update the canonical copy first, then sync every repository copy only after the canonical Avibe change passes review. ## Dependency boundary - Use the `background-watch-hook` skill for every managed wait. It owns the reusable `vibe watch` workflow and the GitHub PR, issue, and Actions waiter implementations. - This skill owns Avibe-specific delivery policy: branch and scope rules, review and CI gates, thread resolution, circuit breaking, authority, and close-out criteria. - Refer to the dependency by skill name. Do not hard-code an installation path, vendor its scripts here, or hand-roll a replacement waiter. It is a workspace-level dependency, not a repository payload: companion repositories must not copy it just to satisfy this policy. The active agent environment must expose it by skill name before a managed wait starts; if it does not, report that environment blocker rather than changing repository scope. ## Roles & authority - Your **orchestrator** is the agent session responsible for integration and scope decisions. For a delegated lane, it is the dispatching session (your run's callback target). In a user-started working session, the current agent is the orchestrator; the user remains the owner and highest authority, not the default decision engine. - Authority order: **user > orchestrator > this standard.** An explicit instruction from the user or your orchestrator overrides any rule below — verify the mechanical gates, then carry it out yourself. - Never spawn another agent to act as orchestrator, final reviewer, or merge-approver. Review authority cannot be conjured by a delegated lane: report decisions to its assigned orchestrator. When the current session is the orchestrator, decide independently; ask the owner only for a major trade-off, irreversible risk, or genuinely ambiguous direction. ## 0. Scope & branch - If the owner or orchestrator gives you an existing branch or worktree, treat it as the assigned task context and continue there after verifying its head; do not create a fresh default-branch lane that abandons or duplicates it. - Otherwise branch from the **latest** origin default branch (`git fetch origin` first). In avibe, use a task worktree under the workspace's `.worktrees/avibe/` directory, or the equivalent sibling worktree directory in a standalone clone. - Stay inside your assigned file scope. A cross-lane interface gap is a **report to the orchestrator**, never an edit to the other lane's files. - No stacked PRs. If you depend on an unmerged PR, build against its documented shapes from the default branch and declare the dependency in your PR body. - **A unique identifier you allocate is a cross-lane contract, even though git treats it as a private file.** Two lanes each add `storage/alembic/versions/_00NN_*.py` picking the same next number; the filenames differ, so git merges both silently into a duplicate revision and a forked head, and the one textual conflict — the shared `HEAD_REVISION` line in `tests/test_sqlite_state_migration.py` — resolves mechanically ("same string, keep it") and erases the evidence. Before final rebase, re-diff your `revision` / `down_revision` against the default branch's newest revision; renumber and re-chain rather than keeping siblings. Same shape for any allocated-id namespace: route paths, feature-flag keys, fixture ids. `git merge-tree --write-tree` will not find it. - **`DIRTY` + zero CI checks is one symptom, not two.** A conflicted PR cannot produce the `refs/pull/N/merge` ref that `pull_request` workflows run against, so no workflow starts at all. Before waiting on a slow queue, check `gh pr view --json mergeStateStatus`. - **A cancel-and-reroute is not a handover.** `vibe runs cancel` is asynchronous and can lose the race, so a brief rerouted to a fresh agent can leave two agents live on one branch. The run table shows nothing wrong; the symptom is commits the current agent did not author and a PR head ahead of its own record. Whoever reroutes owes one of two things: confirmation that the original is dead before the fork starts, or an explicit statement to each side — the fork is told which session it replaces, the original is told it has been superseded. Whoever inherits a branch reads `git log` for authors other than itself before editing, and treats an unexplained commit as a live peer rather than as its own forgotten work. ## 1. Contracts before code (multi-lane work) - Interface shapes live in files both lanes can read (types, example payloads, endpoint lists, or a spec section explicitly named as the contract) — never only in prose. Field names are exact (case included). **Commit them to the base branch before the lanes fork**: a contract that lands after the fork is not a shared reference, it is two divergent copies. - **An untracked contract draft in the main checkout is a defect with a delayed cost.** No lane can read it from `origin/`, and it blocks the post-merge fast-forward of that checkout — which matters because regression deploys the local commit, so the checkout that cannot fast-forward is exactly the one that ships stale code. Commit the contract, or keep it out of the checkout entirely. - **Freeze behavior contracts, not just field shapes.** For every cross-boundary flow, the contract states per field: who produces it, who consumes it, what a signature covers, and — for any handshake/attestation — which side must supply what. A shape-only contract passes both sides' unit tests while the behavior silently disagrees. - Orchestrator duty: before dispatching multi-lane work, walk each end-to-end flow across all boundaries and enumerate every field's produce/consume/sign/supply, surfacing every misalignment at once. On any live integration failure, audit the whole remaining chain in one pass — each fix can mask the next layer. - Require a real end-to-end test that pierces every boundary with real data (including non-ASCII) — two isolated unit suites with the peer mocked prove nothing about the boundary. - Deviating from the contract requires orchestrator sign-off first. - Orchestrator-owned spec/contract files you were given by absolute path live OUTSIDE your branch (often uncommitted). If your PR needs them in-repo, sync the AUTHORITATIVE current content at final rebase (confirm the path with the orchestrator) — never commit the possibly-stale copy you read at kickoff: amendments accumulate while you work. ## 1a. State criteria and tests as invariants, never as enumerations - Every acceptance criterion and every test states the property that must hold, never the list of cases that must not happen. A list reads as complete and never is: the case nobody thought of is, by construction, absent from it. - The mechanical form: seed one row of every shape that already exists, run the change, and assert the rows are unchanged — never list which shapes are skipped. A test naming the skipped ones passes forever while the one shape it never named is silently rewritten; seeding is complete by construction, so a shape added later is covered without editing the test. For a migration, assert after upgrade **and** after downgrade. - Same rule for spec criteria: name the property and let the tests enumerate. An enumeration written into a spec becomes the definition of done, so whatever is missing from it falls out of scope by accident rather than by decision. ## 2. Pre-PR checks - Run the smallest relevant local validation: focused tests for what you touched, lint on changed files, the build gate (`npm run build` for UI work), and any repository-specific required checks. Self-review your own diff once (`git diff origin/...HEAD`) for scope strays and leftovers. - The GitHub Codex bot review (§4) is the review gate. ## 3. Opening the PR - Non-draft (drafts don't trigger review). Title `type(scope): summary`. - Pass the repository's default branch explicitly with `gh pr create --base `; derive it from GitHub rather than trusting a local `branch..gh-merge-base` setting. After creation, read the PR back and verify `baseRefName` is that default branch before treating the PR as open. - Body must include: the changed capability; affected scenario IDs when a catalog exists; evidence layers (unit / contract / scenario / residual manual checks); explicit dependencies ("requires #NNN merged first"); and a **Known-by-design ledger** (§4) when applicable. ## 3a. Turn-final text = a delivered status line When you run as an async lane (`vibe agent run`), the FINAL TEXT of every turn you end is delivered to your orchestrator's conversation as an at-least-once callback. It is a report surface, not scratch space: include a stable PR/head or Run identifier so duplicate callback delivery can be deduplicated, and end every turn with a short, meaningful status line — e.g. `PR #921 head 41c088c5: review triggered (👀 confirmed); watch armed` — never a step narration ("Push branch, confirm repo"), a thinking fragment, or a bare next-action note. If a turn ends because you armed a watch and are waiting, say exactly that. ## 4. Review-loop discipline - The Codex bot usually auto-reviews new pushes, but not reliably. After every push, confirm a review of the new head is in flight within a few minutes; if none appears, comment `@codex review`. An auto-review that finds something submits a review with inline threads, exactly like a triggered one — act on it. Only the passing case is asymmetric: it announces through the PR-body reaction alone, never the sha-bearing pass comment, which is produced by an explicit trigger and by nothing else. Triggering is therefore the only route to comment-shaped head-bound pass evidence. - A trigger only counts once the bot reacts 👀 (`eyes`) to that comment. The trigger is the comment ID your own `gh pr comment --body '@codex review'` call returned — identify it by that ID. Never use `issues//comments --jq '.[-1]'`, and never find one by matching `@codex review` in comment bodies: the bot quotes that phrase in the boilerplate appended to its own verdicts, so the pattern selects its output as readily as your input. Query `repos///issues/comments//reactions` within ~2 minutes and require `content == "eyes"` from the Codex bot (`chatgpt-codex-connector` in the API, often displayed as `chatgpt-codex-connector[bot]`); aggregate counts or other users' reactions do not prove pickup. **The bot withdraws the reaction when the review completes**, so 👀 is evidence only inside that window — afterwards every trigger reads 0, including the reviewed ones. Never infer "it never started" from a reaction count after the fact; look for a current-head verdict instead. - Liveness invariant: at every pause there is either a pending bot review of the current head, or one you just triggered. Never wait on nothing. - Use `background-watch-hook` to create one durable `--forever` combined PR watch for the whole delivery loop. Prefer the bundled `wait_pr.py` with each required `--workflow` and optional `--branch`, but omit `--sha`: every cycle resolves the PR's current head and observes Actions at that exact SHA. A push is a head-change event, not a reason to replace the Watch. Add `--sha` only for an intentionally fixed-head one-shot wait. Use `wait_pr.py` without CI arguments for PR-only monitoring, and reserve `wait_action.py` for Actions targets that are not attached to a PR. - Set the durable PR Watch's per-cycle `--timeout 0`. The Harness default is 21600 seconds, so leaving it implicit turns six quiet hours into a terminal timeout; an idle PR is expected state, not a failed cycle. - The CI waiter matches every distinct Actions run ID for each requested workflow name at the exact SHA and branch. A workflow name is not a unique run identity; do not declare CI complete while a second matching run is pending or failed. - The one-watch invariant is scoped by owner and concern: one live lane/fix watch per PR, plus one independent orchestrator gate watch when work is delegated. Review activity and the PR's exact-head CI are one lane concern and should use the combined `wait_pr.py` watch rather than two sibling waiters. Each genuinely independent concern needs its own state; never share cursor state between concurrent watches or count unrelated global monitors as the lane watch. - Follow `background-watch-hook` for waiter commands, state, baseline seeding, catch-up, filtering, settling, retries, and delivery acknowledgement. Those mechanics belong to the reusable skill; this policy only constrains when and why the watch is armed. - Before the first push or review trigger, use `background-watch-hook` to seed one complete owner-specific state file, then arm the forever Watch with that same file. This is the only routine baseline creation in the loop. Never reseed, rotate, or replace it between rounds: review or CI activity can land while the current follow-up runs, and making the already-arrived event a fresh baseline silently drops it. Catch-up is for deliberately replaying historical activity before the loop starts, and a replacement Watch is recovery from a real failure — neither is normal per-round operation. - Keep the Watch alive while pushing, replying, and resolving threads. Those own actions may produce one extra batched callback, but they cannot consume the Watch or leave the real next review unobserved. End every round by using the `background-watch-hook` management commands to verify exactly one live Watch for this owner, concern, repository, and PR. Do not rely on a remembered Watch ID or one bookkeeping field as proof that its waiter is live. - For whoever gates the PR: a lane run that ended `succeeded` proves nothing about the loop. A watch-triggered run can finish clean having pushed nothing and armed nothing, leaving the PR with new findings and no watcher on either side. When your gate watch fires on a findings review, verify the lane still has a live PR watch before concluding it has the round handled. - The bot has three verdict shapes. Findings arrive as a review with inline threads. A PASS is either (a) a plain issue comment by the Codex bot whose body says "Codex Review: Didn't find any major issues" and names `Reviewed commit: ` equal to the current head, or (b) a `+1` reaction from that bot on the PR body. Nothing else is one: not a contributor comment quoting the pass text, not a reaction by another author, not a reaction on any other comment. - That PR-body reaction is one state slot, not an append-only log: 👀 while a review runs, `+1` once one completes with no comments, each write withdrawing the last. So 👀 there means a review is running right now — a more durable liveness probe than the 👀 on your trigger comment. The `+1` names no sha, but its `created_at` is not stale either: it is when the most recent completed review passed, and the only open question is which head that review ran against. So do not push another head while a review is pending, and accept the reaction when the durable Watch reports it as new after the current head epoch began, the prior-head review was already terminal, and the PR head is unchanged. Never reseed to manufacture that boundary. When the timeline cannot settle it — an intervening head that was never reviewed, say — force the binding rather than infer it: comment `@codex review` on the unchanged head and take whatever verdict returns, a pass comment naming the sha or a findings review naming it. Waiting for that comment instead of triggering waits forever. The slot's 👀 → `+1` transition cannot stand in for it either: the mandated waiter queries PR-body reactions with `?content=+1`, so it never sees the 👀 half. - Do not treat `Reviewed commit:` alone as a pass signal. A findings verdict is a `COMMENTED` review whose body also opens `### 💡 Codex Review … **Reviewed commit:** `, so a gate that merely greps for the sha merges over live findings. Match the bot author, pass phrase, and exact head together for a comment-shaped pass; require the head-bound waiter evidence above for a reaction-shaped one. Neither is enough alone: the bot double-passes commits, so close-out also gates on zero unresolved threads across the entire PR, including threads opened on earlier or outdated heads — never on a quiet latest review. - A review attributed to the repo owner with an **empty body** is a phantom, not a review: replying to a review thread creates a `COMMENTED` review under your own account, stamped with the current head's `commit_id`. Any check that selects reviews by head sha will count it as "the bot has started". Filter it out by author and empty body. - Resolve every thread you address (reply, then resolve). For intentional non-changes the bot keeps re-flagging: keep a **Known-by-design ledger** in the PR body and answer re-flags by linking the entry. - If GitHub refuses your thread reply because another user (often the repo owner) has a **PENDING review** on the PR, never delete or dismiss that pending review to unblock yourself — it may hold unsubmitted draft comments, and deletion destroys them unverifiably. Stop, report the block to the orchestrator/owner, and continue the round without the reply (push, re-arm the watch, note the unreplied threads in your report); reply only after the owner submits or discards the draft themselves. - Same-theme findings = stop patching. The trigger is not a round count, it is whether you can **name the class**: the moment you can enumerate the members the reviewer has not reached yet ("it flagged the projected definition name, and the projection also carries session title, session label, and the callback session"), close the class now — one shared declaration plus a test that asserts the enumeration, so a member added later fails a test instead of costing a review round. Waiting for a third round is paying for a review you have already predicted. Can't name the class? A delegated lane escalates the inventory to its orchestrator; the orchestrator diagnoses whether the closure belongs in a local path, chokepoint, contract, or data model. - The circuit breaker is mechanical, not discretionary. Before editing any findings review, fetch every paginated thread and record the reviewed head, number of findings, and root-cause classes in the status delivered to the orchestrator. If a class appears on a second reviewed head, stop before the next edit or push. After an architecture or data-model rewrite, the third findings-bearing head also stops the lane even when no class has repeated. A delegated lane delivers the complete inventory and waits for its orchestrator. The orchestrator diagnoses the full inventory, records the scope decision, and continues independently when the smallest complete action is clear, reversible, and contract-preserving; it asks the owner only for a major trade-off, irreversible risk, or genuinely ambiguous direction. Local tests, CI, and resolved old threads cannot waive the diagnosis. - Searchable-list invariant, learned the expensive way on #1023 (three rounds): **every string a row disp