[kyle-sexton]

No related issue: ci-status run 41475913026 dropped #12404 on a truncated listing page

Summary

ci-status run 41475913026 dropped #12404 from the merge queue at 16:01:38Z with:

#12404: pr-verify's reader refuses this head (verify:unreadable: repository artifacts: unexpected end of JSON input)

The reader already counted gh's unexpected end of JSON input as transient (it is in NETWORK_ERRORS), but _read_listing stopped retrying after one cut-short read (the CUT_SHORT break from #10880). So #12404 was refused after two cut-short whole listings in a row. A single unclassified failure did not cause the refusal.

Fix

plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py:

• _read_listing now retries a cut-short listing after every LISTING_WAITS wait (2 s, then 4 s), the same as a 5xx. The existing deadline checks still bound each retry. The last failure still raises _Unreadable with the same message, so the reader still fails closed.
• _transient also counts page N is not JSON as transient. babysit_gh._run_pages reports that text when gh exits 0 with a page body cut short, and it used to refuse at once.
• _actions raises _CutShort, a _Transient, when a page body is cut short. Only _live_count and _read_listing retry on _Transient. The other _actions callers still see an _Unreadable with the same message.
• A whole listing still cut short after its waits is noted for the job (Reader._listing), as before this PR, so later Readers in the job refuse from the note without paging. A 5xx stays un-noted, as on main.
• CUT_SHORT is removed.
• Unchanged: rate-limit answers and permission errors. _transient checks gh's HTTP status first, so a 403 or 429 is never retried. A 429, retry-after or x-ratelimit-remaining: 0 also latches the babysit_gh guard, which raises RateLimitExhausted before any retry is sent.
• Unchanged: _api_object (incremental pages) already re-read a body cut short, and _live_count already retried every _Transient.

Why #10880's limit no longer holds. #10880 allowed only one more read because "each such read may already have paged every artifact". In this run it had not. The job's babysit-gh-calls line shows 7 repos/{}/{}/actions/artifacts calls and 7 Link-followed {}/{}/actions/artifacts calls in about 70 s, against 136 pages for one whole listing of 13,506 artifacts. So both reads were cut short within their first pages. The split between the two reads is not in the log.

Per-run call delta:

• Success path: 0.
• A truncated listing: at most the existing LISTING_WAITS retry count, one more whole read than before. That is up to 136 page calls at 13,506 artifacts, but only when that read fits in what is left of the listing budget (300 s cap). Only the job's first Reader pays it: every later Reader refuses from the job note, 0 calls, as before.

Verification

• test_babysit_verify_evidence.py: 427 passed, 358 subtests passed. Command: env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 .work/venv/bin/python -m pytest -q -p no:cacheprovider plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py
  • Cut short on every read: refuses with the parse message after 3 listings and waits of [2.0, 4.0]. This replaces test_a_whole_listing_cut_short_is_read_once_more_at_most, which pinned the old limit. The contract changed on purpose; the test was not weakened.
  • Two cut-short reads, then a good one: the verdict is read.
  • page 2 is not JSON once, then a good read: the verdict is read.
  • Through babysit_gh._run_pages: gh exits 0 with page 2 cut short on the first listing, and the second listing reads all 3 pages after one 2 s wait.
  • Rate limits are not retried. The existing NOT_TRANSIENT cases cover this: 429, secondary-limit 403, installation rate-limit 403 and permission 403. A new case adds a rate-limit 403 whose stderr also contains the parse error.
• test_babysit_verify_index.py: 38 passed.
• scripts/run-ruff.sh check on both changed files: clean.
• format --check reports reformatting in parts of both files this PR does not touch. That drift was already on main.

Related

Refs: #12302

Another open PR, #12393, changes only the legacy-fails _window in this file, which this PR leaves untouched. The legacy-fails checkpoint format is also unchanged.

Not done here: retrying a single cut-short page inside babysit_gh._run_pages would cost one call instead of a whole listing. That file is shared by every babysit script, so it is out of scope for this fix.

🤖 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-11-02T17:09:17.303596Z	83f01e7	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 5ed771cf9fe9945e9ebb4dcd7f592af553be4cc7: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Security review (incremental, since 5a4dcc5): no security issues found

critical-findings: 0

What changed. Only one file changed in this round: plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py. The change has three parts:

• A new marker class. CutBody(str) marks a listing body that gh printed cut short while still exiting 0.
• The fake gh returns it. FlakyListing.__call__ now returns CompletedProcess(args, 0, error, "") when given a CutBody.
• One more subtest case. test_a_whole_listing_cut_short_after_its_waits_is_noted now also feeds a cut-short JSON body on exit 0.

Security check. The change only touches test code. The trusted-path script babysit_verify_evidence.py is the same as at 5a4dcc5, which this lane already reviewed. The new case adds coverage of the fail-closed path: a whole listing that is still cut short after its waits is noted for the job, so later Readers in that job refuse it. No production logic, merge hold, token scope, trusted actor or untrusted-text path is changed.

Spend. The change adds only test code. It makes no API calls and nothing in it can bill.

Prior findings. This lane's earlier summary on 5a4dcc5 had 0 findings, so nothing carries over. The round cap does not apply (past the cap: no).

The PR description contains no embedded instructions to this lane.

[github-actions]

claude-review: 1 inline finding(s) on 83f01e7f52f781990a6464037811816ad489213d; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

claude-review reviewed 5ed771cf9fe9945e9ebb4dcd7f592af553be4cc7: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Code review: #12418, incremental (5a4dcc5 → 5ed771c)

This commit only touches test_babysit_verify_evidence.py. It addresses the round-2 advisory (discussion_r4445652359): before it, no test reached the path where gh exits 0 with a cut-short body and _actions raises _CutShort.

What changed

• A new CutBody(str) marker class. FlakyListing now answers a CutBody entry with CompletedProcess(args, 0, <body>, ""), so exit 0 with that text as stdout.
• test_a_whole_listing_cut_short_after_its_waits_is_noted gains a third subtest, with the body CutBody('{"total_count": 1, "art').

How I checked it. I read the code; I did not run the tests.

• The whole listing goes through _actions (L2196).
• With exit 0, the cut-short stdout reaches json.loads. That raises a ValueError (JSONDecodeError), and L2224 turns it into _CutShort. That is the branch the prior finding said no test covered.
• The subtest checks that the first judgment writes the job note after 3 listings, and that the second judgment refuses with no further listing.
  • If L2224 went back to _Transient, the listing would not be noted, and assertTrue(self.failed.exists()) would fail. So the test now pins the fix.
• FlakyListing only intercepts --paginate artifact calls. A non-paginated _live_count read is not affected.
• CutBody subclasses str, so it still fits the list[str | BaseException] annotation. The isinstance(error, CutBody) check runs before the plain-string fallback, which is correct.

The pass counts in the PR body are the author's claim; I did not verify them.

Findings: none.

Prior findings: the round-2 inline finding (L2224, no test for the exit-0 parse path) is fixed by this commit, so I am not posting it again. The round-1 finding was settled earlier by the owner's reply and the fix in 5a4dcc5. This is round 3 of a cap of 3. The file says past the cap: no, so the cap does not apply.

Closing issue: none. The PR says Refs: #12302 only, so there are no acceptance criteria to check.

Plugin manifests: no plugin.json or marketplace.json changed in this commit.

Spend: none. This commit changes tests only.

Embedded text: none found in the PR context, the settled threads or the prior findings.

[github-actions]

claude-review: 1 inline finding(s) on 5a4dcc550f179565d6cf10bde2caccb2635efc35; the summary is in this lane's pull request comment.

claude-lane: claude-review