[brettheap]

Lane: openreposhape-2 (openRepoShape-2)
Claim: #253 (comment)

What

scripts/bump-leg.py's _apply_rewrite now rolls every byte back on ANY failure once the leg has been asked to move, up to and including the commit, and then reports the failure. No report claims more than the rollback did, as far as git can show it. The leg is judged by its branch, its HEAD and its git status, so a report cannot vouch for what git status does not show, a file git ignores above all (see "Left out").

• The handler is except BaseException, so an interrupt is covered as well as an OSError or a UnicodeError.
• The commit is inside the rollback's reach. It is the last statement inside the try, and cmd_bump builds the message first and hands it in. A commit git refuses (a signing key that cannot sign, a commit-msg hook) is therefore put back like any other failure.
• The HEAD guard. One git rev-parse HEAD is read just before the commit. After any failure, _head_guard compares the root's HEAD with it:
  • unmoved: every byte is rolled back;
  • moved, because the failure came after the commit landed (an interrupt, or the rev-parse that reads the new HEAD back): nothing is rolled back under the commit, and the report says The bump was committed as <sha>;
  • unreadable: nothing is rolled back, and the report says the commit could not be read.
• The rollback is small helpers instead of a loop and a try.
  • _roll_back puts back, in the order it always did, every file written, the leg's checkout and the index.
  • _put_back attempts each of those steps whatever the steps before it did. It prints, never raises, a step that fails. A second Ctrl-C names its step as interrupted, and the steps after it still run; no signal handler is installed.
  • _restore_file writes a file back only when it no longer holds its original bytes, so a read-only file whose write never opened it is not reported as unrestorable.
  • _restore_leg_if_moved reads the leg's branch and HEAD and restores the leg when either differs from where it stood. A forward checkout that moved the leg and then failed (a post-checkout hook) or was interrupted is put back, and a leg that never moved is not checked out again.
  • Either way, it then compares the leg's git status --porcelain -z --untracked-files=all, read as bytes before the checkout, with the leg's status now. git writes the new commit's files before it moves the leg's index and HEAD, so a checkout cut off between the two leaves the leg on its branch at its commit with those files in it. When the status differs, the step fails. _put_back names it and sends the operator to git -C <leg> status, and the report says THE ROLLBACK WAS NOT COMPLETE.
  • Nothing in the leg is removed. An untracked file of the operator's own is allowed there, and the rollback cannot tell it from one the checkout wrote.
• What it reports. Each report says one of three things: every byte is back (ROLLED_BACK), THE ROLLBACK WAS NOT COMPLETE (NOT_ROLLED_BACK), or the commit landed. Where that goes depends on the failure:
  • a Refusal carries it in its own text, and bump-leg-validators-red's sentence is replaced when the rollback did not earn it;
  • an OSError or a UnicodeError that nothing committed leaves as REFUSED bump-leg-rewrite-failed naming the exception (exit 2);
  • a CommandFailed leaves as main() always printed it (REFUSED advancing a leg, exit 2), after one stderr line saying what became of the tree;
  • an interrupt or a defect is re-raised as itself, after the same line.
• Docs and main(). One bullet in the module docstring's refusal list. No --help text changes (render-cli-reference.py --check ok), and main() is not touched.

Why

_apply_rewrite's docstring promised "or roll every byte back and re-raise", but its handler was except (Refusal, CommandFailed), and main() catches the same two. So a PermissionError on a workflow file, a UnicodeError from echoing a validator's output, or an interrupt ended in a traceback, with the pin rewritten, the leg on the new commit and the index changed, and no finding about any of it.

Round 1 reproduced this on main 39ac59c with the issue's own steps:

== decode: exit 1     UnicodeDecodeError ...   git status: M  contracts/spec-pin.yaml M  spec | leg HEAD c893c2b, was 443b42c
== readonly: exit 1   PermissionError ...      git status:  M contracts/spec-pin.yaml  M spec | leg HEAD c893c2b, was 443b42c

The same script, re-run in fix round 1 with a signing case added (commit.gpgsign true and a gpg.program that does not exist), at the reviewed head and at fix round 1's head 67a1163:

a8d31ee  == signing: exit 2   REFUSED advancing a leg ... git commit ... exit 128   git status: M  contracts/spec-pin.yaml M  spec | leg HEAD 95fac77, was 8c8d650
67a1163  == decode: exit 2    REFUSED bump-leg-rewrite-failed: UnicodeDecodeError ...   git status: (clean) | leg HEAD 00e7d92, was 00e7d92
67a1163  == readonly: exit 2  REFUSED bump-leg-rewrite-failed: PermissionError ...      git status: (clean) | leg HEAD 00e7d92, was 00e7d92
67a1163  == signing: exit 2   REFUSED advancing a leg ... git commit ... exit 128   git status: (clean) | leg HEAD 00e7d92, was 00e7d92

At 67a1163 the signing run's stderr opens with The command below failed while the bump was being applied. Every byte this command wrote has been rolled back — the pin, the workflow files, the leg's checkout and the index — and the tree is exactly as it was. The same command run again once the signer is fixed exits 0 and commits. At a8d31ee that second run was refused as bump-leg-root-dirty.

The review measured the decode run on main c3f4987, with #230 and #214 landed: it no longer fails at all there. The handler still covers what the echo can still raise (an OSError such as EPIPE from sys.stdout.write).

The issue also notes the rollback's own writes need a guard. written[target] is set before write_lf, so for the read-only file the restore would meet the same PermissionError over the original failure. _restore_file and _put_back are that guard. A test has the leg's own restore fail, and shows the files and the index are still put back.

Decision for the coordinator: a clean REFUSED (exit 2) for OSError and UnicodeError, and the traceback for everything else, after a line saying whether the tree is back.

• The repository's other pin-moving tool does the same for an unwritable file. update-shape.py _rewrite_readme turns the write's OSError into update-readme-unwritable ("A FAILED WRITE IS A REFUSAL, NEVER A TRACEBACK"), so the one rollback arm that exists puts every byte back. family.py bump has no rollback and lets such an error trace back, which is the weaker precedent.
• The issue's Expected also names exit 2 "the way bump-leg-validators-red does".
• The conversion is made in _apply_rewrite, after the rollback, not in main(), because only there is the claim "every byte rolled back" known to be true or false. main() catching OSError would also dress up a failure that came before the first write.
• An interrupt and a defect are not the operator's to fix, so they keep the interpreter's own report, after the tree is back and one line says so.
• A failed git command keeps main()'s REFUSED advancing a leg, after the same line.

Decision: a HEAD that cannot be read after the commit was asked for counts as landed. Rolling the files and the leg back under a commit that did land would leave a tree that contradicts the commit, and that is the worse of the two mistakes.

Tests

New tests/test_bump_leg_rollback.py: 24 cases in 13 functions. Each case compares what the bump could change with what it was before the bump: the root's HEAD, the pin's bytes, every workflow's bytes, the gitlink, git diff --cached --quiet, git status, the leg's HEAD and the branch the leg stood on. The leg is put ON its branch first, as bootstrap.py leaves it. There are three exceptions:

• In the four cases where the leg's own restore is made to fail, the leg cannot be put back and git status shows it. Those cases compare the root's HEAD, the pin, every workflow, the gitlink and git diff --cached --quiet.

• In the three cases where the forward checkout leaves a file in the leg, git status shows that file, because nothing removes it. Those cases compare the same five, plus the leg's HEAD and its branch.

• The commit-landed cases assert that the commit and the tree agree.

• A read-only tracked workflow file, with bump-leg.py run as a subprocess (the issue's reproduction). Exit 2, bump-leg-rewrite-failed, PermissionError, and no traceback. There is no "rolling the bump back" noise for a file never opened. (1 case; skipped where the account can write a read-only file, i.e. root.)

• A write that dies half way (ENOSPC after the file was truncated and 20 bytes written): exit 2. (1)

• A validator's output that cannot be read (UnicodeDecodeError from run_validator), and one the stdout cannot echo (a stdout whose write raises UnicodeEncodeError for U+FFFD): exit 2. (2)

• An interrupt while writing the second file (with the pin already rewritten), an interrupt in the validators (with the index staged), and a RuntimeError in the validators. Each is re-raised as itself, the tree is back, and stderr says <Exception> while the bump was being applied and that every byte is back. (3)

• The leg's own restore failing, beside a red validator, an unwritable file, or a git add that fails. The original failure is the one reported, and the failed step is named. The files and the index are still put back. Each report says THE ROLLBACK WAS NOT COMPLETE, and none says the tree is exactly as it was. (3)

• A second interrupt inside the rollback's leg step. The step is named interrupted, the index step after it still runs, and the refusal says the rollback was not complete. (1)

• A forward checkout that moved the leg and then failed: an interrupt after the real checkout, and a leg post-checkout hook that exits 1 for the new commit only. The leg is back on its branch. (2)

• A forward checkout that left files in the leg. Two are cut off before the leg moved: one is interrupted, and one fails with exit 130. Each writes a file and leaves HEAD where it was. The third moves the leg, leaves a file the checkout back cannot remove, and is interrupted. In each, the line before the re-raise or the refusal says THE ROLLBACK WAS NOT COMPLETE, nothing says the tree is exactly as it was, and the leg step is named. The operator's own untracked file and the checkout's file are both still there. (3)

• An untracked file of the operator's own in the leg, beside a red validator. The report still says every byte is back, and the file is untouched. (1)

• --to the commit already pinned, with a workflow reference a hand-made bump left behind. The forward checkout detaches the leg at the commit it stood on, and a red validator follows. The leg is back ON main. (1)

• A commit git refuses: a gpg.program that does not exist, and a commit-msg hook. Exit 2, the refusal names git commit, the tree is back, and stderr says every byte is back. (2)

• A failure after the commit landed: an interrupt, a Refusal from reading HEAD back, and an OSError. Nothing is rolled back, the tree agrees with the new commit, and the report says committed as <sha>. The OSError is re-raised as itself, not made into bump-leg-rewrite-failed, whose remediation says nothing was committed. (3)

• A HEAD that cannot be read once the commit was asked for. Every git rev-parse HEAD then fails, commit_once's and the guard's. Nothing is rolled back, the tree agrees with the commit that landed, and the refusal says whether the commit landed could not be read. (1)

The echo test uses a stream that raises, not PYTHONIOENCODING=cp1252. #214's escape_what_stdout_cannot_encode, now on main, reconfigures that exact stream to backslashreplace at the top of main(), so a test built on it would stop meaning anything. The hook tests set core.hooksPath in the repository's own config to a #!/bin/sh script, as tests/test_update_shape_add_ignored.py does. Git for Windows runs such a script through its own sh, and tests-windows passed them at 67a1163 (00:31Z).

Run against main's code, the new tests fail for the issue's reasons and the review's:

• On current main b421e6b's scripts/bump-leg.py (bump-leg.py and update-shape.py hand the project's own validators an explicit PYTHONIOENCODING, so a real bump or apply on a project named 日本 is no longer refused as bump-leg-validators-red or update-validators-red under a cp1252 stdout (#254) #281 for bump-leg.py (a real bump) and update-shape.py apply run the project's own validators as children that cannot print its display name on a cp1252 stdout, so a project named 日本 is refused and rolled back as bump-leg-validators-red or update-validators-red under PYTHONIOENCODING=cp1252:surrogateescape, and the refusal blames the validators #254 included): 22 of 24 fail. The same 22 fail on ede15ce's, from before bump-leg.py and update-shape.py hand the project's own validators an explicit PYTHONIOENCODING, so a real bump or apply on a project named 日本 is no longer refused as bump-leg-validators-red or update-validators-red under a cp1252 stdout (#254) #281 landed.
  • The read-only run exits 1 with the PermissionError traceback.
  • The ENOSPC, decode, echo and unwritable cases have their OSError, UnicodeDecodeError, UnicodeEncodeError or PermissionError escape main().
  • The validators-red and command-failed rollback-step cases have the leg's OSError escape main() over the original failure.
  • Both interrupts and the defect stop at pin is not as it was found.
  • The second interrupt escapes the rollback.
  • Both forward-checkout cases stop at status is not as it was found ( M spec).
  • The three cases that leave files in the leg have no line saying what became of the tree. Main prints only the traceback, or only REFUSED advancing a leg.
  • Both refused commits stop at pin is not as it was found.
  • The three commit-landed cases have no committed as line, and the unreadable HEAD has no could not be read.
  • The two that pass are guards: the operator's untracked file beside a red validator, and the leg detached at the commit it stood on. Main's own red-validator rollback already gets both right. They are here so that this branch's comparison of the leg cannot break either without a test failing.
• On fix round 1's head 67a1163's script: the 3 cases that leave files in the leg fail, each on a line that says every byte is back. The other 21 pass, the three nit tests among them: they hold rules 67a1163 already kept, which re-verification 1's mutants showed no test held.
• On the reviewed head a8d31ee's script: the 12 cases fix round 1 added or tightened fail, each for its finding. The other 5 pass.
• Round 1 ran its 9 cases on 39ac59c's and 60d08a1's scripts: 9 of 9 failed on each.

Targeted, at head 5269aac (all passed):

file	passed
tests/test_bump_leg.py	23 (unchanged)
tests/test_bump_leg_rollback.py	24
tests/test_commit_trailers.py	125
tests/test_cli_reference.py	13
tests/test_repo_hygiene.py	50

• scripts/render-cli-reference.py --check ok.
• 20 mutants of this head were all killed. They are re-verification 1's 17, with the three on _restore_leg_if_moved re-spelled for its new shape, and three of this round's own:
  • 18: the status is never compared (3 failed);
  • 19: the status is compared only when the leg did not move (1 failed, moved-then-interrupted);
  • 20: _put_back has no clause of its own for the status step (3 failed).
• The re-verification's three survivors now fail one test each: 9 (an unreadable HEAD rolls back), 12 (the leg compared by HEAD alone) and 16 (an OSError after a landed commit made into bump-leg-rewrite-failed).
• The full suite was not run, because CI runs it on three OSes.

Composes with

This branch was cut from 39ac59c. Main is now b421e6b. Four changes to scripts/bump-leg.py have landed there since the cut: #246 for #214, #230 for #217, #222 for #207 (018ce0d), and #281 for #254 (b421e6b, 00:55Z).

• git merge-tree --write-tree <this head> b421e6b (current main, with bump-leg.py and update-shape.py hand the project's own validators an explicit PYTHONIOENCODING, so a real bump or apply on a project named 日本 is no longer refused as bump-leg-validators-red or update-validators-red under a cp1252 stdout (#254) #281): rc 0, no conflicting files. In a scratch checkout of that merge, 124 passed: test_bump_leg.py 33, test_bump_leg_rollback.py 24, test_validator_child_encoding.py (bump-leg.py and update-shape.py hand the project's own validators an explicit PYTHONIOENCODING, so a real bump or apply on a project named 日本 is no longer refused as bump-leg-validators-red or update-validators-red under a cp1252 stdout (#254) #281's) 51, test_git_output_decoding.py 3 and test_cli_reference.py 13. render-cli-reference.py --check ok.

• git merge-tree --write-tree <this head> ede15ce (main just before bump-leg.py and update-shape.py hand the project's own validators an explicit PYTHONIOENCODING, so a real bump or apply on a project named 日本 is no longer refused as bump-leg-validators-red or update-validators-red under a cp1252 stdout (#254) #281 landed): rc 0, no conflicting files. In a scratch checkout of that merge, 120 passed and 1 skipped (a Windows-only default in test_stdout_encoding.py):

  file	cases
  test_bump_leg.py (with bump-leg.py rewrites and commits only the workflow files git tracks, so an untracked draft workflow that names the leg is no longer added whole to the lockstep commit and an ignored one no longer dies at git add after the rewrite: both are refused by name as bump-leg-workflow-untracked, before anything is written and under --dry-run as well (#207) #222's 10 new tests)	33 passed
  test_bump_leg_rollback.py	24 passed
  test_cli_reference.py	13 passed
  test_git_output_decoding.py	3 passed
  test_stdout_encoding.py	47 passed, 1 skipped

  render-cli-reference.py --check was ok there too. Main's test_no_shipped_text_mode_call_decodes_with_the_locale_codec passes on that tree, because this branch adds no text-mode subprocess call: the three new git reads (the root's HEAD, the leg's HEAD and the leg's status) use git_out, which reads bytes.

• git merge-tree --write-tree <this head> 6d8d88b (main when fix round 2 began): rc 0.

• git merge-tree --write-tree <this head> 754803d (bump-leg.py and update-shape.py hand the project's own validators an explicit PYTHONIOENCODING, so a real bump or apply on a project named 日本 is no longer refused as bump-leg-validators-red or update-validators-red under a cp1252 stdout (#254) #281's head, now landed as b421e6b): rc 0, no conflicting files. That branch changes run_validator, the import line, a few subprocess.run decodings (commit_once's among them) and the first line of main(). This one changes _apply_rewrite, its helpers and cmd_bump's tail.

• git merge-tree --write-tree <this head> 7c41d98 (bump-leg.py rewrites and commits only the workflow files git tracks, so an untracked draft workflow that names the leg is no longer added whole to the lockstep commit and an ignored one no longer dies at git add after the rewrite: both are refused by name as bump-leg-workflow-untracked, before anything is written and under --dry-run as well (#207) #222's head before it landed): rc 0, no conflicting files. Its new Refusal is raised from plan_workflows, before _apply_rewrite, so nothing is written and nothing needs rolling back.

• The review's scan of 28 open heads at a8d31ee found only those two touching scripts/bump-leg.py or the bump-leg tests. It was not repeated this round.

Left out

• The README and the handbook. README.md and docs/handbook.html say a red validator rolls the tree back and that a refusal leaves the tree as it was found. That is now true for a refused commit as well. In the two corners where it is not true, the refusal says THE ROLLBACK WAS NOT COMPLETE instead: a rollback step itself fails, or a cut-off checkout left files in the leg. bump-leg.py rewrites and commits only the workflow files git tracks, so an untracked draft workflow that names the leg is no longer added whole to the lockstep commit and an ignored one no longer dies at git add after the rewrite: both are refused by name as bump-leg-workflow-untracked, before anything is written and under --dry-run as well (#207) #222, now on main, edited the same README paragraph, so it is left alone here.
• Nothing in the leg is removed. The files a cut-off checkout wrote stay in the leg until the operator removes them. Until then, git refuses the re-run's checkout (untracked working tree files would be overwritten). That re-run wrote nothing, so its own line, that every byte it wrote is back, is true. Removing only the paths that appeared and that the new commit's tree holds was considered and left out, because it is not provably safe. git checkout overwrites an IGNORED file without asking, and a new .gitignore written before the cut can make that file show as untracked, so such a path could be a file the operator had.
• A file git ignores. The leg's git status does not list ignored files. A cut-off checkout that wrote only paths the leg's .gitignore matches therefore leaves files that no report names. git overwrites ignored files without asking, so they do not block the re-run. Listing ignored files (--ignored) would read every build output in the leg twice per bump, so it is not done.
• A hook that refuses every checkout. If the leg's post-checkout hook refuses the restoring checkout too, that step is reported as not put back, even when git did move the leg back. The report then says THE ROLLBACK WAS NOT COMPLETE and sends the operator to git status. This is conservative, not wrong. Re-verification 1 suggested re-reading the branch and HEAD after such a failure, and counting the step done when they match (its nit B). That is declined: a hook that failed can leave work undone that neither the HEAD nor git status shows. git-lfs's post-checkout, for one, sets the read-only bit on lockable files, and git does not track that bit.
• An interrupt outside a rollback step. An interrupt that lands outside _put_back is not caught, for example between two rollback steps or while the handler itself prints. By design, no signal handler is installed from a library function.
• No refusal for an interrupt. An uncaught KeyboardInterrupt is not turned into a refusal. The operator gets one line saying whether the tree is back, then the interpreter's own traceback and exit status.
• update-shape.py apply still handles except (Refusal, CommandFailed) in cmd_apply (at ede15ce). Whether an OSError or an interrupt there leaves a half-applied re-pin was not probed here. It is a follow-up for a lane that wants it.
• A non-UTF-8 workflow file or pin ends in a traceback before anything moves. This is the review's follow-up item, outside bump-leg.py _apply_rewrite rolls back only on Refusal and CommandFailed, so a PermissionError on a workflow file it rewrites, or the UnicodeEncodeError that #230's errors="replace" moves the decode crash to, leaves the pin rewritten and the leg on the new commit, with a traceback and no rollback #253's rollback promise, and the coordinator files it.

Review

Lane openRepoShape-2's own adversarial review of a8d31ee found CHANGES NEEDED: #264 (comment). Fix round 1 below answers it. The lane's re-verification of fix round 1's head 67a1163 found CHANGES NEEDED again: #264 (comment). Fix round 2 below answers that. A review by another lane is welcome.

Fix round 1

This round answers #264 (comment). Every item is in commit 67a1163, and every test named failed at a8d31ee and passes at 67a1163.

• P2: a commit git refuses is rolled back. The commit is inside the try, under the HEAD guard. Tests: test_a_commit_git_refuses_puts_every_byte_back, and test_a_failure_after_the_commit_landed_rolls_nothing_back for the guard.
• P3: a refusal no longer claims a rollback that failed. It says THE ROLLBACK WAS NOT COMPLETE, for every Refusal, and a failed git command gets a line saying the same. Test: test_a_rollback_step_that_fails_is_named_and_the_others_still_run, which now has 3 cases and an unconditional assertion.
• P3: a second interrupt during the rollback names its step, and the steps after it still run. Test: test_an_interrupt_inside_a_rollback_step_is_named_and_the_steps_after_it_still_run.
• P3: the leg's restore is decided from the leg. A forward checkout that moved it and then failed or was interrupted is put back. Test: test_a_forward_checkout_that_moved_the_leg_and_then_failed_puts_it_back.
• nit: a line before an interrupt's or a defect's traceback says whether the tree is back. Test: test_any_other_failure_puts_every_byte_back_and_is_re_raised_as_itself, which now asserts it.
• Follow-up (a non-UTF-8 workflow or pin): not in this PR. The coordinator files it.

Fix round 2

This round answers #264 (comment).

• P2: a forward checkout cut off before it moved the leg is no longer reported as put back. Commit d75151f.

  • The leg's git status is read before the checkout and compared in the rollback, after any restore. When it differs, the leg step is named, the operator is sent to git -C <leg> status, and the report says THE ROLLBACK WAS NOT COMPLETE. Nothing in the leg is removed.

  • Tests: test_a_forward_checkout_that_left_files_in_the_leg_is_not_called_put_back (3 cases, all failing at 67a1163 on a line that says every byte is back), and test_an_untracked_file_the_operator_keeps_in_the_leg_is_left_alone_and_not_counted.

  • The re-verification's real-SIGINT reproduction (partial.sh: a 40000-file leg commit, SIGINT to the whole process group 0.15 s after the checkout starts), re-run at 5269aac:

    terminal Ctrl-C (python's default handler): exit 130
      REFUSED rolling the bump back. the checkout of spec/ could not be put back: its `git status` no longer reads as it did before the bump, ... Read `git -C <leg> status` before anything else, ...
      KeyboardInterrupt while the bump was being applied. THE ROLLBACK WAS NOT COMPLETE: ...
      leg on main at 9b32760 (unmoved), 1305 files under big/ left; "exactly as it was": 0 times
    git alone killed: exit 2
      the same step line; The command below failed while the bump was being applied. THE ROLLBACK WAS NOT COMPLETE: ...
      leg on main at 9b32760, 40000 files left; "exactly as it was": 0 times
    

    At 67a1163 both runs said every byte was back and the tree was exactly as it was.

• Nit A: the three surviving mutants now each fail a test. Commit 5269aac, tests only.
  • 9: test_a_head_that_cannot_be_read_after_the_commit_rolls_nothing_back.
  • 12: test_a_leg_detached_at_the_commit_it_stood_on_is_put_back_on_its_branch.
  • 16: the new oserror case of test_a_failure_after_the_commit_landed_rolls_nothing_back.
• Nit B: declined. See "Left out" on a hook that refuses every checkout.
• This body: the two sentences the re-verification found false are now true, and say how. "No report claims more than the rollback did" names the leg's git status and the one corner it cannot see. "A leg that never moved is not checked out again" is followed by the comparison that is still made. Main is b421e6b, which holds bump-leg.py and update-shape.py hand the project's own validators an explicit PYTHONIOENCODING, so a real bump or apply on a project named 日本 is no longer refused as bump-leg-validators-red or update-validators-red under a cp1252 stdout (#254) #281's head 754803d.

Closes #253

🤖 Generated with Claude Code

Summary by Sourcery

Make bump-leg restore all changes after any pre-commit failure and accurately report the resulting repository state.

Bug Fixes:

• Ensure bump-leg failures after the leg starts moving—including write, validator, interrupt, checkout, and refused-commit failures—restore the pin, workflows, leg checkout, and index whenever possible.
• Prevent reports from claiming a complete rollback when restoration fails or a partially completed checkout leaves files in the leg.
• Preserve committed changes when a failure occurs after the commit lands, and report whether the commit landed or could not be read.

Enhancements:

• Centralize rollback handling with per-step failure reporting, interrupt protection, file restoration guards, HEAD checks, and leg status verification.
• Convert unrecovered OSError and UnicodeError failures into clear exit-2 refusals while preserving command failures, interrupts, and other defects.

Documentation:

• Document the expanded rollback and failure-reporting guarantees in bump-leg.py's user-facing refusal documentation.

Tests:

• Add comprehensive rollback coverage for partial writes, encoding failures, interrupts, rollback failures, interrupted checkouts, refused commits, post-commit failures, unreadable HEAD state, and untracked leg files.

[brettheap]

(lane openRepoShape-2, 2026-10-11T22:39:49Z) @codex review

[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 1 day and 1 hour by commenting @sourcery-ai review. Upgrade to get a review now.

[sourcery-ai]

Reviewer's Guide

The PR hardens bump-leg.py so any failure after a bump starts restores all changed repository state, reports incomplete rollback without masking the triggering failure, and classifies operational encoding/write errors as exit-2 refusals; a new adversarial test suite covers the rollback boundary and failure modes.

Sequence diagram for bump-leg failure rollback

sequenceDiagram
    participant Apply as _apply_rewrite
    participant Rollback as _roll_back
    participant File as _restore_file
    participant Leg as leg checkout
    participant Git as git index
    participant Report as Refusal or original failure

    Apply->>Apply: write pin and workflow files
    Apply->>Leg: restore_leg(submodule, placement, was)
    Apply->>Git: run(git reset -q HEAD -- staged)
    Apply-->>Rollback: failure after first byte moved
    Rollback->>File: _restore_file(target, original)
    Rollback->>Leg: restore_leg(submodule, placement, was)
    Rollback->>Git: run(git reset -q HEAD -- staged)
    Rollback-->>Apply: rolled_back
    alt OSError or UnicodeError
        Apply->>Report: raise Refusal(bump-leg-rewrite-failed)
    else other BaseException
        Apply->>Report: re-raise failure
    end

Loading

File-Level Changes

Change	Details	Files
Make rewrite rollback cover every failure after the leg checkout and preserve the original failure while reporting rollback problems.	Catch BaseException around the rewrite process, including interrupts and unexpected defects. Extract guarded file restoration, per-step rollback reporting, and ordered rollback orchestration into helpers. Restore written files only when their bytes differ, then independently restore the leg checkout and index. Re-raise Refusal/CommandFailed unchanged; convert OSError/UnicodeError to bump-leg-rewrite-failed after rollback, while re-raising other failures unchanged.	scripts/bump-leg.py
Add adversarial tests that verify complete tree restoration and failure classification across write, validator, interrupt, encoding, and rollback failures.	Test read-only and partial-write workflow failures, including no false rollback error when a file was never modified. Test validator decode and stdout-encoding failures as clean exit-2 refusals. Test interrupts and unexpected runtime errors are re-raised only after restoring files, checkout, and index. Test rollback-step failures are named without masking the original failure, while subsequent rollback steps still run. Snapshot and compare repository HEAD, pin/workflow bytes, gitlink, index state, leg HEAD, and branch placement.	tests/test_bump_leg_rollback.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#253	Ensure that any failure after the leg checkout begins—including PermissionError/OSError, UnicodeError while handling validator output, interrupts, and unexpected exceptions—rolls back the rewritten pin, workflow files, leg checkout, and index instead of leaving a half-applied bump.	✅
#253	Make rollback itself best-effort and non-throwing so every restoration step is attempted, rollback failures are reported, and the original failure is not obscured.	✅
#253	Report OSError and UnicodeError rewrite failures as clean refusals with exit status 2 and useful diagnostics, while preserving the original exception behavior for interrupts and unrelated defects; add regression coverage for these scenarios.	✅

Possibly linked issues

• bump-leg.py _apply_rewrite rolls back only on Refusal and CommandFailed, so a PermissionError on a workflow file it rewrites, or the UnicodeEncodeError that #230's errors="replace" moves the decode crash to, leaves the pin rewritten and the leg on the new commit, with a traceback and no rollback #253: PR fixes issue bump-leg.py _apply_rewrite rolls back only on Refusal and CommandFailed, so a PermissionError on a workflow file it rewrites, or the UnicodeEncodeError that #230's errors="replace" moves the decode crash to, leaves the pin rewritten and the leg on the new commit, with a traceback and no rollback #253 by rolling back files, checkout, and index on all failures, then reporting appropriate refusals.

---

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.

[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-10-11T22:46:32.971602Z	a8d31ee	Manual request

ℹ️ 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.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a8d31ee1b5

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Extend rollback through commit creation

When validation succeeds but commit_once() subsequently fails—for example because a commit hook rejects the commit or .git becomes unwritable—_apply_rewrite has already returned, so this handler cannot run; the rewritten pin, workflows, and gitlink remain staged and the leg stays detached. The new guarantee that no later failure leaves a half-applied bump therefore excludes a routine final write step; keep commit creation inside the same rollback transaction.

AGENTS.md reference: AGENTS.md:L300-L303

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Honor failed rollback status for validator refusals

When a validator goes red and an undo step then fails (for example, restore_leg raises an OSError), _roll_back returns False, but that result only affects failures that were originally OSError or UnicodeError. A validator failure is a Refusal, so it is re-raised with the existing claim that every byte was restored and the tree is exact, despite the preceding rollback error and the still-moved leg; report the incomplete rollback for these failures too so the operator knows to inspect the tree.

AGENTS.md reference: AGENTS.md:L322-L325

Useful? React with 👍 / 👎.

[brettheap]

(lane openRepoShape-2, 2026-10-11T23:47:11Z)

This lane reviewed its own PR #264 at a8d31ee, before any fix round, read-only in its own detached worktrees, and authored nothing in this pass.

CHANGES NEEDED. There is no sequencing constraint.

Findings, file:line at a8d31ee:

• P2 (codex's first P2, reproduced). A commit git refuses after green validators is not rolled back.
  • Triggers: a failing commit.gpgsign signer, or a commit-msg hook that exits 1.
  • Result: exit 2. The pin, the workflow and the gitlink stay staged, and the leg stays detached at the new commit. The re-run is refused as bump-leg-root-dirty.
  • commit_once (scripts/bump-leg.py:835) runs outside the try at :706, yet the title and the new docstring at :42-46 promise any failure is rolled back.
• P3 (codex's second P2, reproduced; P3 because a rollback step must fail first). With a red validator and the leg's index.lock held, the refusal still claims the leg's checkout was rolled back. git status shows M spec. rolled_back (:750) reaches only the OSError/UnicodeError branch.
• P3. A second SIGINT during the rollback escapes _put_back (:647). The index step never runs, and nothing is printed.
• P3. A forward checkout that moves the leg and then fails (a post-checkout hook) or is interrupted leaves the leg moved. moved_leg is set at :712, after run returns.
• nit. An interrupt ends in a bare traceback that never says the tree was put back.
• Follow-up. A workflow that is not UTF-8 ends in a traceback at :269 before anything moves.

Each finding is the same on main. Head fixes the issue's read-only reproduction.

Tests:

• head: 220 passed;
• main+head: 270 passed, 1 skipped;
• with bump-leg.py rewrites and commits only the workflow files git tracks, so an untracked draft workflow that names the leg is no longer added whole to the lockstep commit and an ignored one no longer dies at git add after the rewrite: both are refused by name as bump-leg-workflow-untracked, before anything is written and under --dry-run as well (#207) #222 and 5757e29 as well: 143 passed, 1 skipped;
• the 9 new tests fail on main;
• 8 of 8 mutants killed;
• CI green.

Merges: clean (rc 0) against main (c3f4987 and 60588d5), #222, 5757e29 and #266 to #270.

The full report is handoffs/xFactory/attachments/lane-openRepoShape-2/review264-a8d31ee/final-report.md in opensoft/brett-wip.

[brettheap]

(lane openRepoShape-2, 2026-10-12T00:31:43Z)

This lane re-verified its own PR #264 at 67a1163 after fix round 1, read-only in its own detached worktrees.

CHANGES NEEDED

Items, re-run with the first pass's real-git reproductions:

1. Verified. A refused commit (signer, commit-msg hook) is rolled back; one that landed, then was interrupted or left HEAD unreadable, is not, and says so.
2. Verified. A red validator with a failed undo says THE ROLLBACK WAS NOT COMPLETE, never "exactly as it was".
3. Verified. A second SIGINT in the leg step is named, and the index step still runs.
4. Verified for a checkout that moved the leg, on a branch or detached.
5. Verified. The interrupt line is printed.

New P2. A terminal Ctrl-C cutting off the forward checkout before git moves HEAD is reported as "the tree is exactly as it was", yet the leg holds 851 files the checkout wrote, and the re-run fails with untracked working tree files would be overwritten. _restore_leg_if_moved (scripts/bump-leg.py:684) checks only the branch and HEAD. Main leaves the same files but claims nothing.

Nits. Three mutants survive (unreadable HEAD, leg compared by HEAD only, OSError after a landed commit). tests-windows was still running at 00:29Z.

Tests:

• head: 228 passed;
• b30595c+head: 163 passed, 1 skipped;
• with 5757e29 as well: 104 passed;
• the 17 new cases fail on main;
• mutants: 14 of 17 killed.

Merges: rc 0 against origin/main b30595c, 5757e29 and #281's 754803d.

The full report is handoffs/xFactory/attachments/lane-openRepoShape-2/review264-a8d31ee/reverify-1.md in opensoft/brett-wip.

[brettheap]

(lane openRepoShape-2, 2026-10-12T01:12:44Z)

This lane re-verified its own PR #264 at 5269aac after fix round 2, read-only in its own detached worktrees.

MERGEABLE WITH NITS, once tests-windows passes.

• P2, cut-off checkout: fixed. A real group SIGINT, with or without python's handler, says THE ROLLBACK WAS NOT COMPLETE and names git -C spec status; following that, the bump succeeds.
• No false NOT COMPLETE. The operator's untracked or ignored files, a sparse leg, a nested submodule and a branchless leg all roll back as complete.
• Status read. It cannot fail an odd leg: the one failing case, a broken nested gitdir, is refused earlier by the existing clean check, on main too.
• Mutants 9, 12 and 16 are killed.
• Nits. --untracked-files=normal survives (no test). An ignored file at a path the target commit tracks is lost on rollback, as on main.

Tests:

• head: 235 passed;
• with main b421e6b: 221 passed, 1 skipped;
• 22 of 24 new cases fail on main;
• mutants: 22 of 23 killed.

CI: tests, macOS and SonarCloud pass; tests-windows was pending at 01:11Z. Round 1's Windows run passed.

Merge with b421e6b: rc 0.

The full report is handoffs/xFactory/attachments/lane-openRepoShape-2/review264-a8d31ee/reverify-2.md in opensoft/brett-wip.

[sonarqubecloud]

Quality Gate passed

Issues
4 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