Skip to content

pr-pipeline: post-V1 audit and retro of the unattended merge flow #7531

Closed
1 of 1 issue completed
Closed

Description

@kyle-sexton

Ask

The owner's direction (2026-10-12): ship V1 of unattended merge first, then audit, analyze and run a retro on the whole flow end to end, adopting outside practices (Matt Pocock, poteto) only on data and metrics.

Scope, once pr-merge merges live

  • CI/CD performance, the autonomous Actions lanes, and the merge path; find the friction points.
  • From day one of live merges: revert and demotion rate per risk bucket, escaped-defect counts, and merged-and-not-reverted outcomes.
  • Candidates to evaluate on that data: sampled post-merge review, a verifier verdict bound to the head SHA, a mutation pass over changed tests.

Refs: #6202, #4588

Activity

  1. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 15, 2026
  2. github-actions commented on Oct 15, 2026

    @github-actions
    Contributor

    Proposed work class: work-class: read-only
    Basis: automated intake triage; a maintainer confirms or changes it.

    Size: Small
    Complexity: Low
    Uncertainty: Known
    Flags: none
    Blocked by: none

    Understood: a post-V1 audit and retro of the unattended merge flow. The owner's recorded decision settles the open question, so nothing is asked again. The follow-up is tracked in #13601.

    Decision recorded.
    Owner, 2026-11-05 (HITL Backlog session), verbatim: "go with whatever is in line with:" followed by the standing direction s1-s10.

    Question (Q1): what should agent-session merges through the owner login require, given the agent-path data above?
    Decided: (a), measurement only.

    • No new pre-merge gate: agent-session merges stay FAIL-only (docs/conventions/pr-pipeline/README.md:2178-2186). Basis: s10, and the weak causal link (0 of 13 reverts carry a pr-verify FAIL trace).
    • Record agent-authored merges through the owner login in the post-merge accounting under their own path, record only. Tracked in pr-pipeline: record agent-session merges through the owner login in post-merge accounting (record only) #13601, the one follow-up.
    • Rejected: (b) a PASS requirement, (c) routing agent merges through the lanes gate, (d) no change.
      [quote truncated; the full decision is at the link]

    #7531 (comment) is the measurement; the decision is #7531 (comment)

    Automated triage. A maintainer reviews it.

  3. added
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    and removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 15, 2026
  4. kyle-sexton commented on Oct 16, 2026

    @kyle-sexton
    MemberAuthor

    What the article adds for this audit, and the owner's 2026-10-16 decisions. Source: https://claude.dev/blog/solving-code-review-at-anthropic (2026-10-15); ids are digest unit and claim.

    The article: "We don't yet have circuit breakers that tighten approval when those numbers move; engineers here want them before more code moves to zero-human merges" (04 C70). Noticing a bad merge still depends on the author, on-call and the revert and incident dashboards (04 C69). Revert and incident rates are the guardrail (02 C8; 10 C21).

    Owner decision, 2026-10-16: a minimal revert-triggered stop. A revert of an agent-merged PR stops that lane's unattended merges until the owner re-enables them. It stops merges rather than lowering a rung because Phase 9 removes the rung ladder (Q505). It is not a parallel mechanism: a revert of an agent-merged PR feeds the existing post-merge demotion record, post-merge-detect-silent-revert.yml plus the babysit-merge-demotion issues that only a person closes, which already stop unattended merges on a canary or schedule failure. It is built with the #7528 fix and lands before unattended merge widens.

    Owner decision, 2026-10-16: this issue owns the revert and incident counts. #6080 and #6974 read them from here rather than keeping their own.

    Not adopted: a rate-based breaker. Q567 sends confirmed escaped defects to /corrections:codify, not demotion. That is recorded as a departure from 02 C8 and 04 C70; the SRE error-budget policy (halt changes when the budget is exceeded) sides with the article. This issue's per-bucket revert rate is the data that revisits it: on 2026-10-16 the owner kept Q567's unattended merge on a verifier verdict on condition of revisiting it on this data. The comparison to make: the revert and escaped-defect rate on verifier-approved merges, set against the planted-defect miss rate from Phase 9's B4 run.

    Against the originating sources.

    • pr-pipeline: inputs from the 2,000-PRs effort, and the local-lane work waiting on deployed lanes #6202 (the 2,000-PRs effort, from poteto's work) already plans rung lowering after a bad merge; the revert stop is its minimal form once the rungs are gone.
    • This issue's rule, adopt outside practices only on data, agrees with "start strict and loosen with evidence" (10 C14).
    • Order: the revert stop lands before unattended merge widens, which meets the article's engineers part way (04 C70); the rate breaker stays a departure.

    The article's own revert figures are internal and vendor-claimed (00 C10).

    Related: #6080, #6974, #6989, #7570, #7528, #7329, #7639, #7642.

  5. kyle-sexton commented on Oct 18, 2026

    @kyle-sexton
    MemberAuthor

    Gaps moved from #6989, which is closing as completed (owner decision D18, option a, 2026-10-18). They are lifecycle and post-merge items to weigh in this audit and retro. Source for all: the #6989 body, sections named per item.

    Already listed here, so not added: the revert-triggered stop and revert and incident counts (2026-10-16 owner decision above), and "start strict and loosen with evidence" (10 C14), which the risk-tier comment ties to this issue's data. Kill-switch coverage of the review and intake lanes is tracked in #7234.

  6. kyle-sexton commented on Oct 22, 2026

    @kyle-sexton
    MemberAuthor

    This was generated by AI during attend-queue.

    Folded in from #7612 (closed obsolete after #7780 removed the ratification marker): its items 2, 3, 5 and 6, the label-normalization restrictions on admission, belong to this audit.

  7. removed
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    on Oct 24, 2026
  8. kyle-sexton commented on Oct 24, 2026

    @kyle-sexton
    MemberAuthor

    Released for re-triage under the owner's 2026-10-24 rule: an issue stops for a person only for an intent or vision question, a security call, a host or attended run, or spend outside prepaid credit. The hold label came off and intake triage runs again now; it re-applies a hold, with a numbered question, when one of those applies.

    To hold this issue again, add needs-human (or status: needs-decision) and say why in a comment.

  9. added
    status: readyTriaged, unblocked, and fully specified; eligible to pick up.
    status: needs-decisionAwaiting a human or maintainer judgment call.
    and removed
    status: readyTriaged, unblocked, and fully specified; eligible to pick up.
    on Oct 24, 2026
  10. 27 remaining items

  11. added
    status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.
    on Nov 1, 2026
  12. kyle-sexton commented on Nov 2, 2026

    @kyle-sexton
    MemberAuthor

    Status. Two things: the static audit is now filed, and the data retro waits on two conditions. Author: the issue-triage session.

    The static friction audit and the 2026-10-18 gaps checklist are filed as #12284. That is the first criterion of the 2026-10-26 record, with the deliverable from the amendment. Both lane runs reported that they could neither find nor file it.

    The data retro this issue keeps starts when both of these hold:

    • 30 days have passed since the first live pr-merge merge.
    • pr-merge has made 30 live merges.

    Live merges began when report-only turned off on 2026-10-17. That was #7773, commit ad13c59, which set report-only: false in docs/conventions/pr-pipeline.yaml. The lane runs bounded the window by 2026-10-14, when the merge block entered the config as report-only, and that gave 2026-11-13. No live merge could happen before 2026-10-17, so the earliest start is 2026-11-16.

    The record lists the retro under "Checks a person runs", so this issue keeps host-run.

  13. kyle-sexton commented on Nov 5, 2026

    @kyle-sexton
    MemberAuthor

    Decision recorded.
    (agent; not an owner decision)

    Question: run the data retro now, before the "30 live merges or 30 days, whichever is later" trigger from 6412925022 reaches 30 days on 2026-11-16?
    Decided: yes. Run it on every unattended merge so far and state the window. Basis: that trigger was set with "Basis: judgment" and "the retro report states the window it used so the owner can move it"; the merge count is met 22 times over (662); the owner's 2026-11-05 direction to reach actual zero (hub plan, actual-zero-plan-2026-11-05.md) schedules this issue for 11-08.

    Retro: unattended merges, 2026-10-17T18:09Z to 2026-11-05T18:46Z (19 days)

    Window. Start: #7773 set merge.report-only: false; first unattended merge #7801. End: #13567. merge.lane-prs moved gate → native at 2026-10-24 (#9508).

    Definition. A merge the agent identity (melodic-automation-lanes[bot]) enqueued, by gate or after its native auto-merge arm (docs/conventions/pr-pipeline.yaml:1290-1314, docs/conventions/pr-pipeline/README.md:1774-1775). 22 lanes-armed PRs the owner account enqueued are excluded.

    Merge path Merges Reverts Single-PR demotions In range demotions Escaped defects
    Lanes, gate 91 0 4 0 0
    Lanes, native 528 0 3 11 1
    Dependabot 38 0 1 0 0
    Adopted owner PR 5 0 0 0 0
    All 662 0 (0%) 8 (1.2%) 11 (1.7%) 1 (0.15%)
    • Risk bucket is not recorded per merge (no risk:* label; the risk comment is written only when the score holds a PR, .github/scripts/pr-merge/enable-auto-merge.mjs:99-100), so the table splits by merge path. By construction every unattended merge scored LOW or mergeable MED (babysit_risk.py:2304-2335). A per-merge score needs the replay in #12984.
    • Reverts: 13 on main in the window, all of owner-account enqueues; none of an unattended merge. The revert-triggered stop has not fired on live data, by design (README.md:1222-1224).
    • Demotions: 21 records, all closed. Of the 8 single-PR ones: 3 pipeline misses (no dispatch within 6 h, the class #7528 fixed), the rest test-bash newly failing then green (real defect vs flake UNVERIFIED). None led to a revert.
    • Escaped defect: #10810 removed a declaration melodic-software/standards' lockstep check reads; fixed forward in #10964. A cross-repository contract break.

    CI/CD timings (minutes, all 662): opened→merged median 56, p90 612; merge-queue wait median 6.1, p90 10.4. Sample of 29 (every 23rd merge): ci-status job 0.4; ci-status green→merged median 26.9, p90 196. pr-merge.yml last 500 runs: 497 success.

    Friction points.

    1. Demotions held the lane off about 18% of the window (≈84 of 457 h; longest #11916, #12340, #12430), none for a revert.
    2. About 21 min from a green head to the merge outside the queue: the arm waits up to one scheduled interval (README.md:1957-1958); the per-PR wake under #12445 targets it (effect not separated here, UNVERIFIED).
    3. Queue churn: 53 of 528 native merges (10%) left the queue at least once (35 lanes dequeues, 33 failed_checks, 7 merge_conflict).

    Candidate verdicts (data only).

    1. Sampled post-merge review: not supported now (0 reverts, 1 escape that a diff-only review may not catch). Covered by feat(review): replay /review:merge-risk over past unattended merges before wiring demote #12984's replay.
    2. Verifier verdict bound to the head SHA: already built (README.md:1735-1737, :1960-1962). Residual named at babysit_postmerge.py:691-697 (push after arm) has no traced escape.
    3. Mutation pass over changed tests: not supported as a gate; the advisory lane exists (post-merge-test-mutation.yml, #9195); one escape is a data point, not a rate.

    Merge-gate revisit (Q567, Q602: "when #7531's revert and escaped-defect data come in"): the data does not argue for one: 0 reverts and 1 escaped defect in 662 verifier-approved merges.

    Limits. The owner account is used by people and agent sessions alike, so no human-vs-lane revert comparison is possible. Timeline reads capped at 25 events per PR (misclassification possible, UNVERIFIED, rare). Demotion counts before #7528 closed (2026-10-31) may be low. The window is the owner's to move: re-running this read over a longer window repeats the method above.

    The static friction audit and the #6989 gaps checklist moved to the child #12284 per decision 6412925022 item 1, which is closed.

  14. removed
    status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.
    host-runNeeds an unattended run on a host; the host runner takes it, not a person or the issue lane.
    on Nov 5, 2026
  15. kyle-sexton commented on Nov 5, 2026

    @kyle-sexton
    MemberAuthor

    Correction to the retro above; reopening.

    The retro counted only merges the lanes App enqueued. Every one of the 13 reverts in the window hit a PR authored by an agent session and enqueued through the owner account: each of #8254, #8791, #9350, #9410, #11210, #12114, #12121, #12159, #12162, #12186, #12201, #12387 and #13107 carries a <!-- session: … --> or Generated with [Claude Code] body marker and a Co-Authored-By: Claude commit trailer (gh pr view <n> --json body,commits, read 2026-11-05). Agent sessions arm and enqueue through the owner's gh login, so the "owner-account enqueue" group is largely agent merges, not human ones.

    So the retro's "0 reverts" and "the data does not argue for a merge-gate revisit" hold for the lanes App path only. Agent-session merges through the owner account had 13 reverts in the same 19 days. Their denominator (agent-authored PRs enqueued by the owner account) and their escaped-defect count are not measured yet.

    Revert causes on that path, from the table above: 7 were skill cuts that failed a negative control after merge (#12114, #12121, #12159, #12162, #12186, #12201, #12387 — an eval-measurement gap, the class #7296 round 19 now holds with do-not-merge until a before/after eval runs), 2 an owner decision reversed (#9350, #9410), 1 a security finding after merge (#8791), 1 the GITHUB_TOKEN pool exhausted (#11210), 1 a listing timeout (#8254), 1 its own 24 h gate (#13107).

    Next, before this closes again: measure the agent-session path (count of agent-authored PRs merged via owner-account enqueue in the window, reverts 13, escaped defects) and restate the merge-gate verdict for each path.

  16. kyle-sexton commented on Nov 5, 2026

    @kyle-sexton
    MemberAuthor

    Agent-session path measured (window as above, 2026-10-17T18:09Z to 2026-11-05T18:46Z).

    Path Merges Reverts Revert rate Escaped defects (strict) Escape rate
    Lanes App enqueue (verifier PASS required at the head) 662 0 0% 1 (1) 0.15%
    Agent session via the owner account 2,487 13 0.52% 22 (17) 0.88%
    — owner-authored agent PR 2,125 13 0.61% 21 (16) 0.99%
    — lanes App PR enqueued by the owner account (queue still requires a PASS) 360 0 0% 1 0.28%
    No agent marker (36 of 37 are bot PRs) 37 0 0% 0 0%

    Agent PR = body <!-- session: … --> or Generated with [Claude Code] marker plus a Co-Authored-By: Claude trailer. On merges up to 2026-10-29 (every PR had a week to be cited), owner-authored agent PRs escaped at 1.71% (18 of 1,051) against 0.24% (1 of 425) on the lanes path.

    What differs. The lanes path requires a pr-verify PASS at the merged head, a risk score, path/cost holds, and post-merge accounting (docs/conventions/pr-pipeline/README.md:1732-1737, :1917-1925, :1960-1962, :1774-1775). An agent session's gh pr merge --auto through the owner login goes through rulesets, required checks and the client-side guardrails merge guard only (plugins/source-control/reference/providers/github/pull-request/merge.md:121; plugins/guardrails/hooks/block-unresolved-pr-merge.sh:27-59). pr-verify did not judge owner PRs before #8999 (2026-10-22); until #13412 (2026-11-05T01:47Z) its verdict stayed out of ci-status for them; since then the queue holds them on a FAIL only (README.md:2178-2186, owner decision 2026-11-04). Neither the post-merge check nor the revert trigger counts these merges (README.md:1774-1775, :1222-1224), which is why the 13 reverts filed no revert record.

    Against a simple causal reading: none of the 13 reverts carries a pr-verify FAIL trace, and only 3 of the 35 reverted-or-escaped PRs do (#9817, #10252, #12431); 7 of the 13 reverts are skill cuts that failed a negative control, which a diff-only verdict cannot catch (README.md:1763); 51% of owner-authored agent PRs change pipeline or gate code against 18% on the lanes path, a confound.

    Per-path verdicts. Lanes path: unchanged, no candidate supported, no revisit. Agent-session path: sampled post-merge review partly supported on the rate (1.41% reverted or escaped); no open issue covers this path (#12984's replay names lane merges only); mutation pass not supported (6 of 17 strict escapes were workflow/CI breakages it would not run). The data argues for the Q567/Q602 revisit on this path; that rule is the owner's decision, so this issue stays open for it.

  17. kyle-sexton commented on Nov 5, 2026

    @kyle-sexton
    MemberAuthor

    Decision recorded.
    Owner, 2026-11-05 (HITL Backlog session), verbatim: "go with whatever is in line with:" followed by the standing direction s1-s10.

    Question (Q1): what should agent-session merges through the owner login require, given the agent-path data above?
    Decided: (a), measurement only.

    • No new pre-merge gate: agent-session merges stay FAIL-only (docs/conventions/pr-pipeline/README.md:2178-2186). Basis: s10 ("Holds that guard against our own actors … come off now"; "Hardening … is deferred to after that, as configuration edits"), and the weak causal link (0 of 13 reverts carry a pr-verify FAIL trace; 7 of 13 are eval-class skill cuts a diff verdict cannot catch, now held by Run the eval suites deferred by the upstream-pointer sweep PRs #7296's before/after-eval rule).
    • Record agent-authored merges through the owner login in the post-merge accounting under their own path, record only (never demote, trip the breaker or hold), with the path's record/act mode in configuration. Basis: s9 (everything measurable, baselined, comparable), s6 (a later change of direction is a configuration edit). Tracked in pr-pipeline: record agent-session merges through the owner login in post-merge accounting (record only) #13601, the one follow-up.
    • Rejected: (b) a PASS requirement and (c) routing agent merges through the lanes gate, both new holds on our own actors (s10); (d) no change, which leaves 2,487 merges unmeasured (s9).

    Retro verdict per path (this issue's done-when): lanes App path: 0 reverts, 0.15% escapes, no candidate supported, no revisit. Agent-session path: 0.52% reverts, 0.88% escapes; the Q567/Q602 revisit is answered by this decision; sampled post-merge review is partly supported on the rate and is re-judged on #13601's records; mutation pass not supported. Closing as completed.

  18. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Nov 5, 2026
  19. removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority: mediumReal value, no hard deadline; normal backlog flow.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

    Type

    Fields

    Work status

    confirmed

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions