Skip to content

refactor(source-control): drop the verdict reader's unused listing-carry input - #13141

Merged
kyle-sexton merged 4 commits into
mainfrom
chore/drop-listing-carry-input
Nov 4, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
chore/drop-listing-carry-input

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: a security-review P5 follow-up on #13040 (dead reader input). Issues are being driven to zero, so none was filed.

Summary

#13040 removed every workflow's listing carry, but the verdict reader kept its input. LISTING_CARRY_ENV (BABYSIT_VERIFY_LISTING_CARRY) and the carry-path load in _incremental_base stayed behind, and the reader's own docstring says the env "names, which no workflow sets". No file under .github/workflows or .github/actions on main sets it (git grep), and pr-merge's workflow test already asserts that (workflow.test.mjs:900-901).

Fix

  • babysit_verify_evidence.py: remove LISTING_CARRY_ENV, the carry_path field and its load, and the docstring and comment mentions. Index mode and --shared-listing (BABYSIT_VERIFY_LISTING) are untouched. LISTING_CARRY_MAX_AGE stays: it still bounds the incremental base's age (:1539, :1543).
  • verify-cap.test.mjs: drop the two carry-listing tests and the env assertion. verify-cap.mjs needed no edit, since its carrySteps handles only the live BABYSIT_RUN_CACHE and BABYSIT_VERIFY_HEAD_REFS.
  • Python tests: about 50 incremental-listing tests used the carry file as their base. They now seed the run's saved listing instead (class renamed TheListingBuildsOnASavedOne). test_enable_auto_merge now expects 2 listing counts instead of 1, because the test had modelled a carry no workflow sets. Production behaviour is unchanged.

Call delta (R34): 0. The removed path never ran in production.

Verification

  • pytest plugins/source-control/skills/babysit-prs/scripts/tests: 4,111 passed, before and after merging origin/main.
  • node --test on verify-cap and pr-merge workflow tests: 136 pass.
  • ruff check passes.
  • After merge (R21): the next two pr-merge runs' enable-auto-merge and merge steps, and pr-unblock-dispatch's verify-cap, go green.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits November 4, 2026 02:29
… input

No workflow sets BABYSIT_VERIFY_LISTING_CARRY, so the reader's carry path
and its environment constant are removed. Incremental-listing tests now
seed the run's saved listing instead, and the carry-only verify-cap tests
are deleted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

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-04T07:40:15.279056Z c0e1acb PR opened
ℹ️ 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

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

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

Security review: PR #13141, incremental review (c0e1acb → 4ab0d83)

I found no security issues.

Scope: One file is in scope, plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py, and I read every hunk of its change in .claude-lane/incremental.diff.

What changed: The push only changes tests, in the APrMergeTickListsOnlyBeforeAnAllowOrAKeep class:

  • Seed removed: setUp no longer builds the carried-listing seed, and environ() no longer writes a merge-listing.json file for the merge job. Each job now starts with no saved listing.
  • Assertions updated:
    • The skip filter's first listing is now expected in full mode instead of incremental.
    • The expected listing counts each go up by one (2 + allows, 3, 5), to cover the skip filter's top-up page.

This matches the earlier round's removal of the dead BABYSIT_VERIFY_LISTING_CARRY input: the test no longer simulates a carry that no workflow sets.

Trust controls: The merge-hold verdict reader, babysit_verify_evidence.py, did not change in this push. The tests now expect equal or more full listings, never fewer, so nothing here narrows what the reader catches.

Other checks:

  • No instruction-surface deletions.
  • No plugin manifest changes.
  • No changes to token scope, trusted actors or the kill switch.
  • No owner friction holds were touched.
  • The PR context file holds no embedded directives.
  • No spend or cost impact.

Prior finding: The earlier summary comment (#13141 (comment)) reported 0 findings, so nothing carries over from it.

critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

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

Code review: #13141, incremental review (c0e1acb → 4ab0d83)

I read the whole change to the one file in scope, plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py. The incremental diff covers every hunk of it, so the read is complete.

What changed. This push fixes the one finding from my last round: the merge-job fixture still seeded a carried listing that production never has.

  • setUp no longer builds self.seed.
  • environ("merge") no longer writes merge-listing.json, so the merge job starts with no saved listing, the same as arm and as production, where BABYSIT_VERIFY_LISTING points under runner.temp.
  • The skip filter's assertion is now listing_mode == "full".
  • Each listing count goes up by one for the top-up's first page: 2 + allows, slow job 3, slow queued job 5. This is the same +1 that test_enable_auto_merge already makes (self.listings == 2, "One whole listing … and its top-up's first page").
  • The class docstring and the skip-filter comment no longer describe a carried base.

Check. The new counts match the fixture's accounting:

  • The answer() fake counts only page-1 listing requests.
  • A full listing followed by its top-up's first page adds exactly one page-1 request, the same as arm_job.
  • The slow queued arithmetic still matches its comment: 2 skip-filter, plus 2 keeps past the bound, plus 1 allow, makes 5.

I did not run the tests. The owner's reply reports 4,111 babysit-prs passes; that is author-claimed and unverified.

Prior finding. The thread at #discussion_r4464917351 is resolved, and this diff addresses it. I'm not raising it again.

Findings. None new in this increment. The "Minor, not counted" naming notes from last round are outside this increment's scope, and I'm not restating them.

No closing issue and no plugin manifest change in scope. I found no embedded instructions in the PR context, settled-threads or prior-findings files.

kyle-sexton and others added 2 commits November 4, 2026 02:48
…ved listing

The merge job's listing file starts empty each run, so the fixtures no
longer seed a listing from an earlier tick. The skip filter now reads a
whole listing, and the listing counts include its top-up's first page.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit a064c47 Nov 4, 2026
48 checks passed
@kyle-sexton
kyle-sexton deleted the chore/drop-listing-carry-input branch November 4, 2026 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant