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

Lane: openreposhape-5 (openRepoShape-5)

Found by this lane while reviewing #214's fix: the review's follow-up 1 reproduced a UnicodeDecodeError from bump-leg.py's run_validator that leaves the bump half applied, "the rollback does not cover a decode error". Lane 3's PR #230 (fix/decode-git-output-utf8, head fbdf8db) rewrites that run_validator call with errors="replace", so the decode crash itself is covered there and this issue does not ask for it. What #230 does not touch, and what this issue is about, is the rollback: it catches exactly two exception types, so any other failure between the leg's checkout and the end of the validators leaves the bump half applied. Pre-existing on main 54aa69a; the same on #230's head.

What is wrong. _apply_rewrite (scripts/bump-leg.py:617-689) checks the leg out detached (moved_leg = True), writes the pin and each workflow file (:644 written[target] = target.read_bytes(), :652 write_lf), stages them, runs the validators and echoes their output (:663 sys.stdout.write(output)), and rolls every byte back on except (Refusal, CommandFailed) (:675). Its docstring promises "or roll every byte back and re-raise", and the comment above the handler says the rollback "never raises over the refusal". Any other exception (an OSError from a write or read, a UnicodeError from the echo, a KeyboardInterrupt) passes through the handler without a rollback and, since main() (:794-802) catches only Refusal and CommandFailed too, reaches the interpreter as a traceback. The tree is left with the pin rewritten, the leg moved to the new commit and, once the files are staged, the index changed, while the tool prints no finding about any of it.

SHAPE=<openRepoShape checkout>; TMP=$(mktemp -d); cd "$TMP"
GIT='git -c user.name=T -c user.email=t@example.invalid'
# a scaffolded project (local bare remotes, no network), cloned recursively, and one new commit on the spec leg
python3 "$SHAPE/scaffold-project.py" --org testorg --project Northwind --elected-by 'T H' \
    --elected-on 2026-09-02 --local-remote-dir remotes --work-dir work >scaffold.out 2>&1 || exit 1
git -c protocol.file.allow=always clone -q --recurse-submodules remotes/Northwind.git base || exit 1
git clone -q remotes/Northwind-spec.git adv && ( cd adv && printf '# new\n' > NEW.md && git add NEW.md \
    && $GIT commit -qm advance && git push -q origin HEAD:refs/heads/adv ) && NEW=$(git -C adv rev-parse HEAD)
( cd base && git checkout -q -b bump/spec )
# a workflow that names both legs, which the lockstep rewrite edits, made read-only
READONLY='SPEC=$(git rev-parse HEAD:spec); CODE=$(git rev-parse HEAD:code)
printf "on: [pull_request]\njobs:\n  s:\n    uses: testorg/Northwind-spec/.github/workflows/x.yml@%s\n  c:\n    uses: testorg/Northwind-code/.github/workflows/y.yml@%s\n" "$SPEC" "$CODE" > .github/workflows/legs.yml
git add .github/workflows/legs.yml && '"$GIT"' commit -qm workflow && chmod a-w .github/workflows/legs.yml'
bump() {   # bump <label> <prep> [VAR=value ...]: a REAL bump on a fresh copy of the project
  label=$1; prep=$2; shift 2; rm -rf "run-$label"; cp -a base "run-$label"; cd "run-$label"; eval "$prep"
  env "$@" python3 "$SHAPE/scripts/bump-leg.py" --root "$PWD" --leg spec --to "$NEW" \
      --local-remote-dir "$TMP/remotes" >"../out-$label" 2>"../err-$label"
  echo "== $label: exit $?"; grep -a -E '^[A-Za-z]*(Error|Exception):|^REFUSED' "../err-$label" | cut -c1-110 | tail -2
  echo "   git status: $(git status --short | tr '\n' ' ')| leg HEAD $(git -C spec rev-parse --short HEAD), was $(git -C ../base/spec rev-parse --short HEAD)"
  cd "$TMP"
}
bump decode ''       LC_ALL=C.UTF-8 PYTHONIOENCODING=cp1252   # the child writes cp1252, the parent reads UTF-8
bump readonly "$READONLY" LC_ALL=C.UTF-8
chmod -R u+w "$TMP"

Observed on main 54aa69a (the leg's short hashes differ per run):

== decode: exit 1
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x85 in position 115: invalid start byte
   git status: M  contracts/spec-pin.yaml M  spec | leg HEAD 4092a86, was fba5462
== readonly: exit 1
PermissionError: [Errno 13] Permission denied: '<TMP>/run-readonly/.github/workflows/legs.yml'
   git status:  M contracts/spec-pin.yaml  M spec | leg HEAD 4092a86, was fba5462

Observed on fix/decode-git-output-utf8 fbdf8db (PR #230):

== decode: exit 1
UnicodeEncodeError: 'charmap' codec can't encode character '\ufffd' in position 115: character maps to <undefined>
   git status: M  contracts/spec-pin.yaml M  spec | leg HEAD 09a99c3, was cf02cca
== readonly: exit 1
PermissionError: [Errno 13] Permission denied: '<TMP>/run-readonly/.github/workflows/legs.yml'
   git status:  M contracts/spec-pin.yaml  M spec | leg HEAD 09a99c3, was cf02cca

On main the first run is the decode crash the review found: the validator's … (0x85 in the child's cp1252) is not UTF-8, and the pin and the gitlink stay staged. #230's errors="replace" makes that read U+FFFD, and the SAME run now dies one statement later, when sys.stdout.write(output) meets a stdout that cannot encode U+FFFD (here PYTHONIOENCODING=cp1252, strict): a different exception, the same half-applied tree. The second run needs no encoding at all: a workflow file git tracks and the operator cannot write (a read-only attribute, a root-owned checkout) raises PermissionError at the write after the leg was moved and the pin was written: git status shows the pin modified and spec modified (the leg's own checkout is at the new commit, not the one the project pins), and the tool prints a traceback, not a refusal.

Expected. A bump that fails for any reason after the leg moves puts back every byte it wrote, the staging and the leg's checkout, names the failure, and exits with a refusal the way bump-leg-validators-red does: the handler covers every exception the sequence can raise (at least OSError and UnicodeError beside Refusal and CommandFailed; except BaseException: re-raising after the restore also covers an interrupt during the validators, which this issue did not run). The rollback's own writes need a guard of their own: written[target] is set BEFORE write_lf(target, data) (:644 before :652), so for the read-only file above the restore (target.write_bytes(original)) would meet the same PermissionError, so each restore is attempted and a failure is printed, never raised (reasoned from the code, not run). Tests: the read-only workflow, and a stdout that cannot encode the validator's output, each asserting an exit of 2, a clean git status and the leg back at its pinned commit.

Reach. Anyone who runs bump-leg.py (the lockstep command the README documents) on a checkout where a file it rewrites cannot be written, or where the tool's stdout cannot encode what a validator printed. Neither was run on Windows; a read-only attribute and a redirected stdout in the ANSI code page are the likely sources there. No writer in this repository produces either condition. update-shape.py apply has a rollback of the same shape; it was not probed here.

Found by. handoffs/openRepoShape/attachments/lane-openRepoShape-5/review214-report.md in opensoft/brett-wip (candidate follow-up 1), checked against #230's run_validator hunk for this issue. The runs above, on main and on #230's head, are new.

Not claimed.

🤖 Generated with Claude Code