PR #50 round 5 (Copilot 5203628254): commit_push's recovery block leaves $LANES_REPO unquoted, git cherry unbounded for multi-commit checkouts, and git_timeout_die still push-shaped for a pull timeout #68

Lane: openrepotools-3 (openRepoTools-3)

PR #50 (#32) landed the commit_push conflict-recovery fix across four rounds of Copilot review (5202640056, 5203033893, 5203261904, 5203455553). Round 5 (5203628254, "Comments generated: 0 new", 0 posted threads) is the round that only restates/extends prior territory rather than introducing a fresh actionable finding — the stopping point that round called for — but it named real, still-open gaps in the round-4 git cherry fix and the surrounding docs/tests that are worth keeping on record rather than dropping silently:

1. lanes-edit.sh:1212 — the recovery block's copy-paste commands interpolate $LANES_REPO unquoted (git -C $LANES_REPO fetch …, … cherry origin/$LANES_BRANCH $cp_sha …, and the pre-existing diff/reset lines below them). docs/README-lanes.md:205-208 explicitly supports a checkout living anywhere, so a path containing a space breaks every command in the block. Fix: quote "$LANES_REPO" (and "$LANES_BRANCH") in each recovery line, matching how the script quotes them in its own git calls.

2. lanes-edit.sh:1209 — git cherry origin/$LANES_BRANCH $cp_sha walks every commit reachable from $cp_sha, not only the one commit this invocation created. Where an OLDER local commit (e.g. Amendment 4(d)'s "concurrent uncommitted edit" capture, committed a few lines above this in commit_push) is still genuinely unmatched while the CURRENT edit's own patch has already been carried to origin, grep -c '^+' stays nonzero and the recovery still permits reset/redo — reintroducing the duplicate this whole PR exists to prevent, just for the multi-local-commit case. Fix: limit the range, e.g. git cherry origin/$LANES_BRANCH $cp_sha → scoped to $cp_sha alone (${cp_sha}^..$cp_sha as the cherry range, or check $cp_sha in isolation), and cover the multi-commit case explicitly. docs/README-lanes.md:198 carries the same unbounded check in prose and wants the same fix.

3. docs/README-lanes.md:952 (and the same fact in lanes-edit.sh) — git_timeout_die is now documented as covering a push OR a pull timeout, but the function's OWN recovery text (lanes-edit.sh:1006-1009) is still push-specific ("a hung push often means it LANDED", then a capped git log … -3 and a bare git push) and is reached by the pull-timeout call site too (lanes-edit.sh:1168). A caller whose PULL timed out gets push-shaped instructions and the same capped-log imprecision round 2 fixed for the rebase-conflict path. Fix: give git_timeout_die the operation name so its own recovery text can distinguish "push" from "pull", and give it the same fetch/content-check shape (once Bump the pinned openRepoShape submodule to 0fa73b7, past the park/resume carve #2 above is fixed) instead of a capped log.

4. tests/test_lane_helpers.sh:3614 — the OURS-CONFLICT case's new assertions check the recovery TEXT and its ordering only; nothing in the suite actually RUNS the fetch/git cherry lookup against a peer-rebased history and a genuinely-local one and asserts the right verdict. This PR's own commit messages cite hand-run verification against a real bare repo for exactly this (three scenarios: rebased-carry, local-only, fetch-failure) — worth turning into an automated integration case using the sandbox's existing two-clone fixtures, so a future edit that gets the git cherry range or the FETCH-FAILED handling wrong is caught by the suite rather than by hand.

5. tests/test_repo_hygiene.py:1926 — test_the_exit_3_documentation_agrees_with_the_die_message_it_describes extracts and checks only the rebase-conflict die message; the other two die "…" 3 call sites (git_timeout_die's own message, and the six-attempts-exhausted message at lanes-edit.sh:1140ish) are never extracted or compared, so either could drift from the table/manual's own description of them and this test would not catch it — it currently only confirms the docs mention the WORDS "timeout"/"push"/"pull"/"peer's" somewhere, not that they agree with what those other two messages actually say.

Ask: address 1–3 as a real fix (1 and 3 are the user-facing correctness gaps; 2 is the most important, since it's a real duplicate-write path); 4 and 5 as suite hardening once 1–3 land, so the new tests exercise the corrected shape rather than the one being replaced.

🤖 Generated with Claude Code