From 0266a2d1bb6e12a564f5b86df173003669d1a8f6 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 14 Sep 2026 19:48:00 +0000 Subject: [PATCH 1/5] WIP #32: `commit_push`'s true-conflict die stops claiming "nothing was pushed" when a later write from the same shared checkout may already carry the commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Checkpoint commit at the coordinator's request, ahead of merging main's #41 (Amendment 15, canon_lane, 8345d2f) into this branch. `bash -n lanes-edit.sh` and `bash -n tests/test_lane_helpers.sh` are both clean; the full `bash tests/test_lane_helpers.sh` run and `python3 -m pytest tests -q` are still in flight / pending re-run after the merge, and are not yet reflected here. Contents, inherited from the predecessor's uncommitted work and reviewed against issue #32's five occurrences (2026-09-13/14, Eagle, several lanes writing the register concurrently) and its own comments: - `commit_push`'s true-rebase-conflict branch (lanes-edit.sh ~1048-1062) reworded from an unconditional "rebase conflict on origin/main — nothing was pushed." to "…— not pushed by this attempt; a later write from this checkout may already carry it, so look before you retry.": three of the five observed occurrences named a commit that WAS already on origin by the time anyone read the die, carried there by the very next `lanes-edit.sh` write out of the same shared checkout. The RECOVERY lines are reordered to fetch-and-look at origin first, and the diff/reset instructions are now conditional on "only if it is not there". - Two new cases in tests/test_lane_helpers.sh: RACE-RETRY-WINS (a second clone lands a peer commit between this invocation's own pull and its push; asserts the final message names the attempt that actually pushed, never attempt 1's rejection, and that the row's text is written once, not twice) and OURS-CONFLICT (the peer rewrites the exact same row, a real conflict; asserts the abort truly leaves the worktree clean, the new wording appears, the old unconditional "nothing was pushed." does not, and the look-before-reset ordering holds). Lane: openrepotools-3 (openRepoTools-3) Co-Authored-By: Claude Fable 5.1 --- lanes-edit.sh | 15 ++++- tests/test_lane_helpers.sh | 112 +++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 3 deletions(-) diff --git a/lanes-edit.sh b/lanes-edit.sh index aaee25b0..c46bb162 100755 --- a/lanes-edit.sh +++ b/lanes-edit.sh @@ -1045,12 +1045,21 @@ commit_push() { git -C "$LANES_REPO" rebase --abort 2>/dev/null || : note "rebase ABORTED — the worktree is clean and NOT mid-rebase. Your edit is safe in these local commits:" git -C "$LANES_REPO" --no-pager log --oneline "origin/$LANES_BRANCH..HEAD" 2>/dev/null | head -n 10 >&2 || : + # #32 — AN ABORTED PULL IS NOT PROOF THIS COMMIT NEVER REACHED ORIGIN + # (the same rule RV-B3 took for the handoff stamp's own push, in + # lane-start). This checkout is shared with every lane on the + # workstation, so the very next write out of it pulls this dangling + # commit onto its own and pushes both together — three of the four + # "rebase conflict … nothing was pushed" lines seen on Eagle in one + # hour named a commit that was ALREADY on origin by the time anyone + # read them. LOOK before you reset or redo anything. note "RECOVERY (in that order):" - note " git -C $LANES_REPO diff origin/$LANES_BRANCH..HEAD -- ${CP_PATHS[*]} # read back exactly what you wrote" - note " git -C $LANES_REPO reset --hard origin/$LANES_BRANCH # drop the local commits (NOTE: also drops any" + note " git -C $LANES_REPO fetch origin $LANES_BRANCH && git -C $LANES_REPO log --oneline origin/$LANES_BRANCH -3 # a write that ran after this one may already have carried it" + note " git -C $LANES_REPO diff origin/$LANES_BRANCH..HEAD -- ${CP_PATHS[*]} # only if it is not there: read back exactly what you wrote" + note " git -C $LANES_REPO reset --hard origin/$LANES_BRANCH # then drop the local commits (NOTE: also drops any" note " # uncommitted peer edit in this checkout)" note " then re-read LANES.md and redo the edit with lanes-edit.sh, on top of the peer's version." - die "rebase conflict on origin/$LANES_BRANCH — nothing was pushed." 3 + die "rebase conflict on origin/$LANES_BRANCH — not pushed by this attempt; a later write from this checkout may already carry it, so look before you retry." 3 fi attempt=$((attempt + 1)) sleep 3 diff --git a/tests/test_lane_helpers.sh b/tests/test_lane_helpers.sh index 5cecb4e1..abf8d7dc 100755 --- a/tests/test_lane_helpers.sh +++ b/tests/test_lane_helpers.sh @@ -3497,6 +3497,118 @@ else fi rm -rf "$H_LOCK" +# ========================================================================= +# #32 — `commit_push` NAMES THE ATTEMPT THAT ACTUALLY LANDED, NEVER AN +# EARLIER ONE ORIGIN HAS SINCE OVERTAKEN. +# +# Observed on Eagle, three to four times in one hour on 2026-09-13/14 while +# several lanes wrote the register concurrently: `append-row-status` / +# `append-line` printed "rebase conflict on origin/main — nothing was +# pushed" for a commit that WAS on origin by the time anyone read the line — +# carried out by the very next lanes-edit.sh write through this same shared +# checkout, which pulls this dangling local commit onto its own and pushes +# both together. A caller who trusted "nothing was pushed" and redid the +# write wrote a duplicate line (issue #32). The second clone plays the peer +# that moves origin between this invocation's own commit and its push, +# exactly as it does for the lost-claim race above. +# +# THIS SANDBOX'S OWN CHECKOUT IS ALREADY DIRTY, from the rejectgit cases +# above (a stamp whose push or pull was faked to fail leaves its commit +# local, and the fixture files those cases wrote are still uncommitted +# where their own act never got to commit them) — real content, and none of +# it this section's. `dirty_elsewhere` would read it as "someone else's +# uncommitted file" and route every case below through Amendment 5(d)'s +# skip-the-pull branch instead of the rebase this section means to exercise, +# so it is committed first, exactly as its own author would. +# ------------------------------------------------------------------------- +if [ -n "$(git -C "$WIP" diff --name-only 2>/dev/null)" ]; then + git -C "$WIP" add -u -- . + git -C "$WIP" commit -q -m "fixture cleanup: commit content earlier cases left uncommitted, before the #32 race/conflict cases" + git -C "$WIP" pull -q --rebase origin main 2>/dev/null || : + git -C "$WIP" push -q origin main +fi +git -C "$CLONE2" pull -q --rebase origin main 2>/dev/null +RC_ID="aaaa0005-c0c0-4000-8000-aaaa0005c0c0" +"$E" add-row "| \`repoRC-1\` | harness \`$RC_ID\` | Eagle / test / brett | 2026-09-11T00:00Z | none | handoffs/repoRC/x.md | ACTIVE |" >/dev/null 2>&1 + +# -- a race a LATER attempt wins: not a conflict, and the message must name +# the attempt that actually pushed, never attempt 1's rejection — and the +# retry must not write the row's own text twice. +RACE_FLAG="$SANDBOX/race-peer-landed" +rm -f -- "$RACE_FLAG" +cat > "$SANDBOX/racegit" </dev/null 2>&1 + printf 'CLAIMED — lane repoH-2, session %s@Raven, %s, opensoft/repoH#92\n' \ + "$DEAD_ID" "\$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$CLONE2/lanes/log/repoH-2.md" + git -C "$CLONE2" add -- lanes/log/repoH-2.md + git -C "$CLONE2" commit -q -m "LOG(repoH-2@Raven): lands between this attempt's pull and its push" + git -C "$CLONE2" push -q origin main >/dev/null 2>&1 + fi +done +exec git "\$@" +FAKE +chmod +x "$SANDBOX/racegit" + +run env LANES_GIT="$SANDBOX/racegit" "$E" append-row-status repoRC-1 "RACE-RETRY-WINS" +is "a peer landing between this attempt's pull and its push still exits 0" "$rc" 0 +has "…the message names attempt 2, the one that actually pushed" "$err" "pushed origin/main (attempt 2)" +hasnt "…never claiming attempt 1 pushed it" "$err" "pushed origin/main (attempt 1)" +is "…and the retry writes the row's text once, never twice" "$(grep -c 'RACE-RETRY-WINS' "$LANES")" 1 + +# -- a race the rebase truly cannot resolve: the peer rewrote the SAME row. +# `rebase --abort` must leave the tree clean, the message must say so +# truthfully, and — because a write that runs after this one carries the +# dangling commit out regardless of what conflicted here — it must say to +# LOOK before resetting or redoing anything, the same rule RV-B3 took for +# the handoff stamp's own push in `lane-start`. +git -C "$CLONE2" pull -q --rebase origin main 2>/dev/null +PC_LINE="$(grep -n '^| `repoRC-1`' "$CLONE2/lanes/LANES.md" | head -n1 | cut -d: -f1)" +PC_ROW="$(sed -n -e "${PC_LINE}p" "$CLONE2/lanes/LANES.md")" +PC_TMP="$(mktemp -d)/pre" +cat -- "$CLONE2/lanes/LANES.md" > "$PC_TMP" +{ [ "$PC_LINE" -gt 1 ] && head -n "$((PC_LINE - 1))" -- "$PC_TMP" + printf '%s\n' "${PC_ROW%|}· PEER-CONFLICT |" + tail -n "+$((PC_LINE + 1))" -- "$PC_TMP" +} > "$CLONE2/lanes/LANES.md" +git -C "$CLONE2" add -- lanes/LANES.md +git -C "$CLONE2" commit -q -m "LANES(repoRC-1@Raven): peer rewrites the same row" +git -C "$CLONE2" push -q origin main + +run "$E" append-row-status repoRC-1 "OURS-CONFLICT" +is "a peer's rewrite of the SAME row is a real conflict: exit 3" "$rc" 3 +has "…REBASE CONFLICT is still named" "$err" "REBASE CONFLICT on origin/main" +has "…the abort runs and says the tree is clean" "$err" "rebase ABORTED — the worktree is clean and NOT mid-rebase" +is "…and it really is clean" "$(git -C "$WIP" status --porcelain | grep -c .)" 0 +has "…told to fetch and look before resetting or redoing anything" "$err" \ + "log --oneline origin/main -3 # a write that ran after this one may already have carried it" +has "…the reset is now conditional on that look, not unconditional" "$err" \ + "only if it is not there: read back exactly what you wrote" +has "…and the die itself is scoped to this attempt, never a permanent claim" "$err" \ + "not pushed by this attempt; a later write from this checkout may already carry it" +hasnt "…never the old unconditional claim an aborted pull cannot prove" "$err" \ + "rebase conflict on origin/main — nothing was pushed." +pc_look="$(printf '%s\n' "$err" | grep -n -- 'log --oneline origin/main -3' | head -n1 | cut -d: -f1)" +pc_reset="$(printf '%s\n' "$err" | grep -n -- 'reset --hard origin/main' | head -n1 | cut -d: -f1)" +is "…the look printed BEFORE the reset it gates, the same order RV-B3 pins" \ + "$([ -n "$pc_look" ] && [ -n "$pc_reset" ] && [ "$pc_look" -lt "$pc_reset" ] && printf 'look first' || printf "look $pc_look, reset $pc_reset")" "look first" + +# Suite housekeeping: this conflict is real and stays real (the peer's row +# rewrite is still on origin), so nothing later inherits this section's own +# dangling commit — and nothing later finds $WIP behind the peer's push +# either, which every case below this one assumes it never is. Fully synced +# to the CURRENT origin/main (the abort's own fetch already has it), not +# merely one commit back from wherever HEAD happened to be. +git -C "$WIP" fetch -q origin main 2>/dev/null || : +git -C "$WIP" reset --hard origin/main >/dev/null 2>&1 + # --------------------------------------- the pointer file, when it does NOT answer # # AMENDMENT 9(a): "Failing to find it is a refusal, never a guess." Six ways it From d770e4d30987af68a2c062579f48cd04128aa14d Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:18:04 +0000 Subject: [PATCH 2/5] `commit_push`'s exit-3 table and its recovery notes stop contradicting the die message they document, so a reader who already found the commit on origin is told to STOP instead of reset and redo it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot's round-1 review of #50 (5202640056) found two things this PR's own diff introduced without noticing: - lanes-edit.sh:188 (posted thread) — the exit-code table's row 3 still read "nothing was pushed, the edit is a local commit" after the die message a few hundred lines below it picked up the attempt-scoped wording ("not pushed by this attempt; a later write from this checkout may already carry it"). Reworded to match. docs/README-lanes.md:939 carries the same stale claim as the manual's own copy of the table and is reworded too. - lanes-edit.sh:1176-1181 (suppressed comment on :1180) — the RECOVERY block's "only if it is not there" qualifier was attached to the diagnostic `diff` line alone; the `reset --hard` and "redo the edit" lines after it stayed unconditional, so a caller whose fetch/log just showed the commit already on origin was still told, by those two lines, to reset and redo — the exact duplicate #32 exists to prevent. One new line between the look and the reset — "if that commit is already there, STOP — reset or redo now would write it a second time." — gates all three lines below it at once. Tests: - tests/test_lane_helpers.sh's OURS-CONFLICT case (~3600) asserts the STOP line's text and that it prints between the look and the reset it gates. - tests/test_repo_hygiene.py gains test_the_exit_3_documentation_agrees_with_the_die_message_it_describes, tying lanes-edit.sh's table and docs/README-lanes.md's copy to the die message's own wording so the three can't drift apart silently again. Verification: bash -n on both shell files, python3 -m py_compile on the test module, `bash tests/test_lane_helpers.sh` and `python3 -m pytest tests -q` both green (footers quoted in the PR). Lane: openrepotools-3 (openRepoTools-3) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019UFwD7bCS73jaUMJ2vuo5m --- docs/README-lanes.md | 2 +- lanes-edit.sh | 4 +++- tests/test_lane_helpers.sh | 14 ++++++++++++++ tests/test_repo_hygiene.py | 27 +++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 2 deletions(-) diff --git a/docs/README-lanes.md b/docs/README-lanes.md index 8db12a48..c96a868d 100644 --- a/docs/README-lanes.md +++ b/docs/README-lanes.md @@ -936,7 +936,7 @@ else. | 0 | done | | 1 | environment — no register, no writer | | 2 | refusal — bad arguments, the object is held, an unknown alias, a checkout that cannot be rebased | -| 3 | rebase conflict — nothing pushed, the edit is a local commit | +| 3 | rebase conflict — not pushed by this attempt; a later write from this checkout may already carry it to origin, so look before you retry | | 4 | the mutex could not be taken within 60s | | 5 | an edit moved more than one line and was refused | | 6 | `git add` / `commit` / `push` failed | diff --git a/lanes-edit.sh b/lanes-edit.sh index 8c51ec89..ca3dbc1f 100755 --- a/lanes-edit.sh +++ b/lanes-edit.sh @@ -185,7 +185,8 @@ # carry both without the caller guessing, so the newer reads spend a number # of their own and the older half of this file keeps 2 where it always was. # Clause (h)'s table is the contract for which read uses which. -# 3 rebase conflict — nothing was pushed, the edit is a local commit +# 3 rebase conflict — not pushed BY THIS ATTEMPT; a later write from this +# checkout may already carry it to origin, so LOOK before you retry # 4 the mutex could not be taken within 60s # 5 an edit moved more than one line and was refused — or, Amendment 15, a # lane's object log could not be renamed to the row's own spelling @@ -1174,6 +1175,7 @@ EOF # read them. LOOK before you reset or redo anything. note "RECOVERY (in that order):" note " git -C $LANES_REPO fetch origin $LANES_BRANCH && git -C $LANES_REPO log --oneline origin/$LANES_BRANCH -3 # a write that ran after this one may already have carried it" + note " if that commit is already there, STOP — reset or redo now would write it a second time." note " git -C $LANES_REPO diff origin/$LANES_BRANCH..HEAD -- ${CP_PATHS[*]} # only if it is not there: read back exactly what you wrote" note " git -C $LANES_REPO reset --hard origin/$LANES_BRANCH # then drop the local commits (NOTE: also drops any" note " # uncommitted peer edit in this checkout)" diff --git a/tests/test_lane_helpers.sh b/tests/test_lane_helpers.sh index 0ddee6a3..4519c2b2 100755 --- a/tests/test_lane_helpers.sh +++ b/tests/test_lane_helpers.sh @@ -3600,6 +3600,20 @@ pc_reset="$(printf '%s\n' "$err" | grep -n -- 'reset --hard origin/main' | head is "…the look printed BEFORE the reset it gates, the same order RV-B3 pins" \ "$([ -n "$pc_look" ] && [ -n "$pc_reset" ] && [ "$pc_look" -lt "$pc_reset" ] && printf 'look first' || printf "look $pc_look, reset $pc_reset")" "look first" +# Copilot's review of #50 (5202640056), suppressed comment on lanes-edit.sh:1180: +# the gate above was attached only to the diagnostic `diff` line — a caller +# whose fetch/log just showed the commit already on origin was still told, by +# the next two unconditional lines, to reset and redo, which is the exact +# duplicate this PR exists to prevent. A single STOP between the look and the +# reset gates all three lines below it (diff, reset, redo) at once, the same +# job the existing "only if it is not there" qualifier does for the diff line +# alone. +has "…and told to STOP there instead of resetting or redoing a landed commit" "$err" \ + "if that commit is already there, STOP — reset or redo now would write it a second time." +pc_stop="$(printf '%s\n' "$err" | grep -n -- 'STOP — reset or redo now' | head -n1 | cut -d: -f1)" +is "…the STOP sits between the look and the reset, gating both reset and redo" \ + "$([ -n "$pc_look" ] && [ -n "$pc_stop" ] && [ -n "$pc_reset" ] && [ "$pc_look" -lt "$pc_stop" ] && [ "$pc_stop" -lt "$pc_reset" ] && printf 'ordered' || printf "look $pc_look, stop $pc_stop, reset $pc_reset")" "ordered" + # Suite housekeeping: this conflict is real and stays real (the peer's row # rewrite is still on origin), so nothing later inherits this section's own # dangling commit — and nothing later finds $WIP behind the peer's push diff --git a/tests/test_repo_hygiene.py b/tests/test_repo_hygiene.py index 986f56ea..a251579e 100644 --- a/tests/test_repo_hygiene.py +++ b/tests/test_repo_hygiene.py @@ -1883,6 +1883,33 @@ def test_the_exit_code_table_carries_every_code_the_file_exits_with(): assert "64" in documented, "64 is the code every Amendment 11 read uses" +def test_the_exit_3_documentation_agrees_with_the_die_message_it_describes(): + """Copilot's review of PR #50 (5202640056): the `die` for exit 3 picked up + attempt-scoped wording (#32) — *"not pushed by this attempt; a later + write from this checkout may already carry it"* — while the exit-code + table just above it in the same file, and the manual's own copy of that + table (`docs/README-lanes.md`), both still read *"nothing was pushed"* / + *"nothing pushed"*, a claim an aborted pull never proved. A table (or a + manual) that contradicts the code it describes is worse than no table at + all — the same claim `test_the_exit_code_table_carries_every_code_the_ + file_exits_with` makes for a code missing from the table entirely. + """ + src = (REPO / "lanes-edit.sh").read_text(encoding="utf-8") + start = src.index("# EXIT CODES — every subcommand, one table") + end = src.index("# --no-sweep", start) + table = src[start:end] + manual = (REPO / "docs/README-lanes.md").read_text(encoding="utf-8") + for name, text in (("lanes-edit.sh's own exit-code table", table), + ("docs/README-lanes.md's copy of it", manual)): + assert "nothing was pushed" not in text and "nothing pushed" not in text, ( + f"{name} still claims exit 3 means nothing reached origin, which " + "an aborted pull does not prove (#32)") + assert "not pushed BY THIS ATTEMPT" in table, ( + "lanes-edit.sh's exit-3 row no longer names the attempt-scoped wording") + assert "not pushed by this attempt" in manual, ( + "docs/README-lanes.md's exit-3 row no longer names the attempt-scoped wording") + + #: ADOPTION ACT 0, AND THE ONE SHA THAT IS IT. `opensoft/brett-wip#5` merged #: 2026-09-13T19:14:37Z, SQUASHED — so the PR's pre-merge head is not an #: ancestor of `origin/main` and names code that never landed, while the merge From 6a3540eb75c06254c26efc862adcb7860f0d9b23 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:44:59 +0000 Subject: [PATCH 3/5] Round 2: the recovery's own "look" traded a capped `log -3` for a real ancestry check, and the manual's prose paragraph and this test's own blind spot got the same fix as the table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot's round-2 review of #50 (5203033893) found three more things, all downstream of round 1's own fix: - lanes-edit.sh:1177 (posted thread) — `git log --oneline origin/$LANES_BRANCH -3` is not proof of absence: four or more later writes can bury this attempt's commit below a three-line window while it stays an ancestor of origin, and the next line would then wrongly permit reset/redo. Replaced with an actual ancestry test: `git -C $LANES_REPO rev-parse HEAD` is captured right after `rebase --abort` (HEAD is back to this attempt's own commit there) into `cp_sha`, and the look becomes `fetch ... && merge-base --is-ancestor $cp_sha origin/$LANES_BRANCH && echo ALREADY-THERE || echo not-there`. The STOP line now reads "if that printed ALREADY-THERE" rather than depending on what a human sees in a capped list. - docs/README-lanes.md:939 (posted thread, actually about the prose two lines above the table at :185-190) — the manual's own conflict-recovery paragraph still told readers to diff/reset/redo immediately, with no fetch, no ancestry check, no STOP. Reworded to mirror the ordered recovery lanes-edit.sh prints. - tests/test_repo_hygiene.py:1910 (suppressed comment, no thread) — round 1's new test read only the table and the manual, so a regression in the `die` MESSAGE ITSELF back to "nothing was pushed" left both docs unchanged and passed anyway. The die is extracted and checked first now, so all three have to agree with the thing that actually runs, not merely with each other. Tests: tests/test_lane_helpers.sh's OURS-CONFLICT case now asserts the ancestry command and its ALREADY-THERE/not-there framing, that a real commit sha is named (not a placeholder), and the STOP line's new wording; the ordering check (look before reset) now keys off the ancestry line. Verification: bash -n on both shell files, python3 -m py_compile on the test module, `bash tests/test_lane_helpers.sh` and `python3 -m pytest tests -q` both green (footers quoted in the PR). Lane: openrepotools-3 (openRepoTools-3) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019UFwD7bCS73jaUMJ2vuo5m --- docs/README-lanes.md | 9 ++++++++- lanes-edit.sh | 10 +++++++--- tests/test_lane_helpers.sh | 13 +++++++++---- tests/test_repo_hygiene.py | 13 +++++++++++++ 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/docs/README-lanes.md b/docs/README-lanes.md index c96a868d..bcce9960 100644 --- a/docs/README-lanes.md +++ b/docs/README-lanes.md @@ -184,7 +184,14 @@ creating a second one; two rows differing only by case are a refusal naming both **On a rebase conflict it aborts**, leaves the worktree clean and not mid-rebase, prints the conflicting lines and prints the recovery commands. Your -edit survives as a local commit; read it back with +edit survives as a local commit — but an aborted pull is not proof it never +reached origin: this checkout is shared with every lane on the workstation, so +the very next write out of it can pull this dangling commit onto its own and +push both together (#32). LOOK first: `git -C fetch +origin main`, then check ANCESTRY of your own commit rather than eyeballing a +capped log — `git -C merge-base --is-ancestor origin/main` exits 0 once it is already there, in which case STOP. +Only once it answers non-zero do you read your edit back with `git -C diff origin/main..HEAD -- lanes/LANES.md`, then `git -C reset --hard origin/main` and redo it on top of the peer's version. diff --git a/lanes-edit.sh b/lanes-edit.sh index ca3dbc1f..9e08c16e 100755 --- a/lanes-edit.sh +++ b/lanes-edit.sh @@ -1163,6 +1163,7 @@ EOF grep -n -A2 -e '^<<<<<<<' -- "$LANES_REPO/$cp" 2>/dev/null | head -n 40 >&2 || : done git -C "$LANES_REPO" rebase --abort 2>/dev/null || : + cp_sha="$(git -C "$LANES_REPO" rev-parse HEAD 2>/dev/null)" note "rebase ABORTED — the worktree is clean and NOT mid-rebase. Your edit is safe in these local commits:" git -C "$LANES_REPO" --no-pager log --oneline "origin/$LANES_BRANCH..HEAD" 2>/dev/null | head -n 10 >&2 || : # #32 — AN ABORTED PULL IS NOT PROOF THIS COMMIT NEVER REACHED ORIGIN @@ -1172,10 +1173,13 @@ EOF # commit onto its own and pushes both together — three of the four # "rebase conflict … nothing was pushed" lines seen on Eagle in one # hour named a commit that was ALREADY on origin by the time anyone - # read them. LOOK before you reset or redo anything. + # read them. LOOK before you reset or redo anything — by ANCESTRY + # (Copilot round 2 on #50, 5203033893): a log capped at a few entries + # can bury this commit below the window while it stays an ancestor, the + # moment four or more later writes land ahead of it. note "RECOVERY (in that order):" - note " git -C $LANES_REPO fetch origin $LANES_BRANCH && git -C $LANES_REPO log --oneline origin/$LANES_BRANCH -3 # a write that ran after this one may already have carried it" - note " if that commit is already there, STOP — reset or redo now would write it a second time." + note " git -C $LANES_REPO fetch origin $LANES_BRANCH && git -C $LANES_REPO merge-base --is-ancestor $cp_sha origin/$LANES_BRANCH && echo ALREADY-THERE || echo not-there # ancestry, not a capped log: a write that ran after this one, however many commits back, may already have carried it" + note " if that printed ALREADY-THERE, STOP — reset or redo now would write it a second time." note " git -C $LANES_REPO diff origin/$LANES_BRANCH..HEAD -- ${CP_PATHS[*]} # only if it is not there: read back exactly what you wrote" note " git -C $LANES_REPO reset --hard origin/$LANES_BRANCH # then drop the local commits (NOTE: also drops any" note " # uncommitted peer edit in this checkout)" diff --git a/tests/test_lane_helpers.sh b/tests/test_lane_helpers.sh index 4519c2b2..88d58fe4 100755 --- a/tests/test_lane_helpers.sh +++ b/tests/test_lane_helpers.sh @@ -3587,15 +3587,20 @@ is "a peer's rewrite of the SAME row is a real conflict: exit 3" "$rc" 3 has "…REBASE CONFLICT is still named" "$err" "REBASE CONFLICT on origin/main" has "…the abort runs and says the tree is clean" "$err" "rebase ABORTED — the worktree is clean and NOT mid-rebase" is "…and it really is clean" "$(git -C "$WIP" status --porcelain | grep -c .)" 0 -has "…told to fetch and look before resetting or redoing anything" "$err" \ - "log --oneline origin/main -3 # a write that ran after this one may already have carried it" +has "…told to fetch and check ANCESTRY before resetting or redoing anything" "$err" \ + "merge-base --is-ancestor" +has "…a real ancestry test, not a log capped at a few entries (Copilot round 2 on #50)" "$err" \ + "&& echo ALREADY-THERE || echo not-there # ancestry, not a capped log" +cp_sha_printed="$(printf '%s\n' "$err" | grep -oE 'is-ancestor [0-9a-f]{7,40} origin/main' | head -n1 | awk '{print $2}')" +is "…naming a real local commit to check ancestry of, not a placeholder" \ + "$([ -n "$cp_sha_printed" ] && printf 'has-sha' || printf 'MISSING')" "has-sha" has "…the reset is now conditional on that look, not unconditional" "$err" \ "only if it is not there: read back exactly what you wrote" has "…and the die itself is scoped to this attempt, never a permanent claim" "$err" \ "not pushed by this attempt; a later write from this checkout may already carry it" hasnt "…never the old unconditional claim an aborted pull cannot prove" "$err" \ "rebase conflict on origin/main — nothing was pushed." -pc_look="$(printf '%s\n' "$err" | grep -n -- 'log --oneline origin/main -3' | head -n1 | cut -d: -f1)" +pc_look="$(printf '%s\n' "$err" | grep -n -- 'merge-base --is-ancestor' | head -n1 | cut -d: -f1)" pc_reset="$(printf '%s\n' "$err" | grep -n -- 'reset --hard origin/main' | head -n1 | cut -d: -f1)" is "…the look printed BEFORE the reset it gates, the same order RV-B3 pins" \ "$([ -n "$pc_look" ] && [ -n "$pc_reset" ] && [ "$pc_look" -lt "$pc_reset" ] && printf 'look first' || printf "look $pc_look, reset $pc_reset")" "look first" @@ -3609,7 +3614,7 @@ is "…the look printed BEFORE the reset it gates, the same order RV-B3 pins" # job the existing "only if it is not there" qualifier does for the diff line # alone. has "…and told to STOP there instead of resetting or redoing a landed commit" "$err" \ - "if that commit is already there, STOP — reset or redo now would write it a second time." + "if that printed ALREADY-THERE, STOP — reset or redo now would write it a second time." pc_stop="$(printf '%s\n' "$err" | grep -n -- 'STOP — reset or redo now' | head -n1 | cut -d: -f1)" is "…the STOP sits between the look and the reset, gating both reset and redo" \ "$([ -n "$pc_look" ] && [ -n "$pc_stop" ] && [ -n "$pc_reset" ] && [ "$pc_look" -lt "$pc_stop" ] && [ "$pc_stop" -lt "$pc_reset" ] && printf 'ordered' || printf "look $pc_look, stop $pc_stop, reset $pc_reset")" "ordered" diff --git a/tests/test_repo_hygiene.py b/tests/test_repo_hygiene.py index a251579e..d09b6144 100644 --- a/tests/test_repo_hygiene.py +++ b/tests/test_repo_hygiene.py @@ -1893,8 +1893,21 @@ def test_the_exit_3_documentation_agrees_with_the_die_message_it_describes(): manual) that contradicts the code it describes is worse than no table at all — the same claim `test_the_exit_code_table_carries_every_code_the_ file_exits_with` makes for a code missing from the table entirely. + + Round 2 (5203033893) found this test's own gap: it read only the table + and the manual, so a regression in the `die` MESSAGE ITSELF, back to + "nothing was pushed", left both docs unchanged and passed anyway — a + test that enforces two descriptions agree with each other, never that + either agrees with the code. The die is extracted and checked first now, + so all three have to agree with the one thing that actually runs. """ src = (REPO / "lanes-edit.sh").read_text(encoding="utf-8") + m = re.search(r'die "(rebase conflict on origin/\$LANES_BRANCH[^"]*)" 3\b', src) + assert m, "no exit-3 die message found in lanes-edit.sh to check the docs against" + die_text = m.group(1) + assert "not pushed by this attempt" in die_text, ( + "the exit-3 die message itself no longer names the attempt-scoped " + f"wording the table and the manual are checked against: {die_text!r}") start = src.index("# EXIT CODES — every subcommand, one table") end = src.index("# --no-sweep", start) table = src[start:end] From f6cc73019ebd32aad03bd4814024decace410da3 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:10:32 +0000 Subject: [PATCH 4/5] Round 3: the ancestry check's own `&&`/`||` could fold a FAILED fetch into "not-there", and the exit-3 table still named only one of its three causes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot's round-3 review of #50 (5203261904) found one real bug and one real gap, both in round 2's own work: - lanes-edit.sh:1181 (posted thread, real bug) — `fetch && merge-base ... && echo ALREADY-THERE || echo not-there` is left-associative: a FAILED fetch also falls to the trailing `||` and prints the SAME "not-there" a genuine non-ancestor does, silently waving the reset/redo path through against a ref that was never refreshed — recreating the exact duplicate this whole PR exists to prevent. Fixed by grouping the merge-base-and-echo pair in `{ ; }`, so the trailing `|| echo FETCH-FAILED` only fires when fetch itself failed (the group then never runs, against a ref nothing refreshed). Verified by hand against a real bare repo for all three outcomes: ALREADY-THERE (commit is an ancestor), not-there (commit is local-only, fetch succeeded), FETCH-FAILED (fetch against a bad remote). The STOP line now names FETCH-FAILED as its own STOP-and-retry outcome. docs/README-lanes.md:194 carried the same "answers non-zero → safe to redo" gap in its prose and is reworded the same way. - docs/README-lanes.md:946 and lanes-edit.sh:189 (suppressed comments, real completeness gap) — exit 3 is not only the rebase-conflict path: `git_timeout_die`'s network timeout and the six-attempts-exhausted path against a peer's uncommitted file (lanes-edit.sh:1140) both call `die ... 3` too, and the table's own claim is "one table, no two meanings on one number." Both rows now name all three causes; the attempt-scoped rebase-conflict wording (#32, round 1) is kept verbatim as the first of the three rather than replaced. Tests: - tests/test_lane_helpers.sh's OURS-CONFLICT case gains assertions for the FETCH-FAILED outcome (its own line, its own STOP-and-retry wording) and the updated ancestry-line text. - tests/test_repo_hygiene.py's exit-3 documentation test is whitespace- normalized (a phrase can legitimately wrap onto a comment-continuation line now that the row names three causes instead of one line) and gains a check that both the table and the manual name the other two causes ("timeout", "peer's"), not only the rebase-conflict one. Verification: bash -n on both shell files, python3 -m py_compile on the test module, a hand-built bare-repo check of all three ancestry outcomes, `bash tests/test_lane_helpers.sh` and `python3 -m pytest tests -q` both green (footers quoted in the PR). Lane: openrepotools-3 (openRepoTools-3) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019UFwD7bCS73jaUMJ2vuo5m --- docs/README-lanes.md | 13 ++++++++----- lanes-edit.sh | 23 ++++++++++++++++++---- tests/test_lane_helpers.sh | 15 +++++++++++++- tests/test_repo_hygiene.py | 40 +++++++++++++++++++++++++++++++++++--- 4 files changed, 78 insertions(+), 13 deletions(-) diff --git a/docs/README-lanes.md b/docs/README-lanes.md index bcce9960..e83b1be8 100644 --- a/docs/README-lanes.md +++ b/docs/README-lanes.md @@ -188,10 +188,13 @@ edit survives as a local commit — but an aborted pull is not proof it never reached origin: this checkout is shared with every lane on the workstation, so the very next write out of it can pull this dangling commit onto its own and push both together (#32). LOOK first: `git -C fetch -origin main`, then check ANCESTRY of your own commit rather than eyeballing a -capped log — `git -C merge-base --is-ancestor origin/main` exits 0 once it is already there, in which case STOP. -Only once it answers non-zero do you read your edit back with +origin main` — if THAT fails, STOP and retry it; a stale or unreachable origin +proves nothing either way, and is not permission to reset and redo. Only once +the fetch succeeds do you check ANCESTRY of your own commit rather than +eyeballing a capped log — `git -C merge-base +--is-ancestor origin/main` exits 0 once it is already there, in +which case STOP. Only once fetch succeeded AND this answers non-zero do you +read your edit back with `git -C diff origin/main..HEAD -- lanes/LANES.md`, then `git -C reset --hard origin/main` and redo it on top of the peer's version. @@ -943,7 +946,7 @@ else. | 0 | done | | 1 | environment — no register, no writer | | 2 | refusal — bad arguments, the object is held, an unknown alias, a checkout that cannot be rebased | -| 3 | rebase conflict — not pushed by this attempt; a later write from this checkout may already carry it to origin, so look before you retry | +| 3 | the commit stays local, never a permanent loss, but not confirmed on origin from this attempt either — a rebase conflict (not pushed by this attempt; a later write from this checkout may already carry it to origin, so look before you retry), a network timeout on the push itself (a hung push often already landed), or six attempts exhausted because a peer's uncommitted file blocks every rebase | | 4 | the mutex could not be taken within 60s | | 5 | an edit moved more than one line and was refused | | 6 | `git add` / `commit` / `push` failed | diff --git a/lanes-edit.sh b/lanes-edit.sh index 9e08c16e..585da889 100755 --- a/lanes-edit.sh +++ b/lanes-edit.sh @@ -185,8 +185,15 @@ # carry both without the caller guessing, so the newer reads spend a number # of their own and the older half of this file keeps 2 where it always was. # Clause (h)'s table is the contract for which read uses which. -# 3 rebase conflict — not pushed BY THIS ATTEMPT; a later write from this -# checkout may already carry it to origin, so LOOK before you retry +# 3 the commit stays local — never a permanent loss, but not confirmed on +# origin from THIS attempt either. THREE CAUSES (Copilot round 3 on #50, +# 5203261904, naming the other two this row used to leave out): a +# rebase conflict this attempt could not resolve — not pushed BY THIS +# ATTEMPT; a later write from this checkout may already carry it to +# origin, so LOOK by ancestry before you retry; `git_timeout_die`'s +# network timeout on the push itself, where a hung push often already +# landed; or six attempts exhausted because a peer's own uncommitted +# file blocks every rebase. # 4 the mutex could not be taken within 60s # 5 an edit moved more than one line and was refused — or, Amendment 15, a # lane's object log could not be renamed to the row's own spelling @@ -1177,9 +1184,17 @@ EOF # (Copilot round 2 on #50, 5203033893): a log capped at a few entries # can bury this commit below the window while it stays an ancestor, the # moment four or more later writes land ahead of it. + # ROUND 3 (Copilot 5203261904): `fetch && merge-base ... && echo A || + # echo B` is left-associative — a FAILED fetch also falls to the `||` + # and prints the SAME "not-there" a genuine non-ancestor does, so a + # stale or unreachable origin would silently wave the reset/redo path + # through. The merge-base-and-echo pair is grouped in `{ }` so the + # trailing `|| echo FETCH-FAILED` only fires when fetch ITSELF failed + # (the group then never runs at all, against a ref that was never + # refreshed). note "RECOVERY (in that order):" - note " git -C $LANES_REPO fetch origin $LANES_BRANCH && git -C $LANES_REPO merge-base --is-ancestor $cp_sha origin/$LANES_BRANCH && echo ALREADY-THERE || echo not-there # ancestry, not a capped log: a write that ran after this one, however many commits back, may already have carried it" - note " if that printed ALREADY-THERE, STOP — reset or redo now would write it a second time." + note " git -C $LANES_REPO fetch origin $LANES_BRANCH && { git -C $LANES_REPO merge-base --is-ancestor $cp_sha origin/$LANES_BRANCH && echo ALREADY-THERE || echo not-there; } || echo FETCH-FAILED # ancestry, not a capped log — and a FAILED fetch answers NEITHER, never treat it as not-there" + note " if that printed ALREADY-THERE, STOP — reset or redo now would write it a second time. If it printed FETCH-FAILED, STOP and retry the fetch — a stale or unreachable origin proves nothing either way." note " git -C $LANES_REPO diff origin/$LANES_BRANCH..HEAD -- ${CP_PATHS[*]} # only if it is not there: read back exactly what you wrote" note " git -C $LANES_REPO reset --hard origin/$LANES_BRANCH # then drop the local commits (NOTE: also drops any" note " # uncommitted peer edit in this checkout)" diff --git a/tests/test_lane_helpers.sh b/tests/test_lane_helpers.sh index 88d58fe4..607e6652 100755 --- a/tests/test_lane_helpers.sh +++ b/tests/test_lane_helpers.sh @@ -3590,10 +3590,21 @@ is "…and it really is clean" "$(git -C "$WIP" status --porcelain | grep -c has "…told to fetch and check ANCESTRY before resetting or redoing anything" "$err" \ "merge-base --is-ancestor" has "…a real ancestry test, not a log capped at a few entries (Copilot round 2 on #50)" "$err" \ - "&& echo ALREADY-THERE || echo not-there # ancestry, not a capped log" + "&& echo ALREADY-THERE || echo not-there; } || echo FETCH-FAILED # ancestry, not a capped log" cp_sha_printed="$(printf '%s\n' "$err" | grep -oE 'is-ancestor [0-9a-f]{7,40} origin/main' | head -n1 | awk '{print $2}')" is "…naming a real local commit to check ancestry of, not a placeholder" \ "$([ -n "$cp_sha_printed" ] && printf 'has-sha' || printf 'MISSING')" "has-sha" + +# Copilot's round-3 review of #50 (5203261904): `fetch && merge-base ... && +# echo A || echo B` is left-associative, so a FAILED fetch also falls to the +# trailing `||` and prints the SAME "not-there" a genuine non-ancestor does — +# silently waving reset/redo through against a ref that was never refreshed. +# Grouping the merge-base-and-echo pair in `{ }` makes a THIRD outcome +# possible, reached only when fetch itself failed. +has "…and a FAILED fetch is its own THIRD outcome, never folded into not-there" "$err" \ + "|| echo FETCH-FAILED" +has "…named as answering neither ALREADY-THERE nor not-there" "$err" \ + "a FAILED fetch answers NEITHER, never treat it as not-there" has "…the reset is now conditional on that look, not unconditional" "$err" \ "only if it is not there: read back exactly what you wrote" has "…and the die itself is scoped to this attempt, never a permanent claim" "$err" \ @@ -3615,6 +3626,8 @@ is "…the look printed BEFORE the reset it gates, the same order RV-B3 pins" # alone. has "…and told to STOP there instead of resetting or redoing a landed commit" "$err" \ "if that printed ALREADY-THERE, STOP — reset or redo now would write it a second time." +has "…and STOP on a FAILED fetch too, never treated as leave to redo" "$err" \ + "If it printed FETCH-FAILED, STOP and retry the fetch — a stale or unreachable origin proves nothing either way." pc_stop="$(printf '%s\n' "$err" | grep -n -- 'STOP — reset or redo now' | head -n1 | cut -d: -f1)" is "…the STOP sits between the look and the reset, gating both reset and redo" \ "$([ -n "$pc_look" ] && [ -n "$pc_stop" ] && [ -n "$pc_reset" ] && [ "$pc_look" -lt "$pc_stop" ] && [ "$pc_stop" -lt "$pc_reset" ] && printf 'ordered' || printf "look $pc_look, stop $pc_stop, reset $pc_reset")" "ordered" diff --git a/tests/test_repo_hygiene.py b/tests/test_repo_hygiene.py index d09b6144..a056d1ac 100644 --- a/tests/test_repo_hygiene.py +++ b/tests/test_repo_hygiene.py @@ -1900,6 +1900,14 @@ def test_the_exit_3_documentation_agrees_with_the_die_message_it_describes(): test that enforces two descriptions agree with each other, never that either agrees with the code. The die is extracted and checked first now, so all three have to agree with the one thing that actually runs. + + Round 3 (5203261904) named exit 3's other two causes (`git_timeout_die`, + and six push attempts exhausted against a peer's uncommitted file) that + this row used to leave out entirely; the row now names all three, which + means the phrase this test looks for can legitimately wrap onto its own + comment-continuation line — so the comparison text is whitespace- + normalized (one space, however the source wrapped it) rather than + matched as one literal run of characters. """ src = (REPO / "lanes-edit.sh").read_text(encoding="utf-8") m = re.search(r'die "(rebase conflict on origin/\$LANES_BRANCH[^"]*)" 3\b', src) @@ -1912,15 +1920,41 @@ def test_the_exit_3_documentation_agrees_with_the_die_message_it_describes(): end = src.index("# --no-sweep", start) table = src[start:end] manual = (REPO / "docs/README-lanes.md").read_text(encoding="utf-8") + def flatten(text): + # A row that wraps onto a comment-continuation line puts a literal + # `#` back-to-back with the next word once newlines alone are + # collapsed — strip the leading `#` (lanes-edit.sh's own comment + # marker; a no-op on docs/README-lanes.md's markdown lines, which + # never start with one) from every line FIRST, so a phrase that + # wraps still reads as one run of words, not one broken by a stray + # `#`. + lines = (re.sub(r"^#\s*", "", ln) for ln in text.splitlines()) + return re.sub(r"\s+", " ", " ".join(lines)).strip() + for name, text in (("lanes-edit.sh's own exit-code table", table), ("docs/README-lanes.md's copy of it", manual)): - assert "nothing was pushed" not in text and "nothing pushed" not in text, ( + flat = flatten(text) + assert "nothing was pushed" not in flat and "nothing pushed" not in flat, ( f"{name} still claims exit 3 means nothing reached origin, which " "an aborted pull does not prove (#32)") - assert "not pushed BY THIS ATTEMPT" in table, ( + manual_start = manual.index("### Exit codes") + manual_end = manual.index("3 to 6 are the codes this helper already used", manual_start) + manual_table = manual[manual_start:manual_end] + table_flat = flatten(table) + manual_flat = flatten(manual_table) + assert "not pushed BY THIS ATTEMPT" in table_flat, ( "lanes-edit.sh's exit-3 row no longer names the attempt-scoped wording") - assert "not pushed by this attempt" in manual, ( + assert "not pushed by this attempt" in manual_flat, ( "docs/README-lanes.md's exit-3 row no longer names the attempt-scoped wording") + for name, flat in (("lanes-edit.sh's own exit-code table", table_flat), + ("docs/README-lanes.md's own exit-codes table", manual_flat)): + for cause in ("timeout", "peer's"): + assert cause in flat, ( + f"{name} no longer names a {cause!r} cause, one of exit 3's " + "other two causes (#50 round 3, 5203261904) — this table " + "claims to be \"no two meanings on one number\", so a code " + "with three causes has to name all three or it is back to " + "being wrong") #: ADOPTION ACT 0, AND THE ONE SHA THAT IS IT. `opensoft/brett-wip#5` merged From 0d2c243a53e3eb39c01dc7a8490fafa1843e2cc3 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:39:52 +0000 Subject: [PATCH 5/5] =?UTF-8?q?Round=204:=20the=20ancestry=20check=20itsel?= =?UTF-8?q?f=20checked=20the=20wrong=20thing=20=E2=80=94=20a=20peer's=20re?= =?UTF-8?q?base=20of=20the=20dangling=20commit=20changes=20its=20sha,=20so?= =?UTF-8?q?=20`git=20cherry`=20replaces=20`merge-base=20--is-ancestor`=20t?= =?UTF-8?q?o=20compare=20by=20content=20instead?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot's round-4 review of #50 (5203455553) found the deepest issue yet, squarely in the mechanism this whole PR is about: - lanes-edit.sh:1196 (posted thread, real bug in round 2's own fix) — `cp_sha` is captured right after `rebase --abort`, but the very peer write this recovery exists for (the branch just above this one: "the next write out of it pulls this dangling commit onto its own") carries it by REBASING onto a newer base, which gives the SAME change a NEW sha. `merge-base --is-ancestor $cp_sha origin/$LANES_BRANCH` checks the OLD sha, so it still answers "not-there" once the change is published under the new one — recreating the exact duplicate #32 is about, one layer deeper than round 3's fetch-failure bug. Replaced with `git cherry origin/$LANES_BRANCH $cp_sha | grep -c '^+'`, which compares by PATCH rather than sha and finds a rebase-carried commit either way. Verified by hand against a real bare repo: a peer rebases our dangling commit (sha changes, same patch) — `merge-base --is-ancestor` on the original sha answers "not-there" (the bug); `git cherry` on the same original sha answers 0, i.e. found (the fix). Re-verified for a genuinely local-only commit (1, not-there), a plain already-pushed commit with no rebase involved (0, empty `git cherry` output reads as zero `+` lines), and a fetch against a bad remote (its own FETCH-FAILED, never reaching a stale `git cherry`). Fetch and the content check are now two separate RECOVERY lines (matching `lane-start`'s own RV-B3 two-line shape) instead of one line chained with `&&`/`||`, since fetch's own success/failure no longer needs to gate a single compound exit code. - docs/README-lanes.md:196 (suppressed comment) and :949 / lanes-edit.sh:189 (open threads) — the same "SHA vs content" gap in the manual's prose, and a second, narrower completeness gap in both exit-3 rows: `git_timeout_die` fires for a timed-out PUSH (lanes-edit.sh:1112/1141/1160) or a timed-out PULL (lanes-edit.sh:1166), and round 3's own wording only ever named the push. Both rows and the prose are reworded: "a push or a pull", the table's opening no longer flatly claims "the commit stays local" (a hung push may already have landed), and the manual's recovery paragraph now describes the `git cherry` check instead of `merge-base --is-ancestor`. Tests: - tests/test_lane_helpers.sh's OURS-CONFLICT case is rewritten for the new two-line fetch/cherry recovery: the cherry invocation, that it names a real commit sha, the `grep -c '^+'` count, the fetch-then-check-then-reset ordering, and the simplified "if that printed 0, STOP" wording. - tests/test_repo_hygiene.py's exit-3 documentation test now requires both "push" and "pull" alongside "timeout" in each row, closing the gap a bare "timeout" check left open (round 3's own wording would have kept passing it forever). Verification: bash -n on both shell files, python3 -m py_compile on the test module, three hand-built bare-repo scenarios (rebased-carry, local-only, fetch-failure) for the cherry-based check, `bash tests/test_lane_helpers.sh` and `python3 -m pytest tests -q` both green (footers quoted in the PR). Lane: openrepotools-3 (openRepoTools-3) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019UFwD7bCS73jaUMJ2vuo5m --- docs/README-lanes.md | 15 ++++++----- lanes-edit.sh | 37 ++++++++++++++++++--------- tests/test_lane_helpers.sh | 52 ++++++++++++++++++++------------------ tests/test_repo_hygiene.py | 22 +++++++++++----- 4 files changed, 78 insertions(+), 48 deletions(-) diff --git a/docs/README-lanes.md b/docs/README-lanes.md index e83b1be8..01ffec30 100644 --- a/docs/README-lanes.md +++ b/docs/README-lanes.md @@ -190,11 +190,14 @@ the very next write out of it can pull this dangling commit onto its own and push both together (#32). LOOK first: `git -C fetch origin main` — if THAT fails, STOP and retry it; a stale or unreachable origin proves nothing either way, and is not permission to reset and redo. Only once -the fetch succeeds do you check ANCESTRY of your own commit rather than -eyeballing a capped log — `git -C merge-base ---is-ancestor origin/main` exits 0 once it is already there, in -which case STOP. Only once fetch succeeded AND this answers non-zero do you -read your edit back with +the fetch succeeds do you check by CONTENT whether your own commit is already +there, never by sha: the very peer write this recovery exists for carries a +dangling commit by rebasing it onto a newer base, which changes its hash, so +`merge-base --is-ancestor` alone can still say "not there" once the change is +actually published. `git -C cherry origin/main | grep -c '^+'` compares by patch instead and still finds it; 0 means +it is already there, in which case STOP. Only once fetch succeeded AND this +prints something other than 0 do you read your edit back with `git -C diff origin/main..HEAD -- lanes/LANES.md`, then `git -C reset --hard origin/main` and redo it on top of the peer's version. @@ -946,7 +949,7 @@ else. | 0 | done | | 1 | environment — no register, no writer | | 2 | refusal — bad arguments, the object is held, an unknown alias, a checkout that cannot be rebased | -| 3 | the commit stays local, never a permanent loss, but not confirmed on origin from this attempt either — a rebase conflict (not pushed by this attempt; a later write from this checkout may already carry it to origin, so look before you retry), a network timeout on the push itself (a hung push often already landed), or six attempts exhausted because a peer's uncommitted file blocks every rebase | +| 3 | the edit is never a permanent loss, but this attempt cannot confirm whether it also reached origin — a rebase conflict (not pushed by this attempt; a later write from this checkout may already carry it to origin, so look by content before you retry), a network timeout on a push or a pull (a hung one often already landed), or six attempts exhausted because a peer's uncommitted file blocks every rebase | | 4 | the mutex could not be taken within 60s | | 5 | an edit moved more than one line and was refused | | 6 | `git add` / `commit` / `push` failed | diff --git a/lanes-edit.sh b/lanes-edit.sh index 585da889..ed9a8722 100755 --- a/lanes-edit.sh +++ b/lanes-edit.sh @@ -185,15 +185,17 @@ # carry both without the caller guessing, so the newer reads spend a number # of their own and the older half of this file keeps 2 where it always was. # Clause (h)'s table is the contract for which read uses which. -# 3 the commit stays local — never a permanent loss, but not confirmed on -# origin from THIS attempt either. THREE CAUSES (Copilot round 3 on #50, +# 3 the edit is never a permanent loss, but this attempt cannot confirm +# whether it also reached origin. THREE CAUSES (Copilot round 3 on #50, # 5203261904, naming the other two this row used to leave out): a # rebase conflict this attempt could not resolve — not pushed BY THIS # ATTEMPT; a later write from this checkout may already carry it to -# origin, so LOOK by ancestry before you retry; `git_timeout_die`'s -# network timeout on the push itself, where a hung push often already -# landed; or six attempts exhausted because a peer's own uncommitted -# file blocks every rebase. +# origin, so LOOK by CONTENT before you retry (round 4, 5203455553: +# SHA ancestry alone misses a peer's rebase of it, which changes the +# hash but not the patch); `git_timeout_die`'s network timeout on a +# push OR a pull, where a hung one often already landed; or six +# attempts exhausted because a peer's own uncommitted file blocks +# every rebase. # 4 the mutex could not be taken within 60s # 5 an edit moved more than one line and was refused — or, Amendment 15, a # lane's object log could not be renamed to the row's own spelling @@ -1188,13 +1190,24 @@ EOF # echo B` is left-associative — a FAILED fetch also falls to the `||` # and prints the SAME "not-there" a genuine non-ancestor does, so a # stale or unreachable origin would silently wave the reset/redo path - # through. The merge-base-and-echo pair is grouped in `{ }` so the - # trailing `|| echo FETCH-FAILED` only fires when fetch ITSELF failed - # (the group then never runs at all, against a ref that was never - # refreshed). + # through. Fetch is now its own line, checked before anything else. + # ROUND 4 (Copilot 5203455553): SHA ancestry is the wrong test — the + # very write this whole recovery exists to catch (a peer's own + # `commit_push` pulling THIS dangling commit and rebasing it onto a + # newer base, per the very next branch above) gives it a NEW sha, so + # `merge-base --is-ancestor $cp_sha …` answers "not-there" even once + # the CONTENT is published. `git cherry` compares by PATCH, not by + # sha, so a rebase that only replays the same change (no real + # conflict) is still found. Measured against a real bare repo: a + # commit rebased by a peer this way answers `not-there` under + # `merge-base --is-ancestor` and `-` (found, equivalent) under + # `git cherry` — verified for the genuinely-absent case too (`+`) and + # the plain same-sha case (no output at all, which the `-c '^+'` + # count below also reads as zero, i.e. already there). note "RECOVERY (in that order):" - note " git -C $LANES_REPO fetch origin $LANES_BRANCH && { git -C $LANES_REPO merge-base --is-ancestor $cp_sha origin/$LANES_BRANCH && echo ALREADY-THERE || echo not-there; } || echo FETCH-FAILED # ancestry, not a capped log — and a FAILED fetch answers NEITHER, never treat it as not-there" - note " if that printed ALREADY-THERE, STOP — reset or redo now would write it a second time. If it printed FETCH-FAILED, STOP and retry the fetch — a stale or unreachable origin proves nothing either way." + note " git -C $LANES_REPO fetch origin $LANES_BRANCH || echo FETCH-FAILED # if that printed FETCH-FAILED, STOP and retry — a stale or unreachable origin proves nothing either way" + note " git -C $LANES_REPO cherry origin/$LANES_BRANCH $cp_sha | grep -c '^+' # BY CONTENT, not by sha: a peer's rebase changes the hash but git cherry still finds the same patch" + note " if that printed 0, STOP — reset or redo now would write it a second time." note " git -C $LANES_REPO diff origin/$LANES_BRANCH..HEAD -- ${CP_PATHS[*]} # only if it is not there: read back exactly what you wrote" note " git -C $LANES_REPO reset --hard origin/$LANES_BRANCH # then drop the local commits (NOTE: also drops any" note " # uncommitted peer edit in this checkout)" diff --git a/tests/test_lane_helpers.sh b/tests/test_lane_helpers.sh index 607e6652..ae38d749 100755 --- a/tests/test_lane_helpers.sh +++ b/tests/test_lane_helpers.sh @@ -3587,47 +3587,51 @@ is "a peer's rewrite of the SAME row is a real conflict: exit 3" "$rc" 3 has "…REBASE CONFLICT is still named" "$err" "REBASE CONFLICT on origin/main" has "…the abort runs and says the tree is clean" "$err" "rebase ABORTED — the worktree is clean and NOT mid-rebase" is "…and it really is clean" "$(git -C "$WIP" status --porcelain | grep -c .)" 0 -has "…told to fetch and check ANCESTRY before resetting or redoing anything" "$err" \ - "merge-base --is-ancestor" -has "…a real ancestry test, not a log capped at a few entries (Copilot round 2 on #50)" "$err" \ - "&& echo ALREADY-THERE || echo not-there; } || echo FETCH-FAILED # ancestry, not a capped log" -cp_sha_printed="$(printf '%s\n' "$err" | grep -oE 'is-ancestor [0-9a-f]{7,40} origin/main' | head -n1 | awk '{print $2}')" -is "…naming a real local commit to check ancestry of, not a placeholder" \ +has "…told to fetch before resetting or redoing anything" "$err" \ + "fetch origin main || echo FETCH-FAILED" +has "…and STOP named for a FAILED fetch too, never treated as leave to redo" "$err" \ + "if that printed FETCH-FAILED, STOP and retry — a stale or unreachable origin proves nothing either way" + +# Copilot's round-4 review of #50 (5203455553): the very peer write this +# recovery exists for carries a dangling commit by REBASING it onto a newer +# base — which the branch just above this one takes — and a rebase changes +# the commit's sha without changing its patch. `merge-base --is-ancestor +# $cp_sha …` (round 2's own fix) checks the OLD sha, so it can still answer +# "not-there" once the change is published under a new one. `git cherry` +# compares by PATCH, so it finds it either way; verified against a real bare +# repo for a peer-rebased commit (0, correctly ALREADY THERE — where +# `merge-base --is-ancestor` on the same sha answered "not-there"), a +# genuinely local-only one (1, correctly not-there), and a fetch against a +# bad remote (FETCH-FAILED, cherry never run against a stale ref). +has "…checked by CONTENT (git cherry), never by a sha a peer's rebase can change" "$err" \ + "cherry origin/main" +cp_sha_printed="$(printf '%s\n' "$err" | grep -oE 'cherry origin/main [0-9a-f]{7,40}' | head -n1 | awk '{print $3}')" +is "…naming a real local commit to check, not a placeholder" \ "$([ -n "$cp_sha_printed" ] && printf 'has-sha' || printf 'MISSING')" "has-sha" - -# Copilot's round-3 review of #50 (5203261904): `fetch && merge-base ... && -# echo A || echo B` is left-associative, so a FAILED fetch also falls to the -# trailing `||` and prints the SAME "not-there" a genuine non-ancestor does — -# silently waving reset/redo through against a ref that was never refreshed. -# Grouping the merge-base-and-echo pair in `{ }` makes a THIRD outcome -# possible, reached only when fetch itself failed. -has "…and a FAILED fetch is its own THIRD outcome, never folded into not-there" "$err" \ - "|| echo FETCH-FAILED" -has "…named as answering neither ALREADY-THERE nor not-there" "$err" \ - "a FAILED fetch answers NEITHER, never treat it as not-there" +has "…counting only the patches git cherry could not find upstream" "$err" \ + "grep -c '^+'" has "…the reset is now conditional on that look, not unconditional" "$err" \ "only if it is not there: read back exactly what you wrote" has "…and the die itself is scoped to this attempt, never a permanent claim" "$err" \ "not pushed by this attempt; a later write from this checkout may already carry it" hasnt "…never the old unconditional claim an aborted pull cannot prove" "$err" \ "rebase conflict on origin/main — nothing was pushed." -pc_look="$(printf '%s\n' "$err" | grep -n -- 'merge-base --is-ancestor' | head -n1 | cut -d: -f1)" +pc_fetch="$(printf '%s\n' "$err" | grep -n -- 'fetch origin main || echo FETCH-FAILED' | head -n1 | cut -d: -f1)" +pc_look="$(printf '%s\n' "$err" | grep -n -- 'cherry origin/main' | head -n1 | cut -d: -f1)" pc_reset="$(printf '%s\n' "$err" | grep -n -- 'reset --hard origin/main' | head -n1 | cut -d: -f1)" -is "…the look printed BEFORE the reset it gates, the same order RV-B3 pins" \ - "$([ -n "$pc_look" ] && [ -n "$pc_reset" ] && [ "$pc_look" -lt "$pc_reset" ] && printf 'look first' || printf "look $pc_look, reset $pc_reset")" "look first" +is "…fetched, THEN checked by content, THEN reset — the same order RV-B3 pins" \ + "$([ -n "$pc_fetch" ] && [ -n "$pc_look" ] && [ -n "$pc_reset" ] && [ "$pc_fetch" -lt "$pc_look" ] && [ "$pc_look" -lt "$pc_reset" ] && printf 'ordered' || printf "fetch $pc_fetch, look $pc_look, reset $pc_reset")" "ordered" # Copilot's review of #50 (5202640056), suppressed comment on lanes-edit.sh:1180: # the gate above was attached only to the diagnostic `diff` line — a caller -# whose fetch/log just showed the commit already on origin was still told, by +# whose fetch/look just showed the commit already on origin was still told, by # the next two unconditional lines, to reset and redo, which is the exact # duplicate this PR exists to prevent. A single STOP between the look and the # reset gates all three lines below it (diff, reset, redo) at once, the same # job the existing "only if it is not there" qualifier does for the diff line # alone. has "…and told to STOP there instead of resetting or redoing a landed commit" "$err" \ - "if that printed ALREADY-THERE, STOP — reset or redo now would write it a second time." -has "…and STOP on a FAILED fetch too, never treated as leave to redo" "$err" \ - "If it printed FETCH-FAILED, STOP and retry the fetch — a stale or unreachable origin proves nothing either way." + "if that printed 0, STOP — reset or redo now would write it a second time." pc_stop="$(printf '%s\n' "$err" | grep -n -- 'STOP — reset or redo now' | head -n1 | cut -d: -f1)" is "…the STOP sits between the look and the reset, gating both reset and redo" \ "$([ -n "$pc_look" ] && [ -n "$pc_stop" ] && [ -n "$pc_reset" ] && [ "$pc_look" -lt "$pc_stop" ] && [ "$pc_stop" -lt "$pc_reset" ] && printf 'ordered' || printf "look $pc_look, stop $pc_stop, reset $pc_reset")" "ordered" diff --git a/tests/test_repo_hygiene.py b/tests/test_repo_hygiene.py index a056d1ac..dc8aae6e 100644 --- a/tests/test_repo_hygiene.py +++ b/tests/test_repo_hygiene.py @@ -1908,6 +1908,14 @@ def test_the_exit_3_documentation_agrees_with_the_die_message_it_describes(): comment-continuation line — so the comparison text is whitespace- normalized (one space, however the source wrapped it) rather than matched as one literal run of characters. + + Round 4 (5203455553) found this test's own gap in THAT check: it looked + for the bare word "timeout" only, which the round-3 wording ("timeout on + the push itself") already satisfied while omitting `git_timeout_die`'s + OTHER call site — a timed-out `git pull --rebase` (lanes-edit.sh:1166) — + entirely. Checking for "timeout" alone would keep passing even if a push + is all either row ever names again; "push" and "pull" are now checked + for too, so the row has to cover both keywords or the test catches it. """ src = (REPO / "lanes-edit.sh").read_text(encoding="utf-8") m = re.search(r'die "(rebase conflict on origin/\$LANES_BRANCH[^"]*)" 3\b', src) @@ -1948,13 +1956,15 @@ def flatten(text): "docs/README-lanes.md's exit-3 row no longer names the attempt-scoped wording") for name, flat in (("lanes-edit.sh's own exit-code table", table_flat), ("docs/README-lanes.md's own exit-codes table", manual_flat)): - for cause in ("timeout", "peer's"): + for cause in ("timeout", "push", "pull", "peer's"): assert cause in flat, ( - f"{name} no longer names a {cause!r} cause, one of exit 3's " - "other two causes (#50 round 3, 5203261904) — this table " - "claims to be \"no two meanings on one number\", so a code " - "with three causes has to name all three or it is back to " - "being wrong") + f"{name} no longer names a {cause!r} cause, part of exit 3's " + "other two causes (#50 rounds 3 and 4, 5203261904 and " + "5203455553) — `git_timeout_die` fires for a push OR a pull " + "timeout (lanes-edit.sh:1112/1141/1160 and :1166), and this " + "table claims to be \"no two meanings on one number\", so a " + "code with three causes has to name all three or it is back " + "to being wrong") #: ADOPTION ACT 0, AND THE ONE SHA THAT IS IT. `opensoft/brett-wip#5` merged