[brettheap]

Refs #138 (items 6, 13, 15)

The three wrapper items of #138, each a read the wrapper could not make and then took for an answer, plus the older gaps beside two of them and the regression a review found in the first round of this PR's own fix (below). Each is re-proved on main f87fd5a (none was already fixed; main's commits since do not touch these functions), fixed fail-closed the way the issue describes, and has a proving test in the new tests/test_wrapper_hardening_138.py. The other items of #138 (lanes-edit.sh, and lane-start items 3, 14, 16) are not touched.

Item 13: lane, go_live, the window-name collision check

• On main: lane:849-855 asked only register-row, which reads the PUBLISHED row, before renaming a lane-shaped window. A lane whose add-row or session stamp is committed here and not pushed answers 8 there, and its window was renamed out from under it.
• Fix: lane go_live, lines 851-863: when the published read answers 8, ask register-row-local too. A local row refuses (same message as before), 8 passes, any other status refuses with "could not be read from the published or the local register (exit n)". The local read of the target lane just above already did this.
• Tests: test_live_attach_refuses_a_window_named_for_a_row_not_yet_pushed, test_live_attach_fails_closed_when_the_local_row_cannot_be_read (local exits 1 and 2), and test_live_attach_renames_a_window_no_register_row_names (both reads 8 renames and attaches, and asserts that both registers were asked; it fails on main because the local read is not made there). The control is test_live_attach_still_refuses_a_window_named_for_a_published_row.

Item 15: lanes, the empty listing's hidden-row probe

• On main: lanes:566 piped lanes --closed into command grep -q . under set -o pipefail. Past a pipe buffer of rows grep -q exits on the first line, the producer takes SIGPIPE, the pipeline answers 141, e_hidden stays 0, and the listing said "no lane ... is recorded" about a repository with hidden rows.
• Fix: lanes, the case "$rc" arm 8), lines 562-586: the probe reads the whole output into a variable, so nothing closes on the producer, and keeps the producer's status.
• Older gap, same lines: if the --closed read FAILED (exit 1, with or without rows) main also said "no lane of ... is recorded". The status now decides. A status of 0 or 8 is an answer: any output on either is hidden rows (an 8 that printed rows is a contradiction and fails towards showing them), and no output on either is none. Any other status prints "whether closed or dormant lanes are recorded could not be read (lanes-edit.sh lanes --closed exited n)" and claims nothing. The exit stays 8, since nothing is listed.
• Tests: test_empty_listing_says_closed_rows_are_hidden_however_many_there_are (1 row, and 40000 rows, ~5 MB, far past a pipe buffer), test_empty_listing_claims_nothing_when_the_hidden_row_read_fails (exits 1 and 2, with 0 and 3 rows), test_empty_listing_treats_rows_printed_before_an_exit_8_as_hidden, and the control test_empty_listing_of_an_empty_repository_still_says_none_is_recorded.

Item 6: lane-end --retire-dormant, the stderr capture

• On main: lane-end:711-719: when mktemp could not make the capture file the listing ran with its stderr uncaptured, rd_err stayed empty, "session records could not be read" never reached the check that refuses on it, and the sweep went on (dry run and --yes alike).
• Fix: lane-end, the --retire-dormant block, lines 723-745: refuse (exit 1, "Nothing was written.") when the capture file cannot be made; the uncaptured else branch is gone.
• Older gap, same block: the note is relayed with printf '%s' "$rd_err" >&2, and where that write fails (a closed or full stderr) set -e left before the rm and the lane-end-sweep.* file stayed in $TMPDIR. The capture file is now assigned to LE_TMP, which the EXIT trap's cleanup removes, straight from the mktemp (LE_TMP="$(mktemp ...)"), so no signal can land between the file existing and cleanup knowing its name.
• Regression found in review of that, and fixed here: lane-end had one trap, trap cleanup EXIT INT TERM, and cleanup returns rather than exits, so a TERM or INT to the lane-end shell alone resumed the script after deleting LE_TMP. The resumed script read the deleted capture as an empty note, skipped the unread-records refusal, and --yes called retire-rows over a fact that was never read. Now (lane-end 548-557) cleanup is the EXIT trap only, and INT and TERM exit with 130 and 143, so the EXIT trap removes the file once. Nothing in lane-end relied on resuming after a signal (this is its only trap). The same trap also stops a signal from resuming the ordinary path, whose lane-objects read is LE_TMP too.
• Help table: the EXIT table --help prints (lane-end 238-242) now says what exit 8 means ("nothing there": for --retire-dormant, no lane of that repository recorded or none of its rows DORMANT; for --retire <pid|uuid>, nothing of any kind live for the lane) and lists the 130 and 143 above, beside 0, 1 and 2.
• Same block: rd_err is now read with cat -- "$rd_errf" and a failed read-back is a refusal ("could not be read back ... Nothing was written."), not an empty note, which is the one note that never refuses.
• Tests: test_sweep_refuses_when_the_stderr_capture_cannot_be_made (dry run and --yes, a failing mktemp shim on PATH, asserting retire-rows is never called), test_sweep_leaves_no_capture_file_when_its_note_cannot_be_relayed (stderr is /dev/full; skipped where there is none), test_sweep_does_not_resume_after_a_signal_during_the_read (TERM and INT to the shell alone, mid-read, with the unread-records note present: exits 143 and 130, 0 retire-rows calls, no capture file; INT skips where SIGINT is ignored at entry), test_sweep_refuses_when_the_capture_cannot_be_read_back (a cat shim failing on the capture file), test_sweep_leaves_no_capture_file_when_a_signal_lands_as_it_is_made (a mktemp shim that sleeps after making the file; TERM and INT to the shell alone leave no lane-end-sweep.*), plus controls that the unread-records refusal and the ordinary dry run and act still behave.

Lines touched, for lane openRepoTools-5

lane: inside go_live, 851-863 only. lanes: 562-586. lane-end: 238-242 (the help table), 548-557 (the trap) and 723-745 (outside the fenced 200-215 and 1971-2143, and clear of the case "$rd_err" arms below, which #230 touches). No other file besides the new test.

Verification

• The fakes (tmux, mktemp, cat, lanes-edit.sh) are on a controlled PATH; nothing touches a real register or tmux.
• Proof against main, same test file, run under the shared pytest lock with OPENREPOTOOLS_CODE_ROOT pointing at a git archive f87fd5a copy (the first round's 18 tests): 12 failed, 6 passed. The six that pass are the controls: the published-row refusal, item 15's 1-row case and its empty repository, item 6's two unread-records refusals, and the ordinary dry run and act. With that round's fix: 18 passed.
• The regression, by hand against a git archive fe6e38a copy, a fake helper that prints the unread note and sleeps 3 s, and a TERM or INT to the shell alone at 1.2 s: at fe6e38a exit 0 and 1 retire-rows call; with this round's fix exit 143 and 130 and 0 calls.
• Delta round 1 (head a564b18), same file, under the shared lock, against the git archive fe6e38a copy: 3 failed, 18 passed, 1 skipped. The three are the TERM case, the unreadable capture and rows printed before an exit 8; the skip is the INT case, because a background job starts with SIGINT ignored and no script can trap it, so INT was driven by hand (exit 130 on the fix, exit 0 and 1 retire-rows call at fe6e38a). On the fix: 21 passed, 1 skipped. The bash lint tests of tests/test_repo_hygiene.py selected with -k: 100 passed.
• The mktemp window, by hand against a git archive a564b18 copy with a mktemp shim that sleeps after making the file and a TERM or INT to the shell alone: at a564b18 one empty lane-end-sweep.* file is left behind (exit 143 and 130, 0 retire-rows calls); with this round's fix none is.
• Delta rounds 2 and 3 (head ae1b37b, on the merge of main f5c3bcd), under the shared lock with every LANES_* and TMUX* unset: tests/test_wrapper_hardening_138.py and tests/test_repo_hygiene.py together, 213 passed, 0 skipped (the INT cases included), at 80f46b2 and again at ae1b37b, which only rewords the help table.
• CI's Linux tests job is the suite of record; the full suite was not run locally.

🤖 Generated with Claude Code

Summary by Sourcery

Harden lane, lanes, and lane-end wrappers so failed reads and interrupted operations cannot be mistaken for safe answers or proceed to unintended changes.

Bug Fixes:

• Make lane window-name collision checks fail closed when unpublished register rows exist or local register reads fail.
• Prevent lanes from misreporting hidden rows as absent when large listings or failed closed-row reads occur.
• Make lane-end --retire-dormant refuse safely when stderr capture cannot be created or read, and prevent signal handling from continuing into retirement without valid records.

Enhancements:

• Clarify lane-end exit-status help and ensure temporary capture files are cleaned up across errors and signals.

Tests:

• Add comprehensive wrapper hardening tests covering register-read failures, large hidden-row listings, capture failures, signal interruptions, cleanup, and regression controls.

[sourcery-ai]

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 23 hours by commenting @sourcery-ai review. Upgrade to get a review now.

[sourcery-ai]

Reviewer's Guide

Hardens three wrapper reads to fail closed: lane checks unpublished local rows before window changes, lanes fully consumes hidden-row output, and lane-end --retire-dormant refuses when stderr capture cannot be established. A focused test module uses controlled fakes to prove failure paths and preserve existing behavior.

File-Level Changes

Change	Details	Files
The lane window-name collision check now validates both published and local register state before renaming or attaching.	Performs a local-register read when the published read reports no row. Refuses on a local row and fails closed on other local-read errors. Adds controlled tmux/register fakes covering unpublished rows, read failures, and existing published-row behavior.	lane tests/test_wrapper_hardening_138.py
The hidden-row probe now consumes the complete closed-lane stream instead of using an early-exiting grep pipeline.	Replaces grep -q with an awk scan that avoids SIGPIPE/pipefail being interpreted as an empty result. Tests both a single hidden row and a large 40,000-row stream, plus the genuinely empty case.	lanes tests/test_wrapper_hardening_138.py
Dormant-lane retirement now refuses before any write when its stderr capture file cannot be created.	Removes the uncaptured fallback and exits with the existing no-write refusal. Verifies capture-creation failure for dry-run and --yes, while retaining unread-record and normal dry-run/act coverage.	lane-end tests/test_wrapper_hardening_138.py

Possibly linked issues

• PR #93 Copilot rounds past the two-round cap: eighteen fail-open hardenings from rounds 7–21 and the Codex connector (d0310f6–8fc5e1a), filed rather than taken #138: The PR implements the issue’s three specified wrapper fixes: items 6, 13, and 15, with tests for each.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Sourcery assessment

Approved.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud