diff --git a/.changes/pr-pipeline/fix-route-repeated-fail-to-lanes-4b9e17a2.md b/.changes/pr-pipeline/fix-route-repeated-fail-to-lanes-4b9e17a2.md new file mode 100644 index 0000000000..a77cfb08cf --- /dev/null +++ b/.changes/pr-pipeline/fix-route-repeated-fail-to-lanes-4b9e17a2.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Changed + +- `clear-escalations.mjs` `LANE_CAUSES.verdict` adds `verify:repeated-fail` and `verify:too-old`, so under `merge.lane-prs: native` a `needs-human` label `enable-auto-merge` placed for either comes off at its head (`refusal-cleared`) once the reader names only lane causes there. `lane-causes-person-codes.test.mjs` also pins `LANE_CAUSES.verdict` against `babysit_verify_evidence.NEEDS_HUMAN`. diff --git a/.changes/source-control/fix-route-repeated-fail-to-lanes-4b9e17a2.md b/.changes/source-control/fix-route-repeated-fail-to-lanes-4b9e17a2.md new file mode 100644 index 0000000000..1c44ccceb7 --- /dev/null +++ b/.changes/source-control/fix-route-repeated-fail-to-lanes-4b9e17a2.md @@ -0,0 +1,11 @@ +--- +bump: patch +--- + +### Changed + +- `babysit_verify_evidence.NEEDS_HUMAN` is `verify:untrusted-spec` alone (owner rule 2026-10-27: a person only for an intent or design change or a CRITICAL security issue, trust holds kept). `verify:repeated-fail` and `verify:too-old` still refuse the merge for good, now with no `needs-human` label: the pr-merge gate writes no escalation for them, and `enable-auto-merge.mjs` leaves the pull request unarmed with the lane comment and, once it stands, a failing `pr-merge/unarmed` status. + +### Removed + +- The repeated-fail escalation's quote of the head FAIL's summary, now unreachable: `babysit_merge._fail_summary`, `_carried_quote` and `VERIFY_SUMMARY_MAX`, `Reader.fail_summary`, `SummaryUnreadable`, and the report's `headFail` field. diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index 9e342c25cf..99f7e700cb 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -144,21 +144,23 @@ const REFUSAL_CLEARED = "refusal-cleared"; // this script lifts at the same head, and the only ones a lift tolerates // refusing now. Every other code, an unknown one included, is a person's. The // list is explicit, so it fails closed; lane-causes-person-codes.test.mjs pins -// it against babysit_risk.PERSON_CODES and PERSON_PREFIXES (#10249). +// it against babysit_risk.PERSON_CODES and PERSON_PREFIXES (#10249) and +// babysit_verify_evidence.NEEDS_HUMAN. // - risk (an entry ending in ":" is a prefix): babysit_risk.py's holding MED // codes files-incomplete (:938) and evidence-unbound: (:978-998), and // the manifest review (:598). Not the reader's `unreadable: …` score. // files-incomplete and the manifest review join their items with "; ", so // a reason with two or more items reads as unknown codes and stays. -// - verdict: babysit_verify_evidence.py's FAIL, EARLIER_FAIL and SPEC_CHANGED, -// which a push clears. Not untrusted-spec or repeated-fail (a person's), -// too-old (it recurs at the same head; #10249 makes it a person's), missing -// or unreadable (no answer), or ineligible (always paired with a link -// answer, which verifyHold reads first). +// - verdict: every babysit_verify_evidence.py refusal off NEEDS_HUMAN that +// answers: FAIL, EARLIER_FAIL and SPEC_CHANGED, which a push or a new +// judgment clears, and REPEATED_FAIL and TOO_OLD, which never clear but hold +// for the lanes with no label (owner rule 2026-10-27). Not untrusted-spec (a +// person's), missing or unreadable (no answer), or ineligible (always +// paired with a link answer, which verifyHold reads first). // - link: none; an issue link is a person's. export const LANE_CAUSES = Object.freeze({ link: Object.freeze([]), - verdict: Object.freeze(["verify:fail", "verify:earlier-fail", "verify:spec-changed"]), + verdict: Object.freeze(["verify:fail", "verify:earlier-fail", "verify:spec-changed", "verify:repeated-fail", "verify:too-old"]), risk: Object.freeze(["files-incomplete", "evidence-unbound:", "manifest-review-required"]), }); diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index 542a3c9a83..09452e50d1 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -517,6 +517,7 @@ test("a hold marker from github-actions[bot] with no lane footer, or from anothe const UNBOUND = "pr-merge's gate holds it on its risk score (evidence-unbound:claude-review: not bound: rerun-not-allowed)"; const SECURITY = "pr-merge's gate holds it on its risk score (security-finding: 1 open CRITICAL finding)"; const REPEATED = "pr-merge's gate refuses its verifier verdict (verify:repeated-fail)"; +const TOO_OLD = "pr-merge's gate refuses its verifier verdict (verify:too-old)"; const UNTRUSTED = "pr-merge's lane does not take its issue link (untrusted-provenance)"; const UNTRUSTED_SPEC = "pr-merge's gate refuses its verifier verdict (verify:untrusted-spec)"; const COST = "pr-merge's gate holds it on its risk score (cost-review-required: infra/main.tf)"; @@ -553,6 +554,11 @@ test("enable-auto-merge's label comes off when the reader now reads a refusal th [UNBOUND, (pull) => ({ ...clean(pull), reviews: "evidence-unbound:claude-review: not bound: rerun-allowed" })], // A lane verdict now passes, and the risk score it hid names only lane causes. [SPEC_CHANGED, reviewsNow("evidence-unbound:claude-review: not bound: rerun-allowed")], + // Owner rule 2026-10-27: the repeated-FAIL cap and an old pull request + // hold for the lanes, so their labels come off at the head where the + // reader still refuses them (#10134's shape). + [REPEATED, refusing("verify:repeated-fail")], + [TOO_OLD, refusing("verify:too-old")], ]) { const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence }); await run(w, NATIVE); @@ -570,13 +576,10 @@ const PERSON_CLASS = [ risk("high-path:**: a symlink"), risk("high-path:.github/workflows/**: .github/workflows/ci.yml"), UNTRUSTED_SPEC, - REPEATED, UNTRUSTED, // A link reason, the reader's own unread link included. "pr-merge's lane does not take its issue link (unreadable: timed out)", - // Off the lane list: a refusal that recurs at the same head, or one the - // reader always pairs with a link answer. - "pr-merge's gate refuses its verifier verdict (verify:too-old)", + // Off the lane list: one the reader always pairs with a link answer. "pr-merge's gate refuses its verifier verdict (verify:ineligible)", // A code not known to be a lane's, matched whole, or a person's beside a lane's. risk("a-new-code: x"), diff --git a/.github/scripts/pr-merge/enable-auto-merge.mjs b/.github/scripts/pr-merge/enable-auto-merge.mjs index a55fd53ae2..f090d6fa59 100755 --- a/.github/scripts/pr-merge/enable-auto-merge.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.mjs @@ -98,11 +98,11 @@ // person's, and left alone while the reason and head are unchanged // (refusalWrites). The // comment is the record; only a cause a person must clear (owner rule -// 2026-10-26: an intent or design change or a CRITICAL security issue, with -// cost and trust holds and a hold no lane can clear kept: untrusted -// provenance or an untrusted spec, the repeated-FAIL cap, a pull request too -// old to verify, a security finding, a high path, a trusted-actor widening, -// a real charge or a listing too long to check, see verifyClears; a hold path, a configured denied path +// 2026-10-26 and 2026-10-27: an intent or design change or a CRITICAL +// security issue, with cost and trust holds kept: untrusted +// provenance or an untrusted spec, a security finding, a high path, a +// trusted-actor widening, a real charge or a listing too long to check, see +// verifyClears; a hold path, a configured denied path // or the floor, which only a trusted approval lifts; and the lane's own hold // line, owner decision Q-HOLDLINE) also gets the human-gated label, put back // while the comment stands unless a person removed it since. Every other @@ -508,9 +508,9 @@ export function verifyHold(evidence, head) { // read whole, more than the 100 the read lists, which no later read changes. export const TRUST_REFUSALS = Object.freeze(["untrusted-provenance", "untrusted-provenance-removed-by-non-human", "issue-labels-unreadable"]); -// Who clears the refusal verifyHold reads in `evidence` at `head` (owner rule -// 2026-10-26: a person only for an intent or design change, or a CRITICAL -// security issue; cost and trust holds and a hold no lane can clear stay a +// Who clears the refusal verifyHold reads in `evidence` at `head` (owner +// rules 2026-10-26 and 2026-10-27: a person only for an intent or design +// change, or a CRITICAL security issue; cost and trust holds stay a // person's): `person` // for an issue link the trust hold refuses (TRUST_REFUSALS), a verifier // refusal the gate marks `needsHuman` (babysit_verify_evidence NEEDS_HUMAN) diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index 6db87849bc..331ab5e086 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -370,18 +370,17 @@ test("a floor path no configured glob names is commented on and labeled human-ga ); }); -// Owner rule 2026-10-26: a person only for intent, design, a CRITICAL -// security issue or a hold no lane can clear. The gate's `needsHuman` -// (babysit_verify_evidence NEEDS_HUMAN: an untrusted spec, the terminal -// repeated-FAIL cap, a pull request too old to verify) decides, so a changed -// spec is a lane's. -test("only a verifier refusal the gate marks needsHuman is a person's; a changed spec is a lane's", async () => { +// Owner rules 2026-10-26 and 2026-10-27: a person only for intent, design or +// a CRITICAL security issue, trust holds kept. The gate's `needsHuman` +// (babysit_verify_evidence NEEDS_HUMAN: an untrusted spec) decides, so a +// changed spec, the repeated-FAIL cap and an old pull request are the lanes'. +test("only a verifier refusal the gate marks needsHuman is a person's; the cap and an old pull request are the lanes'", async () => { const refused = (refusal, needsHuman) => ({ refusal, needsHuman, head: HEAD, ineligible: null, reviews: null }); const { notes } = await run([ pull(1, { verdict: refused("verify:untrusted-spec", true) }), pull(2, { verdict: refused("verify:spec-changed", false) }), - pull(3, { verdict: refused("verify:repeated-fail", true) }), - pull(4, { verdict: refused("verify:too-old", true) }), + pull(3, { verdict: refused("verify:repeated-fail", false) }), + pull(4, { verdict: refused("verify:too-old", false) }), pull(5, { verdict: { ...MISSING, refusal: "verify:ineligible", ineligible: "it closes 2 issues" } }), ]); const refusing = (code) => `pr-merge's gate refuses its verifier verdict (${code})`; @@ -389,10 +388,8 @@ test("only a verifier refusal the gate marks needsHuman is a person's; a changed { method: "POST", path: `${ISSUES}/1/comments`, body: { body: personNote(refusing("verify:untrusted-spec")) } }, { method: "POST", path: `${ISSUES}/1/labels`, body: { labels: ["needs-human"] } }, { method: "POST", path: `${ISSUES}/2/comments`, body: { body: laneNote(refusing("verify:spec-changed")) } }, - { method: "POST", path: `${ISSUES}/3/comments`, body: { body: personNote(refusing("verify:repeated-fail")) } }, - { method: "POST", path: `${ISSUES}/3/labels`, body: { labels: ["needs-human"] } }, - { method: "POST", path: `${ISSUES}/4/comments`, body: { body: personNote(refusing("verify:too-old")) } }, - { method: "POST", path: `${ISSUES}/4/labels`, body: { labels: ["needs-human"] } }, + { method: "POST", path: `${ISSUES}/3/comments`, body: { body: laneNote(refusing("verify:repeated-fail")) } }, + { method: "POST", path: `${ISSUES}/4/comments`, body: { body: laneNote(refusing("verify:too-old")) } }, { method: "POST", path: `${ISSUES}/5/comments`, body: { body: laneNote("pr-merge's lane does not take its issue link (it closes 2 issues)") } }, ]); }); @@ -1502,7 +1499,7 @@ test("no pass at the head, a FAIL or the repeated-FAIL cap arms nothing", async const { code, writes, verified } = await run([ pull(1, { verdict: { refusal: "verify:missing", head: HEAD } }), pull(2, { verdict: { refusal: "verify:fail", head: HEAD } }), - pull(3, { verdict: { refusal: "verify:repeated-fail", head: HEAD, needsHuman: true } }), + pull(3, { verdict: { refusal: "verify:repeated-fail", head: HEAD, needsHuman: false } }), pull(4, { verdict: { refusal: "verify:earlier-fail", head: HEAD } }), pull(5, { verdict: { refusal: null, head: "c".repeat(40) } }), pull(6, { verdict: new Error("gh: rate limited") }), @@ -2515,11 +2512,11 @@ test("a kept refusal on an armed pull request disarms it", async () => { }); test("a kept refusal writes no label back; a changed reason is still written", async () => { - const reason = "pr-merge's gate refuses its verifier verdict (verify:repeated-fail)"; - const repeated = { refusal: "verify:repeated-fail", needsHuman: true, head: HEAD, ineligible: null }; + const reason = "pr-merge's gate refuses its verifier verdict (verify:untrusted-spec)"; + const untrusted = { refusal: "verify:untrusted-spec", needsHuman: true, head: HEAD, ineligible: null }; const standing = (verdict) => pull(2, { verdict, discussion: { comments: [lanesSaid(personNote(reason))] } }); - assert.deepEqual((await run([standing(repeated)])).notes, [{ method: "POST", path: `${ISSUES}/2/labels`, body: { labels: ["needs-human"] } }]); - const kept = await run([standing({ ...repeated, cached: true })]); + assert.deepEqual((await run([standing(untrusted)])).notes, [{ method: "POST", path: `${ISSUES}/2/labels`, body: { labels: ["needs-human"] } }]); + const kept = await run([standing({ ...untrusted, cached: true })]); assert.deepEqual(kept.notes, []); assert.equal(kept.calls.filter(({ body }) => body?.query === UNLABELED_QUERY).length, 0); const moved = await run([pull(1, { verdict: CACHED_MISSING, discussion: { comments: [lanesSaid(laneNote("an earlier reason"))] } })]); diff --git a/.github/scripts/pr-merge/lane-causes-person-codes.test.mjs b/.github/scripts/pr-merge/lane-causes-person-codes.test.mjs index e598f54768..1652650164 100644 --- a/.github/scripts/pr-merge/lane-causes-person-codes.test.mjs +++ b/.github/scripts/pr-merge/lane-causes-person-codes.test.mjs @@ -1,7 +1,7 @@ // Expected values come from the invariant in clear-escalations.mjs's LANE_CAUSES // comment: the causes the lanes lift are never ones only a person clears. The -// person set is read from babysit_risk.py (PERSON_CODES, PERSON_PREFIXES), the -// one list the merge gate reads. +// person sets are read from babysit_risk.py (PERSON_CODES, PERSON_PREFIXES) and +// babysit_verify_evidence.py (NEEDS_HUMAN), the lists the merge gate reads. import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; import { test } from "node:test"; @@ -10,7 +10,9 @@ import { fileURLToPath } from "node:url"; import { LANE_CAUSES } from "./clear-escalations.mjs"; const scripts = fileURLToPath(new URL("../../../plugins/source-control/skills/babysit-prs/scripts", import.meta.url)); -const read = "import json,sys\nsys.path.insert(0, sys.argv[1])\nimport babysit_risk as b\nprint(json.dumps({'codes': sorted(b.PERSON_CODES), 'prefixes': list(b.PERSON_PREFIXES)}))"; +const read = + "import json,sys\nsys.path.insert(0, sys.argv[1])\nimport babysit_risk as b\nimport babysit_verify_evidence as v\n" + + "print(json.dumps({'codes': sorted(b.PERSON_CODES), 'prefixes': list(b.PERSON_PREFIXES), 'verdicts': list(v.NEEDS_HUMAN)}))"; function personSets() { const python = spawnSync("python3", ["-I", "-c", read, scripts], { encoding: "utf8" }); @@ -39,3 +41,19 @@ test("no LANE_CAUSES.risk entry is a reason only a person clears", () => { } } }); + +test("no LANE_CAUSES.verdict entry is a verifier refusal only a person clears", () => { + const { verdicts } = personSets(); + // Owner rule 2026-10-27: an untrusted spec is a trust boundary, a person's. + assert.ok(verdicts.includes("verify:untrusted-spec"), "NEEDS_HUMAN lost verify:untrusted-spec"); + const overlapping = LANE_CAUSES.verdict.filter((entry) => verdicts.includes(entry)); + assert.deepEqual(overlapping, [], `lane verdicts ${overlapping} are in NEEDS_HUMAN`); +}); + +test("the repeated-FAIL cap and an old pull request are lane causes the clear lifts", () => { + // Owner rule 2026-10-27: neither is intent, design or security, so a label + // either one placed comes off at its head. + for (const refusal of ["verify:repeated-fail", "verify:too-old"]) { + assert.ok(LANE_CAUSES.verdict.includes(refusal), `${refusal} is not a lane cause`); + } +}); diff --git a/docs/conventions/pr-pipeline.yaml b/docs/conventions/pr-pipeline.yaml index 5c53feecf6..b3ed59d947 100644 --- a/docs/conventions/pr-pipeline.yaml +++ b/docs/conventions/pr-pipeline.yaml @@ -955,9 +955,10 @@ merge: skip-unchanged: max-minutes: 60 max-skips: 3 - # pr-verify FAILs at this many distinct heads of one PR hold it for a human - # whatever a later pass says, so pushing until the verifier passes once - # does not merge it (owner decision 2026-10-21). + # pr-verify FAILs at this many distinct heads of one PR hold it, never to + # merge unattended, whatever a later pass says, so pushing until the + # verifier passes once does not merge it (owner decision 2026-10-21). The + # hold is the lanes', with no label (owner rule 2026-10-27). max-verify-fails: 3 # Days a suite failing on main outside the merges' affected set stays # quarantined: its quarantine issue excuses its failures from demotion diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index b409e0b706..95b0d667fb 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -553,7 +553,7 @@ head: | `unresolved-conflict`, `protected-path-conflict` | GitHub reports the PR mergeable | | `feedback-protected-path` | the PR changes nothing under `.github/workflows/` | | `head-moved` (pr-merge's `babysit-merge-verify-escalation`, `babysit-merge-risk` and `babysit-merge-shared-copy-escalation`, with no pr-unblock escalation since the label, and the marked App comment written last before the label is pr-merge's and within a minute of it) | the newest App comment carrying each marker names, as ``at head `` ``, a 40-hex head other than the current one; pr-merge then judges the new head and escalates again if its refusal still holds | -| `refusal-cleared` (under `merge.lane-prs: native`, `enable-auto-merge`'s own unedited person refusal comment on a lanes App pull request, naming the current head, whose recorded verifier or risk cause is one the lanes clear: every code on `LANE_CAUSES`) | the gate's reader at that head finds nothing refusing, or only causes the lanes clear, read at least as far as the recorded one. A recorded cause a person clears (an issue link, a security finding, a trust-gate widening, a cost hold, a high path, an untrusted spec, the repeated-fail cap, any code off that list) is never read or lifted at its head (owner decision Q-CLEAR B): a person removes the label, or the `head-moved` clear does once the head moves. At most three reader runs a run | +| `refusal-cleared` (under `merge.lane-prs: native`, `enable-auto-merge`'s own unedited person refusal comment on a lanes App pull request, naming the current head, whose recorded verifier or risk cause is one the lanes clear: every code on `LANE_CAUSES`) | the gate's reader at that head finds nothing refusing, or only causes the lanes clear, read at least as far as the recorded one. A recorded cause a person clears (an issue link, a security finding, a trust-gate widening, a cost hold, a high path, an untrusted spec, any code off that list) is never read or lifted at its head (owner decision Q-CLEAR B): a person removes the label, or the `head-moved` clear does once the head moves. At most three reader runs a run | It removes only a label whose newest `labeled` event has the lanes App as its actor, and keeps it when a person commented, reviewed or left a review comment since that event (a bot account, such @@ -1242,12 +1242,11 @@ any of those, and does not turn it back on once a person has turned it off. A sc hand-started run that leaves a lanes App pull request unarmed on report-outcome's own `Hold: a human merges this` line, a hold path, a cost hold, the gate's verdict or its risk score says why in one `` comment, edited in place, and adds the human-gated -label only for a cause a person must clear (owner rule 2026-10-26: an intent or design change or a -CRITICAL security issue, with cost and trust holds and a hold no lane can clear kept): that hold +label only for a cause a person must clear (owner rules 2026-10-26 and 2026-10-27: an intent or +design change or a CRITICAL security issue, with cost and trust holds kept): that hold line (owner decision Q-HOLDLINE), a configured denied path or a [path floor](#merge-authority) path, -a cost hold, untrusted provenance, an issue whose labels cannot be read whole, an untrusted spec, the -repeated-FAIL round cap, a pull request too old to verify, a security finding, a high path, a -trusted-actor widening or a listing past 300 files. It puts the label back while the comment stands +a cost hold, untrusted provenance, an issue whose labels cannot be read whole, an untrusted spec, a +security finding, a high path, a trusted-actor widening or a listing past 300 files. It puts the label back while the comment stands unless a person removed it since; a person's own hold text gets neither. Every other refusal, a listing read that failed included, is the lanes': later scheduled runs judge it again, and once a lanes' refusal has stood at the head for `merge.risk-escalation-minutes`, timed from the first one @@ -1826,16 +1825,15 @@ exists (#7711). `verify:fail` and `verify:earlier-fail` only wait: the fail's notes go to an `address-feedback` round (`pr-feedback-dispatch.yml`, above), whose push the verifier judges again. -`verify:too-old`, `verify:repeated-fail` and `verify:untrusted-spec` need a human; -`verify:spec-changed` holds for the lanes (owner rule 2026-10-26). While `merge.report-only` is -false the lane escalates such a -refusal on the pull request ([loop-lane §2](../loop-lane/README.md#2-escalation-contract)): it -keeps one `` comment naming the refusal and the head, and -for `verify:repeated-fail` with a `fail` at the head quoting as data the first 1500 characters of -the `summary` in the newest head `fail` verdict when its record can be read and names this pull -request. The comment is rewritten only when any of these changes. A `verify:repeated-fail` with no -summary to quote keeps the summary the lane's last such comment quoted, under the head it was given -at, so a failed read rewrites nothing and the reason outlives a push. Then the lane adds the tracker's human-gated role label (`needs-human` unless +Only `verify:untrusted-spec` needs a human, since the spec's provenance is a trust boundary. +`verify:spec-changed`, `verify:repeated-fail` and `verify:too-old` hold for the lanes (owner rules +2026-10-26 and 2026-10-27): each still refuses the merge, with no label. The last two never clear, +so such a pull request never merges unattended, and address-feedback's own round cap bounds the +fix rounds on it. While `merge.report-only` is +false the lane escalates `verify:untrusted-spec` +on the pull request ([loop-lane §2](../loop-lane/README.md#2-escalation-contract)): it +keeps one `` comment naming the refusal and the head, +rewritten only when either changes. Then the lane adds the tracker's human-gated role label (`needs-human` unless `.work-item-tracker.json` maps it) when no label matches it ignoring case. The comment goes first, so a label never stands unexplained, and a failed label is added by the next run unless a person removed it after the comment last changed (see the reasons comment above); a rerun that diff --git a/docs/conventions/pr-pipeline/pr-pipeline.schema.json b/docs/conventions/pr-pipeline/pr-pipeline.schema.json index bfa7f50ddb..376118befa 100644 --- a/docs/conventions/pr-pipeline/pr-pipeline.schema.json +++ b/docs/conventions/pr-pipeline/pr-pipeline.schema.json @@ -1001,7 +1001,7 @@ "type": "integer", "minimum": 1, "default": 3, - "description": "Counted pr-verify fail verdicts at this many distinct heads of one pull request refuse it `verify:repeated-fail`, for a human, whatever a later pass says (README.md, Verifier verdict). Any other value is 1, so any earlier fail holds." + "description": "Counted pr-verify fail verdicts at this many distinct heads of one pull request refuse it `verify:repeated-fail`, held for the lanes with no label, whatever a later pass says (README.md, Verifier verdict). Any other value is 1, so any earlier fail holds." }, "verify-cache": { "type": "string", diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py index b1ef939801..55353b2d4b 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py @@ -194,7 +194,6 @@ gh_capture, gh_http_status, gh_json, - low_headroom, normalized_rest_author, parse_repo_number, pull_request_commits_once, @@ -2957,9 +2956,6 @@ def queued_entry_hold( "the pr-merge lane added the human-gated label." ) VERIFY_ESCALATION_MARKER = "" -# The most of a verdict's summary a verify:repeated-fail escalation quotes -# (judgment). -VERIFY_SUMMARY_MAX = 1500 QUEUE_EJECTION_MARKER = "" MAJOR_ESCALATION_MARKER = "" # Cleared by clear-escalations.mjs once the head moves, like the verify one. @@ -3544,63 +3540,17 @@ def upsert_marker_comment( return outcome -def verify_escalation_body( - refusal: str, - head: str, - summary: str | None = None, - carried: tuple[str, str] | None = None, -) -> str: +def verify_escalation_body(refusal: str, head: str) -> str: """The needs-human comment for a verifier refusal: a function of the - refusal, the head and the quoted fail summary only, so a rerun renders the - same body. The summary is model text, quoted as data in a text fence and - cut to `VERIFY_SUMMARY_MAX` characters. `carried`, the (head, text) an - earlier comment quoted (`_carried_quote`), is quoted as it was when there - is no `summary`.""" - lines = [ - VERIFY_ESCALATION_MARKER, - f"Unattended merge: the verifier verdict refuses `{refusal}` at head " - f"`{head}`. That refusal needs a human, who merges in person; the " - "pr-merge lane will not merge this pull request.", - ] - quote = carried - if summary: - text = _CONTROL.sub("?", summary.replace("\r\n", "\n")) - if len(text) > VERIFY_SUMMARY_MAX: - text = text[:VERIFY_SUMMARY_MAX] + " [cut]" - # A lone surrogate from a JSON escape cannot be encoded for `gh`. - text = text.encode("utf-8", "replace").decode("utf-8") - quote = (head, text.replace("```", "'''")) - if quote: - lines += [ - "", - f"The verifier's summary of its fail verdict at `{quote[0]}`, as it " - "wrote it:", - "", - "```text", - quote[1], - "```", + refusal and the head only, so a rerun renders the same body.""" + return "\n".join( + [ + VERIFY_ESCALATION_MARKER, + f"Unattended merge: the verifier verdict refuses `{refusal}` at head " + f"`{head}`. That refusal needs a human, who merges in person; the " + "pr-merge lane will not merge this pull request.", ] - return "\n".join(lines) - - -_QUOTE_RE = re.compile( - r"\nThe verifier's summary of its fail verdict at `([0-9a-f]{40})`, as it " - r"wrote it:\n\n```text\n(.*)\n```\Z", - re.S, -) - - -def _carried_quote( - comments: list[dict[str, Any]], identity: str -) -> tuple[str, str] | None: - """The (head, text) the identity's newest verify escalation quotes, read - back verbatim so a rerun that cannot read the summary renders the same - body, and an earlier-fail keeps the reason of the fail before it.""" - newest = _newest_marker_comment(comments, identity, VERIFY_ESCALATION_MARKER) - if newest is None: - return None - match = _QUOTE_RE.search(str(newest.get("body") or "")) - return (match[1], match[2]) if match else None + ) def major_escalation_body(majors: str, head: str) -> str: @@ -3653,51 +3603,13 @@ def escalate_verify( identity: str, refusal: str, inputs: Mapping[str, Any], - summary: str | None = None, ) -> dict[str, Any]: - """`escalate` for a needs-human verifier refusal. A - `verify:repeated-fail` with no `summary` (unreadable, or no fail at the - head) quotes the last fail summary its comment carried, so an unreadable - artifact rewrites nothing and the reason outlives the head.""" + """`escalate` for a needs-human verifier refusal.""" head = str(json_object(json_object(inputs.get("pr")).get("head")).get("sha")) - carried = None - if summary is None and refusal == verify_evidence.REPEATED_FAIL: - comments = inputs.get("comments") - if comments is None: - comments = fetch_paginated_api( - f"repos/{repo}/issues/{number}/comments?per_page=100", - f"#{number} comments", - ) - inputs = {**inputs, "comments": comments} - carried = _carried_quote(comments, identity) - body = verify_escalation_body(refusal, head, summary, carried) + body = verify_escalation_body(refusal, head) return escalate(repo, number, identity, body, VERIFY_ESCALATION_MARKER, inputs) -def _fail_summary( - verify: Mapping[str, Any], inputs: Mapping[str, Any], number: int -) -> str | None: - """The summary of the head FAIL a `verify:repeated-fail` refusal names, - read through the gate's verdict reader; None for any other refusal, with - no FAIL at the head, when the record cannot be read, or below the read - floor: the quote is report-only, and the escalation stands without it.""" - fail = json_object(verify.get("headFail")) - reader = inputs.get("verify_reader") - run_id, name = fail.get("run"), fail.get("artifact") - if ( - low_headroom() - or verify.get("refusal") != verify_evidence.REPEATED_FAIL - or not isinstance(reader, verify_evidence.Reader) - or not isinstance(run_id, int) - or not isinstance(name, str) - ): - return None - try: - return reader.fail_summary(run_id, name, number, str(verify.get("head"))) - except verify_evidence.SummaryUnreadable: - return None - - def escalate( repo: str, number: int, @@ -4132,7 +4044,6 @@ def judge(n: int) -> str | None: identity, str(verify.get("refusal")), inputs, - _fail_summary(verify, inputs, number), ) except (RuntimeError, ValueError, json.JSONDecodeError) as exc: report["errors"].append(f"escalation: {exc}") diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py index 5f7cfc15ec..4f61477e93 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py @@ -585,8 +585,8 @@ class PipelineConfig: # `RISK_ESCALATION_MINUTES`. risk_escalation_minutes: int = RISK_ESCALATION_MINUTES # `merge.max-verify-fails`: counted pr-verify FAILs at distinct heads of - # one pull request at which it holds for a human whatever a later pass - # says (`babysit_verify_evidence`). Absent is `MAX_VERIFY_FAILS`; anything + # one pull request at which it holds, never to merge unattended, whatever + # a later pass says (`babysit_verify_evidence`). Absent is `MAX_VERIFY_FAILS`; anything # but a whole number of at least 1 is 1, so any earlier FAIL holds. max_verify_fails: int = MAX_VERIFY_FAILS # `merge.check-inputs`: git pathspecs of what the pull request checks diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index 5d14441583..bca7d13e4d 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py @@ -76,12 +76,15 @@ for a new head; - otherwise `verify:missing`. -`verify:too-old`, `verify:repeated-fail` and `verify:untrusted-spec` need a -human (`NEEDS_HUMAN`), who merges in person; a `verify:repeated-fail` -escalation quotes the newest head FAIL's summary (`Reader.fail_summary`). -`verify:spec-changed` holds for the lanes (owner rule 2026-10-26), and -enable-auto-merge.mjs shows one that stands as a failing `pr-merge/unarmed` -status. A `verify:fail` or `verify:earlier-fail` only waits: +Only `verify:untrusted-spec` needs a human (`NEEDS_HUMAN`), who merges in +person: the spec's provenance is a trust boundary. Every other refusal holds +for the lanes (owner rule 2026-10-27: a person only for an intent or design +change or a CRITICAL security issue), and enable-auto-merge.mjs shows +one that stands as a failing `pr-merge/unarmed` status. `verify:spec-changed` +waits for the lanes to judge the head again. `verify:repeated-fail` and +`verify:too-old` never clear, so the gate never merges such a pull request +unattended, and address-feedback's own round cap bounds the fix rounds. A +`verify:fail` or `verify:earlier-fail` only waits: pr-feedback-dispatch.yml posts a FAIL's notes on the pull request as review comments for an address-feedback round, whose push gives a head the verifier judges again. Any read that fails refuses @@ -248,14 +251,14 @@ def verdict_name(name: str) -> re.Match[str] | None: # A verdict's `intent` when its pull request closed no issue in this # repository: the title and body were the spec (write-verdict.mjs). INTENT_PULL_REQUEST = "pull-request" -# The refusals only a person clears (owner rule 2026-10-26: a person only -# for an intent or design change or a CRITICAL security issue, with trust -# holds kept): an untrusted spec (provenance), and the two no lane can clear, -# the repeated-FAIL cap (terminal: `merge.max-verify-fails`) and an old pull -# request (judged from its creation, so it never clears). A changed spec -# holds for the lanes, which judge it again. The one list the merge gate and -# enable-auto-merge.mjs (`needsHuman`) read. -NEEDS_HUMAN = (REPEATED_FAIL, TOO_OLD, UNTRUSTED_SPEC) +# The refusals only a person clears (owner rule 2026-10-27: a person only +# for an intent or design change or a CRITICAL security issue, with +# trust holds kept): an untrusted spec (provenance). Every other refusal, +# the repeated-FAIL cap and an old pull request included, holds for the +# lanes: it still refuses the merge, with no label. The one list the merge +# gate and enable-auto-merge.mjs (`needsHuman`) read; clear-escalations.mjs +# LANE_CAUSES.verdict shares nothing with it (lane-causes-person-codes.test.mjs). +NEEDS_HUMAN = (UNTRUSTED_SPEC,) ISSUE_FIELDS = ( "number lastEditedAt repository{nameWithOwner} " "labels(first:100){totalCount nodes{name}} " @@ -513,11 +516,6 @@ class _Transient(_Unreadable): """A failed read that may succeed if repeated.""" -class SummaryUnreadable(Exception): - """`Reader.fail_summary` could not read the record, which is not the - same as a record with nothing to quote.""" - - def _transient(detail: str) -> bool: """A 5xx, or a network error with no HTTP response. A rate limit is not: a fixed short wait outlasts neither a Retry-After nor an hourly reset, and @@ -575,9 +573,6 @@ class Evidence: verdicts: tuple[Verdict, ...] ignored: int detail: str | None = None - # The newest counted FAIL at the head, for `verify:fail` and - # `verify:repeated-fail`: its run id and artifact name. - head_fail: tuple[int, str] | None = None @property def needs_human(self) -> bool: @@ -598,11 +593,6 @@ def report(self) -> dict[str, Any]: ), "ignored": self.ignored, "detail": self.detail, - "headFail": ( - {"run": self.head_fail[0], "artifact": self.head_fail[1]} - if self.head_fail - else None - ), } @@ -1463,7 +1453,6 @@ def _evidence(self, number: int, head: str, pr: dict[str, Any] | None) -> Eviden commits = self._commits(number, head) | {head} verdicts: list[Verdict] = [] passes: list[tuple[int, str, Any]] = [] - head_fails: list[tuple[int, str]] = [] # Counted FAILs by head: run id, artifact name and id (`_wider_pass`). fail_artifacts: dict[str, list[tuple[int, str, Any]]] = {} ignored = 0 @@ -1542,8 +1531,6 @@ def _evidence(self, number: int, head: str, pr: dict[str, Any] | None) -> Eviden continue verdicts.append(Verdict(sha, verdict, run_id)) fail_artifacts.setdefault(sha, []).append((run_id, name, item.get("id"))) - if sha == head and verdict == "fail": - head_fails.append((run_id, name)) fails = [v for v in verdicts if v.verdict == "fail"] failed = {v.sha for v in fails} refusal: str | None @@ -1599,16 +1586,7 @@ def _evidence(self, number: int, head: str, pr: dict[str, Any] | None) -> Eviden ) else: refusal = MISSING - return Evidence( - refusal, - head, - tuple(verdicts), - ignored, - detail, - max(head_fails) - if head_fails and refusal in (FAIL, REPEATED_FAIL) - else None, - ) + return Evidence(refusal, head, tuple(verdicts), ignored, detail) def _off_grammar( self, item: dict[str, Any], name: str, tied: bool @@ -1798,37 +1776,6 @@ def _wider_pass( return False return True - def _listed_id(self, run_id: int, name: str) -> Any: - """The id the listing this Reader judged on gives the artifact named - `name` of run `run_id`, or None (it is then fetched by run and name).""" - for item in map(json_object, self._artifacts or []): - if item.get("name") == name and _is_id( - json_object(item.get("workflow_run")).get("id"), run_id - ): - return item.get("id") - return None - - def fail_summary( - self, run_id: int, name: str, number: int, head: str - ) -> str | None: - """The `summary` a FAIL artifact's verdict.json records for pull - request `number` at `head`, or None when it records another pull - request, head or verdict. Raises `SummaryUnreadable` when it cannot be - read. Untrusted model text: the caller frames it as data.""" - try: - record = self._record(run_id, name, self._listed_id(run_id, name)) - except _Unreadable as exc: - raise SummaryUnreadable(str(exc)) from exc - summary = record.get("summary") - if ( - record.get("pr") != number - or record.get("head_sha") != head - or record.get("verdict") != "fail" - or not isinstance(summary, str) - ): - return None - return summary - def _pass_refusal( self, run_id: int, diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_gh_rate_guard.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_gh_rate_guard.py index 06d97a9213..5ea34e3dfa 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_gh_rate_guard.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_gh_rate_guard.py @@ -25,7 +25,6 @@ sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) import babysit_gh as gh -import babysit_merge as merge import babysit_verify_evidence as verify_evidence SCRIPTS = pathlib.Path(__file__).resolve().parent.parent @@ -291,20 +290,12 @@ def test_a_set_share_is_read_and_an_unset_one_is_the_file_default(self) -> None: self.assertEqual(shares["writer-pause"], 0.3) self.assertEqual(shares["read-floor"], file_default("read-floor")) - def test_below_the_read_floor_report_only_reads_are_skipped(self) -> None: + def test_below_the_read_floor_headroom_is_low(self) -> None: floor = self.shares("github-token")["read-floor"] just_below = int(floor * 5000) - 1 self.answer(0, head(200, **core(just_below)) + "{}") gh.gh_capture(["api", "repos/o/r/pulls/1"]) self.assertTrue(gh.low_headroom()) - reader = mock.Mock(spec=verify_evidence.Reader) - verify = { - "refusal": verify_evidence.REPEATED_FAIL, - "headFail": {"run": 7, "artifact": "a"}, - "head": "h", - } - self.assertIsNone(merge._fail_summary(verify, {"verify_reader": reader}, 1)) - reader.fail_summary.assert_not_called() def test_at_the_read_floor_the_read_runs(self) -> None: os.environ["API_BUDGET_TOKEN"] = "lanes-app" diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_answers.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_answers.py index 686a66bf80..74396ad4a2 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_answers.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_answers.py @@ -59,7 +59,6 @@ "earlierFails": [], "ignored": 0, "detail": None, - "headFail": None, "ineligible": None, } ALLOW = MISSING | { diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index 477dcd6c1e..97d40d4260 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -8,7 +8,7 @@ from a completed success at this pull request's head; a FAIL at another head of the pull request goes to a human unless a counted pass at the head clears every check; an unreadable FAIL blocks; a pull request -older than 80 days goes to a human; a pass needs every linked issue written +older than 80 days is refused; a pass needs every linked issue written and last edited by a trusted human, the pull request's author included, unchanged since the judgment; the Actions reads use the Actions token; a Dependabot pull request needs no verdict. @@ -463,83 +463,6 @@ def test_a_fail_waits_for_a_fix_round_not_a_human(self) -> None: evidence = read(fake) self.assertEqual(evidence.refusal, "verify:fail") self.assertFalse(evidence.needs_human) - self.assertEqual( - evidence.report()["headFail"], {"run": 1, "artifact": named(HEAD, "fail")} - ) - - def summary(self, fake: FakeGitHub, run_id: int, name: str) -> str | None: - with mock.patch.object(postmerge, "gh_capture", side_effect=fake): - return ve.Reader(REPO, "main", now=NOW).fail_summary(run_id, name, PR, HEAD) - - def test_the_newest_head_fail_names_the_summary_to_quote(self) -> None: - older, newer = named(HEAD, "fail", 7), named(HEAD, "fail") - fake = FakeGitHub( - [artifact(newer, 5), artifact(older, 2)], - {2: run(2), 5: run(5)}, - { - older: record(7, verdict="fail", summary="old"), - newer: record(verdict="fail", summary="Spec item 2 is unmet."), - }, - ) - evidence = read(fake) - self.assertEqual(evidence.head_fail, (5, newer)) - self.assertEqual(self.summary(fake, 5, newer), "Spec item 2 is unmet.") - - def test_another_pull_requests_fail_counts_but_is_not_quoted(self) -> None: - other = named(HEAD, "fail", 99) - fake = FakeGitHub( - [artifact(other, 5)], - {5: run(5)}, - {other: record(99, verdict="fail", summary="PR 99's reason.")}, - ) - evidence = read(fake) - self.assertEqual(evidence.refusal, "verify:fail") - self.assertEqual(evidence.head_fail, (5, other)) - self.assertIsNone(self.summary(fake, 5, other)) - - def test_a_newer_head_pass_is_never_the_fail_to_quote(self) -> None: - fail, passed = named(HEAD, "fail"), named(HEAD, "pass", 99) - fake = FakeGitHub( - [artifact(passed, 5), artifact(fail, 2)], - {2: run(2), 5: run(5)}, - {passed: record(99)}, - ) - evidence = read(fake) - self.assertEqual(evidence.refusal, "verify:fail") - self.assertEqual(evidence.head_fail, (2, fail)) - - def test_a_newer_fail_at_an_earlier_head_is_never_the_fail_to_quote( - self, - ) -> None: - fail, earlier = named(HEAD, "fail"), named(EARLIER, "fail") - fake = FakeGitHub( - [artifact(earlier, 5), artifact(fail, 2)], - {2: run(2), 5: run(5)}, - ) - evidence = read(fake) - self.assertEqual(evidence.refusal, "verify:fail") - self.assertEqual(evidence.head_fail, (2, fail)) - - def test_an_unreadable_record_is_not_a_record_with_nothing_to_quote( - self, - ) -> None: - name = named(HEAD, "fail") - fake = FakeGitHub([artifact(name, 1)], {1: run(1)}) - with self.assertRaises(ve.SummaryUnreadable): - self.summary(fake, 1, name) - - def test_a_foreign_record_quotes_no_summary(self) -> None: - name = named(HEAD, "fail") - fake = FakeGitHub([artifact(name, 1)], {1: run(1)}) - for other in ( - record(99, verdict="fail"), - record(sha=EARLIER, verdict="fail"), - record(verdict="pass"), - record(verdict="fail", summary=7), - ): - with self.subTest(other): - fake.records[name] = other - self.assertIsNone(self.summary(fake, 1, name)) def test_a_fail_beside_a_pass_at_the_same_head_blocks(self) -> None: fake = passing(artifact(named(HEAD, "fail"), 2), runs={2: run(2)}) @@ -1161,7 +1084,7 @@ def sibling_fails(*shas: str, number: int = SIBLING) -> FakeGitHub: def test_three_fails_on_a_closed_pull_request_refuse(self) -> None: evidence = read(self.sibling_fails(*SIBLING_HEADS)) self.assertEqual(evidence.refusal, "verify:repeated-fail") - self.assertTrue(evidence.needs_human) + self.assertFalse(evidence.needs_human) def test_two_there_and_one_here_refuse(self) -> None: fake = self.sibling_fails(*SIBLING_HEADS[:2]) @@ -1169,16 +1092,14 @@ def test_two_there_and_one_here_refuse(self) -> None: fake.runs[9] = judged(9, OLDER_START) self.assertEqual(read(fake).refusal, "verify:repeated-fail") - def test_two_there_and_a_fail_at_the_head_go_to_a_human(self) -> None: - # Not to a fix round whose push could never merge. + def test_two_there_and_a_fail_at_the_head_reach_the_cap(self) -> None: + # Not verify:fail: a fix round whose push could never merge. fake = self.sibling_fails(*SIBLING_HEADS[:2]) fake.artifacts.append(artifact(named(HEAD, "fail"), 9)) fake.runs[9] = judged(9, OLDER_START) evidence = read(fake) self.assertEqual(evidence.refusal, "verify:repeated-fail") - self.assertEqual( - evidence.report()["headFail"], {"run": 9, "artifact": named(HEAD, "fail")} - ) + self.assertFalse(evidence.needs_human) def test_one_there_and_a_fail_at_the_head_wait_for_a_fix_round(self) -> None: fake = self.sibling_fails(*SIBLING_HEADS[:1]) @@ -1242,18 +1163,16 @@ def test_three_fails_on_another_refs_pull_request_refuse(self) -> None: fake = self.sibling_fails(*SIBLING_HEADS) evidence = read(fake) self.assertEqual(evidence.refusal, "verify:repeated-fail") - self.assertTrue(evidence.needs_human) + self.assertFalse(evidence.needs_human) self.assertEqual(downloads(fake), 0) - def test_two_there_and_a_fail_at_the_head_go_to_a_human(self) -> None: + def test_two_there_and_a_fail_at_the_head_reach_the_cap(self) -> None: fake = self.sibling_fails(*SIBLING_HEADS[:2]) fake.artifacts.append(artifact(named(HEAD, "fail"), 9)) fake.runs[9] = judged(9, OLDER_START) evidence = read(fake) self.assertEqual(evidence.refusal, "verify:repeated-fail") - self.assertEqual( - evidence.report()["headFail"], {"run": 9, "artifact": named(HEAD, "fail")} - ) + self.assertFalse(evidence.needs_human) def test_two_there_leave_the_pass_to_decide(self) -> None: self.assertIsNone(read(self.sibling_fails(*SIBLING_HEADS[:2])).refusal) @@ -1313,18 +1232,16 @@ def named_fails(*shas: str, suffix: str = "-i7") -> FakeGitHub: def test_three_named_fails_refuse(self) -> None: evidence = read(self.named_fails(*SIBLING_HEADS)) self.assertEqual(evidence.refusal, "verify:repeated-fail") - self.assertTrue(evidence.needs_human) + self.assertFalse(evidence.needs_human) self.assertEqual(downloads(self.named_fails(*SIBLING_HEADS)), 0) - def test_two_named_and_a_fail_at_the_head_go_to_a_human(self) -> None: + def test_two_named_and_a_fail_at_the_head_reach_the_cap(self) -> None: fake = self.named_fails(*SIBLING_HEADS[:2]) fake.artifacts.append(artifact(named(HEAD, "fail"), 9)) fake.runs[9] = judged(9, OLDER_START) evidence = read(fake) self.assertEqual(evidence.refusal, "verify:repeated-fail") - self.assertEqual( - evidence.report()["headFail"], {"run": 9, "artifact": named(HEAD, "fail")} - ) + self.assertFalse(evidence.needs_human) def test_two_named_leave_the_pass_to_decide(self) -> None: self.assertIsNone(read(self.named_fails(*SIBLING_HEADS[:2])).refusal) @@ -1436,10 +1353,11 @@ def test_a_foreign_run_never_counts(self) -> None: self.assertIsNone(read(fake).refusal) -class RepeatedFailsGoToAHuman(unittest.TestCase): +class RepeatedFailsHoldForTheLanes(unittest.TestCase): """FAILs at `merge.max-verify-fails` (default 3) distinct heads of one - pull request hold it for a human whatever a later pass says (owner - decision 2026-10-21); fewer leave the pass to decide.""" + pull request hold it, never to merge unattended, whatever a later pass + says (owner decision 2026-10-21), for the lanes with no label (owner rule + 2026-10-27); fewer leave the pass to decide.""" @staticmethod def failed_at(*shas: str) -> FakeGitHub: @@ -1454,7 +1372,7 @@ def failed_at(*shas: str) -> FakeGitHub: def test_fails_at_three_heads_refuse_despite_a_later_pass(self) -> None: evidence = read(self.failed_at(EARLIER, IN_PR, UNRELATED)) self.assertEqual(evidence.refusal, "verify:repeated-fail") - self.assertTrue(evidence.needs_human) + self.assertFalse(evidence.needs_human) self.assertIn("3", evidence.detail or "") def test_the_cap_fails_closed_when_its_fails_cannot_be_read(self) -> None: @@ -1466,7 +1384,7 @@ def test_the_cap_fails_closed_when_its_fails_cannot_be_read(self) -> None: self.assertEqual(evidence.refusal, "verify:unreadable") def test_its_own_fails_cannot_expire_before_it_is_too_old(self) -> None: - # Artifacts last 90 days here; a pull request is refused for a person + # Artifacts last 90 days here; a pull request is refused # past TOO_OLD_DAYS, before any of its own FAILs can expire, so an # expired FAIL never lifts the cap of a pull request still judged. self.assertLess(ve.TOO_OLD_DAYS, 90) @@ -1479,7 +1397,7 @@ def test_its_own_fails_cannot_expire_before_it_is_too_old(self) -> None: ) evidence = read(expired, pull(created_at="2026-07-27T23:00:00Z")) self.assertEqual(evidence.refusal, "verify:too-old") - self.assertTrue(evidence.needs_human) + self.assertFalse(evidence.needs_human) def test_fails_at_two_heads_leave_the_pass_to_decide(self) -> None: self.assertIsNone(read(self.failed_at(EARLIER, IN_PR)).refusal) @@ -1494,15 +1412,12 @@ def test_fails_at_one_head_count_once(self) -> None: self.assertEqual(len([v for v in evidence.verdicts if v.verdict == "fail"]), 3) def test_a_fail_at_the_head_that_reaches_the_cap_is_repeated_fail(self) -> None: - # The third failing head goes to a human, not to a fourth fix round, - # and names its own FAIL for the escalation to quote. + # The third failing head is refused for good, not sent to a fourth + # fix round. fake = self.failed_at(EARLIER, IN_PR, HEAD) evidence = read(fake) self.assertEqual(evidence.refusal, "verify:repeated-fail") - self.assertTrue(evidence.needs_human) - self.assertEqual( - evidence.report()["headFail"], {"run": 4, "artifact": named(HEAD, "fail")} - ) + self.assertFalse(evidence.needs_human) def test_a_fail_at_the_head_under_the_cap_stays_verify_fail(self) -> None: fake = self.failed_at(EARLIER, HEAD) @@ -1707,11 +1622,11 @@ def test_a_failed_change_read(self) -> None: self.assertFalse(evidence.needs_human) -class AnOldPullRequestGoesToAHuman(unittest.TestCase): +class AnOldPullRequestIsRefused(unittest.TestCase): def test_older_than_80_days_is_too_old(self) -> None: evidence = read(passing(), pull(created_at="2026-07-27T23:00:00Z")) self.assertEqual(evidence.refusal, "verify:too-old") - self.assertTrue(evidence.needs_human) + self.assertFalse(evidence.needs_human) def test_79_days_is_judged(self) -> None: self.assertIsNone( @@ -2892,16 +2807,8 @@ def send(args, *, timeout_seconds, env_overrides, binary): # --- The --authority pr-merge path ------------------------------------------------ -def evidence( - refusal: str | None, head_fail: tuple[int, str] | None = None -) -> ve.Evidence: - return ve.Evidence( - refusal=refusal, - head=unattended.HEAD, - verdicts=(), - ignored=0, - head_fail=head_fail, - ) +def evidence(refusal: str | None) -> ve.Evidence: + return ve.Evidence(refusal=refusal, head=unattended.HEAD, verdicts=(), ignored=0) # A manifest the floor switch's manifest review holds; the floor itself holds @@ -2962,11 +2869,13 @@ def escalation_comment(self) -> dict[str, Any]: "id": 90, "user": {"login": unattended.IDENTITY, "type": "Bot"}, "created_at": "2026-10-12T10:00:00Z", - "body": merge.verify_escalation_body("verify:too-old", unattended.HEAD), + "body": merge.verify_escalation_body( + "verify:untrusted-spec", unattended.HEAD + ), } def test_a_failed_comment_adds_no_label(self) -> None: - self.verdicts[unattended.PR] = evidence("verify:too-old") + self.verdicts[unattended.PR] = evidence("verify:untrusted-spec") self.failing = {merge.VERIFY_ESCALATION_MARKER} _, out = self.lane() self.assertFalse( @@ -2983,7 +2892,7 @@ def test_a_failed_comment_adds_no_label(self) -> None: def test_a_failed_label_is_added_on_the_next_run_without_a_second_comment( self, ) -> None: - self.verdicts[unattended.PR] = evidence("verify:too-old") + self.verdicts[unattended.PR] = evidence("verify:untrusted-spec") self.failing = {"/labels"} _, out = self.lane() self.assertTrue( @@ -3004,7 +2913,7 @@ def test_a_failed_label_is_added_on_the_next_run_without_a_second_comment( ) def test_a_label_differing_only_in_case_is_not_added_again(self) -> None: - self.verdicts[unattended.PR] = evidence("verify:too-old") + self.verdicts[unattended.PR] = evidence("verify:untrusted-spec") self.pr_labels = [{"name": "Needs-Human"}] self.comments = [self.escalation_comment()] _, out = self.lane() @@ -3053,7 +2962,7 @@ def test_the_gate_reads_verdicts_under_the_configured_cap(self) -> None: def test_the_label_is_the_trackers_human_gated_role(self) -> None: self.role_label.return_value = "status: needs-decision" - self.verdicts[unattended.PR] = evidence("verify:too-old") + self.verdicts[unattended.PR] = evidence("verify:untrusted-spec") self.lane() self.assertIn("labels[]=status: needs-decision", self.escalation_writes()[-1]) @@ -3086,151 +2995,28 @@ def test_a_rerun_writes_no_second_label_or_comment(self) -> None: # names the verifier refusal. self.assertEqual(out["unattended"]["reason"], "verify:untrusted-spec") - FAIL_RUN = (7, f"pr-verify-{unattended.PR}-{unattended.HEAD}-fail") - - def fail_with_summary(self, summary: str | None | Exception) -> mock.Mock: - """A `verify:repeated-fail` with a FAIL at the head whose verdict - records `summary`, or whose record read raises it.""" - self.verdicts[unattended.PR] = evidence("verify:repeated-fail", self.FAIL_RUN) - answer = ( - {"side_effect": summary} - if isinstance(summary, Exception) - else {"return_value": summary} - ) - patcher = mock.patch.object(ve.Reader, "fail_summary", **answer) - read = patcher.start() - self.addCleanup(patcher.stop) - return read - def test_a_fail_at_the_head_writes_no_escalation(self) -> None: # A fix round answers it (pr-feedback-dispatch.yml); a label would # hold that round as awaiting-human. - self.verdicts[unattended.PR] = evidence("verify:fail", self.FAIL_RUN) + self.verdicts[unattended.PR] = evidence("verify:fail") code, out = self.lane() self.assert_no_merge(code) self.assertEqual(out["unattended"]["reason"], "verify:fail") self.assertEqual(self.escalation_writes(), []) self.assertNotIn("escalation", out["unattended"]["writes"]) - def test_a_repeated_fail_escalates_quoting_the_verdict_summary(self) -> None: - read = self.fail_with_summary("Spec item 2 is unmet.") - code, out = self.lane() - self.assert_no_merge(code) - read.assert_called_once_with(*self.FAIL_RUN, unattended.PR, unattended.HEAD) - comment, label = self.escalation_writes() - self.assertIn("labels[]=needs-human", label) - body = comment[-1].removeprefix("body=") - self.assertTrue(body.startswith(merge.VERIFY_ESCALATION_MARKER + "\n")) - self.assertIn("`verify:repeated-fail`", body) - self.assertIn(unattended.HEAD, body) - self.assertTrue(body.endswith("```text\nSpec item 2 is unmet.\n```")) - self.assertEqual( - out["unattended"]["writes"]["escalation"], - {"label": "added", "comment": "created"}, - ) - - def test_a_repeated_fail_rerun_at_the_same_head_writes_nothing(self) -> None: - self.fail_with_summary("Spec item 2 is unmet.") - self.lane() - comment, _ = self.escalation_writes() - self.capture.reset_mock() - # The next run reads the comment and the label the first one wrote. - self.pr_labels = [{"name": "needs-human"}] - self.comments = [ - { - "id": 90, - "user": {"login": unattended.IDENTITY, "type": "Bot"}, - "created_at": "2026-10-12T10:00:00Z", - "body": comment[-1].removeprefix("body="), - } - ] - _, out = self.lane() - self.assertEqual(self.escalation_writes(), []) - self.assertEqual( - out["unattended"]["writes"]["escalation"], - {"label": "present", "comment": "unchanged"}, - ) - - def test_the_quoted_summary_stays_inside_its_fence_and_is_cut(self) -> None: - summary = "a```b\r\n@someone\x1b" + "x" * 1500 - body = merge.verify_escalation_body( - "verify:repeated-fail", unattended.HEAD, summary - ) - quoted = body.split("```text\n", 1)[1] - # 1500 characters kept: the 15 of "a'''b\n@someone?" and 1485 x's. - self.assertEqual(quoted, "a'''b\n@someone?" + "x" * 1485 + " [cut]\n```") - - def test_a_lone_surrogate_in_the_summary_still_posts(self) -> None: - body = merge.verify_escalation_body( - "verify:repeated-fail", unattended.HEAD, "a\ud800b" - ) - self.assertTrue(body.endswith("```text\na?b\n```")) - body.encode("utf-8") - - def assert_escalates_without_a_quote(self, summary: Any) -> None: - self.fail_with_summary(summary) - _, out = self.lane() - comment, _ = self.escalation_writes() - body = comment[-1].removeprefix("body=") - self.assertIn("`verify:repeated-fail`", body) - self.assertNotIn("```", body) - self.assertEqual( - out["unattended"]["writes"]["escalation"]["comment"], "created" - ) - - def test_a_repeated_fail_with_no_summary_to_quote_still_escalates(self) -> None: - self.assert_escalates_without_a_quote(None) - - def test_a_repeated_fail_whose_record_cannot_be_read_still_escalates(self) -> None: - self.assert_escalates_without_a_quote( - ve.SummaryUnreadable("gh: boom (HTTP 502)") - ) - - def test_a_failed_read_keeps_the_quote_a_same_head_comment_carries(self) -> None: - quoted = merge.verify_escalation_body( - "verify:repeated-fail", unattended.HEAD, "Spec item 2 is unmet." - ) - self.fail_with_summary(ve.SummaryUnreadable("gh: boom (HTTP 502)")) - self.pr_labels = [{"name": "needs-human"}] - self.comments = [ - { - "id": 90, - "user": {"login": unattended.IDENTITY, "type": "Bot"}, - "created_at": "2026-10-12T10:00:00Z", - "body": quoted, - } - ] - _, out = self.lane() - self.assertEqual(self.escalation_writes(), []) - self.assertEqual( - out["unattended"]["writes"]["escalation"], - {"label": "present", "comment": "unchanged"}, - ) - - def test_a_failed_read_still_reposts_another_heads_comment(self) -> None: - self.fail_with_summary(ve.SummaryUnreadable("gh: boom (HTTP 502)")) - self.comments = [ - { - "id": 90, - "user": {"login": unattended.IDENTITY, "type": "Bot"}, - "created_at": "2026-10-12T10:00:00Z", - "body": merge.verify_escalation_body( - "verify:repeated-fail", "c" * 40, "Old head's reason." - ), - } - ] - _, out = self.lane() - self.assertEqual( - out["unattended"]["writes"]["escalation"]["comment"], "created" - ) - # The old head's reason stays, under the head it was given at. - (comment, _) = self.escalation_writes() - self.assertTrue( - comment[-1].endswith( - f"fail verdict at `{'c' * 40}`, as it wrote it:\n\n" - "```text\nOld head's reason.\n```" - ) - ) + def test_the_cap_and_an_old_pull_request_hold_with_no_escalation(self) -> None: + # Owner rule 2026-10-27: neither is an intent, design or security + # question, so each holds for the lanes. The gate still refuses. + for refusal in ("verify:repeated-fail", "verify:too-old"): + with self.subTest(refusal): + self.verdicts[unattended.PR] = evidence(refusal) + code, out = self.lane() + self.assert_no_merge(code) + self.assertEqual(out["unattended"]["reason"], refusal) + self.assertIs(out["unattended"]["verify"]["needsHuman"], False) + self.assertEqual(self.escalation_writes(), []) + self.assertNotIn("escalation", out["unattended"]["writes"]) def test_report_only_reports_a_needs_human_refusal_and_writes_nothing(self) -> None: self.config = dataclasses.replace(self.config, report_only=True) @@ -3676,7 +3462,7 @@ def test_a_tied_fail_after_the_pass_holds(self) -> None: # Only the untied FAILs' records: the pass decides nothing. self.assertEqual(download_requests(fake), requests) - def test_a_third_failed_head_goes_to_a_human(self) -> None: + def test_a_third_failed_head_reaches_the_cap(self) -> None: for ids in (True, False): with self.subTest(ids=ids): fake = self.heads(ids=ids) @@ -3687,7 +3473,7 @@ def test_a_third_failed_head_goes_to_a_human(self) -> None: fake.runs[5] = run(5) evidence = read(fake) self.assertEqual(evidence.refusal, "verify:repeated-fail") - self.assertTrue(evidence.needs_human) + self.assertFalse(evidence.needs_human) def test_an_unreadable_untied_record_refuses(self) -> None: for ids in (True, False): @@ -3729,7 +3515,10 @@ class TheByIdFetchDecidesAsGhRunDownloadDid(unittest.TestCase): the reader at 6c4b552396, which fetched every verdict with `gh run download`, run on these same fixtures. The by-id fetch must give the same report in each of the six cases and spend half the download requests. - Break `_record` on purpose and these fail.""" + Two fields were changed since by design, not by the fetch: `needsHuman` + is false for the repeated-FAIL cap (owner rule 2026-10-27), and the + report no longer carries `headFail`. Break `_record` on purpose and these + fail.""" @staticmethod def report( @@ -3737,13 +3526,12 @@ def report( ) -> dict[str, Any]: return { "refusal": refusal, - "needsHuman": refusal == "verify:repeated-fail", + "needsHuman": False, "head": HEAD, "atHead": at_head, "earlierFails": sorted(earlier), "ignored": 0, "detail": None, - "headFail": None, **extra, } @@ -3798,7 +3586,6 @@ def cases(self) -> dict[str, tuple[FakeGitHub, dict[str, Any], int, int]]: ["fail", "pass"], both, detail=capped, - headFail={"run": 6, "artifact": named(HEAD, "fail")}, ), 4, 2, @@ -3813,22 +3600,6 @@ def test_each_case_reports_as_before_on_half_the_requests(self) -> None: self.assertEqual(after * 2, before) -class AFailSummaryIsFetchedByItsListedId(unittest.TestCase): - def test_the_judging_reader_quotes_the_head_fail_in_one_request(self) -> None: - name = named(HEAD, "fail") - fake = FakeGitHub( - [artifact(name, 5)], - {5: run(5)}, - {name: record(verdict="fail", summary="Why.")}, - ) - with mock.patch.object(postmerge, "gh_capture", side_effect=fake): - reader = ve.Reader(REPO, "main", now=NOW) - self.assertEqual(reader.evidence(PR, HEAD, pull()).refusal, "verify:fail") - fake.calls.clear() - self.assertEqual(reader.fail_summary(5, name, PR, HEAD), "Why.") - self.assertEqual(download_requests(fake), 1) - - class ACorruptArchiveIsAnUnreadableRecord(unittest.TestCase): """A deflate stream that does not inflate raises zlib.error, not BadZipFile; it must refuse like any unreadable record, and be shared.""" @@ -4369,16 +4140,17 @@ def test_a_repeated_fail_prints_its_refusal(self) -> None: ve.Evidence(ve.REPEATED_FAIL, HEAD, (), 0, "fails at 3 heads"), ) self.assertEqual(report["refusal"], ve.REPEATED_FAIL) - self.assertTrue(report["needsHuman"]) + self.assertIs(report["needsHuman"], False) - def test_a_changed_spec_alone_goes_back_to_the_lanes(self) -> None: - # Owner rule 2026-10-26: a person only where no lane can clear the - # refusal. An untrusted spec, the terminal repeated-FAIL cap and an - # old pull request stay a person's; a changed spec is judged again. + def test_only_an_untrusted_spec_goes_to_a_person(self) -> None: + # Owner rule 2026-10-27: a person only for intent, a design-altering + # finding or a CRITICAL security issue, trust holds kept. An untrusted + # spec is a trust boundary; the cap, an old pull request and a changed + # spec hold for the lanes. for refusal, person in ( (ve.UNTRUSTED_SPEC, True), - (ve.REPEATED_FAIL, True), - (ve.TOO_OLD, True), + (ve.REPEATED_FAIL, False), + (ve.TOO_OLD, False), (ve.SPEC_CHANGED, False), ): with self.subTest(refusal):