diff --git a/.github/scripts/enqueue-guard.test.mjs b/.github/scripts/enqueue-guard.test.mjs index cf5615b91..928637590 100644 --- a/.github/scripts/enqueue-guard.test.mjs +++ b/.github/scripts/enqueue-guard.test.mjs @@ -914,3 +914,31 @@ test('#1202: a fork needs no checkpoint — nobody there could claim it', () => assert.equal(r.fork, true) assert.equal(r.ok, true, r.reasons.join('; ')) }) + +// #1335 (#1325): the guard read only the newest `Verdict:` line, so a `Correction from :` that +// withdrew the PASS was invisible and the head still read "passed, ready to enqueue". These cases run the +// whole way through `main` — the comment list, `judge`, `decide` and `guard` — because the defect was one +// reader disagreeing with another, not a parse detail. +const correction = ({ by = 'reviewer-agent', tip = HEAD, at = '2026-10-21T11:00:00Z', say = 'is withdrawn' } = {}) => + ({ user: { login: 'reviewer-agent' }, body: `Correction from ${by}: the PASS at \`${tip}\` ${say}.\n\nThis is not a second verdict.`, created_at: at, author_association: 'MEMBER' }) + +test('#1335: a PASS its own writer withdrew does not enter the queue', () => { + const r = run({ comments: [comment(), correction()] }) + assert.equal(r.ok, false) + assert.match(r.reasons.join('; '), new RegExp(`the PASS at ${HEAD} was withdrawn by reviewer-agent`)) + assert.match(r.message, /^do not enqueue #566: .*withdrawn by reviewer-agent/) + // The PASS is still on the pull request: the refusal has to be readable as "withdrawn", not "no PASS". + assert.doesNotMatch(r.reasons.join('; '), /no Verifier report names/) +}) + +test('#1335: a withdrawal from a name that wrote no PASS here changes nothing', () => { + // Dev2 is a `writers` Bot name but wrote no PASS at this tip and is not a `reviewers` login. + const r = run({ comments: [comment(), correction({ by: 'Dev2' })] }) + assert.equal(r.ok, true, r.reasons.join('; ')) + assert.equal(r.command, `gh pr merge 566 --auto --squash --match-head-commit ${HEAD}`) +}) + +test('#1335: a withdrawal naming another tip leaves this tip enqueueable', () => { + const r = run({ comments: [comment(), correction({ tip: OLD })] }) + assert.equal(r.ok, true, r.reasons.join('; ')) +}) diff --git a/.github/scripts/review-gate.mjs b/.github/scripts/review-gate.mjs index fa974b1ef..498a78901 100644 --- a/.github/scripts/review-gate.mjs +++ b/.github/scripts/review-gate.mjs @@ -36,6 +36,16 @@ // still describes this head, and `change-set.mjs` answers that. It fails closed, so an unreadable // tree or a changed blob is the same refusal as before. // +// #1335: a PASS can be withdrawn, and until this both gates read the withdrawal as ordinary prose. A +// body that opens `Correction from : the PASS at is withdrawn` carries no `Verdict:` line, +// so `parseReport` reported no conclusion, `decide` never put it in the newest-wins scan and an earlier +// PASS stayed the newest word — which is how #1325 read as "passed, ready to enqueue" after its own +// reviewer had withdrawn it. The correction is a conclusion line now, read from the header like a +// `Verdict:` header and carrying the tip it withdraws, so it joins that one scan instead of becoming a +// second reader of the same text. The refusal says `the PASS at was withdrawn by `. It +// counts only from the Bot that wrote the PASS it withdraws, or from a `reviewers` login: otherwise +// anyone with write access could freeze a healthy pull request with one sentence. +// // The pull request is read only through the GitHub API; no PR code is checked out or run. import { readFileSync, appendFileSync } from 'node:fs' import { execFileSync } from 'node:child_process' @@ -97,6 +107,24 @@ export function adminsFor({ gh, repo, reports, policy, author } = {}) { return admins } +// #1335: `Correction from : the PASS at is withdrawn` — the one other conclusion header. +// The name and the tip are read here, and nothing is decided: whether that name may withdraw that PASS +// is `decide`'s identity rule, because only it sees every report at the tip. Markdown emphasis is +// already stripped from the line by the caller, so the real body (`the PASS at `` is **withdrawn**`) +// reads the same as a plain one. A `Correction from …:` line that names no tip, or never says +// `withdrawn`, is ordinary prose — a correction that agrees with something is not a conclusion. +const CORRECTION_HEADER = /^Correction\s+from\s+([^:]{1,64}?)\s*:\s*(.+)$/i +const CORRECTION_TIP = /the PASS at\s+([0-9a-f]{7,40})\b/i +const CORRECTION_WORD = /\bwithdrawn\b/i +function parseCorrection(line) { + const header = CORRECTION_HEADER.exec(String(line ?? '').trim()) + if (!header) return null + const by = header[1].trim() + const tip = CORRECTION_TIP.exec(header[2]) + if (!by || !tip || !CORRECTION_WORD.test(header[2])) return null + return { by, tip: tip[1].toLowerCase() } +} + // The last Verifier block in one body: a comment may hold an earlier review that a later // one replaced, and only the last block is the verdict the reviewer stands behind. export function parseReport(body) { @@ -104,7 +132,7 @@ export function parseReport(body) { // markdown emphasis, line for line, for everything else. const raw = String(body ?? '').split('\n') const lines = raw.map((line) => line.replace(/[*`]/g, '')) - const out = { tip: null, verdict: null, unknownVerdict: null, ran: [], hasNotRun: false, writer: null, attestation: null } + const out = { tip: null, verdict: null, unknownVerdict: null, withdraw: null, ran: [], hasNotRun: false, writer: null, attestation: null } // The fixed format (#637): the first non-empty line is the whole conclusion, // `Verdict: PASS at ` (or INC / REQUEST_CHANGES). The attribution line and the evidence go // below it, so a verdict Chief posts for a Reviewer with no write channel parses the same way. @@ -116,6 +144,8 @@ export function parseReport(body) { // The tip is read here too, which is what makes it a conclusion at that tip rather than a stray // sentence — a word with no tip below is still an ordinary comment. const firstUnknown = first ? null : /^Verdict\s*:\s*(\S+?)\s+(?:at\s+)?([0-9a-f]{7,40})\s*$/i.exec(firstLine.trim()) + // #1335: `Correction from : the PASS at is withdrawn` is the other conclusion header. + const firstCorrection = first || firstUnknown ? null : parseCorrection(firstLine) let start = -1 for (let i = 0; i < lines.length; i++) { if (/^Tip SHA\s*:/i.test(lines[i].trim())) start = i @@ -126,6 +156,9 @@ export function parseReport(body) { } else if (firstUnknown) { out.unknownVerdict = firstUnknown[1] out.tip = firstUnknown[2].toLowerCase() + } else if (firstCorrection) { + out.withdraw = firstCorrection + out.tip = firstCorrection.tip } // #643, #640: once the first line is the conclusion, the loop below must write neither field again. // A body quotes older reports (a `Tip SHA: ` block with an older `Verdict: PASS`), and letting @@ -133,8 +166,9 @@ export function parseReport(body) { // tip that `decide` then dropped while the PASS it refused came back to life. Both fail open. let ranSeen = false // An unreadable first-line conclusion is still the header: the quote rules below keep a later block - // from overwriting the first one (#643, #640), and they protect this one the same way. - const firstIsVerdict = first !== null || firstUnknown !== null + // from overwriting the first one (#643, #640), and they protect this one the same way. A correction + // header is protected the same way — the `Correction` body quotes the `Verdict: PASS` it withdraws. + const firstIsVerdict = first !== null || firstUnknown !== null || firstCorrection !== null // With the first line as the conclusion the evidence is below it, so scan from the top; a // legacy body is judged by its last block. const from = firstIsVerdict || start < 0 ? 0 : start @@ -331,7 +365,7 @@ export function decide({ head, author, reports, policy, admins, body, owners, re for (const r of reports ?? []) { const parsed = parseReport(r.body) if (!parsed.tip && !parsed.verdict && !parsed.unknownVerdict) continue // not a Verifier report - const entry = { id: r.id, login: r.login, verdict: parsed.verdict, unknownVerdict: parsed.unknownVerdict, tip: parsed.tip, at: r.at, kind: r.kind, writer: parsed.writer, parsed, counts: false, why: null } + const entry = { id: r.id, login: r.login, verdict: parsed.verdict, unknownVerdict: parsed.unknownVerdict, withdraw: parsed.withdraw, tip: parsed.tip, at: r.at, kind: r.kind, writer: parsed.writer, parsed, counts: false, why: null } judged.push(entry) if (!parsed.tip) { reasons.push(`${where(r)}: no Tip SHA`); continue } // #1112: the comparison is the whole sha, not a prefix. A conclusion written `at abaf938` is a @@ -360,8 +394,9 @@ export function decide({ head, author, reports, policy, admins, body, owners, re entry.carriedFrom = parsed.tip } // #1239: an unreadable word is not "no verdict" — it is a conclusion at this tip that nobody can - // read, and the newest one has to be able to veto an older PASS. - if (!parsed.verdict && !parsed.unknownVerdict) continue // a `Tip SHA` block with no verdict is not a conclusion + // read, and the newest one has to be able to veto an older PASS. #1335: a correction is the same + // shape — it carries no `Verdict:` word, but it names this tip's PASS and must reach the scan. + if (!parsed.verdict && !parsed.unknownVerdict && !entry.withdraw) continue // a `Tip SHA` block with no verdict is not a conclusion // #943: refused before the association test, with its own reason: a bot account is not a // conclusion for a reason unlisted logins cannot explain, and the log has to say which defect fired. if (isBotAccount(r)) { @@ -373,7 +408,9 @@ export function decide({ head, author, reports, policy, admins, body, owners, re // conclusion. It stays out of `atTip`, so it neither passes nor refuses an older PASS, and the summary // lists it as unverified. Reading it here — before the association, writer and reviewers checks — is // what makes the rule one cut rather than one more refusal stacked on the old path. - if (signing.enforced) { + // #1335: a correction signs no verdict — there is no `Verdict:` line to put in `canonical()` — and it + // can only ever refuse, never pass, so the identity rule below is what keeps it honest. + if (signing.enforced && !entry.withdraw) { const signed = verifyReport({ repo, pr, head: headSha, verdict: parsed.verdict, writer: parsed.writer, body: r.body, signers: signing }) if (!signed.ok) { entry.why = `${where(r)}: unverified — ${signed.reason}` @@ -400,6 +437,9 @@ export function decide({ head, author, reports, policy, admins, body, owners, re // newer refusal invisible: an INC with no `Written-by:` line left an older PASS in force, and a // human REQUEST_CHANGES from an unlisted login did the same. atTip.push(entry) + // #1335: a correction joins the scan the same way, and its identity rule is settled after the loop, + // because only then is it known which PASSes this tip carries and who wrote them. + if (entry.withdraw) continue // #1239: a word outside the four the gate reads counts for nothing, but it still takes its place in // the newest-word scan, carrying a reason that names it. Dropping the line is the defect: an INC the // reviewer meant became invisible and the older PASS it withdrew stayed in force on #1221. @@ -427,7 +467,28 @@ export function decide({ head, author, reports, policy, admins, body, owners, re } if (!reviewers.size) return { ok: false, verdict: null, by: null, tip: null, reason: 'no reviewer is listed in .github/agent/review-policy.json', judged, unverified } - if (!atTip.length) { + // #1335: a correction is a conclusion only from the Bot that wrote the PASS it withdraws, or from a + // `reviewers` login. Without that, anyone with write access could freeze a healthy pull request with + // one sentence — the withdrawal would be a veto with no signature behind it. Read here rather than in + // the loop because the PASS it withdraws may be listed after it. + // Only a PASS the gate actually counted can be withdrawn by its writer: an invalid PASS is already + // refused on its own ground, so its `Written-by:` name must not become a licence to withdraw. + const passWriters = new Set( + atTip.filter((e) => e.verdict === 'PASS' && e.counts && e.writer).map((e) => String(e.writer).toLowerCase()), + ) + const effective = [] + for (const e of atTip) { + if (!e.withdraw) { effective.push(e); continue } + const by = String(e.withdraw.by).toLowerCase() + if (!passWriters.has(by) && !reviewers.has(by)) { + e.why = `${where({ kind: e.kind, login: e.login })}: Correction from ${e.withdraw.by} withdraws a PASS it did not write, and that name is not a reviewer listed in .github/agent/review-policy.json, so it is not a conclusion` + continue + } + e.why = `${where({ kind: e.kind, login: e.login })}: the PASS at ${e.withdraw.tip} was withdrawn by ${e.withdraw.by}` + e.counts = false + effective.push(e) + } + if (!effective.length) { const reason = reasons.length ? reasons.join('; ') : `no Verifier report names the tip ${headSha.slice(0, SHORT)}` return { ok: false, verdict: null, by: null, tip: null, reason, judged, unverified } } @@ -435,8 +496,8 @@ export function decide({ head, author, reports, policy, admins, body, owners, re // #1317: a conclusion that names the head itself always decides, and the carried ones are read only // when the head carries none. That is what keeps an INC at the head a veto: a carried PASS can never // outrank a conclusion written at this tip, whatever the two timestamps say. - const exact = atTip.filter((e) => !e.carriedFrom) - const candidates = exact.length ? exact : atTip + const exact = effective.filter((e) => !e.carriedFrom) + const candidates = exact.length ? exact : effective // The newest word at this tip decides, and that includes a conclusion the checks refused: an invalid // PASS is not evidence, and an invalid INC is still someone saying not to merge (#676). Equal // timestamps fall back to the order the API returned them in (oldest first), so the later one wins. @@ -487,13 +548,16 @@ export function render(result, { pr, head, author } = {}) { out.push('', 'Unverified — not read as conclusions:') for (const e of unverifiedReports.slice(0, 10)) out.push(`- unverified: ${e.login} (${e.kind}): ${e.verdict ?? 'no verdict'} at ${(e.tip ?? 'no tip').slice(0, SHORT)} — ${String(e.why ?? '').replace(/^[^:]+: unverified — /, '')}`) } - const seen = result.judged.filter((e) => e.counts || e.verdict || e.unknownVerdict) + const seen = result.judged.filter((e) => e.counts || e.verdict || e.unknownVerdict || e.withdraw) if (seen.length) { out.push('', 'Verifier reports read:') for (const e of seen.slice(0, 10)) { // #1239: an unreadable word is named with its reason, so the next reader does not have to rediscover - // that `Incorrect` was never read as `INC`. - const word = e.unknownVerdict ? `"${e.unknownVerdict}", which is not a verdict word` : (e.verdict ?? 'no verdict') + // that `Incorrect` was never read as `INC`. #1335: a withdrawal is named the same way, with the tip + // it withdraws and the Bot that wrote it, so a reader can see why the PASS beside it does not count. + const word = e.withdraw + ? `withdrawal of the PASS at ${(e.withdraw.tip ?? 'no tip').slice(0, SHORT)} by ${e.withdraw.by}` + : e.unknownVerdict ? `"${e.unknownVerdict}", which is not a verdict word` : (e.verdict ?? 'no verdict') const why = e.unknownVerdict ? ` — ${String(e.why ?? '').replace(/^[^:]+:\s*/, '')}` : '' out.push(`- ${e.login} (${e.kind}): ${word} at ${(e.tip ?? 'no tip').slice(0, SHORT)}, written-by ${e.writer ?? 'nothing'}${e.counts ? '' : ' (not counted)'}${why}`) } diff --git a/.github/scripts/review-gate.test.mjs b/.github/scripts/review-gate.test.mjs index 645e95512..42ac593aa 100644 --- a/.github/scripts/review-gate.test.mjs +++ b/.github/scripts/review-gate.test.mjs @@ -896,3 +896,90 @@ test('#644: a conclusion from a different GitHub login needs no attestation line }) assert.equal(other.ok, true, other.reason) }) + +// #1335: a PASS its own reviewer withdrew stayed the newest word, because `Correction from :` is not +// a `Verdict:` line — `parseReport` reported no conclusion, so `decide` never put the withdrawal in the +// newest-wins scan. On #1325 that read as "passed, ready to enqueue" 93 seconds after the writer withdrew. +const withdraw = ({ by = 'Reviewer', tip = HEAD, at = '2026-10-18T02:00:00Z', login = 'reviewer-agent', kind = 'comment', say = 'is **withdrawn**' } = {}) => + ({ login, kind, at, association: 'MEMBER', body: [`Correction from ${by}: the PASS at \`${tip}\` ${say}.`, '', 'This is not a second verdict — the tip still carries one `Verdict:` line.'].join('\n') }) + +test('#1335: the Bot that wrote a PASS can withdraw it at this tip, and the gate refuses', () => { + const parsed = parseReport(withdraw().body) + assert.deepEqual(parsed.withdraw, { by: 'Reviewer', tip: HEAD }) + assert.equal(parsed.tip, HEAD, 'the correction names the tip it withdraws') + assert.equal(parsed.verdict, null, 'a withdrawal is not a verdict word') + const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('reviewer-agent', report(), '2026-10-18T01:00:00Z'), withdraw()], policy }) + assert.equal(r.ok, false, 'a withdrawn PASS must not pass this tip') + assert.equal(r.verdict, null) + assert.match(r.reason, new RegExp(`the PASS at ${HEAD} was withdrawn by Reviewer`)) + // The report says why the PASS beside it decided nothing. + const view = render(r, { pr: 1335, head: HEAD, author: 'opencode-agent' }) + assert.match(view, /withdrawal of the PASS at/) + assert.match(view, /by Reviewer/) +}) + +test('#1335: a withdrawal that names a different tip does not touch the PASS at this one', () => { + const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('reviewer-agent', report(), '2026-10-18T01:00:00Z'), withdraw({ tip: OLD })], policy }) + assert.equal(r.ok, true, r.reason) + assert.equal(r.by, 'reviewer-agent') +}) + +test('#1335: a Correction from someone who did not write the PASS is not a conclusion', () => { + // Chief is a listed writer but wrote no PASS here, and is not a `reviewers` login: one sentence from any + // other name must not park a healthy pull request. + const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('reviewer-agent', report(), '2026-10-18T01:00:00Z'), withdraw({ by: 'Chief' })], policy }) + assert.equal(r.ok, true, r.reason) + const stray = r.judged.find((e) => e.withdraw) + assert.equal(stray.counts, false) + assert.match(stray.why, /withdraws a PASS it did not write/) +}) + +test('#1335: a user in review-policy.json reviewers may withdraw a PASS another name wrote', () => { + const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('reviewer-agent', report(), '2026-10-18T01:00:00Z'), withdraw({ by: 'hsliuustc0106' })], policy }) + assert.equal(r.ok, false) + assert.match(r.reason, /was withdrawn by hsliuustc0106/) +}) + +test('#1335: a later PASS supersedes a withdrawal — the newest word at the tip still wins', () => { + const reports = [ + comment('reviewer-agent', report(), '2026-10-18T01:00:00Z'), + withdraw({ at: '2026-10-18T02:00:00Z' }), + comment('reviewer-agent', report(), '2026-10-18T03:00:00Z'), + ] + const r = decide({ head: HEAD, author: 'opencode-agent', reports, policy }) + assert.equal(r.ok, true, r.reason) +}) + +test('#1335: the #1325 shape — a self-PASS with an Attestation, withdrawn by its writer', () => { + const selfPolicy = { reviewers: ['Birfy', 'hsliuustc0106'], selfReview: ['Birfy'], writers: ['Reviewer', 'Birfy'] } + const reports = [ + comment('Birfy', report({ attest: true }), '2026-10-27T04:14:03Z'), + withdraw({ by: 'Reviewer', login: 'Birfy', at: '2026-10-27T04:15:36Z' }), + ] + const r = decide({ head: HEAD, author: 'Birfy', reports, policy: selfPolicy, owners: ['Dev'] }) + assert.equal(r.ok, false) + assert.match(r.reason, /was withdrawn by Reviewer/) +}) + +test('#1335: a Correction quoted inside a body does not withdraw anything — the header decides', () => { + const body = [ + `Verdict: PASS at ${HEAD}`, + 'Written-by: Reviewer', + '', + 'Ran:', + ' - npm run verify → pass', + '', + `Quoted from my earlier comment: Correction from Reviewer: the PASS at \`${HEAD}\` is withdrawn.`, + ].join('\n') + const parsed = parseReport(body) + assert.equal(parsed.withdraw, null, 'only the header is a conclusion line') + assert.equal(parsed.verdict, 'PASS') + const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('reviewer-agent', body)], policy }) + assert.equal(r.ok, true, r.reason) +}) + +test('#1335: a Correction that never says withdrawn is prose, not a conclusion', () => { + const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('reviewer-agent', report(), '2026-10-18T01:00:00Z'), withdraw({ say: 'still stands' })], policy }) + assert.equal(r.ok, true, r.reason) + assert.equal(parseReport(withdraw({ say: 'still stands' }).body).withdraw, null) +}) diff --git a/AGENTS.md b/AGENTS.md index 20128b1c3..61c78ff50 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,7 @@ A person talks to the **Chief** (the Main Bot). The Chief hands the work to the 6. **Gate, then queue.** Five checks, in this order. Each one can refuse. 1. **Tier.** `node .github/scripts/risk-gate.mjs --pr `, or quote the `risk-tier:` line of the job summary. **T0/T1** need green `tests-ok` at the tip, a mergeable pull request and design-conformance — no PASS. **T2/T3** need a **PASS at the tip SHA** written by the Reviewer ([`review-pr`](.claude/skills/review-pr/SKILL.md): PASS / FAIL / INC), never by the implementer ([Merge gates](#merge-gates)). Neither a PASS nor green CI replaces the other. - 2. **One conclusion per tip.** A tip carries at most one `Verdict:` block. A second conclusion at the same tip is an incident, not a better answer; a new commit voids the PASS **unless it changes no file of the reviewed change set** — a tip that moved only because `main` was merged in keeps it, and the guard names the sha it accepted and why (#1317); a correction reads `Correction from : …` and is not a verdict. + 2. **One conclusion per tip.** A tip carries at most one `Verdict:` block. A second conclusion at the same tip is an incident, not a better answer; a new commit voids the PASS **unless it changes no file of the reviewed change set** — a tip that moved only because `main` was merged in keeps it, and the guard names the sha it accepted and why (#1317); a correction reads `Correction from : …` and is not a verdict, but one that names this tip and says its PASS is withdrawn voids that PASS when `` wrote it or is a `reviewers` login, and the guard refuses with `the PASS at was withdrawn by ` (#1335). 3. **Enqueue.** `node .github/scripts/enqueue-guard.mjs --pr `, then `gh pr merge --auto --squash --match-head-commit `. The guard re-reads the live head and refuses (exit 1) on a `hold`, on a newest conclusion that is not a PASS, and on a report marked "record only — do not merge", so a PASS is never a queue permit on its own (#602). `main` requires a merge queue: the command enqueues the pull request (gh may answer `The merge strategy for main is set by the merge queue` or `already queued`), the queue performs the squash merge, and a plain `--squash` merge is refused — do not retry ([merge plumbing](docs/autonomous-dev.md#merge-plumbing-merge_group-merge-queue-auto-merge)). 4. **Judge on the merge tree.** Whether the pull request can merge is decided on the queue entry — the pull request merged onto the current `main`, built and tested again — never on the pull request page. The page's green is the head alone: #759 was 9/9 green, was kicked out of the queue twice, and went red on `main` at `core/test/follow-through.test.js:264`. 5. **Never bypass.** No admin-merge, no bypass, no ruleset change. A `Docs-not-needed:` line goes in the PR body as well as in the branch commit, and you check both at the tip ([why, and how](#docs-not-needed-why-both-places)). Run `verify` with `TZ=UTC`. @@ -200,7 +200,7 @@ Four rules decide what a queue entry must pass. The mechanical judge runs where A review's `state` is not read at all: approvals are not a gate any more (user decision 2026-10-22), and the account cannot approve its own pull requests, so its reviews stay `COMMENTED` even for an INC — the INC on #610 was a comment rather than a review. -**One conclusion per tip.** A tip carries at most one conclusion: one `Verdict:` block naming one SHA — the whole 40 characters, because a prefix names every head that starts that way and the newest of them is the tip being judged (#1112). **A tip that moved without changing the reviewed change set keeps its conclusion** (#1317): when no conclusion names the head itself, the gate compares `(path → blob sha)` at the reviewed tip and at the head over the pull request's changed paths, and accepts the PASS only when every one matches — naming both shas and the comparison it made — while a changed blob, a path missing on either side, a truncated tree or an unreadable one refuses exactly as before. A conclusion that names the head always decides, so an INC written at the head still vetoes a carried PASS. Two conclusions at the same tip is an incident, not a tie to be broken — nobody re-verdicts to win, the Reviewer that wrote the later one retracts it in a comment, and the pull request waits. A correction reads `Correction from : …` and carries no `Verdict:` line, so it never competes with the standing conclusion; the replacement PASS for a missing `Ran:` ([above](#gates)) is the only other body allowed at one tip. **A GitHub bot account's `Verdict:` is never a conclusion** — `user.type` is `Bot`, or the login ends `[bot]` — whatever its body says (#943, `cursor[bot]` at #940's tip); the rule is the account type and not a login list, because every Bot writes as the shared `Birfy` account and a list containing it would void the whole pipeline, so the real Reviewer voids such a report at that tip with `Correction from Reviewer:`. +**One conclusion per tip.** A tip carries at most one conclusion: one `Verdict:` block naming one SHA — the whole 40 characters, because a prefix names every head that starts that way and the newest of them is the tip being judged (#1112). **A tip that moved without changing the reviewed change set keeps its conclusion** (#1317): when no conclusion names the head itself, the gate compares `(path → blob sha)` at the reviewed tip and at the head over the pull request's changed paths, and accepts the PASS only when every one matches — naming both shas and the comparison it made — while a changed blob, a path missing on either side, a truncated tree or an unreadable one refuses exactly as before. A conclusion that names the head always decides, so an INC written at the head still vetoes a carried PASS. Two conclusions at the same tip is an incident, not a tie to be broken — nobody re-verdicts to win, the Reviewer that wrote the later one retracts it in a comment, and the pull request waits. A correction reads `Correction from : …` and carries no `Verdict:` line, so it is not a second verdict; the replacement PASS for a missing `Ran:` ([above](#gates)) is the only other body allowed at one tip. **A withdrawal is the one correction the gates read** (#1335): `Correction from : the PASS at is withdrawn`, in the first line the way a `Verdict:` header is, joins the same newest-wins scan at the tip it names, and the gate refuses with `the PASS at was withdrawn by `. It counts only from the Bot that wrote that PASS or from a `reviewers` login — one sentence from any other writer is prose, so a healthy pull request cannot be parked by whoever can comment. A later PASS at the tip supersedes it, exactly as a later FAIL vetoes an earlier PASS, and a correction quoted inside a body is a quote, not a withdrawal. **A GitHub bot account's `Verdict:` is never a conclusion** — `user.type` is `Bot`, or the login ends `[bot]` — whatever its body says (#943, `cursor[bot]` at #940's tip); the rule is the account type and not a login list, because every Bot writes as the shared `Birfy` account and a list containing it would void the whole pipeline, so the real Reviewer voids such a report at that tip with `Correction from Reviewer:`. **Judged on the merge tree, never on the pull request page.** The page answers "does this commit pass alone"; the queue answers "does this commit pass merged onto the current `main`", and the second one is the merge decision. #759 was green on every check, was kicked out of the queue twice, and went red on `main` at `core/test/follow-through.test.js:264`. A red queue entry that names files you did not touch is the stale base of [step 2](#pipeline-contract), not bad luck: fix the base, do not re-enqueue blindly. diff --git a/docs/autonomous-dev.md b/docs/autonomous-dev.md index 53f98486a..bb77e1929 100644 --- a/docs/autonomous-dev.md +++ b/docs/autonomous-dev.md @@ -156,13 +156,22 @@ What counts as a reviewed tip, all of it: counting; that report is then listed as "not counted", with the sha it named. 5. The **newest** report that names this tip decides, so a later FAIL at the same tip vetoes an earlier PASS, and a later PASS at the same tip replaces a FAIL. -6. **A GitHub bot account writes no conclusion** (#943): `user.type` is `Bot`, or the login ends `[bot]`, and the +6. **A withdrawal is a conclusion line** (#1335): `Correction from : the PASS at is withdrawn`, as the + body's first line, is read by the same `parseReport` and enters the same newest-wins scan at the tip it names, + so the gate refuses with `the PASS at was withdrawn by ` and both `enqueue-guard` and `merge-gate` + see it without a second parser. It counts only when `` is the `Written-by:` of the PASS it withdraws or a + `reviewers` login in `.github/agent/review-policy.json`; any other name is prose and changes nothing, which is + what keeps one comment from parking a healthy pull request. A later PASS at the tip supersedes the withdrawal, + a correction that names another tip does not touch this one, and a correction quoted below a blank line is a + quote. `#1325` is the incident: its PASS was withdrawn at 04:15:36Z and the head still read "passed, ready to + enqueue" because a `Correction from …:` line carries no `Verdict:` word. +7. **A GitHub bot account writes no conclusion** (#943): `user.type` is `Bot`, or the login ends `[bot]`, and the report joins `judged` with `counts: false` and a reason naming the account, whatever its body says — the real Reviewer voids it at that tip with `Correction from Reviewer:`. The rule is the account type and not a login list: every Bot writes as the shared `Birfy` account, so a list containing it would void the whole pipeline. `cursor[bot]`'s report at #940's tip is the case (`Correction from Reviewer`, [comment `6381646389`](https://github.com/ThinkFlowLab/FlyBot/pull/940#issuecomment-6381646389)). -7. **The report can be signed by the workstation it names, and today it is not** (#1110, #1111): +8. **The report can be signed by the workstation it names, and today it is not** (#1110, #1111): `Signed-by:` + `Nonce:` + `Signature:` against `.github/agent/signers.json` for the full head being judged is built and switched **off** — the policy has no `signers` roster and the signers file does not exist, so no conclusion is asked for those lines and none is dropped as `unverified:`. See [Signed conclusions](#signed-conclusions-1110)