[kyle-sexton]

No related issue: regression from #12558 (FAIL index mode); issues are being driven to zero, so none was filed.

Summary

Since #12558 turned on FAIL index mode, merge-group ci-status fails every passing lanes App pull request with "the verdict reader read no artifact listing" (pr-require-checks.yml, lanes App hold step). In index mode the verdict reader reads by exact name and writes no artifact listing, so this step read the missing listing as unreadable, failed the head and dropped it and its group from the queue. Index mode was reported working on call counts alone (a 24-call reader run) while this next step failed.

Fix

When the legacy FAILs file (verify-legacy-fails.json) sits beside the reader, the step reads the head's four pass names (pass, pass-<digest>, pass-notes, pass-notes-<digest>) by exact name into the listing file and judges them as before. Without that file the step still fails closed with the same error. The reader-refusal path is unchanged.

Call delta: +4 exact-name artifact reads (counter B) per lanes App head per merge-group run, against the whole-listing reads index mode removed. Every read is bounded (one page of an exact name).

Verification

• bash scripts/ci-lanes-pr-hold.test.sh: 303/303 pass; the 3 new cases (index mode with a pass at the head passes, its pass names take 1-4 exact-name reads, index mode with no pass holds) fail without the change. The existing no-listing case still fails closed when no legacy file is present.
• plugins/source-control/skills/babysit-prs/scripts/tests/test_verify_rules_sync.py lifts the hold step's lookup loop out of the workflow; it now also lifts the pass_names array the loop reads (856d563). Without that, CI at 864f696 failed 23 subtests (test-python (8), test-bash (1)). After the fix: 18 passed, 206 subtests; engine.test.sh 4029 tests OK; both re-run green after merging main (cfa6620).
• After merge: the next two merge_group runs of pr-require-checks.yml are watched (lanes App PRs included) and the change is reverted on two consecutive failures caused by it.

Related

• feat(source-control): commit the legacy FAILs file to turn on FAIL index mode #12558 (FAIL index mode), feat(docs-hygiene): rename-references auto-applies the Certain bucket and reports it #12604 (lanes App PR ejected by this defect; re-armed after merge).

🤖 Generated with Claude Code

https://claude.ai/code/session_01VcupomBPg7kuobwLoNpxEW

[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-11-03T05:55:52.193417Z	864f696	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-review reviewed cfa662042b9e98a312af3df1f66b06150092054b: read 4 of 4 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #12717, reading a lanes App head's pass by exact name in FAIL index mode

No findings. I read every hunk in all 4 changed files: .github/workflows/pr-require-checks.yml, scripts/ci-lanes-pr-hold.test.sh, plugins/source-control/skills/babysit-prs/scripts/tests/test_verify_rules_sync.py and the changelog fragment .changes/source-control/fix-lanes-hold-index-mode-pass-read-3f9c2a71.md.

The previous round (at 864f696) covered the workflow and the shell test. Its notes on index-mode detection, building the listing, error handling and the test cases still hold for those lines. This round also covers the two files added since then.

What I checked

• pass_names uses $digest after it is set. digest is assigned at L6339, before the array at L6439. Both the new index-mode branch and the existing loop therefore get the full digest-suffixed names.
• read_api in this step (L6219) passes ${@:2} on to the reader, so --jq '.artifacts[]' reaches gh api. If all three tries fail, it prints ::error::could not read … and returns 1, which || exit 1 then turns into a failed step.
• Sync test extraction. queue_lookup_script now copies the pass_names=( line through the first line ending in ). The array's first line ends in ", so the search stops at the second line. Without this change, the copied loop for name in "${pass_names[@]}" would loop over an unset array and the sync tests could not find a pass. The test's bash -e -c harness handles arrays, and the index-mode branch is not copied, so the test's read_api stub (which reads $2 as a run path) is never called with an artifact query.
• Changelog fragment. bump: none fits: the only plugin file touched is a test, and the workflow and scripts/ test do not ship in the plugin.

Not verified by me: the PR's 303/303 pass count for scripts/ci-lanes-pr-hold.test.sh comes from the author. I had no shell, so I did not run it or the Python sync tests.

Closing issue: none. The PR says no issue was filed (it fixes a regression from #12558), so there are no acceptance criteria to check.

Prior findings: the only earlier entry is the summary comment from the last round, which had 0 findings. Nothing is restated.

Spend (not a finding): up to 4 more GITHUB_TOKEN exact-name artifact reads per lanes App head per merge-group run. This uses only the API rate pool and does not cost money.

[github-actions]

claude-security-review reviewed cfa662042b9e98a312af3df1f66b06150092054b: read 4 of 4 changed file(s); 0 finding(s), 0 as inline comments.

Security review: #12717 (head cfa6620)

No security issues found.

Scope: I read every hunk of all 4 changed files:

• .changes/source-control/fix-lanes-hold-index-mode-pass-read-3f9c2a71.md: a changelog entry with bump: none.
• .github/workflows/pr-require-checks.yml: a trusted path, and one of the ci-status merge-hold steps for lanes App pull requests. I read its whole change and the step around it.
• plugins/source-control/skills/babysit-prs/scripts/tests/test_verify_rules_sync.py: test only.
• scripts/ci-lanes-pr-hold.test.sh: test only.

What changed since the last review (head 864f696): The four pass-artifact names are now built once, in the pass_names array (L6439-L6440). Both the new index-mode exact-name read and the existing verdict loop (L6463) use that array. The loop's old inline list held the same four names, so the loop behaves the same. The sync test now also extracts the array along with the loop. Nothing that ships changes there.

Merge-hold check (repeated against this head):

• The index-mode branch can't be turned on from a pull request. It runs only when verify-legacy-fails.json exists beside $VERIFY_EVIDENCE_SCRIPT (L6445). That variable points into the .base/ checkout (L6147), so pull request content can't create the file.
• A pass still counts only when its run checks out. Each artifact found has its run read live. It counts only for a successful .github/workflows/pr-verify.yml workflow_run run on main in this repository (L6476-L6483). The names contain this pull request's number, head and digest, and the API's name= filter matches exactly, so an artifact from another pull request can't be counted.
• Failures still hold the head. If an exact-name read fails, the step exits 1 (L6450). If the listing doesn't parse, it exits 1. If no pass is found, the head is held. Outside index mode, a reader that allows a head but saves no listing still fails closed. The refusal path is unchanged.
• No injection surface. The interpolated values are pr (checked numeric), head (checked 40-hex) and digest (a sha256 hex prefix). All are validated before this point in the step.

No merge hold is weakened. No trust control is widened. No token scope changes. No manifest changes.

Spend: each lanes App head in a merge-group run makes up to 4 more GITHUB_TOKEN REST reads. This uses API rate budget only and costs no money.

Other:

• The PR description contains no embedded instructions to the lane.
• Prior findings: the earlier summary comment (head 864f696) reported 0 findings, so nothing carries over. Round cap not reached.

critical-findings: 0