[kyle-sexton]

No related issue: enable-auto-merge batch and pr-merge GITHUB_TOKEN audit items 2 and 5

Summary

Cuts pr-merge's GitHub reads, from GitHub Support's read-only GITHUB_TOKEN audit (items 2, 5 and 7), and records a keep or drop verdict for each item of the enable-auto-merge batch plan against current main. Since that plan was written, #11058 (182cde0, merge.diff-check.path-holds: false) turned off every path hold.

This PR builds:

• Audit item 2: enable-auto-merge skips re-reading an unchanged lane refusal, and the discussion read now lists the changed files.
• Audit item 5: a pull_request_target run handles only its own pull request.
• Audit item 7: the merge job lists candidates with one gh pr list instead of one per author.

Audit item 3 and plan items 1-4 are not built. The table below gives each verdict and its basis.

Verdicts

Item	Verdict	Basis (current main, ba9eab0)
Plan 1: liftBlocker counts the lanes App's stale-review-hold removal	Drop	Nothing on main writes the input it parses. clear-escalations.mjs:182 defines clearedMarker(reason, head), but no stale-review-hold reason exists (grep of .github/scripts/pr-merge/*.mjs is empty), because #10959 is not merged. The item also serves only approvalLift's lift of a hold-path refusal (enable-auto-merge.mjs approvalLift, which requires holdPathOnly), and §10 removes path holds.
Plan 2: the widen read runs decide() under another lane's label	Drop	The plan itself (lines 154-160) says the posted pr-merge marker makes clear-escalations keep pr-unblock's label as a foreign marker (clear-escalations.mjs:772 foreignMarker(appComments, since, known)). Labels pr-unblock clears on its own would then wait for a person, and every reader run adds a discussion read and a verifier read. Both work against §10 and against this PR's goal.
Plan 3, (a): the risk comment repeats the path and the remedy	Drop	The repeat lives only in review-path: reasons. With path-holds: false (docs/conventions/pr-pipeline.yaml:1101), babysit_risk.py:2266-2268 moves every review-path: reason to skipped (is_path_hold, :2283). The comment text at babysit_verify_evidence.py:2512 is built from held, which never draws on skipped. The duplicated remedy can no longer reach a comment in this repository. The brief named (a) as a keep; this verdict overrides it on that code basis.
Plan 4, (b): wrap the refusal comment	Not done, own PR	Still live: lane, cost and verifier refusals keep producing long reason lines. It changes a format that personReasons, holdPathOnly, refusalWrites, clear-escalations nativeRefusal, prMergeEscalations and laneCause, and the ci-status jq all invert. The plan's parent decision (plan, "Parent decisions": "PR 2, separate: item 4") keeps that format change out of a read-cut PR.
Audit 2: skip an unchanged lane refusal; files in DETAIL_QUERY	Build	See Fix.
Audit 3: build the verdict-artifact listing once per tick	Drop	pr-merge.yml:1392 and :2210 each hand the reader BABYSIT_VERIFY_LISTING. Each job already lists incrementally from the carried listing (babysit_verify_evidence.py:1089-1100, pages newer than the base only, about 1 page a tick). A shared listing goes stale after babysit_run_cache.py:31 MAX_AGE = 2 minutes, and enable-auto-merge reads verdicts for up to 5 minutes (VERIFY_BUDGET_MS). The jobs would mostly list again anyway, so it saves about 1 GITHUB_TOKEN core call a tick (estimate) at the price of a cross-job hand-off.
Audit 5: an event run handles only its own pull request	Build	See Fix.
Audit 7: one candidate list in place of one per author	Build	See Fix.

Fix

• One candidate listing (audit item 7). The merge job makes one gh pr list --base main --state open --limit 1000 --json ...,author call and keeps each listed author's pull requests with jq. Login matching ignores case, and gh names an App app/<slug>, as the earlier --author arguments did. A list of 1000 may be cut short, so it stops the job red. With no author at all, it calls nothing, as before.

• Event runs handle one pull request (audit item 5). Both jobs take EVENT_PR: ${{ github.event.pull_request.number }} through env:. The merge job keeps only that number from its API listing; enable-auto-merge does the same with listPulls's answer. The payload never builds a candidate, and the schedule and a hand-started run keep the full sweep. A pending pull_request_target run no longer supersedes another one (superseded.mjs eligible), because each now does different work. Run artifacts from event runs are never read back (rotation.mjs:57 accepts only schedule and workflow_dispatch runs), so a one-PR run cannot reset the cursor or the skip records.

• Files in the discussion read (audit item 2). DETAIL_QUERY now lists files(first: 100) { totalCount nodes { path changeType } }. listedFiles uses that list when it is complete, holds fewer than 100 entries and contains no RENAMED or COPIED change (only REST's previous_filename names the old path). Otherwise the REST pages are read as before.

• Skip-unchanged in enable-auto-merge (audit item 2). This mirrors babysit_merge_skip.py. A reader run keeps, for each unarmed lanes App pull request it left on a lane refusal in SKIPPABLE (a lane risk-score hold, or a pr-unblock stop) and wrote nothing for, a key covering:

  • its head, updatedAt and labels;
  • its head's check and status rollup (each context's id or name with its status and conclusion; a status's time is left out, so the same state posted again is no change);
  • its review decision and count, its newest 50 comments' ids and edit times, and its review threads' resolved and outdated flags, as babysit_merge_skip.pr_state keys them;
  • main's tree ids for the config and scripts;
  • the resolved config, the escalation minutes and the rung the gate scores at (pythonVerify's rung, plus RUNG_CAP).

  These come from one aliased GraphQL query per 20 pull requests, since a check, a status, a resolved thread or an edited comment need not move updatedAt. The next reader run leaves a pull request whose key still matches on that refusal, with no discussion, files, statuses or verifier read.

  A skip never arms, and none happens:

  • for an armed pull request, on a widen read, under a room hold, or after a rate stop;
  • past merge.skip-unchanged max-minutes (60) or max-skips (3), which reuse the merge gate's settings;
  • past the time the lane comment turns into a failing pr-merge/unarmed status.

  A verdict refusal (verify:*) is never kept, because a new pr-verify artifact changes nothing in the key. Neither is a whole-review request, an ejection comment, or any run that wrote a note.

  The records ride in the existing pr-merge-verify-answers artifact as eam-skip.json. verify-answers.mjs reads them from the download it already makes (ghDownload now extracts once per run id), so the hand-off adds no call. It reads them before the answers file, so a run whose reader read nothing (every pull request skipped or held, so no answers file) still hands its records on. A record from another repository, or one dated in the future, malformed or expired, skips nothing.

Metrics

All figures are estimates (UNVERIFIED) until the after measurement. Each names its token pool.

Cut	Saved per tick (estimate)	Pool	Log line the after measurement reads
Audit 2 skip	For each skipped PR: 1 statuses read plus that PR's verifier reads (GITHUB_TOKEN core); 1 files page (lanes App core); 1+ DETAIL_QUERY (GraphQL). With 2-5 of the 8 open lanes App PRs unlabeled and unchanged, that is 2-5 times (1 + V) GITHUB_TOKEN core calls, where V is the PR's github-token= on its last enable-auto-merge: #N calls=... cost line. The key costs 1 GraphQL query per 20 PRs (no core call).	GITHUB_TOKEN core, lanes App core, GraphQL	enable-auto-merge: skip-unchanged skipped=N recorded=M keyed=K of L, plus one #N left, unchanged since <time> (skip k of 3) line per skip
Audit 2 files fold	1 REST files page for each PR read without a rename	lanes App core	/pulls/<n>/files GETs in the job's API budget log
Audit 5 event scope	Per human event, not per tick: every other candidate's gate process in the merge job (each one "tens of lane-token and job-token calls", babysit_merge_skip.py docstring), and in enable-auto-merge one discussion, files and statuses read for each other armed lanes PR	GITHUB_TOKEN core, lanes App core	the merge job's sweep-cost=... candidates=N line on a pull_request_target run (N=1 or 0)
Offset: event runs no longer supersede each other	A burst of N human events on pull requests costs N event runs, each with its fixed cost (who check, lane token, one listing), where a pending event run used to absorb the others. Each run now gates one pull request.	GITHUB_TOKEN core, lanes App	count of pull_request_target runs that reach the gate step
Audit 7 one listing	11 calls per merge-job run (12 authors under lane-prs: native: 11 trusted actors other than the lanes App, plus Dependabot; trusted-actors.json lists 13 logins), on every tick and every event	search (UNVERIFIED: gh pr list --author runs a search query)	gh pr list count in the merge job's API budget log

Verification

• Tests written before each change. The listing, event-scope, files-fold, superseded and verify-answers tests went red on behavior before their fix. The skip tests first went red only on a missing export, and the fake state query first matched nothing, so three skip tests passed with no key read. The fake was fixed, and the behavioral before-evidence for the skip is the mutant runs below. The tests are:
  • enable-auto-merge.test.mjs:
    • an event run handles only its own pull request;
    • the discussion read's file list stands in for the files page unless a rename or a cut list needs it;
    • an unchanged lane risk hold is left with no read and counted;
    • a changed check rollup alone, with the same updatedAt, is read again;
    • a resolved thread, a review or an edited comment alone, with the same updatedAt, is read again;
    • a changed head, update time, label set, main or rung is read again, and an armed one always is;
    • max-minutes, max-skips and 0;
    • escalation;
    • a refusal that wrote, a verdict refusal and an event run keep no record;
    • forged records.
  • merge-job.test.mjs: the listing tests rewritten to one list; an event run lists only its own pull request; a list that may be cut short stops red.
  • superseded.test.mjs: an event run supersedes no other event run.
  • verify-answers.test.mjs: skip records beside the answers, none left planted; a run that kept only skip records still hands them on, with no warning.
• Mutants confirmed to fail a test, each found by a failing run:
  • rollup dropped from the key;
  • labels dropped from the key;
  • the escalation bound removed;
  • the skip disabled;
  • the write check removed;
  • the SKIPPABLE whitelist removed;
  • the future-time check removed.
• node --test over enable-auto-merge, merge-job, workflow, superseded, verify-answers, rotation, listing-carry, head-ref-map, carry-run-cache, lane-records and clear-escalations: 557 pass, 0 fail.
• scripts/check-changelog-fragments.sh --check and --check-required origin/main: pass, with fragment .changes/pr-pipeline/perf-enable-auto-merge-batch-7c4e2a91.md.
• actionlint .github/workflows/pr-merge.yml: clean. zizmor --offline .github/workflows/pr-merge.yml: no findings.

Related

• feat(pr-pipeline): turn path-based merge holds off behind one switch (owner Q15) #11058 (merge.diff-check.path-holds), which this re-check is against.
• perf(pr-pipeline): record main-review stops and cut merge-job lane reads #11084 (merged), whose files this PR leaves untouched: babysit_merge.py, babysit_gh.py, babysit_review_evidence.py, pr-review-main.yml.
• Plan item 4 (refusal comment wrapping) stays for its own PR.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-30T00:28:49.533406Z	220f137	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-security-review reviewed 8e83e8dda15bb1e1d66858a2999f78154fb500ee: 0 finding(s), 0 as inline comments.

No security findings in the incremental change (verify-answers.mjs and verify-answers.test.mjs, since 220f137).

What changed: readAnswers used to swallow every download error and return undefined. It now returns undefined only for an ENOENT from open, which is what readFileSync raises for a missing file in ghDownload (rotation.mjs:76). Any other error, such as a failed gh run download (execFileSync at rotation.mjs:70), is rethrown.

Why it doesn't weaken a gate:

• main() catches the rethrown error (verify-answers.mjs:109-111), writes no answers file, and emits a ::warning. escapeCommand strips \r, \n and % from the message, so the error text can't inject workflow commands.
• A failed download therefore ends the same way as before: no cache file, and the verifier reads each pull request in full. Only the log message differs.
• The change can't make the verifier allow anything. It stays fail-closed, and the answers file is still checked for repository and shape (verify-answers.mjs:69-71) before use.
• VERIFY_ANSWERS_PATH is still removed at verify-answers.mjs:85, before any download. The artifact source (trusted main run, head on main) is unchanged.
• The skip-records path (verify-answers.mjs:95-101) still swallows all errors. That is intended, since a skip only repeats a refusal.

Tests: the new missing() helper mimics the ENOENT/open shape, and a new download-fails case checks that a failed download warns and writes nothing. They match the code behavior.

Spend: none.

[github-actions]

claude-review reviewed 8e83e8dda15bb1e1d66858a2999f78154fb500ee: 0 finding(s), 0 as inline comments.

Incremental review of .github/scripts/pr-merge/verify-answers.mjs and its test since 220f137. I found nothing to flag.

The change: readAnswers used to turn any download error into "no answers". It now does that only for a missing file (ENOENT from open). Any other failure is rethrown, so main writes nothing and emits the ::warning title=verifier answers:: line.

Checks I made by reading the code:

• The only download implementation, ghDownload in rotation.mjs:65-78, raises a failed gh run download through execFileSync. That error has no code of ENOENT with syscall open, so it now surfaces as a warning.
• The same function reads the file with readFileSync, which does raise ENOENT with syscall: "open" when the artifact has no answers file. That case still returns undefined and logs "kept no verifier answers".
• The skip-records path in main (lines 95-101) has its own try/catch. It still swallows every error there, and the change leaves it alone.
• The test double matches both failure kinds: missing() mimics readFileSync, and "download-fails" mimics gh run download. The new case sits in the "unreadable or foreign answers file writes nothing and warns" loop. That loop checks that nothing is written and that a warning is emitted. The existing missing-file tests keep their behavior through missing().

I did not run the tests; this is a read-only review.

There is no hold, no manifest change, and no cost impact.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 220f137d2f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kyle-sexton]

claude-review finding on 220f137 (the readAnswers download error is swallowed): VALID, fixed in 8e83e8d. readAnswers now returns undefined only for a file missing from the artifact (ENOENT on open, which is what ghDownload's readFileSync throws). Any other error, such as a failed gh run download, reaches main's ::warning title=verifier answers::. Test: verify-answers.test.mjs adds a download-fails case to "an unreadable or foreign answers file writes nothing and warns". It failed before the fix and passes after; 362 tests pass across the four pr-merge suites.