From f73feec8e3fc51b19e17838e3959d11c6109d3f6 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 11 Oct 2026 22:20:06 +0000 Subject: [PATCH 01/14] bump-leg.py: a bump that fails for any reason once the leg has moved rolls the pin, the workflow files, the leg's checkout and the index back, so a workflow file it cannot write, a validator line the stdout cannot encode or an interrupt no longer leaves the pin rewritten and the leg on the new commit behind a traceback (#253) `_apply_rewrite` rolled back on `except (Refusal, CommandFailed)` and nothing else. A `PermissionError` on a workflow file, a `UnicodeError` from echoing a validator's output (the crash #230's `errors="replace"` moves the decode error to) or a `KeyboardInterrupt` passed through it: `main()` catches the same two types, so the run ended in a traceback with the pin rewritten, the leg on the new commit and, once staged, the index changed. The handler is now `except BaseException`, and the rollback it runs is three helpers instead of a loop and a `try`. `_put_back` attempts each step (every restored file, the leg's checkout, the index) whatever the steps before it did, and prints, never raises, a step that fails; `_restore_file` writes a file back only when it no longer holds its original bytes, so a read-only file the write never opened is not reported as unrestorable. A `Refusal` or a `CommandFailed` is re-raised as it was. An `OSError` or a `UnicodeError` leaves as `REFUSED bump-leg-rewrite-failed` naming the exception and saying whether the tree is back, the way `update-shape.py` turns an unwritable README into `update-readme-unwritable`; anything else, an interrupt included, is re-raised as itself once the tree is back. tests/test_bump_leg_rollback.py: a read-only workflow file, a stdout that cannot encode a validator's line, a write that dies half way, an interrupt while writing and while validating, a defect in the tool, and a rollback step that itself fails. Each compares the root's HEAD, the pin, every workflow, the gitlink, `git diff --cached --quiet` and the leg's HEAD and branch with what they were before the bump. All eight fail on the code before this change. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- scripts/bump-leg.py | 109 +++++++++++-- tests/test_bump_leg_rollback.py | 277 ++++++++++++++++++++++++++++++++ 2 files changed, 371 insertions(+), 15 deletions(-) create mode 100644 tests/test_bump_leg_rollback.py diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 1e9aac2e..c3449a64 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -39,6 +39,11 @@ * a validator that goes red after the rewrite. Every byte is rolled back first — the pin file, the workflow files, the leg's checkout and the index — so a refusal leaves the tree exactly as it was found. + * any other failure once the first byte has moved — a workflow file it cannot + write, a stdout that cannot encode what a validator printed — is rolled + back the same way and then reported, as a refusal naming the exception + for an `OSError` or a `UnicodeError`, and as itself for anything else + (an interrupt included), so no failure leaves a half-applied bump. EXIT CODES: 0 the commit was made, or a `--dry-run` printed a move it would make · 1 NOTHING TO DO — every fact already names that commit, under @@ -53,6 +58,7 @@ import re import subprocess import sys +from functools import partial from pathlib import Path SHAPE_ROOT = Path(__file__).resolve().parents[1] @@ -614,6 +620,62 @@ def _print_trailers(trailers: list[str]) -> None: print("trailers none") +def _restore_file(target: Path, original: bytes) -> None: + """Write `original` back, but only over a file that no longer holds it. + + A write that never opened the file — a read-only attribute, a permission + the operator does not have — changed nothing, and writing the same bytes + back would meet the same refusal and report a rollback failure for a file + that was never touched. + """ + if target.read_bytes() != original: + target.write_bytes(original) + + +def _put_back(what: str, undo) -> bool: + """Attempt ONE step of a rollback; say so, and carry on, if it fails. + + Each step is attempted whatever the steps before it did, and none of them + raises: a rollback that stopped at the first byte it could not restore + would leave the leg moved and the index staged behind a file the operator + cannot write. Returns whether the step succeeded. + """ + try: + undo() + except CommandFailed as exc: + print(exc.loudly("rolling the bump back"), file=sys.stderr) + except Exception as exc: # noqa: BLE001 - nothing may escape a rollback + print(f"REFUSED rolling the bump back. {what} could not be put back: " + f"{type(exc).__name__}: {exc}", file=sys.stderr) + else: + return True + return False + + +def _roll_back(root: Path, submodule: Path, placement: str | None, was: str, + staged: list[str], written: dict[Path, bytes], + moved_leg: bool) -> bool: + """Put back every byte `_apply_rewrite` changed: the files it wrote, the + leg's checkout and the index, in that order. Returns whether ALL of it + went back. + + THE ROLLBACK NEVER RAISES OVER THE FAILURE THAT STARTED IT. Whatever it + cannot put back is printed and the original failure is still the one the + caller re-raises: a rollback failure that replaced the validator's finding + would leave a person with a changed tree and no idea what the gate + objected to. + """ + steps = [(target.relative_to(root).as_posix(), + partial(_restore_file, target, original)) + for target, original in written.items()] + if moved_leg: + steps.append((f"the checkout of {submodule.name}/", + partial(restore_leg, submodule, placement, was))) + steps.append(("the index", partial( + run, ["git", "reset", "-q", "HEAD", "--", *staged], cwd=root))) + return all([_put_back(what, undo) for what, undo in steps]) + + def _apply_rewrite(root: Path, submodule: Path, commit: str, was: str, pin_path: Path, pin_text: str, digest: str, plans: list, staged: list[str]) -> None: @@ -626,8 +688,17 @@ def _apply_rewrite(root: Path, submodule: Path, commit: str, was: str, its order are unchanged: the leg is checked out detached BEFORE any file is written, the rewritten files are staged AFTER they are all written and BEFORE the validators run (`recorded_gitlink` reads the - INDEX), and a caught `Refusal` or `CommandFailed` rolls every byte back - before it is re-raised, never swallowed. + INDEX), and a failure of ANY kind rolls every byte back before it is + re-raised, never swallowed (#253: the handler named `Refusal` and + `CommandFailed` alone, so a `PermissionError` on a workflow file, a + `UnicodeError` from the validator echo or an interrupt left the pin + rewritten, the leg on the new commit and the index changed). + + A `Refusal` and a `CommandFailed` leave as they are. An `OSError` or a + `UnicodeError` leaves as a `Refusal` naming it — the repository's own + answer to a write that could not happen, `update-shape.py`'s + `update-readme-unwritable`, never a traceback — and anything else, an + interrupt included, is re-raised as itself once the tree is back. """ written: dict[Path, bytes] = {} placement = leg_placement(submodule) @@ -672,19 +743,27 @@ def _apply_rewrite(root: Path, submodule: Path, commit: str, was: str, "Remediation: read the finding above. A tool that left a " "project red would have moved the cost from this command " "to somebody else's pull request.") - except (Refusal, CommandFailed): - # THE ROLLBACK NEVER RAISES OVER THE REFUSAL. Whatever it cannot put - # back is printed and the original exit is re-raised: a rollback - # failure that replaced the validator's finding would leave a person - # with a changed tree and no idea what the gate objected to. - for target, original in written.items(): - target.write_bytes(original) - try: - if moved_leg: - restore_leg(submodule, placement, was) - run(["git", "reset", "-q", "HEAD", "--", *staged], cwd=root) - except CommandFailed as undo: - print(undo.loudly("rolling the bump back"), file=sys.stderr) + except BaseException as failure: + # `BaseException`, NOT `Exception`: an interrupt during the checkout + # or the validators is the failure a person causes on purpose, and it + # leaves the same half-applied bump as any other. + rolled_back = _roll_back(root, submodule, placement, was, staged, + written, moved_leg) + if isinstance(failure, (OSError, UnicodeError)): + raise Refusal( + "bump-leg-rewrite-failed", + f"{type(failure).__name__} after the leg was moved: {failure}. " + + ("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." + if rolled_back else + "THE ROLLBACK WAS NOT COMPLETE: what it could not put " + "back is named above, so read `git status` before " + "anything else."), + "Remediation: fix what the exception names — a file this " + "command cannot write, a stdout that cannot encode what a " + "validator printed — and run the same command again. " + "Nothing was committed.") from failure raise diff --git a/tests/test_bump_leg_rollback.py b/tests/test_bump_leg_rollback.py new file mode 100644 index 00000000..afc46a38 --- /dev/null +++ b/tests/test_bump_leg_rollback.py @@ -0,0 +1,277 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`scripts/bump-leg.py`: a bump that fails for ANY reason once its first byte +has moved puts every byte back (#253). + +`_apply_rewrite` checks the leg out, writes the pin and each workflow file, +stages them and runs the validators. Its rollback used to be a handler for +`Refusal` and `CommandFailed` alone, so a `PermissionError` on a workflow +file, a `UnicodeError` from echoing a validator's output or an interrupt left +the pin rewritten, the leg on the new commit and the index changed, behind a +traceback. `tests/test_bump_leg.py` holds the red-validator rollback; this +file holds the failures that rollback never saw. + +EVERY TEST COMPARES THE WHOLE TREE, not the one thing its failure touched: +the root's HEAD, the pin's bytes, every workflow's bytes, the gitlink the +index records, `git diff --cached --quiet`, the leg's HEAD and the branch the +leg stood on. A rollback that put the pin back and forgot the index is the +bug, so a test that read only the pin could not fail for it. + +The failures that need an operating system (a read-only file, a stdout that +cannot encode what a validator printed) run `bump-leg.py` as a subprocess. +The ones that need a failure at an exact step (a write that dies half way, an +interrupt, a rollback step that itself fails) call `main` in-process with ONE +module attribute replaced, and everything else, git included, real. +""" + +from __future__ import annotations + +import errno +import os +import stat +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import PROJECT, git +# A fixture is used by being named in this module's namespace, which is what +# the import of `bump_leg_module` is for. +from test_bump_leg import ( # noqa: F401 + BUMP, branched, bump, bump_leg_module, reference_both_legs, +) + +#: Where the advanced spec commit is pushed. Not `test_bump_leg.py`'s branch: +#: both modules share the session's bare remotes, and a second, different +#: commit pushed to the same branch name would be refused as non-fast-forward. +ADVANCE_BRANCH = "bump-leg-rollback-advance" + +#: Both validators the bump runs are the project's own copies. This one prints +#: a character no Windows ANSI code page or Latin-1 terminal can show, as +#: UTF-8 bytes straight to the pipe so the child's own stdout encoding has +#: no say in it. +CHATTY_VALIDATOR = '''\ +import sys + +sys.stdout.buffer.write("validator says: \\u0394 changed\\n".encode("utf-8")) +''' + + +@pytest.fixture(scope="module") +def advanced(scaffolded, tmp_path_factory) -> str: + """A real new commit on the spec leg's bare remote; returns its sha.""" + work = tmp_path_factory.mktemp("spec-advance-rollback") / f"{PROJECT}-spec" + proc = subprocess.run( + ["git", "clone", "-q", + str(scaffolded["remotes"] / f"{PROJECT}-spec.git"), str(work)], + capture_output=True, text=True, check=False) + assert proc.returncode == 0, proc.stderr + (work / "ROLLBACK-CONTRACT.md").write_text("# a contract the leg gained\n") + git("add", "--", "ROLLBACK-CONTRACT.md", cwd=work) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Advance the spec leg for the rollback tests", cwd=work) + git("push", "-q", "origin", f"HEAD:refs/heads/{ADVANCE_BRANCH}", cwd=work) + return git("rev-parse", "HEAD", cwd=work).stdout.strip() + + +def snapshot(project: Path) -> dict: + """Everything a bump may change and a rollback has to put back.""" + leg = project / "spec" + workflows = sorted((project / ".github" / "workflows").glob("*.y*ml")) + return { + "head": git("rev-parse", "HEAD", cwd=project).stdout.strip(), + "pin": (project / "contracts" / "spec-pin.yaml").read_bytes(), + "workflows": {path.name: path.read_bytes() for path in workflows}, + "gitlink": git("ls-files", "-s", "--", "spec", cwd=project).stdout, + "status": git("status", "--porcelain", cwd=project).stdout, + "leg_head": git("rev-parse", "HEAD", cwd=leg).stdout.strip(), + "leg_branch": git("symbolic-ref", "--quiet", "HEAD", cwd=leg, + check=False).stdout.strip(), + } + + +def assert_nothing_changed(project: Path, before: dict) -> None: + after = snapshot(project) + for key, value in before.items(): + assert after[key] == value, f"{key} is not as it was found" + assert git("diff", "--cached", "--quiet", cwd=project, + check=False).returncode == 0, "the index carries a staged change" + + +def prepared(project: Path) -> dict: + """A bump-ready root: a workflow naming both legs, committed; the spec leg + ON its branch, as `bootstrap.py` leaves it, so a rollback that restored the + commit but left the leg detached is a failure too; a bump branch.""" + reference_both_legs(project) + git("checkout", "-q", "-B", "main", cwd=project / "spec") + branched(project) + return snapshot(project) + + +def run_main(module, project: Path, scaffolded: dict, to: str) -> int: + return module.main(["--root", str(project), "--leg", "spec", "--to", to, + "--local-remote-dir", str(scaffolded["remotes"])]) + + +def raiser(exc: BaseException): + def raise_it(*_args, **_kwargs): + raise exc + return raise_it + + +# --- the operating system's failures, run for real -------------------------- + +def test_a_workflow_file_the_operator_cannot_write_is_refused_and_put_back( + project, scaffolded, advanced): + """THE ISSUE'S OWN REPRODUCTION: a tracked workflow file with a + read-only attribute. The leg was moved and the pin written before the + write failed; the file itself was never opened, so it is the one thing + the rollback must NOT report as unrestorable.""" + before = prepared(project) + legs = project / ".github" / "workflows" / "legs.yml" + legs.chmod(stat.S_IREAD) + try: + if os.access(legs, os.W_OK): + pytest.skip("this account can write a read-only file (root)") + result = bump(project, scaffolded, advanced) + finally: + legs.chmod(stat.S_IREAD | stat.S_IWRITE) + + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED bump-leg-rewrite-failed" in result.stderr + assert "PermissionError" in result.stderr + assert "Traceback" not in result.stderr, "a refusal, never a traceback" + assert "rolled back" in result.stderr + assert "rolling the bump back" not in result.stderr, ( + "the file was never changed, so there was nothing it could not put " + "back") + assert_nothing_changed(project, before) + + +def test_a_validator_line_the_stdout_cannot_encode_is_refused_and_put_back( + project, scaffolded, advanced): + """The echo is the statement after the validator ran, with the index + already staged: the crash #230's `errors="replace"` moves the decode error + to. UTF-8 in, a Latin-1 stdout out, on every platform.""" + (project / "scripts" / "validate-pins.py").write_text( + CHATTY_VALIDATOR, encoding="utf-8") + git("add", "--", "scripts/validate-pins.py", cwd=project) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "A validator with something to say", cwd=project) + before = prepared(project) + + # READ AS BYTES: the tool's own stdout is cp1252 here, so the suite's + # UTF-8 `text=True` reading would fail on the `…` it prints before the + # validators run, for a reason that has nothing to do with the tool. + result = subprocess.run( + [sys.executable, str(BUMP), "--root", str(project), "--leg", "spec", + "--to", advanced, "--local-remote-dir", str(scaffolded["remotes"])], + capture_output=True, check=False, stdin=subprocess.DEVNULL, + env={**os.environ, "PYTHONUTF8": "1", "PYTHONIOENCODING": "cp1252"}) + stderr = result.stderr.decode("cp1252", errors="replace") + + assert result.returncode == 2, stderr + assert "REFUSED bump-leg-rewrite-failed" in stderr + assert "UnicodeEncodeError" in stderr + assert "Traceback" not in stderr + assert_nothing_changed(project, before) + + +# --- a failure at an exact step, called in-process -------------------------- + +def test_a_write_that_dies_half_way_leaves_the_file_as_it_was( + project, scaffolded, advanced, bump_leg_module, monkeypatch, capsys): + """A full disk after the file was truncated and part of it written: the + bytes on disk are neither the old nor the new ones, and only the bytes + kept before the write can put the file back.""" + before = prepared(project) + real_write = bump_leg_module.write_lf + + def disk_full(path, text): + if path.name != "legs.yml": + return real_write(path, text) + path.write_bytes(text.encode("utf-8")[:20]) + raise OSError(errno.ENOSPC, os.strerror(errno.ENOSPC), str(path)) + + monkeypatch.setattr(bump_leg_module, "write_lf", disk_full) + code = run_main(bump_leg_module, project, scaffolded, advanced) + + err = capsys.readouterr().err + assert code == 2 + assert "REFUSED bump-leg-rewrite-failed" in err + assert os.strerror(errno.ENOSPC) in err + assert_nothing_changed(project, before) + + +@pytest.mark.parametrize("step, failure", [ + ("write_lf", KeyboardInterrupt()), + ("run_validator", KeyboardInterrupt()), + ("run_validator", RuntimeError("a defect in this tool")), +], ids=["interrupt-writing", "interrupt-validating", "defect-validating"]) +def test_any_other_failure_puts_every_byte_back_and_is_re_raised_as_itself( + step, failure, project, scaffolded, advanced, bump_leg_module, + monkeypatch): + """An interrupt, or a defect that is not the operator's to fix, is not + made into a refusal: it reaches the interpreter as what it is, AFTER the + tree is back. `write_lf` fails on the SECOND file, with the pin already + rewritten; `run_validator` fails with the index staged.""" + before = prepared(project) + if step == "write_lf": + real_write = bump_leg_module.write_lf + + def second_write_fails(path, text): + if path.name == "legs.yml": + raise failure + return real_write(path, text) + monkeypatch.setattr(bump_leg_module, "write_lf", second_write_fails) + else: + monkeypatch.setattr(bump_leg_module, "run_validator", raiser(failure)) + + with pytest.raises(type(failure)) as raised: + run_main(bump_leg_module, project, scaffolded, advanced) + + assert raised.value is failure + assert_nothing_changed(project, before) + + +@pytest.mark.parametrize("failure", ["validators-red", "unwritable"]) +def test_a_rollback_step_that_fails_is_named_and_the_others_still_run( + failure, project, scaffolded, advanced, bump_leg_module, monkeypatch, + capsys): + """THE ROLLBACK NEVER RAISES OVER THE FAILURE: with the leg's checkout the + one step that cannot be put back, the files and the index still are, the + step is named, and the failure the person sees is the original one.""" + before = prepared(project) + monkeypatch.setattr(bump_leg_module, "restore_leg", raiser( + OSError(errno.EIO, "the leg's checkout is gone"))) + if failure == "validators-red": + monkeypatch.setattr(bump_leg_module, "run_validator", + lambda root, script: (1, "pin-digest-mismatch\n")) + original = "REFUSED bump-leg-validators-red" + else: + real_write = bump_leg_module.write_lf + + def refused(path, text): + if path.name == "legs.yml": + raise PermissionError(errno.EACCES, "Permission denied", + str(path)) + return real_write(path, text) + monkeypatch.setattr(bump_leg_module, "write_lf", refused) + original = "REFUSED bump-leg-rewrite-failed" + + code = run_main(bump_leg_module, project, scaffolded, advanced) + + err = capsys.readouterr().err + assert code == 2 + assert original in err, "the original failure is the one reported" + assert "the checkout of spec/ could not be put back" in err + assert "the leg's checkout is gone" in err + if failure == "unwritable": + assert "THE ROLLBACK WAS NOT COMPLETE" in err, ( + "a refusal does not claim a tree it did not restore") + after = snapshot(project) + for key in ("head", "pin", "workflows", "gitlink"): + assert after[key] == before[key], f"{key} is not as it was found" + assert git("diff", "--cached", "--quiet", cwd=project, + check=False).returncode == 0, ( + "the index was still put back, though the leg's step failed before it") From 3c499c51e0cb218f89e289863fa7b4b9d68f337b Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 11 Oct 2026 22:28:52 +0000 Subject: [PATCH 02/14] bump-leg.py: the rollback tests raise the echo's failure from a stream, not from a codec setting, so they keep meaning the same thing once `main` escapes what a strict stdout cannot encode (#253) The first cut of the echo test ran `bump-leg.py` with `PYTHONIOENCODING=cp1252`. #214's `escape_what_stdout_cannot_encode` reconfigures exactly that stream to `backslashreplace` at the top of `main`, so that test would have started to see a bump that succeeds the day #214 landed. The echo test is now in-process with a stdout whose `write` raises `UnicodeEncodeError` for what cp1252 cannot hold, beside a `UnicodeDecodeError` raised from the validator read, so both ways a validator's output can fail the statement after the staging are covered whichever of #214 and #230 is in. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_bump_leg_rollback.py | 100 ++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 45 deletions(-) diff --git a/tests/test_bump_leg_rollback.py b/tests/test_bump_leg_rollback.py index afc46a38..7254dd3f 100644 --- a/tests/test_bump_leg_rollback.py +++ b/tests/test_bump_leg_rollback.py @@ -16,16 +16,26 @@ leg stood on. A rollback that put the pin back and forgot the index is the bug, so a test that read only the pin could not fail for it. -The failures that need an operating system (a read-only file, a stdout that -cannot encode what a validator printed) run `bump-leg.py` as a subprocess. -The ones that need a failure at an exact step (a write that dies half way, an -interrupt, a rollback step that itself fails) call `main` in-process with ONE -module attribute replaced, and everything else, git included, real. +The failure that needs the operating system (a read-only file) runs +`bump-leg.py` as a subprocess. The ones that need a failure at an exact step +(a write that dies half way, a validator's output that cannot be decoded or +echoed, an interrupt, a rollback step that itself fails) call `main` +in-process with ONE module attribute replaced, and everything else, git +included, real. + +THE ECHO IS A STREAM THAT RAISES, NOT A CODEC SETTING. A real `strict` stdout +(`PYTHONIOENCODING=cp1252`) is what #253 was found with, but +`escape_what_stdout_cannot_encode` (#214) reconfigures exactly that stream to +`backslashreplace` at the top of `main`, so a test built on it would stop +meaning anything the day #214 lands. A stream that raises +`UnicodeEncodeError` from `write` stays a failure in the echo whichever of +those fixes are in. """ from __future__ import annotations import errno +import io import os import stat import subprocess @@ -38,7 +48,7 @@ # A fixture is used by being named in this module's namespace, which is what # the import of `bump_leg_module` is for. from test_bump_leg import ( # noqa: F401 - BUMP, branched, bump, bump_leg_module, reference_both_legs, + branched, bump, bump_leg_module, reference_both_legs, ) #: Where the advanced spec commit is pushed. Not `test_bump_leg.py`'s branch: @@ -46,16 +56,6 @@ #: commit pushed to the same branch name would be refused as non-fast-forward. ADVANCE_BRANCH = "bump-leg-rollback-advance" -#: Both validators the bump runs are the project's own copies. This one prints -#: a character no Windows ANSI code page or Latin-1 terminal can show, as -#: UTF-8 bytes straight to the pipe so the child's own stdout encoding has -#: no say in it. -CHATTY_VALIDATOR = '''\ -import sys - -sys.stdout.buffer.write("validator says: \\u0394 changed\\n".encode("utf-8")) -''' - @pytest.fixture(scope="module") def advanced(scaffolded, tmp_path_factory) -> str: @@ -119,6 +119,15 @@ def raise_it(*_args, **_kwargs): return raise_it +class CannotEncode(io.StringIO): + """A stdout that holds what cp1252 holds and raises on anything else, as a + redirected Windows stdout does: `…` is fine, U+FFFD is not.""" + + def write(self, text): + text.encode("cp1252") + return super().write(text) + + # --- the operating system's failures, run for real -------------------------- def test_a_workflow_file_the_operator_cannot_write_is_refused_and_put_back( @@ -148,35 +157,6 @@ def test_a_workflow_file_the_operator_cannot_write_is_refused_and_put_back( assert_nothing_changed(project, before) -def test_a_validator_line_the_stdout_cannot_encode_is_refused_and_put_back( - project, scaffolded, advanced): - """The echo is the statement after the validator ran, with the index - already staged: the crash #230's `errors="replace"` moves the decode error - to. UTF-8 in, a Latin-1 stdout out, on every platform.""" - (project / "scripts" / "validate-pins.py").write_text( - CHATTY_VALIDATOR, encoding="utf-8") - git("add", "--", "scripts/validate-pins.py", cwd=project) - git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", - "A validator with something to say", cwd=project) - before = prepared(project) - - # READ AS BYTES: the tool's own stdout is cp1252 here, so the suite's - # UTF-8 `text=True` reading would fail on the `…` it prints before the - # validators run, for a reason that has nothing to do with the tool. - result = subprocess.run( - [sys.executable, str(BUMP), "--root", str(project), "--leg", "spec", - "--to", advanced, "--local-remote-dir", str(scaffolded["remotes"])], - capture_output=True, check=False, stdin=subprocess.DEVNULL, - env={**os.environ, "PYTHONUTF8": "1", "PYTHONIOENCODING": "cp1252"}) - stderr = result.stderr.decode("cp1252", errors="replace") - - assert result.returncode == 2, stderr - assert "REFUSED bump-leg-rewrite-failed" in stderr - assert "UnicodeEncodeError" in stderr - assert "Traceback" not in stderr - assert_nothing_changed(project, before) - - # --- a failure at an exact step, called in-process -------------------------- def test_a_write_that_dies_half_way_leaves_the_file_as_it_was( @@ -203,6 +183,36 @@ def disk_full(path, text): assert_nothing_changed(project, before) +@pytest.mark.parametrize("what", ["decoded", "echoed"]) +def test_a_validator_output_that_cannot_be_decoded_or_echoed_is_refused_and_put_back( + what, project, scaffolded, advanced, bump_leg_module, monkeypatch, + capsys): + """The statement after the leg moved, the files were written and the + index was staged: reading the validator's output (the decode error #230 + turns into a replacement character) and writing it to a stdout that + cannot hold that character (the crash it moves to).""" + before = prepared(project) + if what == "decoded": + monkeypatch.setattr(bump_leg_module, "run_validator", raiser( + UnicodeDecodeError("utf-8", b"\x85", 0, 1, "invalid start byte"))) + expected = "UnicodeDecodeError" + else: + monkeypatch.setattr( + bump_leg_module, "run_validator", + lambda root, script: (0, "validator says: \ufffd\n")) + monkeypatch.setattr(sys, "stdout", CannotEncode()) + expected = "UnicodeEncodeError" + + code = run_main(bump_leg_module, project, scaffolded, advanced) + + err = capsys.readouterr().err + assert code == 2 + assert "REFUSED bump-leg-rewrite-failed" in err + assert expected in err + assert "rolled back" in err + assert_nothing_changed(project, before) + + @pytest.mark.parametrize("step, failure", [ ("write_lf", KeyboardInterrupt()), ("run_validator", KeyboardInterrupt()), From a8d31ee1b57a2a886fea6d459b8fedcc49653c57 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 11 Oct 2026 22:37:32 +0000 Subject: [PATCH 03/14] bump-leg.py: `REFUSED bump-leg-rewrite-failed` says the failure came while the bump was being applied, so an `OSError` from the leg's checkout itself is no longer described as happening after the leg was moved (#253) The refusal said "after the leg was moved", which is false for an `OSError` raised by the checkout that moves it (a `git` that cannot be started). The rollback it follows is unchanged; only the sentence is. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- scripts/bump-leg.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index c3449a64..2512a2bd 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -752,7 +752,8 @@ def _apply_rewrite(root: Path, submodule: Path, commit: str, was: str, if isinstance(failure, (OSError, UnicodeError)): raise Refusal( "bump-leg-rewrite-failed", - f"{type(failure).__name__} after the leg was moved: {failure}. " + f"{type(failure).__name__} while the bump was being applied: " + f"{failure}. " + ("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." From 67a1163bcbff0a9a66c8226440ce8e39a5733b6e Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sun, 11 Oct 2026 23:58:30 +0000 Subject: [PATCH 04/14] bump-leg.py: a commit git refuses, a checkout that moved the leg and then failed, and a second interrupt during the rollback are all put back, and no report claims a rollback that was not complete, so a signing key that cannot sign no longer leaves the bump staged for the re-run to refuse as a dirty root (#253) Lane openRepoShape-2's review of a8d31ee found the rollback stopped short of the PR's own promise in four places, and one report said too little: - The commit ran after `_apply_rewrite` returned, outside the rollback. A commit git refuses (`commit.gpgsign` with a signer that fails, a `commit-msg` hook) left the pin, the workflows and the gitlink staged and the leg on the new commit, and the same command run again once the signer was fixed was refused as `bump-leg-root-dirty`. The commit is now the last statement inside the `try`. One `git rev-parse HEAD` is read just before it, and `_head_guard` compares the root's HEAD with it after any failure: unmoved, every byte goes back; moved (or unreadable), the bump is a commit, nothing is rolled back under it, and the report says it was committed. - A `bump-leg-validators-red` refusal said every byte was rolled back even when a rollback step had failed. The sentence is now one constant, `ROLLED_BACK`, and the handler replaces it with `NOT_ROLLED_BACK` (THE ROLLBACK WAS NOT COMPLETE) when the rollback did not earn it, in every `Refusal`; a `CommandFailed` gets the same sentence on a line before `main()` prints it. - `_put_back` caught `Exception` only, so a second Ctrl-C during the leg's checkout skipped the index step and left `MM` on the pin, the workflow and the gitlink. It now catches `KeyboardInterrupt` too, names the step as interrupted, and the steps after it still run. No signal handler: this is a library function. - The leg was restored only if the forward checkout returned, but git moves HEAD before `post-checkout` runs, so a failing hook (git-lfs's without `git-lfs` on PATH) or an interrupt while the checkout was being waited on left the leg on the new commit. `_restore_leg_if_moved` reads the leg's branch and HEAD and restores only when either differs, so an unmoved leg is not checked out again through the hook that refused. - An interrupt or a defect was re-raised with nothing saying whether the tree was back. One stderr line now says so before the traceback. tests/test_bump_leg_rollback.py: a refused commit (a signer that does not exist, and a `commit-msg` hook), a failure after the commit landed (an interrupt, and a refusal), a forward checkout that moved the leg (an interrupt, and a `post-checkout` hook that refuses the new commit only), an interrupt inside a rollback step, a `git add` that fails beside a failing rollback step, and the line before an interrupt's or a defect's traceback. Twelve cases fail on a8d31ee's script, each for its finding's reason. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- scripts/bump-leg.py | 197 +++++++++++++++++++++------- tests/test_bump_leg_rollback.py | 226 ++++++++++++++++++++++++++++++-- 2 files changed, 365 insertions(+), 58 deletions(-) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 2512a2bd..28c1108a 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -39,11 +39,16 @@ * a validator that goes red after the rewrite. Every byte is rolled back first — the pin file, the workflow files, the leg's checkout and the index — so a refusal leaves the tree exactly as it was found. - * any other failure once the first byte has moved — a workflow file it cannot - write, a stdout that cannot encode what a validator printed — is rolled - back the same way and then reported, as a refusal naming the exception - for an `OSError` or a `UnicodeError`, and as itself for anything else - (an interrupt included), so no failure leaves a half-applied bump. + * any other failure once the leg has been asked to move, up to and + including a commit git refuses (a signing key that cannot sign, a + `commit-msg` hook) — a workflow file it cannot write, a stdout that + cannot encode what a validator printed, an interrupt — is rolled back + the same way and then reported: as a refusal naming the exception for an + `OSError` or a `UnicodeError`, and as itself for anything else, an + interrupt included, after a line saying whether the tree is back. No + report claims more than the rollback did: a step it could not do is + named, and the report says THE ROLLBACK WAS NOT COMPLETE. A failure + after the commit has landed rolls nothing back, and says so. EXIT CODES: 0 the commit was made, or a `--dry-run` printed a move it would make · 1 NOTHING TO DO — every fact already names that commit, under @@ -620,6 +625,18 @@ def _print_trailers(trailers: list[str]) -> None: print("trailers none") +#: What a report says when the rollback put every byte back, and what it says +#: when it did not. ONE SPELLING EACH: a refusal written before the rollback +#: ran (`bump-leg-validators-red`) carries `ROLLED_BACK`, and the handler +#: replaces exactly that sentence when the rollback did not earn it. +ROLLED_BACK = ("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.") +NOT_ROLLED_BACK = ("THE ROLLBACK WAS NOT COMPLETE: what it could not put " + "back is named above, so read `git status` before " + "anything else.") + + def _restore_file(target: Path, original: bytes) -> None: """Write `original` back, but only over a file that no longer holds it. @@ -639,11 +656,23 @@ def _put_back(what: str, undo) -> bool: raises: a rollback that stopped at the first byte it could not restore would leave the leg moved and the index staged behind a file the operator cannot write. Returns whether the step succeeded. + + AN INTERRUPT IS CAUGHT HERE TOO. A second Ctrl-C while the rollback checks + a large leg out again is the likeliest next keystroke, and let through it + skipped every step after the one it landed in, leaving the new pin, the + new workflow and the new gitlink staged with nothing naming the index. + Caught per step rather than by a signal handler, which a library function + has no business installing: the step it landed in is named, the steps + after it (a file, the index: short ones) still run, and the first failure + is still the one re-raised. """ try: undo() except CommandFailed as exc: print(exc.loudly("rolling the bump back"), file=sys.stderr) + except KeyboardInterrupt: + print(f"REFUSED rolling the bump back. {what} could not be put back: " + "interrupted", file=sys.stderr) except Exception as exc: # noqa: BLE001 - nothing may escape a rollback print(f"REFUSED rolling the bump back. {what} could not be put back: " f"{type(exc).__name__}: {exc}", file=sys.stderr) @@ -652,12 +681,32 @@ def _put_back(what: str, undo) -> bool: return False +def _restore_leg_if_moved(submodule: Path, placement: str | None, + was: str) -> None: + """Put the leg back where it stood, if it no longer stands there. + + DECIDED FROM THE LEG, NOT FROM WHETHER THE CHECKOUT RETURNED. git moves + the leg's HEAD before it runs `post-checkout`, so a hook that exits + non-zero (git-lfs's does, with no `git-lfs` on PATH) fails a checkout + that has already moved the leg, and an interrupt can land while the + checkout is still being waited on after git has moved it. Either way the + leg is on the new commit and the checkout never returned. A leg that + still stands where it stood, on the same branch at the same commit, is + left alone, so a checkout that never moved it does not go through a + refusing hook a second time. + """ + if (leg_placement(submodule) == placement + and git_out(["rev-parse", "HEAD"], + cwd=submodule).lower() == was.lower()): + return + restore_leg(submodule, placement, was) + + def _roll_back(root: Path, submodule: Path, placement: str | None, was: str, - staged: list[str], written: dict[Path, bytes], - moved_leg: bool) -> bool: + staged: list[str], written: dict[Path, bytes]) -> bool: """Put back every byte `_apply_rewrite` changed: the files it wrote, the - leg's checkout and the index, in that order. Returns whether ALL of it - went back. + leg's checkout (when the leg no longer stands where it stood) and the + index, in that order. Returns whether ALL of it went back. THE ROLLBACK NEVER RAISES OVER THE FAILURE THAT STARTED IT. Whatever it cannot put back is printed and the original failure is still the one the @@ -668,19 +717,56 @@ def _roll_back(root: Path, submodule: Path, placement: str | None, was: str, steps = [(target.relative_to(root).as_posix(), partial(_restore_file, target, original)) for target, original in written.items()] - if moved_leg: - steps.append((f"the checkout of {submodule.name}/", - partial(restore_leg, submodule, placement, was))) + steps.append((f"the checkout of {submodule.name}/", + partial(_restore_leg_if_moved, submodule, placement, was))) steps.append(("the index", partial( run, ["git", "reset", "-q", "HEAD", "--", *staged], cwd=root))) return all([_put_back(what, undo) for what, undo in steps]) +def _head_guard(root: Path, head_before: str | None) -> str | None: + """None when the rollback is to run; otherwise the sentence the report + carries instead, saying why it must not. + + THE ROOT'S HEAD DECIDES, NOT WHICH STATEMENT RAISED. Until the commit is + asked for (`head_before` is None) nothing can have been committed. Once + it has been, an unmoved HEAD means git refused the commit, and every + byte goes back as for any other failure. A moved HEAD means the bump IS + a commit and the failure came after it (the `git rev-parse` that reads + it back, an interrupt); putting the files, the leg and the index back + under that commit would leave a tree that contradicts it. A HEAD that + cannot be read is treated the same way, since a rollback over a commit + that landed is the worse of the two mistakes. + """ + if head_before is None: + return None + try: + head = git_out(["rev-parse", "HEAD"], cwd=root).lower() + except BaseException as exc: # noqa: BLE001 - an interrupt included + return ("Whether the commit landed could not be read " + f"({type(exc).__name__}), so nothing was rolled back: read " + "`git log -1` and `git status` before anything else.") + if head == head_before: + return None + return (f"The bump was committed as {head[:12]} before this failed, so " + "nothing was rolled back: that commit holds the whole bump, and " + "`git log -1` shows it.") + + +def _with_verdict(detail: str, verdict: str) -> str: + """A refusal's `detail`, saying what the rollback actually did: in place + of the `ROLLED_BACK` it was written with, or else after it.""" + if ROLLED_BACK in detail: + return detail.replace(ROLLED_BACK, verdict, 1) + return f"{detail}\n{verdict}" + + def _apply_rewrite(root: Path, submodule: Path, commit: str, was: str, pin_path: Path, pin_text: str, digest: str, plans: list, - staged: list[str]) -> None: - """Rewrite the pin and every workflow file, stage them and validate — - or roll every byte back and re-raise. + staged: list[str], message: str, + trailers: list[str]) -> str: + """Rewrite the pin and every workflow file, stage them, validate them and + commit them — or roll every byte back and re-raise. Returns the commit. Split from `cmd_bump` for #136: the whole rewrite-or-roll-back sequence is one question, asked once, instead of being the tail half of a @@ -694,22 +780,35 @@ def _apply_rewrite(root: Path, submodule: Path, commit: str, was: str, `UnicodeError` from the validator echo or an interrupt left the pin rewritten, the leg on the new commit and the index changed). - A `Refusal` and a `CommandFailed` leave as they are. An `OSError` or a - `UnicodeError` leaves as a `Refusal` naming it — the repository's own - answer to a write that could not happen, `update-shape.py`'s - `update-readme-unwritable`, never a traceback — and anything else, an - interrupt included, is re-raised as itself once the tree is back. + THE COMMIT IS INSIDE IT TOO, as the last statement. A commit git refuses + (a signing key that cannot sign, a `commit-msg` hook) is a failure after + the leg moved like any other; made after this returned, it left the bump + staged and the leg on the new commit, and the operator's next act, the + same command once the signer was fixed, was refused as a dirty root. One + `git rev-parse HEAD` is read just before it, and `_head_guard` compares + the root's HEAD with that after any failure: a failure after the commit + landed is not rolled back. + + WHAT IT REPORTS never claims more than the rollback did: every byte back + (`ROLLED_BACK`), THE ROLLBACK WAS NOT COMPLETE, or the commit landed and + nothing was rolled back. A `Refusal` carries that in its own text. An + `OSError` or a `UnicodeError` that nothing committed leaves as a + `Refusal` naming it — the repository's own answer to a write that could + not happen, `update-shape.py`'s `update-readme-unwritable`, never a + traceback. A `CommandFailed`, an interrupt and anything else leave as + themselves, after one line on stderr saying it, because neither + `main()`'s report of a git command nor a traceback says what became of + the tree. """ written: dict[Path, bytes] = {} placement = leg_placement(submodule) - moved_leg = False + head_before: str | None = None try: # A BUMP LEAVES THE LEG AT THE COMMIT, detached, and does not move any # branch. `bootstrap.py` is what places a leg ON its tracking branch — # at the pin — and a bump that also moved `main` in the leg would be # this tool pushing somebody's branch around from inside a submodule. run(["git", "checkout", "-q", "--detach", commit], cwd=submodule) - moved_leg = True for target, data in [(pin_path, rewrite_pin(pin_text, commit, digest)) ] + [(wf_path, text) for wf_path, text, _ in plans]: written[target] = target.read_bytes() @@ -733,38 +832,48 @@ def _apply_rewrite(root: Path, submodule: Path, commit: str, was: str, code, output = run_validator(root, script) sys.stdout.write(output) if code != 0: + # `ROLLED_BACK` is written before the rollback has run, and + # the handler below replaces it when the rollback did not + # earn it. raise Refusal( "bump-leg-validators-red", - f"{script} exited {code} after the rewrite. 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.\n--- its output ---\n" + f"{script} exited {code} after the rewrite. " + f"{ROLLED_BACK}\n--- its output ---\n" + output.rstrip() + "\n--- end output ---", "Remediation: read the finding above. A tool that left a " "project red would have moved the cost from this command " "to somebody else's pull request.") + + head_before = git_out(["rev-parse", "HEAD"], cwd=root).lower() + return commit_once(root, message, staged, trailers=trailers) except BaseException as failure: # `BaseException`, NOT `Exception`: an interrupt during the checkout # or the validators is the failure a person causes on purpose, and it # leaves the same half-applied bump as any other. - rolled_back = _roll_back(root, submodule, placement, was, staged, - written, moved_leg) - if isinstance(failure, (OSError, UnicodeError)): + landed = _head_guard(root, head_before) + if landed is None: + verdict = (ROLLED_BACK if _roll_back(root, submodule, placement, + was, staged, written) + else NOT_ROLLED_BACK) + else: + verdict = landed + if isinstance(failure, Refusal): + raise Refusal(failure.code, _with_verdict(failure.detail, verdict), + failure.remediation) from failure + if landed is None and isinstance(failure, (OSError, UnicodeError)): raise Refusal( "bump-leg-rewrite-failed", f"{type(failure).__name__} while the bump was being applied: " - f"{failure}. " - + ("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." - if rolled_back else - "THE ROLLBACK WAS NOT COMPLETE: what it could not put " - "back is named above, so read `git status` before " - "anything else."), + f"{failure}. {verdict}", "Remediation: fix what the exception names — a file this " "command cannot write, a stdout that cannot encode what a " "validator printed — and run the same command again. " "Nothing was committed.") from failure + lead = ("The command below failed" + if isinstance(failure, CommandFailed) + else type(failure).__name__) + print(f"{lead} while the bump was being applied. {verdict}", + file=sys.stderr) raise @@ -812,12 +921,10 @@ def cmd_bump(args) -> int: "untouched.") return 0 - # ---- the rewrite, remembered byte for byte so a red validator undoes it + # ---- the rewrite and the commit, remembered byte for byte so a failure + # anywhere up to the commit undoes them staged: list[str] = [path, pin_rel] + [ wf_path.relative_to(root).as_posix() for wf_path, _, _ in plans] - _apply_rewrite(root, submodule, commit, was, pin_path, pin_text, digest, - plans, staged) - body = ( f"{was[:12]} -> {commit[:12]}, tree {digest}.\n\n" + (f"{sum(count for _, _, count in plans)} workflow `@` " @@ -832,9 +939,11 @@ def cmd_bump(args) -> int: "request, and on somebody else's change.\n\nWritten by " "openRepoShape's `scripts/bump-leg.py`; the digest is recomputed " "from the leg's own objects, not adjusted.\n") - head = commit_once( - root, f"Bump {role} leg to {commit[:12]} in {project.display_name}\n\n" - + body, staged, trailers=args.trailer) + head = _apply_rewrite( + root, submodule, commit, was, pin_path, pin_text, digest, plans, + staged, + f"Bump {role} leg to {commit[:12]} in {project.display_name}\n\n" + + body, args.trailer) print(f"\n committed {head[:12]}: " + ", ".join(staged)) print(f" {path}/ is left DETACHED at {commit[:12]}; `make bootstrap` in " "the root re-places it on its tracking branch AT the new pin") diff --git a/tests/test_bump_leg_rollback.py b/tests/test_bump_leg_rollback.py index 7254dd3f..bb7edd00 100644 --- a/tests/test_bump_leg_rollback.py +++ b/tests/test_bump_leg_rollback.py @@ -3,12 +3,19 @@ has moved puts every byte back (#253). `_apply_rewrite` checks the leg out, writes the pin and each workflow file, -stages them and runs the validators. Its rollback used to be a handler for -`Refusal` and `CommandFailed` alone, so a `PermissionError` on a workflow -file, a `UnicodeError` from echoing a validator's output or an interrupt left -the pin rewritten, the leg on the new commit and the index changed, behind a -traceback. `tests/test_bump_leg.py` holds the red-validator rollback; this -file holds the failures that rollback never saw. +stages them, runs the validators and commits. Its rollback used to be a +handler for `Refusal` and `CommandFailed` alone, so a `PermissionError` on a +workflow file, a `UnicodeError` from echoing a validator's output or an +interrupt left the pin rewritten, the leg on the new commit and the index +changed, behind a traceback. `tests/test_bump_leg.py` holds the red-validator +rollback; this file holds the failures that rollback never saw: those, a +commit git refuses, a forward checkout that moved the leg and then failed, +and a rollback step that fails or is interrupted itself. It also holds the +one failure that must NOT be rolled back: one after the commit landed. + +A REPORT NEVER CLAIMS A TREE THE ROLLBACK DID NOT RESTORE. Every failure +says, as a refusal's own text or as a line before the traceback, either that +every byte is back or that THE ROLLBACK WAS NOT COMPLETE. EVERY TEST COMPARES THE WHOLE TREE, not the one thing its failure touched: the root's HEAD, the pin's bytes, every workflow's bytes, the gitlink the @@ -56,6 +63,11 @@ #: commit pushed to the same branch name would be refused as non-fast-forward. ADVANCE_BRANCH = "bump-leg-rollback-advance" +#: The two things a report may say about the tree, spelled here rather than +#: imported, so a test reads what the operator reads. +ROLLED_BACK = "and the tree is exactly as it was" +NOT_COMPLETE = "THE ROLLBACK WAS NOT COMPLETE" + @pytest.fixture(scope="module") def advanced(scaffolded, tmp_path_factory) -> str: @@ -220,11 +232,15 @@ def test_a_validator_output_that_cannot_be_decoded_or_echoed_is_refused_and_put_ ], ids=["interrupt-writing", "interrupt-validating", "defect-validating"]) def test_any_other_failure_puts_every_byte_back_and_is_re_raised_as_itself( step, failure, project, scaffolded, advanced, bump_leg_module, - monkeypatch): + monkeypatch, capsys): """An interrupt, or a defect that is not the operator's to fix, is not made into a refusal: it reaches the interpreter as what it is, AFTER the tree is back. `write_lf` fails on the SECOND file, with the pin already - rewritten; `run_validator` fails with the index staged.""" + rewritten; `run_validator` fails with the index staged. + + The traceback alone does not say whether the tree is back, so one line + before it does: an operator who pressed Ctrl-C should not have to run + `git status` to learn that nothing is left half-applied.""" before = prepared(project) if step == "write_lf": real_write = bump_leg_module.write_lf @@ -242,15 +258,26 @@ def second_write_fails(path, text): assert raised.value is failure assert_nothing_changed(project, before) + err = capsys.readouterr().err + assert (f"{type(failure).__name__} while the bump was being applied" + in err), err + assert ROLLED_BACK in err, "the line says the tree is back" + assert NOT_COMPLETE not in err -@pytest.mark.parametrize("failure", ["validators-red", "unwritable"]) +@pytest.mark.parametrize("failure", + ["validators-red", "unwritable", "command-failed"]) def test_a_rollback_step_that_fails_is_named_and_the_others_still_run( failure, project, scaffolded, advanced, bump_leg_module, monkeypatch, capsys): """THE ROLLBACK NEVER RAISES OVER THE FAILURE: with the leg's checkout the one step that cannot be put back, the files and the index still are, the - step is named, and the failure the person sees is the original one.""" + step is named, and the failure the person sees is the original one. + + AND NOTHING CLAIMS THE TREE IS BACK. A red validator's refusal, the + refusal an `OSError` becomes and the line printed above a git command + that failed all say THE ROLLBACK WAS NOT COMPLETE, and none of them says + the tree is exactly as it was.""" before = prepared(project) monkeypatch.setattr(bump_leg_module, "restore_leg", raiser( OSError(errno.EIO, "the leg's checkout is gone"))) @@ -258,7 +285,7 @@ def test_a_rollback_step_that_fails_is_named_and_the_others_still_run( monkeypatch.setattr(bump_leg_module, "run_validator", lambda root, script: (1, "pin-digest-mismatch\n")) original = "REFUSED bump-leg-validators-red" - else: + elif failure == "unwritable": real_write = bump_leg_module.write_lf def refused(path, text): @@ -268,6 +295,16 @@ def refused(path, text): return real_write(path, text) monkeypatch.setattr(bump_leg_module, "write_lf", refused) original = "REFUSED bump-leg-rewrite-failed" + else: + real_run = bump_leg_module.run + + def add_fails(args, *rest, **kwargs): + if args[:2] == ["git", "add"]: + raise bump_leg_module.CommandFailed( + args, project, 128, "fatal: the index is locked") + return real_run(args, *rest, **kwargs) + monkeypatch.setattr(bump_leg_module, "run", add_fails) + original = "REFUSED advancing a leg" code = run_main(bump_leg_module, project, scaffolded, advanced) @@ -276,12 +313,173 @@ def refused(path, text): assert original in err, "the original failure is the one reported" assert "the checkout of spec/ could not be put back" in err assert "the leg's checkout is gone" in err - if failure == "unwritable": - assert "THE ROLLBACK WAS NOT COMPLETE" in err, ( - "a refusal does not claim a tree it did not restore") + assert NOT_COMPLETE in err, ( + "a refusal does not claim a tree it did not restore") + assert ROLLED_BACK not in err, ( + "and nothing says the tree is exactly as it was") after = snapshot(project) for key in ("head", "pin", "workflows", "gitlink"): assert after[key] == before[key], f"{key} is not as it was found" assert git("diff", "--cached", "--quiet", cwd=project, check=False).returncode == 0, ( "the index was still put back, though the leg's step failed before it") + + +def test_an_interrupt_inside_a_rollback_step_is_named_and_the_steps_after_it_still_run( + project, scaffolded, advanced, bump_leg_module, monkeypatch, capsys): + """A SECOND Ctrl-C, while the rollback checks the leg out again — the + likeliest next keystroke when a large leg is slow to check out. The step + it lands in is named as not put back; the index step after it still + runs, so the new pin, workflow and gitlink are not left staged; and the + failure reported is still the first one, saying the rollback was not + complete.""" + before = prepared(project) + monkeypatch.setattr(bump_leg_module, "run_validator", + lambda root, script: (1, "pin-digest-mismatch\n")) + monkeypatch.setattr(bump_leg_module, "restore_leg", + raiser(KeyboardInterrupt())) + + try: + code = run_main(bump_leg_module, project, scaffolded, advanced) + except KeyboardInterrupt: + pytest.fail("the interrupt in a rollback step escaped the rollback, " + "so no step after it ran") + + err = capsys.readouterr().err + assert code == 2 + assert "REFUSED bump-leg-validators-red" in err + assert "the checkout of spec/ could not be put back: interrupted" in err + assert NOT_COMPLETE in err + assert ROLLED_BACK not in err + after = snapshot(project) + for key in ("head", "pin", "workflows", "gitlink"): + assert after[key] == before[key], f"{key} is not as it was found" + assert git("diff", "--cached", "--quiet", cwd=project, + check=False).returncode == 0, ( + "the index step ran after the interrupted one") + + +# --- the leg, decided from the leg ------------------------------------------ + +def hook(repository: Path, hooks: Path, name: str, body: bytes) -> None: + """A hook for `repository`, set the way `test_update_shape_add_ignored.py` + sets one: `core.hooksPath` in the repository's own config, so a global + one cannot replace it; a shebang script, which Git for Windows runs + through its own `sh`; and a directory outside the tree, so `git status` + never sees it.""" + hooks.mkdir(exist_ok=True) + script = hooks / name + script.write_bytes(b"#!/bin/sh\n" + body) + script.chmod(0o755) + git("config", "core.hooksPath", hooks.as_posix(), cwd=repository) + + +@pytest.mark.parametrize("how", ["interrupted", "hook-failed"]) +def test_a_forward_checkout_that_moved_the_leg_and_then_failed_puts_it_back( + how, project, scaffolded, advanced, bump_leg_module, monkeypatch, + tmp_path): + """git moves the leg's HEAD BEFORE it runs `post-checkout`, so a checkout + can fail after the move: a hook that exits non-zero (git-lfs's, with no + `git-lfs` on PATH), or an interrupt that lands while the tool is still + waiting on git. Whether the leg is put back is read off the leg, not off + whether the checkout returned. The hook here refuses the NEW commit only, + so the checkout that puts the leg back is not refused too.""" + before = prepared(project) + if how == "interrupted": + real_run = bump_leg_module.run + + def moved_then_interrupted(args, *rest, **kwargs): + out = real_run(args, *rest, **kwargs) + if args[:4] == ["git", "checkout", "-q", "--detach"] and \ + args[-1] == advanced: + raise KeyboardInterrupt() + return out + monkeypatch.setattr(bump_leg_module, "run", moved_then_interrupted) + with pytest.raises(KeyboardInterrupt): + run_main(bump_leg_module, project, scaffolded, advanced) + assert_nothing_changed(project, before) + return + + hook(project / "spec", tmp_path / "leg-hooks", "post-checkout", + b'[ "$2" = "' + advanced.encode("ascii") + b'" ] || exit 0\n' + b"echo 'post-checkout: git-lfs was not found on your path' >&2\n" + b"exit 1\n") + result = bump(project, scaffolded, advanced) + + assert_nothing_changed(project, before) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED advancing a leg" in result.stderr + assert f"checkout -q --detach {advanced}" in result.stderr + assert ROLLED_BACK in result.stderr, result.stderr + + +# --- the commit, inside the rollback's reach -------------------------------- + +@pytest.mark.parametrize("refusal", ["signing", "commit-msg"]) +def test_a_commit_git_refuses_puts_every_byte_back( + refusal, project, scaffolded, advanced, tmp_path): + """The validators were green, so everything was written, staged and + checked; then git refused the commit itself. A signing key that cannot + sign and a `commit-msg` hook are both common on a developer's machine, + and the operator's next act is to fix that and run the same command + again, which a root left with the bump staged refuses as dirty. The + signer is a program that does not exist, which needs no hook and runs + the same on every platform.""" + before = prepared(project) + if refusal == "signing": + git("config", "commit.gpgsign", "true", cwd=project) + git("config", "gpg.program", (tmp_path / "no-signer").as_posix(), + cwd=project) + else: + hook(project, tmp_path / "root-hooks", "commit-msg", + b"echo 'commit-msg: not this message' >&2\nexit 1\n") + + result = bump(project, scaffolded, advanced) + + assert_nothing_changed(project, before) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED advancing a leg" in result.stderr + assert "git commit" in result.stderr, "the refusal names the commit" + assert ROLLED_BACK in result.stderr, result.stderr + assert NOT_COMPLETE not in result.stderr + + +@pytest.mark.parametrize("after", ["interrupt", "refusal"]) +def test_a_failure_after_the_commit_landed_rolls_nothing_back( + after, project, scaffolded, advanced, bump_leg_module, monkeypatch, + capsys): + """THE HEAD GUARD. Once the root's HEAD has moved, the bump IS a commit, + and putting the files, the leg and the index back under it would leave a + tree that contradicts the commit just made. An interrupt after `git + commit` returned, or the `git rev-parse HEAD` that reads it back failing, + rolls nothing back, and says the commit landed.""" + before = prepared(project) + real_commit = bump_leg_module.commit_once + failure = (KeyboardInterrupt() if after == "interrupt" else + bump_leg_module.Refusal("git-failed", + "`git rev-parse HEAD` exited 128")) + + def lands_then_fails(*args, **kwargs): + real_commit(*args, **kwargs) + raise failure + monkeypatch.setattr(bump_leg_module, "commit_once", lands_then_fails) + + if after == "interrupt": + with pytest.raises(KeyboardInterrupt): + run_main(bump_leg_module, project, scaffolded, advanced) + else: + assert run_main(bump_leg_module, project, scaffolded, advanced) == 2 + + head = git("rev-parse", "HEAD", cwd=project).stdout.strip() + assert head != before["head"], "the commit landed" + assert git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() == \ + advanced + assert git("status", "--porcelain", cwd=project).stdout == "", ( + "the tree agrees with the commit: nothing was rolled back under it") + assert git("rev-parse", "HEAD", cwd=project / "spec").stdout.strip() == \ + advanced + err = capsys.readouterr().err + assert f"committed as {head[:12]}" in err, err + assert ROLLED_BACK not in err + if after == "refusal": + assert "REFUSED git-failed" in err From d75151fefd59b60ca28b1ddc50b6f99f26d4ea6f Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:41:18 +0000 Subject: [PATCH 05/14] bump-leg.py: the rollback compares the leg's `git status` with the one read before the checkout, so a checkout cut off before it moved the leg is reported as THE ROLLBACK WAS NOT COMPLETE, not as a tree exactly as it was (#253) Re-verification 1 of #264 found it with a real Ctrl-C. git writes the new commit's files before it moves the leg's index and HEAD, and a terminal's SIGINT reaches git and this tool together, so a large forward checkout cut off between the two leaves the leg on its branch at its commit with the files it wrote in it. `_restore_leg_if_moved` read only the branch and the HEAD, found the leg where it stood, and `_roll_back` returned True: the report said every byte was back and the tree was exactly as it was, while the leg held 851 files of the cut-off checkout, and the operator's re-run failed on "untracked working tree files would be overwritten". Main prints only the traceback there, so the false claim was new in fix round 1. `_apply_rewrite` now reads the leg's `git status --porcelain -z --untracked-files=all` as bytes, next to `placement` and before the checkout. `_restore_leg_if_moved` restores the leg when its branch or HEAD moved, as before, and then compares the leg's status with that one. When it differs the step raises `LegNotAsFound`, `_put_back` names the step with a sentence sending the operator to `git -C status`, and the verdict is THE ROLLBACK WAS NOT COMPLETE. The comparison runs after a restore as well, so a checkout that moved the leg and left a file the checkout back cannot remove is reported the same way. Nothing in the leg is removed. An untracked file of the operator's own is allowed there (`_ensure_root_clean` ignores untracked leg content), and removing only the paths that appeared and that the new commit's tree holds 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. Tests: the cut-off checkout, interrupted and failing with exit 130, and a checkout that moved the leg, left a file and was interrupted. Each asserts the line says THE ROLLBACK WAS NOT COMPLETE, nothing says the tree is exactly as it was, the step is named, and the operator's own untracked file and the checkout's file are both still there. One more puts an operator's untracked file in the leg beside a red validator, and shows the rollback still says every byte is back and leaves the file alone. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- scripts/bump-leg.py | 68 ++++++++++++++++++----- tests/test_bump_leg_rollback.py | 97 ++++++++++++++++++++++++++++++++- 2 files changed, 148 insertions(+), 17 deletions(-) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 28c1108a..96aeb414 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -47,8 +47,10 @@ `OSError` or a `UnicodeError`, and as itself for anything else, an interrupt included, after a line saying whether the tree is back. No report claims more than the rollback did: a step it could not do is - named, and the report says THE ROLLBACK WAS NOT COMPLETE. A failure - after the commit has landed rolls nothing back, and says so. + named, and so is a leg whose `git status` no longer reads as it did + before the bump (a checkout cut off part way leaves the files it wrote), + and the report says THE ROLLBACK WAS NOT COMPLETE. A failure after the + commit has landed rolls nothing back, and says so. EXIT CODES: 0 the commit was made, or a `--dry-run` printed a move it would make · 1 NOTHING TO DO — every fact already names that commit, under @@ -649,6 +651,21 @@ def _restore_file(target: Path, original: bytes) -> None: target.write_bytes(original) +class LegNotAsFound(Exception): + """The leg's `git status` does not read as it did before the bump.""" + + +def _leg_status(submodule: Path) -> bytes: + """The leg's `git status`, untracked files one by one, as raw bytes. + + `-z` and bytes, so nothing strips the leading space of ` M `, and + `--untracked-files=all`, so a file written into a directory that already + held an untracked file of the operator's is a line of its own. + """ + return git_out(["status", "--porcelain", "-z", "--untracked-files=all"], + cwd=submodule, binary=True) + + def _put_back(what: str, undo) -> bool: """Attempt ONE step of a rollback; say so, and carry on, if it fails. @@ -673,6 +690,9 @@ def _put_back(what: str, undo) -> bool: except KeyboardInterrupt: print(f"REFUSED rolling the bump back. {what} could not be put back: " "interrupted", file=sys.stderr) + except LegNotAsFound as exc: + print(f"REFUSED rolling the bump back. {what} could not be put back: " + f"{exc}", file=sys.stderr) except Exception as exc: # noqa: BLE001 - nothing may escape a rollback print(f"REFUSED rolling the bump back. {what} could not be put back: " f"{type(exc).__name__}: {exc}", file=sys.stderr) @@ -682,8 +702,9 @@ def _put_back(what: str, undo) -> bool: def _restore_leg_if_moved(submodule: Path, placement: str | None, - was: str) -> None: - """Put the leg back where it stood, if it no longer stands there. + was: str, status: bytes) -> None: + """Put the leg back where it stood, if it no longer stands there, and + fail the step if its `git status` no longer reads as `status` did. DECIDED FROM THE LEG, NOT FROM WHETHER THE CHECKOUT RETURNED. git moves the leg's HEAD before it runs `post-checkout`, so a hook that exits @@ -692,21 +713,36 @@ def _restore_leg_if_moved(submodule: Path, placement: str | None, checkout is still being waited on after git has moved it. Either way the leg is on the new commit and the checkout never returned. A leg that still stands where it stood, on the same branch at the same commit, is - left alone, so a checkout that never moved it does not go through a - refusing hook a second time. + not checked out again, so a checkout that never moved it does not go + through a refusing hook a second time. + + AND FROM THE LEG'S `git status`, NOT ITS BRANCH AND HEAD ALONE. git + writes the new commit's files before it moves the leg's index and HEAD, + so a checkout cut off between the two (a terminal's Ctrl-C reaches git + and this tool together) leaves the leg on its branch at its commit with + those files in it. Nothing is removed here: an untracked file of the + operator's own is allowed in the leg, and only the operator can tell it + from one the checkout wrote, so the step fails and says where to look. """ - if (leg_placement(submodule) == placement + if not (leg_placement(submodule) == placement and git_out(["rev-parse", "HEAD"], cwd=submodule).lower() == was.lower()): - return - restore_leg(submodule, placement, was) + restore_leg(submodule, placement, was) + if _leg_status(submodule) != status: + raise LegNotAsFound( + "its `git status` no longer reads as it did before the bump, " + "which is what a checkout cut off part way leaves behind. Read " + f"`git -C {submodule} status` before anything else, and remove " + "only what you did not put there yourself.") def _roll_back(root: Path, submodule: Path, placement: str | None, was: str, - staged: list[str], written: dict[Path, bytes]) -> bool: + status: bytes, staged: list[str], + written: dict[Path, bytes]) -> bool: """Put back every byte `_apply_rewrite` changed: the files it wrote, the - leg's checkout (when the leg no longer stands where it stood) and the - index, in that order. Returns whether ALL of it went back. + leg's checkout (when the leg no longer stands where it stood, and checked + against the leg's `git status` from before the bump) and the index, in + that order. Returns whether ALL of it went back. THE ROLLBACK NEVER RAISES OVER THE FAILURE THAT STARTED IT. Whatever it cannot put back is printed and the original failure is still the one the @@ -718,7 +754,8 @@ def _roll_back(root: Path, submodule: Path, placement: str | None, was: str, partial(_restore_file, target, original)) for target, original in written.items()] steps.append((f"the checkout of {submodule.name}/", - partial(_restore_leg_if_moved, submodule, placement, was))) + partial(_restore_leg_if_moved, submodule, placement, was, + status))) steps.append(("the index", partial( run, ["git", "reset", "-q", "HEAD", "--", *staged], cwd=root))) return all([_put_back(what, undo) for what, undo in steps]) @@ -802,6 +839,9 @@ def _apply_rewrite(root: Path, submodule: Path, commit: str, was: str, """ written: dict[Path, bytes] = {} placement = leg_placement(submodule) + # Read BEFORE the checkout, as `placement` is: the rollback compares the + # leg with it (`_restore_leg_if_moved`). + status = _leg_status(submodule) head_before: str | None = None try: # A BUMP LEAVES THE LEG AT THE COMMIT, detached, and does not move any @@ -853,7 +893,7 @@ def _apply_rewrite(root: Path, submodule: Path, commit: str, was: str, landed = _head_guard(root, head_before) if landed is None: verdict = (ROLLED_BACK if _roll_back(root, submodule, placement, - was, staged, written) + was, status, staged, written) else NOT_ROLLED_BACK) else: verdict = landed diff --git a/tests/test_bump_leg_rollback.py b/tests/test_bump_leg_rollback.py index bb7edd00..75c9189d 100644 --- a/tests/test_bump_leg_rollback.py +++ b/tests/test_bump_leg_rollback.py @@ -9,9 +9,10 @@ interrupt left the pin rewritten, the leg on the new commit and the index changed, behind a traceback. `tests/test_bump_leg.py` holds the red-validator rollback; this file holds the failures that rollback never saw: those, a -commit git refuses, a forward checkout that moved the leg and then failed, -and a rollback step that fails or is interrupted itself. It also holds the -one failure that must NOT be rolled back: one after the commit landed. +commit git refuses, a forward checkout that moved the leg and then failed or +was cut off before it moved it, and a rollback step that fails or is +interrupted itself. It also holds the one failure that must NOT be rolled +back: one after the commit landed. A REPORT NEVER CLAIMS A TREE THE ROLLBACK DID NOT RESTORE. Every failure says, as a refusal's own text or as a line before the traceback, either that @@ -413,6 +414,96 @@ def moved_then_interrupted(args, *rest, **kwargs): assert ROLLED_BACK in result.stderr, result.stderr +@pytest.mark.parametrize("how", [ + "cut-off-interrupted", "cut-off-command-failed", "moved-then-interrupted"]) +def test_a_forward_checkout_that_left_files_in_the_leg_is_not_called_put_back( + how, project, scaffolded, advanced, bump_leg_module, monkeypatch, + capsys): + """git writes the new commit's files BEFORE it moves the leg's index and + HEAD, and a terminal's Ctrl-C reaches git and this tool together, so a + large checkout can be cut off with the leg's branch and HEAD unmoved and + the files it wrote left in the leg. Read off the branch and the HEAD + alone, that leg looked put back, the report said the tree was exactly as + it was, and the re-run then failed on `untracked working tree files would + be overwritten`. The `cut-off` cases are that checkout, interrupted or + failed; `moved-then-interrupted` is one that did move the leg and left a + file the checkout back cannot remove, so the comparison is made after a + restore as well. + + The leg's `git status` is read before the checkout and compared in the + rollback: the step is named, the operator is sent to `git -C + 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.""" + prepared(project) + leg = project / "spec" + mine = leg / "OPERATOR-NOTES.md" + mine.write_bytes(b"the operator's own untracked file\n") + before = snapshot(project) + real_run = bump_leg_module.run + + def cut_off(args, *rest, **kwargs): + if args == ["git", "checkout", "-q", "--detach", advanced]: + if how == "moved-then-interrupted": + real_run(args, *rest, **kwargs) + (leg / "PARTIAL.md").write_bytes(b"# written before the cut\n") + if how == "cut-off-command-failed": + raise bump_leg_module.CommandFailed(args, leg, 130, "") + raise KeyboardInterrupt() + return real_run(args, *rest, **kwargs) + monkeypatch.setattr(bump_leg_module, "run", cut_off) + + if how != "cut-off-command-failed": + with pytest.raises(KeyboardInterrupt): + run_main(bump_leg_module, project, scaffolded, advanced) + lead = "KeyboardInterrupt" + else: + assert run_main(bump_leg_module, project, scaffolded, advanced) == 2 + lead = "The command below failed" + + err = capsys.readouterr().err + assert (f"{lead} while the bump was being applied. {NOT_COMPLETE}" + in err), err + assert ROLLED_BACK not in err, "nothing says the tree is exactly as it was" + assert "the checkout of spec/ could not be put back: its `git status`" \ + in err, "the step is named" + assert "spec status` before anything else" in err, err + after = snapshot(project) + for key in ("head", "pin", "workflows", "gitlink", "leg_head", + "leg_branch"): + assert after[key] == before[key], f"{key} is not as it was found" + assert git("diff", "--cached", "--quiet", cwd=project, + check=False).returncode == 0, "the index was put back" + assert mine.read_bytes() == b"the operator's own untracked file\n" + assert (leg / "PARTIAL.md").is_file(), ( + "the rollback removes nothing from the leg: only the operator can " + "tell a file of theirs from one the checkout wrote") + + +def test_an_untracked_file_the_operator_keeps_in_the_leg_is_left_alone_and_not_counted( + project, scaffolded, advanced, bump_leg_module, monkeypatch, capsys): + """The other side of the comparison above: an untracked file that was in + the leg before the bump is in its `git status` before and after, so a + rollback that put everything back still says so, and the file is not + touched.""" + prepared(project) + mine = project / "spec" / "OPERATOR-NOTES.md" + mine.write_bytes(b"the operator's own untracked file\n") + before = snapshot(project) + monkeypatch.setattr(bump_leg_module, "run_validator", + lambda root, script: (1, "pin-digest-mismatch\n")) + + code = run_main(bump_leg_module, project, scaffolded, advanced) + + err = capsys.readouterr().err + assert code == 2 + assert "REFUSED bump-leg-validators-red" in err + assert ROLLED_BACK in err, err + assert NOT_COMPLETE not in err + assert_nothing_changed(project, before) + assert mine.read_bytes() == b"the operator's own untracked file\n" + + # --- the commit, inside the rollback's reach -------------------------------- @pytest.mark.parametrize("refusal", ["signing", "commit-msg"]) From 5269aace77c71385b0ea6918ed83d2faaa2d7a0f Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:41:52 +0000 Subject: [PATCH 06/14] bump-leg.py: the rollback tests cover a HEAD that cannot be read after the commit, an `OSError` after it, and a leg detached at the commit it stood on, so the three rules re-verification 1 could break without a test failing are now held by one each (#253) Re-verification 1 of #264 ran 17 mutants of 67a1163 and three survived: - 9: `_head_guard` rolls back when HEAD cannot be read after the commit was asked for. `test_a_head_that_cannot_be_read_after_the_commit_rolls_nothing_back` makes every `git rev-parse HEAD` fail once the commit has been asked for (`commit_once`'s read-back and the guard's), as the reviewer's real `post-commit` reproduction did, and asserts the commit landed, the tree agrees with it, and the refusal says the commit could not be read. - 12: the leg compared by its HEAD alone. `test_a_leg_detached_at_the_commit_it_stood_on_is_put_back_on_its_branch` bumps `--to` the commit already pinned with a workflow reference left behind, so the forward checkout detaches the leg at the same commit; a red validator follows, and the leg must be back ON `main`. - 16: an `OSError` after a landed commit made into `bump-leg-rewrite-failed`, whose remediation says nothing was committed. `test_a_failure_after_the_commit_landed_rolls_nothing_back` gains an `oserror` case: the error is re-raised as itself after a line saying the bump was committed. The landed-commit assertions are one helper, shared by the guard's tests. No code changes. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_bump_leg_rollback.py | 124 +++++++++++++++++++++++++++----- 1 file changed, 106 insertions(+), 18 deletions(-) diff --git a/tests/test_bump_leg_rollback.py b/tests/test_bump_leg_rollback.py index 75c9189d..f46c42f4 100644 --- a/tests/test_bump_leg_rollback.py +++ b/tests/test_bump_leg_rollback.py @@ -504,6 +504,38 @@ def test_an_untracked_file_the_operator_keeps_in_the_leg_is_left_alone_and_not_c assert mine.read_bytes() == b"the operator's own untracked file\n" +def test_a_leg_detached_at_the_commit_it_stood_on_is_put_back_on_its_branch( + project, scaffolded, bump_leg_module, monkeypatch, capsys): + """`--to` the commit already pinned, with a workflow reference left + behind by a bump made by hand: not `nothing to do`, and the forward + checkout detaches the leg AT THE COMMIT IT STOOD ON. Its HEAD is where it + was and its branch is not, so a rollback that compared the HEAD alone + would leave the leg detached, which `bootstrap.py` never leaves it.""" + prepared(project) + pinned = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + legs = project / ".github" / "workflows" / "legs.yml" + spec_ref = f"{PROJECT}-spec/.github/workflows/x.yml@" + text = legs.read_text(encoding="utf-8") + assert text.count(spec_ref + pinned) == 1 + legs.write_text(text.replace(spec_ref + pinned, spec_ref + "0" * 40), + encoding="utf-8") + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "A workflow reference the last bump missed", "--", + ".github/workflows/legs.yml", cwd=project) + before = snapshot(project) + assert before["leg_branch"] == "refs/heads/main" + monkeypatch.setattr(bump_leg_module, "run_validator", + lambda root, script: (1, "pin-digest-mismatch\n")) + + code = run_main(bump_leg_module, project, scaffolded, pinned) + + err = capsys.readouterr().err + assert code == 2 + assert "REFUSED bump-leg-validators-red" in err + assert ROLLED_BACK in err, err + assert_nothing_changed(project, before) + + # --- the commit, inside the rollback's reach -------------------------------- @pytest.mark.parametrize("refusal", ["signing", "commit-msg"]) @@ -535,42 +567,98 @@ def test_a_commit_git_refuses_puts_every_byte_back( assert NOT_COMPLETE not in result.stderr -@pytest.mark.parametrize("after", ["interrupt", "refusal"]) +def assert_the_commit_landed_and_nothing_was_rolled_back( + project: Path, before: dict, advanced: str) -> str: + head = git("rev-parse", "HEAD", cwd=project).stdout.strip() + assert head != before["head"], "the commit landed" + assert git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() == \ + advanced + assert git("status", "--porcelain", cwd=project).stdout == "", ( + "the tree agrees with the commit: nothing was rolled back under it") + assert git("rev-parse", "HEAD", cwd=project / "spec").stdout.strip() == \ + advanced + return head + + +@pytest.mark.parametrize("after", ["interrupt", "refusal", "oserror"]) def test_a_failure_after_the_commit_landed_rolls_nothing_back( after, project, scaffolded, advanced, bump_leg_module, monkeypatch, capsys): """THE HEAD GUARD. Once the root's HEAD has moved, the bump IS a commit, and putting the files, the leg and the index back under it would leave a tree that contradicts the commit just made. An interrupt after `git - commit` returned, or the `git rev-parse HEAD` that reads it back failing, - rolls nothing back, and says the commit landed.""" + commit` returned, the `git rev-parse HEAD` that reads it back failing, or + an `OSError` once it landed rolls nothing back, and says the commit + landed. The `OSError` is not made into `bump-leg-rewrite-failed`, whose + remediation says nothing was committed.""" before = prepared(project) real_commit = bump_leg_module.commit_once - failure = (KeyboardInterrupt() if after == "interrupt" else - bump_leg_module.Refusal("git-failed", - "`git rev-parse HEAD` exited 128")) + failure = { + "interrupt": KeyboardInterrupt(), + "refusal": bump_leg_module.Refusal("git-failed", + "`git rev-parse HEAD` exited 128"), + "oserror": OSError(errno.EIO, "the disk went away after the commit"), + }[after] def lands_then_fails(*args, **kwargs): real_commit(*args, **kwargs) raise failure monkeypatch.setattr(bump_leg_module, "commit_once", lands_then_fails) - if after == "interrupt": - with pytest.raises(KeyboardInterrupt): - run_main(bump_leg_module, project, scaffolded, advanced) - else: + if after == "refusal": assert run_main(bump_leg_module, project, scaffolded, advanced) == 2 + else: + with pytest.raises(type(failure)) as raised: + run_main(bump_leg_module, project, scaffolded, advanced) + assert raised.value is failure - head = git("rev-parse", "HEAD", cwd=project).stdout.strip() - assert head != before["head"], "the commit landed" - assert git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() == \ - advanced - assert git("status", "--porcelain", cwd=project).stdout == "", ( - "the tree agrees with the commit: nothing was rolled back under it") - assert git("rev-parse", "HEAD", cwd=project / "spec").stdout.strip() == \ - advanced + head = assert_the_commit_landed_and_nothing_was_rolled_back( + project, before, advanced) err = capsys.readouterr().err assert f"committed as {head[:12]}" in err, err assert ROLLED_BACK not in err if after == "refusal": assert "REFUSED git-failed" in err + if after == "oserror": + assert "OSError while the bump was being applied" in err, err + assert "bump-leg-rewrite-failed" not in err + assert "Nothing was committed" not in err + + +def test_a_head_that_cannot_be_read_after_the_commit_rolls_nothing_back( + project, scaffolded, advanced, bump_leg_module, monkeypatch, capsys): + """THE HEAD GUARD'S THIRD ANSWER. Once the commit has been asked for, + every `git rev-parse HEAD` in the root fails: the one `commit_once` + reads the new commit back with, and the guard's own. Whether the commit + landed is then unknown, and a rollback under a commit that did land is + the worse mistake, so nothing is rolled back and the report says it + could not be read.""" + before = prepared(project) + asked = [] + real_check = bump_leg_module.check_program + real_git_out = bump_leg_module.git_out + + def notes_the_commit(args): + if args[:2] == ["git", "commit"]: + asked.append(args) + return real_check(args) + + def head_unreadable_once_asked(args, cwd, binary=False): + if asked and args == ["rev-parse", "HEAD"]: + raise bump_leg_module.Refusal( + "git-failed", "`git rev-parse HEAD` exited 128: bad HEAD") + return real_git_out(args, cwd=cwd, binary=binary) + monkeypatch.setattr(bump_leg_module, "check_program", notes_the_commit) + monkeypatch.setattr(bump_leg_module, "git_out", head_unreadable_once_asked) + + code = run_main(bump_leg_module, project, scaffolded, advanced) + + assert asked, "the commit was asked for" + assert code == 2 + assert_the_commit_landed_and_nothing_was_rolled_back( + project, before, advanced) + err = capsys.readouterr().err + assert "REFUSED git-failed" in err + assert "Whether the commit landed could not be read" in err, err + assert ROLLED_BACK not in err + assert NOT_COMPLETE not in err From 923cfd374a2d5abbf675c1182bdba348f5adb74d Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:12:10 +0000 Subject: [PATCH 07/14] bump-leg.py: a workflow file or a leg pin that is not UTF-8 is refused by name before anything moves, so a Latin-1 comment no longer ends the run in a UnicodeDecodeError traceback (#289) `plan_workflows` read every `.yml` and `.yaml` file under `.github/workflows/`, and `cmd_bump` read `contracts/-pin.yaml`, with `read_text(encoding="utf-8")` and nothing that caught the decode error, so a committed Latin-1 `# caf\xe9` or a UTF-16 file with a byte-order mark ended the run in a traceback, exit 1, naming Python's codec and not the file. `--dry-run` died the same way, since both reads come before its early return. `main()` catches `Refusal` and `CommandFailed` only, and `UnicodeDecodeError` is a `ValueError`. Each is now a refusal of its own, exit 2, under `--dry-run` as well: `bump-leg-workflow-unreadable` names every workflow file that is not UTF-8 by its path in the repository, the first bad byte and the reason (the words `load_yaml` uses), and `bump-leg-pin-unreadable` names `contracts/-pin.yaml` the same way. A workflow file is read whether or not it names the leg, because `validate-pins.py` reads every one on disk. The read is before the leg moves, so #253's rollback is not involved and its promises are untouched. Refused, not read lossily: `errors="replace"` would let the run go on, but the rewritten text is written back and the lockstep commit would carry U+FFFD where the operator's byte was. Not skipped either: the validator would still fail on the file. Left out: the project's own `validate-pins.py` still raises the same traceback on a non-UTF-8 workflow when it is run by hand or by CI. That file is a pinned template copy, so a fix moves the sha256 `contracts/shape-pin.yaml` records for it in every project already cut. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- scripts/bump-leg.py | 100 +++++++++++- tests/test_bump_leg_unreadable.py | 262 ++++++++++++++++++++++++++++++ 2 files changed, 357 insertions(+), 5 deletions(-) create mode 100644 tests/test_bump_leg_unreadable.py diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 96aeb414..76d16584 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -36,6 +36,14 @@ * the root standing on its own tracking branch. The next step this prints would then be a push to the default branch, and these organisations are pull-request only. + * a workflow file under `.github/workflows/`, or the leg's + `contracts/-pin.yaml`, that is not UTF-8 (a Latin-1 comment, a + UTF-16 file with a byte-order mark): `bump-leg-workflow-unreadable` and + `bump-leg-pin-unreadable`, naming each file and its first bad byte, before + anything is written and under `--dry-run` as well (#289). Never read + lossily — the rewritten text is written back, and would carry U+FFFD + where the operator's byte was — and never skipped, since `validate-pins.py` + reads every workflow file on disk and cannot read it either. * a validator that goes red after the rewrite. Every byte is rolled back first — the pin file, the workflow files, the leg's checkout and the index — so a refusal leaves the tree exactly as it was found. @@ -241,6 +249,90 @@ def rewrite_pin(text: str, commit: str, digest: str) -> str: return "\n".join(out) + "\n" +def _not_utf8(root: Path, path: Path, exc: UnicodeDecodeError) -> str: + """`: not UTF-8 at byte N ()`, in the + words `repo_shape.load_yaml` gives the same failure (#192), so a file this + tool cannot read is named the way every other reader names one.""" + return (f"{path.relative_to(root).as_posix()}: not UTF-8 at byte " + f"{exc.start} ({exc.reason})") + + +def _leg_pin_text(root: Path, pin_path: Path) -> str: + """The leg's `contracts/-pin.yaml` as text, or a refusal naming it + when it is not UTF-8 (#289). + + Read here rather than through `load_yaml` because `rewrite_pin` is a LINE + REWRITE of the file's own text, which a parsed mapping cannot be. A + `UnicodeDecodeError` is a `ValueError`: nothing on the path catches it and + `main()` names `Refusal` and `CommandFailed` alone, so it ended the run in + a traceback, exit 1, under `--dry-run` as well, because this read comes + before that early return. It comes before anything is written, so the + refusal promises only what is true: the working tree, the index and the + leg's checkout are as they were. (The leg's object store was fetched into + already, which `--dry-run` says it does.) + """ + try: + return pin_path.read_text(encoding="utf-8") + except UnicodeDecodeError as exc: + raise Refusal( + "bump-leg-pin-unreadable", + _not_utf8(root, pin_path, exc), + "Remediation: this tool rewrites `commit:` and " + "`digests.tree_sha256` in this file as UTF-8 text, and reads no " + "file lossily, because the rewritten text is written back and " + "would carry U+FFFD where your byte was. Byte " + f"{exc.start}, counting from 0 at the start of the file, is the " + "first that is not UTF-8: re-save the file as UTF-8, or replace " + "that byte with the character it was meant to be, commit it, and " + "run the same command again. Nothing in the working tree, the " + "index or the leg's checkout was changed.") from exc + + +def _workflow_texts(root: Path, workflows: Path) -> list[tuple[Path, str]]: + """Every `.yml` and `.yaml` file in `workflows`, read as UTF-8 — or ONE + refusal naming each that is not (#289). + + EVERY FILE IS READ, whether or not it names the leg, so a file that is not + UTF-8 is REFUSED BY NAME and never skipped, never read lossily. + `read_text(encoding="utf-8")` on a Latin-1 comment or a UTF-16 file with a + byte-order mark raised `UnicodeDecodeError`, which nothing caught: a + traceback, exit 1, under `--dry-run` too, because this read comes before + that early return. Skipping the file would leave `validate-pins.py`, which + reads every workflow on disk, unable to read it either, and + `errors="replace"` would write U+FFFD over the operator's byte into the + lockstep commit. All of them are named at once, so the operator re-saves + each in one pass instead of meeting them one run at a time. The read is + before anything is written, so the refusal promises only what is true: + the working tree, the index and the leg's checkout are as they were. + """ + texts: list[tuple[Path, str]] = [] + unreadable: list[str] = [] + for path in sorted(workflows.iterdir()): + if path.suffix not in (".yml", ".yaml") or not path.is_file(): + continue + try: + texts.append((path, path.read_text(encoding="utf-8"))) + except UnicodeDecodeError as exc: + unreadable.append(_not_utf8(root, path, exc)) + if unreadable: + raise Refusal( + "bump-leg-workflow-unreadable", + f"{len(unreadable)} workflow file(s) are not UTF-8:\n" + + "\n".join(f" {line}" for line in unreadable), + "Remediation: this tool rewrites each workflow file as UTF-8 " + "text and reads none lossily, because the rewritten text is " + "written back and would carry U+FFFD where your byte was; " + "`scripts/validate-pins.py` reads every workflow file on disk as " + "UTF-8 too, so skipping one would only move the failure there. " + "In each line above, the byte is counted from 0 at the start of " + "the file and is the first that is not UTF-8: re-save every file " + "named as UTF-8 (a UTF-16 file with a byte-order mark needs " + "converting, not editing), whether or not it names this leg, " + "commit it, and run the same command again. Nothing in the " + "working tree, the index or the leg's checkout was changed.") + return texts + + def plan_workflows(root: Path, repository: str, commit: str) -> list[tuple[Path, str, int]]: """Every workflow file whose `@` refs to THIS leg have to move. @@ -253,14 +345,13 @@ def plan_workflows(root: Path, repository: str, `.yaml` is read as well as `.yml` because `validate-pins.py` reads both, and a file this rewrote nothing in is a file the gate still calls red. + A file that is not UTF-8 is a refusal (`_workflow_texts`, #289). """ plans: list[tuple[Path, str, int]] = [] workflows = root / ".github" / "workflows" if not workflows.is_dir(): return plans - for path in sorted(workflows.iterdir()): - if path.suffix not in (".yml", ".yaml") or not path.is_file(): - continue + for path, text in _workflow_texts(root, workflows): moved = 0 def swap(match: re.Match) -> str: @@ -273,7 +364,6 @@ def swap(match: re.Match) -> str: return (f"{match.group('repo')}{match.group('path') or ''}" f"@{commit}") - text = path.read_text(encoding="utf-8") rewritten = WORKFLOW_REF_RE.sub(swap, text) if moved: plans.append((path, rewritten, moved)) @@ -938,7 +1028,7 @@ def cmd_bump(args) -> int: digest = tree_digest(submodule, commit) pin_rel = f"contracts/{role}-pin.yaml" pin_path = _leg_pin_path(root, pin_rel) - pin_text = pin_path.read_text(encoding="utf-8") + pin_text = _leg_pin_text(root, pin_path) plans = plan_workflows(root, repository, commit) print(f"commit {was[:12]} -> {commit[:12]}") diff --git a/tests/test_bump_leg_unreadable.py b/tests/test_bump_leg_unreadable.py new file mode 100644 index 00000000..3bc80dce --- /dev/null +++ b/tests/test_bump_leg_unreadable.py @@ -0,0 +1,262 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`scripts/bump-leg.py`: a committed file it must READ and cannot is refused +by name, before anything moves (#289). + +The tool reads `contracts/-pin.yaml` and every `.yml` and `.yaml` file +under `.github/workflows/` as UTF-8, to rewrite them as text. A Latin-1 +comment (`# caf\\xe9`) or a UTF-16 file with a byte-order mark is not UTF-8, +and `read_text(encoding="utf-8")` raised `UnicodeDecodeError` on it, which +`main()` does not catch: a traceback, exit 1, with no word about which file, +and `--dry-run` died the same way because both reads run before its early +return. Now each is a refusal of its own, exit 2: `bump-leg-workflow- +unreadable` for a workflow file and `bump-leg-pin-unreadable` for the pin. + +EVERY TEST COMPARES THE WHOLE TREE, as `tests/test_bump_leg_rollback.py` does, +because "nothing moved" is the promise: the root's HEAD, the pin's and every +workflow's bytes, the gitlink the index records, the root's `git status`, an +empty `git diff --cached`, and the leg's HEAD and branch. + +THE BYTES ARE WRITTEN WITH `write_bytes`, never `write_text`, so what a test +commits is the same bytes on every platform. A control keeps the refusal from +being wider than the failure: a UTF-8 file with a non-ASCII comment, with a +byte-order mark, and a non-UTF-8 file the validator never reads, all still +bump. +""" + +from __future__ import annotations + +import codecs +import subprocess +from pathlib import Path + +import pytest + +from conftest import PROJECT, git +from test_bump_leg import branched, bump, reference_both_legs, validate +from test_bump_leg_rollback import assert_nothing_changed, snapshot + +#: Where the advanced spec commit is pushed. Not another module's branch: the +#: bare remotes are session-scoped, and a second, different commit pushed to +#: a name already taken is refused as non-fast-forward. +ADVANCE_BRANCH = "bump-leg-unreadable-advance" + +WORKFLOWS = Path(".github") / "workflows" + +#: A Latin-1 comment: `é` is the single byte 0xE9, a UTF-8 lead byte with no +#: continuation after it. +LATIN1_COMMENT = b"# caf\xe9\n" + +#: A whole workflow saved as UTF-16 with a byte-order mark (what a Windows +#: editor's "Unicode" save writes). `ff fe` is not a UTF-8 start byte, so the +#: first byte is already the bad one. +UTF16_WORKFLOW = codecs.BOM_UTF16_LE + "on: [pull_request]\n".encode("utf-16-le") + + +@pytest.fixture(scope="module") +def advanced(scaffolded, tmp_path_factory) -> str: + """A real new commit on the spec leg's bare remote; returns its sha.""" + work = tmp_path_factory.mktemp("spec-advance-unreadable") / f"{PROJECT}-spec" + proc = subprocess.run( + ["git", "clone", "-q", + str(scaffolded["remotes"] / f"{PROJECT}-spec.git"), str(work)], + capture_output=True, text=True, check=False) + assert proc.returncode == 0, proc.stderr + (work / "UNREADABLE-CONTRACT.md").write_text("# a contract the leg gained\n") + git("add", "--", "UNREADABLE-CONTRACT.md", cwd=work) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Advance the spec leg for the unreadable-file tests", cwd=work) + git("push", "-q", "origin", f"HEAD:refs/heads/{ADVANCE_BRANCH}", cwd=work) + return git("rev-parse", "HEAD", cwd=work).stdout.strip() + + +def commit_bytes(project: Path, relative: Path, data: bytes, *, + append: bool = False) -> None: + """Write `data` at `relative` (or append it) and COMMIT it, so the root is + clean when the bump starts and the file is one git tracks.""" + target = project / relative + if append: + data = target.read_bytes() + data + target.write_bytes(data) + git("add", "--", relative.as_posix(), cwd=project) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + f"Commit {relative.as_posix()}", cwd=project) + + +def prepared(project: Path) -> None: + """A bump-ready root: a workflow naming both legs, committed; the spec leg + ON its branch, as `bootstrap.py` leaves it; a bump branch.""" + reference_both_legs(project) + git("checkout", "-q", "-B", "main", cwd=project / "spec") + branched(project) + + +def refused_and_untouched(project: Path, scaffolded: dict, to: str, code: str, + *names: str, extra: tuple = ()) -> str: + """Bump, and assert EVERYTHING the issue's table asks of the row: exit 2, + the refusal's id, every file named, no traceback, and the whole tree as it + was. Returns stderr for a test that asserts more.""" + before = snapshot(project) + result = bump(project, scaffolded, to, *extra) + output = result.stderr + result.stdout + assert result.returncode == 2, output + assert f"REFUSED {code}" in result.stderr, output + for name in names: + assert name in result.stderr, f"{name} is not named:\n{output}" + assert "Traceback" not in output, output + assert "UnicodeDecodeError" not in output, output + assert_nothing_changed(project, before) + return result.stderr + + +MODES = pytest.mark.parametrize("extra", [(), ("--dry-run",)], + ids=["bump", "dry-run"]) + + +# --- a workflow file --------------------------------------------------------- + +@MODES +def test_a_latin1_byte_in_a_workflow_naming_the_leg_is_refused_by_name( + project, scaffolded, advanced, extra): + """THE ISSUE'S FIRST ROW: the file names the leg, holds one Latin-1 byte + and is committed, so the tracked-files rule is met and only the read + stands in the way. The refusal names the file by its path in the + repository, the byte and the reason, in the words `load_yaml` uses.""" + prepared(project) + commit_bytes(project, WORKFLOWS / "legs.yml", LATIN1_COMMENT, append=True) + offset = (project / WORKFLOWS / "legs.yml").read_bytes().index(b"\xe9") + + stderr = refused_and_untouched( + project, scaffolded, advanced, "bump-leg-workflow-unreadable", + ".github/workflows/legs.yml", + f"not UTF-8 at byte {offset} (invalid continuation byte)", + extra=extra) + assert "re-save" in stderr and "UTF-8" in stderr, ( + "the remediation says what to do about it") + + +@MODES +@pytest.mark.parametrize("name", ["other.yml", "other.yaml"]) +def test_a_utf16_workflow_naming_no_leg_is_refused_by_name( + project, scaffolded, advanced, extra, name): + """THE REFUSAL COVERS THE FILE WHETHER OR NOT IT NAMES THE LEG. The file + holds no `@`, so no reference of its own would move; it is read + anyway, because `validate-pins.py` reads every workflow on disk and could + not read it either. `.yaml` is read as well as `.yml`, as the validator + does. Skipping it would be a silent pass over a file the gate dies on.""" + prepared(project) + commit_bytes(project, WORKFLOWS / name, UTF16_WORKFLOW) + + refused_and_untouched( + project, scaffolded, advanced, "bump-leg-workflow-unreadable", + f".github/workflows/{name}", + "not UTF-8 at byte 0 (invalid start byte)", extra=extra) + + +def test_every_unreadable_workflow_is_named_in_one_refusal(project, scaffolded, + advanced): + """Two bad files, one run: the operator re-saves both rather than meeting + them one refusal at a time.""" + prepared(project) + commit_bytes(project, WORKFLOWS / "legs.yml", LATIN1_COMMENT, append=True) + commit_bytes(project, WORKFLOWS / "other.yml", UTF16_WORKFLOW) + + stderr = refused_and_untouched( + project, scaffolded, advanced, "bump-leg-workflow-unreadable", + ".github/workflows/legs.yml", ".github/workflows/other.yml") + assert "2 workflow file(s) are not UTF-8" in stderr + + +def test_an_unreadable_workflow_is_refused_before_nothing_to_do( + project, scaffolded): + """EXIT 1 MUST NOT HIDE IT. `--to` the commit every fact already names is + `nothing to do` (exit 1), but the reads come before that decision: a tree + the next real bump cannot read is not one this run may call settled.""" + prepared(project) + commit_bytes(project, WORKFLOWS / "legs.yml", LATIN1_COMMENT, append=True) + pinned = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + + refused_and_untouched(project, scaffolded, pinned, + "bump-leg-workflow-unreadable", + ".github/workflows/legs.yml") + + +# --- the pin ----------------------------------------------------------------- + +@MODES +def test_a_latin1_byte_in_the_legs_pin_is_refused_by_name( + project, scaffolded, advanced, extra): + """The pin is read OUTSIDE `load_yaml` (the bump rewrites its lines, not a + parsed mapping), so it needs the refusal `load_yaml` gives every other + reader. It names `contracts/spec-pin.yaml`.""" + prepared(project) + commit_bytes(project, Path("contracts") / "spec-pin.yaml", LATIN1_COMMENT, + append=True) + offset = (project / "contracts" / "spec-pin.yaml").read_bytes().index( + b"\xe9") + + stderr = refused_and_untouched( + project, scaffolded, advanced, "bump-leg-pin-unreadable", + "contracts/spec-pin.yaml", + f"not UTF-8 at byte {offset} (invalid continuation byte)", extra=extra) + assert "re-save" in stderr and "UTF-8" in stderr + + +def test_the_other_legs_pin_stays_the_validators_refusal(project, scaffolded, + advanced): + """THE SHAPE THE ISSUE HELD UP AS RIGHT, kept: the CODE leg's pin is not + read by this tool at all, so a Latin-1 byte in it reaches the project's + own validator after the rewrite, which refuses `yaml-unreadable` and the + bump rolls every byte back. This change must not turn that into a second, + earlier refusal for a file the tool never reads.""" + prepared(project) + commit_bytes(project, Path("contracts") / "code-pin.yaml", LATIN1_COMMENT, + append=True) + + stderr = refused_and_untouched( + project, scaffolded, advanced, "bump-leg-validators-red", + "yaml-unreadable", "contracts/code-pin.yaml") + assert "bump-leg-pin-unreadable" not in stderr + assert "bump-leg-workflow-unreadable" not in stderr + + +# --- the controls: the refusal is not wider than the failure ----------------- + +def test_a_utf8_workflow_with_non_ascii_comments_still_bumps_byte_for_byte( + project, scaffolded, advanced): + """A non-ASCII file is not a non-UTF-8 file. The comment survives the + lockstep rewrite BYTE FOR BYTE, and so does the BOM of a second file a + Windows editor saved as 'UTF-8 with BOM': a tool that refused or + normalised either would be a tool nobody could run in that project.""" + prepared(project) + comment = "# café, 日本, ☃\n".encode("utf-8") + commit_bytes(project, WORKFLOWS / "legs.yml", comment, append=True) + bom = codecs.BOM_UTF8 + b"on: [pull_request]\n# caf\xc3\xa9\n" + commit_bytes(project, WORKFLOWS / "bom.yml", bom) + before = (project / WORKFLOWS / "legs.yml").read_bytes() + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + assert "Traceback" not in result.stderr + + after = (project / WORKFLOWS / "legs.yml").read_bytes() + assert after.endswith(comment), "the comment survives, byte for byte" + assert after != before, "and the reference did move" + assert (project / WORKFLOWS / "bom.yml").read_bytes() == bom + assert validate(project).returncode == 0 + assert git("status", "--porcelain", cwd=project).stdout == "" + + +def test_a_non_utf8_file_the_validator_never_reads_does_not_stop_the_bump( + project, scaffolded, advanced): + """The refusal is over the set `validate-pins.py` reads: `.yml` and + `.yaml` under `.github/workflows/`. A Latin-1 `notes.txt` beside them is + nobody's input, and a refusal over it would be the tool having an opinion + about a file it never opens.""" + prepared(project) + commit_bytes(project, WORKFLOWS / "notes.txt", LATIN1_COMMENT) + commit_bytes(project, Path("NOTES.md"), LATIN1_COMMENT) + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + assert validate(project).returncode == 0 + assert (project / WORKFLOWS / "notes.txt").read_bytes() == LATIN1_COMMENT From ac281c6c8204d86a45cde68d7c008665595a7edc Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:18:00 +0000 Subject: [PATCH 08/14] bump-leg.py: the unreadable-file tests wrap three lines that ran past the width of the file, so the module reads as the rest of the suite does (#289) No behaviour change: three lines of tests/test_bump_leg_unreadable.py were 81 characters, in a suite that holds to 79. The 13 tests are unchanged and pass. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_bump_leg_unreadable.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_bump_leg_unreadable.py b/tests/test_bump_leg_unreadable.py index 3bc80dce..4b8f0c9c 100644 --- a/tests/test_bump_leg_unreadable.py +++ b/tests/test_bump_leg_unreadable.py @@ -49,19 +49,21 @@ #: A whole workflow saved as UTF-16 with a byte-order mark (what a Windows #: editor's "Unicode" save writes). `ff fe` is not a UTF-8 start byte, so the #: first byte is already the bad one. -UTF16_WORKFLOW = codecs.BOM_UTF16_LE + "on: [pull_request]\n".encode("utf-16-le") +UTF16_WORKFLOW = (codecs.BOM_UTF16_LE + + "on: [pull_request]\n".encode("utf-16-le")) @pytest.fixture(scope="module") def advanced(scaffolded, tmp_path_factory) -> str: """A real new commit on the spec leg's bare remote; returns its sha.""" - work = tmp_path_factory.mktemp("spec-advance-unreadable") / f"{PROJECT}-spec" + work = (tmp_path_factory.mktemp("spec-advance-unreadable") + / f"{PROJECT}-spec") proc = subprocess.run( ["git", "clone", "-q", str(scaffolded["remotes"] / f"{PROJECT}-spec.git"), str(work)], capture_output=True, text=True, check=False) assert proc.returncode == 0, proc.stderr - (work / "UNREADABLE-CONTRACT.md").write_text("# a contract the leg gained\n") + (work / "UNREADABLE-CONTRACT.md").write_text("# a contract gained\n") git("add", "--", "UNREADABLE-CONTRACT.md", cwd=work) git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", "Advance the spec leg for the unreadable-file tests", cwd=work) From 8828bfd2579b5ca70485217b76920414416f0433 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:54:27 +0000 Subject: [PATCH 09/14] bump-leg.py: a workflow file that is not UTF-8 is refused with each line marked untracked or ignored when git does not track it, and the remediation tells a tracked file's operator to commit it and an untracked one's not to, so no operator commits a draft onto the bump branch to clear the refusal (#289) The review of #352 (107242d) found that the `bump-leg-workflow-unreadable` remediation ended "re-save every file named as UTF-8 ... commit it, and run the same command again" for every file it named. That is true of a file git tracks and false of one it does not: re-saving an untracked draft is enough (the bump then goes through and the draft is not in the lockstep commit), committing it puts a draft workflow on the bump branch, which #207's `bump-leg-workflow-untracked` refusal exists to prevent, and `git add` refuses an ignored file outright. `_workflow_texts` now takes `tracked` and `listed`, the two sets `plan_workflows` already holds before it calls it, and asks `_tracked_spelling` of each file it refuses. A line git does not track ends `(untracked or ignored)`, and the remediation gives each kind its own advice: "Commit each file git tracks." for a tracked one, and "needs no commit, only the re-save" with the move-it-out alternative for the others. The question is asked only of a file already refused and reads no file, so every read still comes before anything is written, `--dry-run` included. `_tracked_spelling` is the question #344 settled, so a tracked file the disk spells another way is not marked untracked. Tests, in tests/test_bump_leg_unreadable.py (19, from 13): - an untracked and an ignored draft holding `# caf\xe9`, bump and `--dry-run` (4): exit 2, the line marked, the remediation without "commit it", then the file re-saved as UTF-8 and still untracked bumps and is not in the commit; at 107242d these fail on the unmarked line and on "commit it"; - an unreadable tracked file and an unreadable untracked one in one refusal (1); - a tracked `legs.yml` holding a Latin-1 byte, renamed outside git to `Legs.yml`, through #344's `_same_file` seam so it runs on every OS (1): refused `bump-leg-workflow-unreadable` under the disk's spelling, not marked untracked, bytes unchanged. This pins the composition the review confirmed by probe; it passes at 107242d. Mutants of the marking, each killed: an unreadable untracked file skipped (the one that survived the review), every line marked tracked, every line marked untracked, the exact-spelling comparison, and each half of the advice dropped. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- scripts/bump-leg.py | 46 ++++++++-- tests/test_bump_leg_unreadable.py | 145 +++++++++++++++++++++++++++++- 2 files changed, 180 insertions(+), 11 deletions(-) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index cfe5c0fc..2ba13832 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -55,7 +55,10 @@ anything is written and under `--dry-run` as well (#289). Never read lossily — the rewritten text is written back, and would carry U+FFFD where the operator's byte was — and never skipped, since `validate-pins.py` - reads every workflow file on disk and cannot read it either. + reads every workflow file on disk and cannot read it either. A workflow + file git does not track is refused the same way and marked `(untracked or + ignored)`: it needs the re-save and no commit, where a tracked one is + committed after it. * a validator that goes red after the rewrite. Every byte is rolled back first — the pin file, the workflow files, the leg's checkout and the index — so a refusal leaves the tree exactly as it was found. @@ -315,7 +318,8 @@ def _leg_pin_text(root: Path, pin_path: Path) -> str: "index or the leg's checkout was changed.") from exc -def _workflow_texts(root: Path, workflows: Path) -> list[tuple[Path, str]]: +def _workflow_texts(root: Path, workflows: Path, tracked: set[str], + listed: set[str]) -> list[tuple[Path, str]]: """Every `.yml` and `.yaml` file in `workflows`, read as UTF-8 — or ONE refusal naming each that is not (#289). @@ -331,21 +335,44 @@ def _workflow_texts(root: Path, workflows: Path) -> list[tuple[Path, str]]: each in one pass instead of meeting them one run at a time. The read is before anything is written, so the refusal promises only what is true: the working tree, the index and the leg's checkout are as they were. + + EACH LINE SAYS WHETHER GIT TRACKS THE FILE, because the advice differs. + A tracked file is committed after the re-save. One git does not track, + untracked or ignored, needs only the re-save: committing it would put a + draft on the bump branch, which `plan_workflows` refuses an untracked + file for (#207), and `git add` refuses an ignored one. `tracked` and + `listed` are `plan_workflows`' own, and `_tracked_spelling` answers with + them, so a tracked file the disk spells another way (#332) is not marked. + It is asked only of a file already refused and reads no file (`tracked` is + in hand, `_same_file` is a `stat`), so every read still comes before + anything is written. """ texts: list[tuple[Path, str]] = [] - unreadable: list[str] = [] + unreadable: list[tuple[str, bool]] = [] for path in sorted(workflows.iterdir()): if path.suffix not in (".yml", ".yaml") or not path.is_file(): continue try: texts.append((path, path.read_text(encoding="utf-8"))) except UnicodeDecodeError as exc: - unreadable.append(_not_utf8(root, path, exc)) + held = _tracked_spelling(root, path, tracked, listed) is not None + unreadable.append((_not_utf8(root, path, exc), held)) if unreadable: + advice = "" + if any(held for _, held in unreadable): + advice += "Commit each file git tracks. " + if not all(held for _, held in unreadable): + advice += ("A file marked (untracked or ignored) needs no " + "commit, only the re-save: committing it would put " + "it on this branch, so either leave it uncommitted " + "or move it out of .github/workflows/ until the bump " + "has landed. ") raise Refusal( "bump-leg-workflow-unreadable", f"{len(unreadable)} workflow file(s) are not UTF-8:\n" - + "\n".join(f" {line}" for line in unreadable), + + "\n".join(f" {line}" + ("" if held + else " (untracked or ignored)") + for line, held in unreadable), "Remediation: this tool rewrites each workflow file as UTF-8 " "text and reads none lossily, because the rewritten text is " "written back and would carry U+FFFD where your byte was; " @@ -354,9 +381,10 @@ def _workflow_texts(root: Path, workflows: Path) -> list[tuple[Path, str]]: "In each line above, the byte is counted from 0 at the start of " "the file and is the first that is not UTF-8: re-save every file " "named as UTF-8 (a UTF-16 file with a byte-order mark needs " - "converting, not editing), whether or not it names this leg, " - "commit it, and run the same command again. Nothing in the " - "working tree, the index or the leg's checkout was changed.") + "converting, not editing), whether or not it names this leg. " + + advice + + "Then run the same command again. Nothing in the working " + "tree, the index or the leg's checkout was changed.") return texts @@ -461,7 +489,7 @@ def plan_workflows(root: Path, repository: str, tracked = _tracked_workflows(root, workflows) listed = {child.name for child in workflows.iterdir()} untracked: list[tuple[str, int]] = [] - for path, text in _workflow_texts(root, workflows): + for path, text in _workflow_texts(root, workflows, tracked, listed): moved = 0 def swap(match: re.Match) -> str: diff --git a/tests/test_bump_leg_unreadable.py b/tests/test_bump_leg_unreadable.py index 4b8f0c9c..0189af33 100644 --- a/tests/test_bump_leg_unreadable.py +++ b/tests/test_bump_leg_unreadable.py @@ -21,6 +21,19 @@ being wider than the failure: a UTF-8 file with a non-ASCII comment, with a byte-order mark, and a non-UTF-8 file the validator never reads, all still bump. + +THE REFUSAL SAYS WHICH ADVICE APPLIES. A file git does not track (untracked, +or ignored) is refused all the same, because `validate-pins.py` reads every +file on disk, but re-saving it is enough: committing it would put a draft on +the bump branch, which `bump-leg-workflow-untracked` exists to prevent, and +`git add` refuses an ignored one. Each such line is marked, and the +remediation tells a tracked file's operator to commit it and an untracked +one's not to. + +THE #344 COMPOSITION is pinned through #344's own seam (`_same_file`), so it +runs on every disk: a tracked workflow renamed by case alone outside git that +is not UTF-8 is refused under the DISK's spelling, because every file is read +before any spelling question is asked. """ from __future__ import annotations @@ -31,8 +44,13 @@ import pytest -from conftest import PROJECT, git -from test_bump_leg import branched, bump, reference_both_legs, validate +from conftest import ORG, PROJECT, git +# A fixture is used by being named in this module's namespace, which is what +# the import of `bump_leg_module` is for. +from test_bump_leg import ( # noqa: F401 + branched, bump, bump_leg_module, committed, ignore_locally, ignoring_case, + reference_both_legs, respelled, validate, +) from test_bump_leg_rollback import assert_nothing_changed, snapshot #: Where the advanced spec commit is pushed. Not another module's branch: the @@ -134,6 +152,10 @@ def test_a_latin1_byte_in_a_workflow_naming_the_leg_is_refused_by_name( extra=extra) assert "re-save" in stderr and "UTF-8" in stderr, ( "the remediation says what to do about it") + assert "Commit each file git tracks" in stderr, ( + "a file git tracks is committed after the re-save") + assert "(untracked or ignored)" not in stderr + assert "needs no commit" not in stderr @MODES @@ -168,6 +190,88 @@ def test_every_unreadable_workflow_is_named_in_one_refusal(project, scaffolded, assert "2 workflow file(s) are not UTF-8" in stderr +DRAFT_UNREADABLE = b"on: [push]\n" + LATIN1_COMMENT +DRAFT_RESAVED = "on: [push]\n# café\n".encode("utf-8") + + +@MODES +@pytest.mark.parametrize("how", ["untracked", "ignored"]) +def test_an_unreadable_workflow_git_does_not_track_is_not_told_to_be_committed( + project, scaffolded, advanced, extra, how): + """A DRAFT THE OPERATOR NEVER COMMITTED IS REFUSED ALL THE SAME, and told + the truth about it. The file names no leg, holds a Latin-1 byte and is + untracked (or ignored through the clone's own `info/exclude`). + `validate-pins.py` reads every file on disk, so the bump cannot go past + it; but the remediation said "commit it", which for an untracked file + puts a draft workflow on the bump branch, the thing the untracked + refusal exists to prevent, and for an ignored one `git add` refuses. The + line is marked, the remediation says no commit is needed, and once the + file is re-saved as UTF-8 and STILL not committed the bump goes through + without it: the draft is not in the lockstep commit. + + It also kills the mutant that skips an unreadable file git does not + track: the draft would then reach the validator, which dies on it in a + traceback inside `bump-leg-validators-red`. + """ + prepared(project) + relative = ".github/workflows/draft.yml" + draft = project / WORKFLOWS / "draft.yml" + draft.write_bytes(DRAFT_UNREADABLE) + if how == "ignored": + ignore_locally(project, relative) + assert git("check-ignore", "-q", relative, cwd=project, + check=False).returncode == 0, "the file is ignored" + assert git("ls-files", "--", relative, cwd=project).stdout == "", ( + "git does not track it") + offset = DRAFT_UNREADABLE.index(b"\xe9") + + stderr = refused_and_untouched( + project, scaffolded, advanced, "bump-leg-workflow-unreadable", + f"{relative}: not UTF-8 at byte {offset} (invalid continuation byte)", + extra=extra) + assert "commit it" not in stderr, ( + "an operator who commits the draft puts it on the bump branch") + assert "Commit each file git tracks" not in stderr + assert "needs no commit" in stderr, ( + "the remediation says what is true of a file git does not track") + line = next(line for line in stderr.splitlines() if relative in line) + assert line.rstrip().endswith("(untracked or ignored)"), line + + draft.write_bytes(DRAFT_RESAVED) + result = bump(project, scaffolded, advanced, *extra) + assert result.returncode == 0, result.stderr + result.stdout + assert relative not in result.stdout, "the draft is nobody's business now" + assert draft.read_bytes() == DRAFT_RESAVED, "left exactly as it was saved" + assert git("ls-files", "--", relative, cwd=project).stdout == "" + if not extra: + assert relative not in committed(project), ( + "re-saving was enough: the draft is not in the lockstep commit") + assert validate(project).returncode == 0 + + +def test_each_unreadable_workflow_line_says_whether_git_tracks_it( + project, scaffolded, advanced): + """A tracked file and an untracked one in ONE refusal: the tracked one's + line is unmarked, the untracked one's is marked, and the remediation + gives each its own advice, so neither operator is told the other's.""" + prepared(project) + commit_bytes(project, WORKFLOWS / "legs.yml", LATIN1_COMMENT, append=True) + (project / WORKFLOWS / "draft.yml").write_bytes(DRAFT_UNREADABLE) + + stderr = refused_and_untouched( + project, scaffolded, advanced, "bump-leg-workflow-unreadable", + ".github/workflows/legs.yml", ".github/workflows/draft.yml") + assert "2 workflow file(s) are not UTF-8" in stderr + tracked_line = next(line for line in stderr.splitlines() + if ".github/workflows/legs.yml" in line) + draft_line = next(line for line in stderr.splitlines() + if ".github/workflows/draft.yml" in line) + assert "(untracked or ignored)" not in tracked_line + assert draft_line.rstrip().endswith("(untracked or ignored)") + assert "Commit each file git tracks" in stderr + assert "needs no commit" in stderr + + def test_an_unreadable_workflow_is_refused_before_nothing_to_do( project, scaffolded): """EXIT 1 MUST NOT HIDE IT. `--to` the commit every fact already names is @@ -182,6 +286,43 @@ def test_an_unreadable_workflow_is_refused_before_nothing_to_do( ".github/workflows/legs.yml") +def test_a_respelled_unreadable_workflow_is_refused_as_the_disk_spells_it( + bump_leg_module, project, advanced, monkeypatch): + """THE #344 COMPOSITION, through #344's own seam, so it runs on every + disk. A tracked `legs.yml` holding a Latin-1 byte, renamed outside git to + `Legs.yml`. `_workflow_texts` reads every file before the loop asks + `_tracked_spelling` anything, so the file is refused under the spelling + the DISK gives it, `.github/workflows/Legs.yml`, and never reaches the + question; a UTF-8 file respelled the same way is planned under git's + (`test_a_respelled_tracked_workflow_is_planned_under_gits_spelling`). + The line is not marked untracked: the file is tracked, under git's + spelling, which is what `_tracked_spelling` is asked. On main this is a + `UnicodeDecodeError`. The file is not touched.""" + reference_both_legs(project) + commit_bytes(project, WORKFLOWS / "legs.yml", LATIN1_COMMENT, append=True) + workflows = project / WORKFLOWS + respelled(workflows, "legs.yml", "Legs.yml") + monkeypatch.setattr(bump_leg_module, "_same_file", ignoring_case, + raising=False) + before = (workflows / "Legs.yml").read_bytes() + offset = before.index(b"\xe9") + + with pytest.raises(bump_leg_module.Refusal) as refused: + bump_leg_module.plan_workflows(project, f"{ORG}/{PROJECT}-spec", + advanced) + + assert refused.value.code == "bump-leg-workflow-unreadable" + assert (f".github/workflows/Legs.yml: not UTF-8 at byte {offset} " + "(invalid continuation byte)") in refused.value.detail, ( + "named under the disk's spelling, with the file's own offset") + assert ".github/workflows/legs.yml" not in refused.value.detail, ( + "not under git's") + assert "(untracked or ignored)" not in refused.value.detail, ( + "it is the tracked file, whichever way the disk spells it") + assert (workflows / "Legs.yml").read_bytes() == before, ( + "nothing was written") + + # --- the pin ----------------------------------------------------------------- @MODES From 797c96b2d2defd90c0a17b0f308c6e87288c3b32 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:54:41 +0000 Subject: [PATCH 10/14] docs/handbook.html: the refusal list for `bump-leg.py` names a workflow file or the leg's pin that is not UTF-8, and the page's change line records it, so a reader of the handbook meets the refusal in the list and not at the command line (#289) The review of #352 found that the handbook's "Refuses ..." list for `bump-leg.py` omitted the two refusals this change adds (`bump-leg-workflow-unreadable` and `bump-leg-pin-unreadable`), and that its change line recorded #207's refusal the same way and not this one. Nothing the page said was false. README.md is at its 1419-line cap, so it is left as it is, and the README blob the footer names does not move. The list gains one clause, with the one fact an operator needs about a file git does not track (it needs the re-save and no commit), and the change line gains (#289). Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- docs/handbook.html | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/handbook.html b/docs/handbook.html index ceaba2fe..5eddf825 100644 --- a/docs/handbook.html +++ b/docs/handbook.html @@ -1312,7 +1312,9 @@

Advancing a leg

Refuses a --to that is not exactly 40 hex; a commit no branch of the leg’s remote contains (a pin the rest of the world cannot fetch is a root it cannot bootstrap); a dirty root; an untracked or ignored workflow file it would - rewrite; a root standing on its own tracking branch (the next step is a pull request, never a + rewrite; a workflow file or the leg’s own pin that is not UTF-8, naming each file and its + first bad byte before anything is written (a file git does not track needs the re-save and no + commit); a root standing on its own tracking branch (the next step is a pull request, never a push to the default branch); and a validator that goes red — rolling the pin, the workflow files, the leg’s checkout and the index back first, so a refusal leaves the tree exactly as it was found.

@@ -1786,8 +1788,10 @@

For AI assistants

family.yaml beside project.yaml as a manifest kinds finding added (#211); bump-leg.py rewriting tracked workflow files only, and refusing an untracked or ignored one it would rewrite, added (#207); the assistant rules - saying that some scaffold refusals name a value to change, added (#288) — a page cannot - name the commit that contains it, and the blob below is the fact a test checks. + saying that some scaffold refusals name a value to change, added (#288); + bump-leg.py refusing a workflow file or the leg’s pin that is not UTF-8, by + name, added (#289) — a page cannot name the commit that contains it, and the blob below + is the fact a test checks. README.md blob e8da93817b0c1a195300800373a2895c0e0280e5. Every command, flag, exit code and file name on this page is quoted from that README and from AGENTS.md.

From 20dfdfa0b81bb5e3b5ce96cddf744ef78ff31253 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:50:53 +0000 Subject: [PATCH 11/14] bump-leg.py: a workflow file, the workflows directory or the leg's pin that this user cannot open is refused as `bump-leg-workflow-unreadable` or `bump-leg-pin-unreadable`, naming the path and the system's reason, so the run ends in exit 2 with a remedy and not a PermissionError traceback, under `--dry-run` as well (#383) `cmd_bump` reads the leg's pin and `plan_workflows` lists `.github/workflows/` and reads every `.yml` and `.yaml` file in it with no handler, and `main()` catches `Refusal` and `CommandFailed` only. A file or a directory this user cannot open raised `PermissionError` (an `OSError`) out of `read_text`, `iterdir` or `is_file`, and the run ended in a traceback, exit 1, naming Python's error and no file, `--dry-run` included, because every one of those reads comes before its early return. Git reads such a tree as clean when it does not re-read the file (`core.trustctime=false`, `--assume-unchanged`, or a directory it cannot list, which `git status` reports on stderr only), so `_ensure_root_clean` lets the run through to the read. #352 (#289) put those reads in `_leg_pin_text` and `_workflow_texts` and refused a file that is not UTF-8 by name. This adds the `OSError` clause beside its `UnicodeDecodeError`, under the same two ids, so one id says "this tool cannot read this file" whichever the reason: - the line is `: cannot be read: ` (`Permission denied`), and the directory is named with a trailing `/` and `cannot be listed`; - the remedy is cause-specific and is never the UTF-8 re-save: make the file readable by this user (`chmod u+r`, `chmod u+rx` for a directory, or fix who owns it) for a `PermissionError`, and for any other `OSError` (a file gone between the listing and the read, an I/O error) put a readable file back, with no `chmod`; - a file that is not UTF-8 and one that cannot be opened meet in ONE refusal, each group under its own header with its own advice; a refusal of the first kind alone is byte for byte what #352 printed; - #352's tracked/untracked marking is kept: an unopenable file git does not track is marked `(untracked or ignored)` and is told it is not in the repository, never to commit it (restoring a mode changes no content, so nothing is committed for a tracked file either); - both `iterdir()` calls go through `_workflow_children`, and the two `stat`s that can be refused (`is_file` on a workflow in a directory this user can list but not search, and on the pin in an unsearchable `contracts/`) are inside the same guards, because they are the same failure one frame earlier. `docs/handbook.html` gains the refusal in the list and a clause in the change line, as #352 did for its half. The default-configuration row (git reports the mode-000 file ` M` and `bump-leg-root-dirty` fires) is another refusal, right about the tree, and is not changed. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- docs/handbook.html | 10 +- scripts/bump-leg.py | 237 +++++++++++-- tests/test_bump_leg_unopenable.py | 537 ++++++++++++++++++++++++++++++ 3 files changed, 746 insertions(+), 38 deletions(-) create mode 100644 tests/test_bump_leg_unopenable.py diff --git a/docs/handbook.html b/docs/handbook.html index 5eddf825..11b4688f 100644 --- a/docs/handbook.html +++ b/docs/handbook.html @@ -1314,7 +1314,10 @@

Advancing a leg

is a root it cannot bootstrap); a dirty root; an untracked or ignored workflow file it would rewrite; a workflow file or the leg’s own pin that is not UTF-8, naming each file and its first bad byte before anything is written (a file git does not track needs the re-save and no - commit); a root standing on its own tracking branch (the next step is a pull request, never a + commit); a workflow file, the .github/workflows/ directory or the leg’s pin + that this user cannot open, naming the path and the system’s reason, with the same ids + and the remedy of making it readable and running the command again; a root standing on its + own tracking branch (the next step is a pull request, never a push to the default branch); and a validator that goes red — rolling the pin, the workflow files, the leg’s checkout and the index back first, so a refusal leaves the tree exactly as it was found.

@@ -1790,8 +1793,9 @@

For AI assistants

refusing an untracked or ignored one it would rewrite, added (#207); the assistant rules saying that some scaffold refusals name a value to change, added (#288); bump-leg.py refusing a workflow file or the leg’s pin that is not UTF-8, by - name, added (#289) — a page cannot name the commit that contains it, and the blob below - is the fact a test checks. + name, added (#289); bump-leg.py refusing a workflow file, the workflows directory + or the leg’s pin that this user cannot open, by name, added (#383) — a page cannot + name the commit that contains it, and the blob below is the fact a test checks. README.md blob e8da93817b0c1a195300800373a2895c0e0280e5. Every command, flag, exit code and file name on this page is quoted from that README and from AGENTS.md.

diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 2ba13832..28965662 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -59,6 +59,15 @@ file git does not track is refused the same way and marked `(untracked or ignored)`: it needs the re-save and no commit, where a tracked one is committed after it. + * the same two ids for a file or a directory this user cannot OPEN (#383): + a workflow file, `.github/workflows/` itself or the leg's pin that the + operating system refuses (`Permission denied`) or fails to read, naming + the path and the system's own reason. The remedy is to make it readable + by this user (`chmod u+r`, or fix who owns it) and run the same command + again, not a re-save. Refused before anything is written, under + `--dry-run` as well, whether or not the file names the leg, and a line + for a file git does not track is marked as it is for a file that is not + UTF-8. * a validator that goes red after the rewrite. Every byte is rolled back first — the pin file, the workflow files, the leg's checkout and the index — so a refusal leaves the tree exactly as it was found. @@ -287,9 +296,68 @@ def _not_utf8(root: Path, path: Path, exc: UnicodeDecodeError) -> str: f"{exc.start} ({exc.reason})") +def _os_reason(exc: OSError) -> str: + """The operating system's own words for `exc` (`Permission denied`), or + what Python made of it when there are none (an `OSError` raised with no + errno carries no `strerror`).""" + return exc.strerror or str(exc) + + +def _cannot_open(root: Path, path: Path, exc: OSError, *, + directory: bool = False) -> str: + """`: cannot be read: ` (`cannot be listed` + for a directory, named with a trailing `/`), in the shape `_not_utf8` + gives a file this tool cannot decode, so a line says what failed on which + file and the two causes read alike (#383).""" + shown = path.relative_to(root).as_posix() + ("/" if directory else "") + return (f"{shown}: cannot be {'listed' if directory else 'read'}: " + f"{_os_reason(exc)}") + + +def _cannot_open_advice(errors: list[OSError]) -> str: + """What to do about a file or a directory the operating system refused, + for each cause present in `errors`, ending in a space so it joins the + sentence after it (#383). + + NOT the re-save advice a file that is not UTF-8 is given: its bytes were + never the trouble, and no edit to them makes a file readable. A refusal + of permission (`PermissionError`, which is `EACCES` and `EPERM`) is put + right by the mode or the owner; any other `OSError` (a file removed or + replaced while this ran, an I/O error, a path that stopped being a file) + is not a permission and is not told to `chmod`. + """ + advice = "" + if any(isinstance(error, PermissionError) for error in errors): + advice += ("Make what this user may not open readable by this " + "user: `chmod u+r` on a file, `chmod u+rx` on a directory " + "(and on every directory above it inside the root), or " + "fix who owns it. ") + if any(not isinstance(error, PermissionError) for error in errors): + advice += ("Where the operating system gave another reason, put a " + "regular file this user can open back in its place, or " + "take away one nobody wants, and make sure nothing is " + "changing the directory while this runs. ") + return advice + + +def _pin_unopened_refusal(root: Path, pin_path: Path, + exc: OSError) -> Refusal: + """`bump-leg-pin-unreadable` for a pin the operating system refused, the + id a pin that is not UTF-8 has, with this cause's own remedy (#383).""" + return Refusal( + "bump-leg-pin-unreadable", + _cannot_open(root, pin_path, exc), + "Remediation: this tool must read the leg's pin to rewrite " + "`commit:` and `digests.tree_sha256` in it, and " + "`scripts/validate-pins.py` reads it too. " + + _cannot_open_advice([exc]) + + "Then run the same command again. Nothing in the working " + "tree, the index or the leg's checkout was changed.") + + def _leg_pin_text(root: Path, pin_path: Path) -> str: """The leg's `contracts/-pin.yaml` as text, or a refusal naming it - when it is not UTF-8 (#289). + when it is not UTF-8 (#289) or this user cannot open it (#383). Read here rather than through `load_yaml` because `rewrite_pin` is a LINE REWRITE of the file's own text, which a parsed mapping cannot be. A @@ -300,9 +368,18 @@ def _leg_pin_text(root: Path, pin_path: Path) -> str: refusal promises only what is true: the working tree, the index and the leg's checkout are as they were. (The leg's object store was fetched into already, which `--dry-run` says it does.) + + THE OPERATING SYSTEM'S REFUSAL IS THE SAME REFUSAL, BY ANOTHER CAUSE + (#383). A pin this user may not open raised `PermissionError`, which is + an `OSError` and no `Refusal`: the same traceback, exit 1, from this same + read, with `--dry-run` no different. The id is the one a pin that is not + UTF-8 has, so one id says "this tool cannot read the leg's pin" whatever + the reason, with the cause's own detail and its own remedy. """ try: return pin_path.read_text(encoding="utf-8") + except OSError as exc: + raise _pin_unopened_refusal(root, pin_path, exc) from exc except UnicodeDecodeError as exc: raise Refusal( "bump-leg-pin-unreadable", @@ -318,10 +395,47 @@ def _leg_pin_text(root: Path, pin_path: Path) -> str: "index or the leg's checkout was changed.") from exc +def _workflow_children(root: Path, workflows: Path) -> list[Path]: + """The entries of `workflows`, sorted — or a refusal naming the directory + when this user cannot list it (#383). + + `plan_workflows` lists the directory for the names git is compared with, + and `_workflow_texts` lists it again to read the files, so both go through + here. A directory this user may not list raised `PermissionError` out of + `iterdir`, which `main()` does not catch: a traceback, exit 1, naming + Python's error and no refusal. It reads as CLEAN to git + (`git status --porcelain` prints its `Permission denied` on stderr, writes + nothing to stdout and exits 0), so `_ensure_root_clean` lets the run + through to here. The listing is before anything is written, under + `--dry-run` as well, so the refusal promises only what is true: the + working tree, the index and the leg's checkout are as they were. + """ + try: + return sorted(workflows.iterdir()) + except OSError as exc: + raise _workflows_dir_refusal(root, workflows, exc) from exc + + +def _workflows_dir_refusal(root: Path, workflows: Path, + exc: OSError) -> Refusal: + """`bump-leg-workflow-unreadable` for the DIRECTORY, named as one.""" + return Refusal( + "bump-leg-workflow-unreadable", + _cannot_open(root, workflows, exc, directory=True), + "Remediation: this tool reads every workflow file in the directory " + "named above, whether or not it names this leg, and " + "`scripts/validate-pins.py` reads them too, so a directory it cannot " + "list is one it cannot bump. " + + _cannot_open_advice([exc]) + + "Then run the same command again. Nothing in the working tree, " + "the index or the leg's checkout was changed.") + + def _workflow_texts(root: Path, workflows: Path, tracked: set[str], listed: set[str]) -> list[tuple[Path, str]]: """Every `.yml` and `.yaml` file in `workflows`, read as UTF-8 — or ONE - refusal naming each that is not (#289). + refusal naming each that is not (#289) or that this user cannot open + (#383). EVERY FILE IS READ, whether or not it names the leg, so a file that is not UTF-8 is REFUSED BY NAME and never skipped, never read lossily. @@ -346,43 +460,83 @@ def _workflow_texts(root: Path, workflows: Path, tracked: set[str], It is asked only of a file already refused and reads no file (`tracked` is in hand, `_same_file` is a `stat`), so every read still comes before anything is written. + + THE OPERATING SYSTEM'S REFUSAL IS THE SAME REFUSAL, BY ANOTHER CAUSE + (#383). A file this user may not open raised `PermissionError`, an + `OSError` that `main()` does not catch either: the same traceback, exit 1, + `--dry-run` included. Any `OSError` of the read or of the `stat` before + it (`is_file`, which a directory this user can list but not search + refuses) is named the same way, as a line of its own with the system's + reason, whether or not the file names the leg; a file that is not UTF-8 + and one that cannot be opened meet in ONE refusal, each group with its own + header and its own advice, because the re-save the first needs does + nothing for the second. Each line says whether git tracks the file as + before, and a file that was only unreadable is never told to be + committed: restoring a mode changes no content, so there is nothing to + commit for it, tracked or not. """ texts: list[tuple[Path, str]] = [] unreadable: list[tuple[str, bool]] = [] - for path in sorted(workflows.iterdir()): - if path.suffix not in (".yml", ".yaml") or not path.is_file(): - continue + unopened: list[tuple[str, bool, OSError]] = [] + for path in _workflow_children(root, workflows): try: + if path.suffix not in (".yml", ".yaml") or not path.is_file(): + continue texts.append((path, path.read_text(encoding="utf-8"))) except UnicodeDecodeError as exc: held = _tracked_spelling(root, path, tracked, listed) is not None unreadable.append((_not_utf8(root, path, exc), held)) - if unreadable: + except OSError as exc: + held = _tracked_spelling(root, path, tracked, listed) is not None + unopened.append((_cannot_open(root, path, exc), held, exc)) + if unreadable or unopened: + marked = " (untracked or ignored)" + detail: list[str] = [] advice = "" - if any(held for _, held in unreadable): - advice += "Commit each file git tracks. " - if not all(held for _, held in unreadable): - advice += ("A file marked (untracked or ignored) needs no " - "commit, only the re-save: committing it would put " - "it on this branch, so either leave it uncommitted " - "or move it out of .github/workflows/ until the bump " - "has landed. ") + if unreadable: + detail.append( + f"{len(unreadable)} workflow file(s) are not UTF-8:\n" + + "\n".join(f" {line}" + ("" if held else marked) + for line, held in unreadable)) + advice += ( + "this tool rewrites each workflow file as UTF-8 " + "text and reads none lossily, because the rewritten text is " + "written back and would carry U+FFFD where your byte was; " + "`scripts/validate-pins.py` reads every workflow file on " + "disk as UTF-8 too, so skipping one would only move the " + "failure there. In each line above, the byte is counted from " + "0 at the start of the file and is the first that is not " + "UTF-8: re-save every file named as UTF-8 (a UTF-16 file " + "with a byte-order mark needs converting, not editing), " + "whether or not it names this leg. ") + if any(held for _, held in unreadable): + advice += "Commit each file git tracks. " + if not all(held for _, held in unreadable): + advice += ("A file marked (untracked or ignored) needs no " + "commit, only the re-save: committing it would " + "put it on this branch, so either leave it " + "uncommitted or move it out of " + ".github/workflows/ until the bump has landed. ") + if unopened: + detail.append( + f"{len(unopened)} workflow file(s) cannot be opened:\n" + + "\n".join(f" {line}" + ("" if held else marked) + for line, held, _ in unopened)) + advice += ( + "Each file named as cannot be opened is one this tool " + "cannot rewrite, and `scripts/validate-pins.py` reads every " + "workflow file on disk too, whether or not it names this " + "leg, so skipping one would only move the failure there. " + + _cannot_open_advice([error for _, _, error in unopened]) + + "Nothing is committed for such a file, because one made " + "readable has no change to commit. ") + if not all(held for _, held, _ in unopened): + advice += ("A file marked (untracked or ignored) is not in " + "the repository: it can also be moved out of " + ".github/workflows/ until the bump has landed. ") raise Refusal( - "bump-leg-workflow-unreadable", - f"{len(unreadable)} workflow file(s) are not UTF-8:\n" - + "\n".join(f" {line}" + ("" if held - else " (untracked or ignored)") - for line, held in unreadable), - "Remediation: this tool rewrites each workflow file as UTF-8 " - "text and reads none lossily, because the rewritten text is " - "written back and would carry U+FFFD where your byte was; " - "`scripts/validate-pins.py` reads every workflow file on disk as " - "UTF-8 too, so skipping one would only move the failure there. " - "In each line above, the byte is counted from 0 at the start of " - "the file and is the first that is not UTF-8: re-save every file " - "named as UTF-8 (a UTF-16 file with a byte-order mark needs " - "converting, not editing), whether or not it names this leg. " - + advice + "bump-leg-workflow-unreadable", "\n".join(detail), + "Remediation: " + advice + "Then run the same command again. Nothing in the working " "tree, the index or the leg's checkout was changed.") return texts @@ -480,14 +634,20 @@ def plan_workflows(root: Path, repository: str, file through it, read through the disk's name and written through git's, which the filesystem has already said are the same file. - A file that is not UTF-8 is a refusal (`_workflow_texts`, #289). + A file that is not UTF-8, or that this user cannot open, is a refusal + (`_workflow_texts`, #289 and #383), and so is the directory itself when + this user cannot list it (`_workflow_children`, #383). """ plans: list[tuple[Path, str, int]] = [] workflows = root / ".github" / "workflows" - if not workflows.is_dir(): - return plans + try: + if not workflows.is_dir(): + return plans + except OSError as exc: + # a directory above `workflows` this user cannot search (#383) + raise _workflows_dir_refusal(root, workflows, exc) from exc tracked = _tracked_workflows(root, workflows) - listed = {child.name for child in workflows.iterdir()} + listed = {child.name for child in _workflow_children(root, workflows)} untracked: list[tuple[str, int]] = [] for path, text in _workflow_texts(root, workflows, tracked, listed): moved = 0 @@ -841,12 +1001,19 @@ def _recorded_leg_commit(root: Path, path: str) -> str: def _leg_pin_path(root: Path, pin_rel: str) -> Path: - """The leg's `contracts/-pin.yaml`, refused if it does not exist. + """The leg's `contracts/-pin.yaml`, refused if it does not exist or + cannot be reached (#383). Split from `cmd_bump` for #136. """ pin_path = root / pin_rel - if not pin_path.is_file(): + try: + present = pin_path.is_file() + except OSError as exc: + # `contracts/` this user cannot search: the `stat` is refused, which + # is not an absent file (#383) + raise _pin_unopened_refusal(root, pin_path, exc) from exc + if not present: raise Refusal( "bump-leg-pin-missing", f"{pin_rel} does not exist, so half of the invariant has nothing " diff --git a/tests/test_bump_leg_unopenable.py b/tests/test_bump_leg_unopenable.py new file mode 100644 index 00000000..c3f42410 --- /dev/null +++ b/tests/test_bump_leg_unopenable.py @@ -0,0 +1,537 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`scripts/bump-leg.py`: a file or a directory it must READ and this user +cannot OPEN is refused by name, before anything moves (#383). + +`tests/test_bump_leg_unreadable.py` (#289) covers a file whose BYTES are not +UTF-8. This is the other way a read fails: the operating system refuses it. +The tool reads `contracts/-pin.yaml` and every `.yml` and `.yaml` file +under `.github/workflows/`, and lists that directory, with no handler, and +`main()` catches `Refusal` and `CommandFailed` only. A `PermissionError` (an +`OSError`) ended the run in a traceback, exit 1, naming Python's error and not +a refusal, under `--dry-run` as well, because every one of those reads comes +before its early return. Now each is the refusal of its own kind, exit 2, with +the ids #289 introduced, so one id says "this tool cannot read this file" +whichever the reason: `bump-leg-workflow-unreadable` for a workflow file or the +directory, `bump-leg-pin-unreadable` for the pin. Each names the path and the +operating system's reason, and the remedy is to make it readable by this user +and run the same command again, never the re-save a file that is not UTF-8 +is given. + +HOW THE TREE COMES TO BE ONE THE TOOL GETS THIS FAR WITH. With git's default +configuration `chmod 000` on a tracked file changes its ctime, git must re-read +it to compare, cannot, and reports ` M`: the run is then refused +`bump-leg-root-dirty`, which is another refusal and is not under test here. +The traceback is what the operator got when git read the tree as clean, so +each mode-000 test says `git update-index --assume-unchanged` of every tracked +file it makes unreadable, which needs no sleep and no second status, asserts +that `git status --porcelain` is empty WHILE the file is unreadable, and clears +the flag when the mode is restored (a flag left on is a `git add` that adds +nothing). The mode is restored in a `finally`, so the temporary directory can +be removed. + +THE TESTS THAT NEED A MODE SKIP ON WINDOWS and when the suite runs as root, +who reads anything. The tests that make the operating system fail for another +reason (a file gone between the listing and the read, an I/O error) do it +with a stand-in for the call, so they run everywhere, root included. + +EVERY MODE-000 TEST COMPARES THE WHOLE TREE, as +`tests/test_bump_leg_rollback.py` does, from a snapshot taken BEFORE the mode +is changed (the snapshot reads the files): the root's HEAD, the pin's and +every workflow's bytes, the gitlink, the root's `git status`, an empty +`git diff --cached`, and the leg's HEAD and branch. A refused bump is then run +AGAIN with the mode restored, to show that the remedy the refusal gives is the +whole of it. +""" + +from __future__ import annotations + +import contextlib +import errno +import os +import stat +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import ORG, PROJECT, git +# A fixture is used by being named in this module's namespace, which is what +# the import of `bump_leg_module` is for. +from test_bump_leg import ( # noqa: F401 + bump, bump_leg_module, committed, ignore_locally, ignoring_case, + reference_both_legs, respelled, validate, +) +from test_bump_leg_rollback import assert_nothing_changed, snapshot +from test_bump_leg_unreadable import ( + LATIN1_COMMENT, MODES, WORKFLOWS, commit_bytes, prepared, +) + +#: Where the advanced spec commit is pushed. Not another module's branch: the +#: bare remotes are session-scoped, and a second, different commit pushed to +#: a name already taken is refused as non-fast-forward. +ADVANCE_BRANCH = "bump-leg-unopenable-advance" + +needs_a_mode = pytest.mark.skipif( + sys.platform == "win32" or (hasattr(os, "geteuid") and os.geteuid() == 0), + reason="a mode that refuses the owner is a POSIX mode, and root reads " + "through every one (tests/test_family_add_refusals.py)") + +#: A workflow that names no leg: no `@` of its own would ever move. +NAMES_NO_LEG = b"on: [pull_request]\njobs: {}\n" + + +@pytest.fixture(scope="module") +def advanced(scaffolded, tmp_path_factory) -> str: + """A real new commit on the spec leg's bare remote; returns its sha.""" + work = (tmp_path_factory.mktemp("spec-advance-unopenable") + / f"{PROJECT}-spec") + proc = subprocess.run( + ["git", "clone", "-q", + str(scaffolded["remotes"] / f"{PROJECT}-spec.git"), str(work)], + capture_output=True, text=True, check=False) + assert proc.returncode == 0, proc.stderr + (work / "UNOPENABLE-CONTRACT.md").write_text("# a contract gained\n") + git("add", "--", "UNOPENABLE-CONTRACT.md", cwd=work) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Advance the spec leg for the unopenable-file tests", cwd=work) + git("push", "-q", "origin", f"HEAD:refs/heads/{ADVANCE_BRANCH}", cwd=work) + return git("rev-parse", "HEAD", cwd=work).stdout.strip() + + +@contextlib.contextmanager +def made_unopenable(project: Path, *relatives: str, mode: int = 0): + """Set `mode` on each of `relatives` (000 by default: no read, no search) + while git still reads the tree as clean, and put everything back. + + Git first: `--assume-unchanged` on every file git tracks at or under each + path, so `git status` neither re-reads a file it cannot nor reports it + changed. The deepest path takes its mode first and the shallowest gives + it back first, so a file inside a directory made mode 000 is never + reached through that directory. + """ + flagged: list[str] = [] + for relative in relatives: + listing = git("ls-files", "-z", "--", relative, cwd=project).stdout + for name in filter(None, listing.split("\0")): + git("update-index", "--assume-unchanged", "--", name, cwd=project) + flagged.append(name) + paths = [project / relative for relative in relatives] + modes = {path: stat.S_IMODE(path.stat().st_mode) for path in paths} + try: + for path in sorted(paths, key=lambda item: len(item.parts), + reverse=True): + path.chmod(mode) + yield + finally: + for path in sorted(modes, key=lambda item: len(item.parts)): + path.chmod(modes[path]) + for name in flagged: + git("update-index", "--no-assume-unchanged", "--", name, + cwd=project) + + +def refused_while_unopenable(project: Path, scaffolded: dict, to: str, + code: str, *relatives: str, extra: tuple = (), + mode: int = 0) -> str: + """Make `relatives` unopenable, bump, and assert EVERYTHING the issue's + table asks of the row: git reads the tree as clean (so it is the read that + fails and not `bump-leg-root-dirty`), exit 2, the refusal's id, "Permission + denied", no traceback and no UTF-8 advice, the tree still clean while the + mode is on, and, once it is back, the whole tree as it was found. + Returns stderr for a test that asserts more.""" + before = snapshot(project) + with made_unopenable(project, *relatives, mode=mode): + assert git("status", "--porcelain", cwd=project).stdout == "", ( + "git must read the tree as clean, or the refusal under test is " + "`bump-leg-root-dirty`") + result = bump(project, scaffolded, to, *extra) + assert git("status", "--porcelain", cwd=project).stdout == "", ( + "the tree is as clean after the run as before it") + output = result.stderr + result.stdout + assert result.returncode == 2, output + assert f"REFUSED {code}" in result.stderr, output + assert "Permission denied" in result.stderr, output + assert "Traceback" not in output, output + assert "PermissionError" not in output, output + assert "re-save" not in result.stderr, ( + "the file's bytes are not the trouble") + assert "UTF-8" not in result.stderr, output + assert_nothing_changed(project, before) + return result.stderr + + +def refused_again_fine(project: Path, scaffolded: dict, to: str, extra: tuple, + *moved: str) -> None: + """THE REMEDY IS THE WHOLE OF IT: with the mode back, the same command + runs, and a bump leaves a root the project's own validator passes, with + every file in `moved` in the lockstep commit.""" + result = bump(project, scaffolded, to, *extra) + assert result.returncode == 0, result.stderr + result.stdout + assert "Traceback" not in result.stderr + if not extra: + assert set(moved) <= committed(project) + assert validate(project).returncode == 0 + assert git("status", "--porcelain", cwd=project).stdout == "" + + +# --- a workflow file --------------------------------------------------------- + +@needs_a_mode +@MODES +def test_a_workflow_naming_the_leg_that_cannot_be_opened_is_refused( + project, scaffolded, advanced, extra): + """THE ISSUE'S ROW (a) AND (b): the file names the leg and is committed, + so the tracked-files rule is met and only the read stands in the way. On + the base it is `PermissionError` out of `plan_workflows`, exit 1, bump and + `--dry-run` alike. The refusal names the file by its path in the + repository and the system's reason, and says to make it readable and run + the same command again; the tracked file's line is not marked untracked.""" + prepared(project) + + stderr = refused_while_unopenable( + project, scaffolded, advanced, "bump-leg-workflow-unreadable", + ".github/workflows/legs.yml", extra=extra) + + assert (".github/workflows/legs.yml: cannot be read: Permission denied" + in stderr) + assert "1 workflow file(s) cannot be opened" in stderr + assert "chmod u+r" in stderr and "fix who owns it" in stderr + assert "Then run the same command again" in stderr + assert "(untracked or ignored)" not in stderr + assert "Commit each file" not in stderr + assert "commit it" not in stderr + refused_again_fine(project, scaffolded, advanced, extra, + ".github/workflows/legs.yml") + + +@needs_a_mode +@MODES +@pytest.mark.parametrize("name", ["other.yml", "other.yaml"]) +def test_a_workflow_naming_no_leg_that_cannot_be_opened_is_refused( + project, scaffolded, advanced, extra, name): + """THE REFUSAL COVERS THE FILE WHETHER OR NOT IT NAMES THE LEG, as #289's + does: `plan_workflows` reads every file in the directory, and + `validate-pins.py` does too. This file holds no `@`, so none of its + own would move; it is read anyway. `.yaml` is read as well as `.yml`.""" + prepared(project) + commit_bytes(project, WORKFLOWS / name, NAMES_NO_LEG) + + refused_while_unopenable( + project, scaffolded, advanced, "bump-leg-workflow-unreadable", + f".github/workflows/{name}", extra=extra) + + +@needs_a_mode +def test_every_workflow_that_cannot_be_opened_is_named_in_one_refusal( + project, scaffolded, advanced): + """Two unopenable files, one run: the operator fixes both rather than + meeting them one refusal at a time.""" + prepared(project) + commit_bytes(project, WORKFLOWS / "other.yml", NAMES_NO_LEG) + + stderr = refused_while_unopenable( + project, scaffolded, advanced, "bump-leg-workflow-unreadable", + ".github/workflows/legs.yml", ".github/workflows/other.yml") + + assert "2 workflow file(s) cannot be opened" in stderr + assert ".github/workflows/legs.yml: cannot be read" in stderr + assert ".github/workflows/other.yml: cannot be read" in stderr + + +@needs_a_mode +@MODES +@pytest.mark.parametrize("how", ["untracked", "ignored"]) +def test_an_unopenable_draft_is_marked_and_not_told_to_be_committed( + project, scaffolded, advanced, extra, how): + """#352's TRACKED/UNTRACKED MARKING, KEPT FOR THIS CAUSE. A draft nobody + committed (or that the clone's own `info/exclude` ignores) names no leg + and cannot be opened. It is refused all the same, because + `validate-pins.py` reads every file on disk, but its line is marked, and + nothing in the remediation tells its operator to commit it: that puts a + draft workflow on the bump branch, which `bump-leg-workflow-untracked` + exists to prevent, and `git add` refuses an ignored one. Making it + readable, or moving it away, is enough, and with the mode back the bump + goes through and leaves the draft out of the lockstep commit.""" + prepared(project) + relative = ".github/workflows/draft.yml" + draft = project / WORKFLOWS / "draft.yml" + draft.write_bytes(NAMES_NO_LEG) + if how == "ignored": + ignore_locally(project, relative) + assert git("check-ignore", "-q", relative, cwd=project, + check=False).returncode == 0, "the file is ignored" + assert git("ls-files", "--", relative, cwd=project).stdout == "", ( + "git does not track it") + + before = snapshot(project) + with made_unopenable(project, relative): + result = bump(project, scaffolded, advanced, *extra) + stderr = result.stderr + assert result.returncode == 2, stderr + result.stdout + assert "REFUSED bump-leg-workflow-unreadable" in stderr + line = next(line for line in stderr.splitlines() if relative in line) + assert line.rstrip().endswith("(untracked or ignored)"), line + assert "Permission denied" in line + assert "commit it" not in stderr and "Commit each file" not in stderr, ( + "an operator who commits the draft puts it on the bump branch") + assert "is not in the repository" in stderr + assert "Traceback" not in stderr + assert_nothing_changed(project, before) + + result = bump(project, scaffolded, advanced, *extra) + assert result.returncode == 0, result.stderr + result.stdout + assert relative not in result.stdout, "the draft is nobody's business now" + assert git("ls-files", "--", relative, cwd=project).stdout == "" + if not extra: + assert relative not in committed(project) + assert validate(project).returncode == 0 + + +@needs_a_mode +def test_a_non_utf8_file_and_an_unopenable_one_meet_in_one_refusal( + project, scaffolded, advanced): + """TWO CAUSES, ONE REFUSAL, EACH WITH ITS OWN ADVICE. A tracked Latin-1 + file and a tracked file made unreadable: both are named under their own + header, the re-save advice and the commit advice are given for the first, + and `chmod` for the second, which is told nothing about re-saving. The + count line #352 wrote is unchanged for the first.""" + prepared(project) + commit_bytes(project, WORKFLOWS / "latin1.yml", LATIN1_COMMENT) + commit_bytes(project, WORKFLOWS / "other.yml", NAMES_NO_LEG) + offset = (project / WORKFLOWS / "latin1.yml").read_bytes().index(b"\xe9") + + before = snapshot(project) + with made_unopenable(project, ".github/workflows/other.yml"): + result = bump(project, scaffolded, advanced) + stderr = result.stderr + assert result.returncode == 2, stderr + result.stdout + assert "REFUSED bump-leg-workflow-unreadable" in stderr + assert "Traceback" not in stderr + assert "1 workflow file(s) are not UTF-8:" in stderr + assert (f".github/workflows/latin1.yml: not UTF-8 at byte {offset} " + "(invalid continuation byte)") in stderr + assert "1 workflow file(s) cannot be opened:" in stderr + assert (".github/workflows/other.yml: cannot be read: Permission denied" + in stderr) + assert stderr.index("are not UTF-8") < stderr.index("cannot be opened"), ( + "the same order every run: not UTF-8 first") + assert "re-save" in stderr and "Commit each file git tracks" in stderr + assert "chmod u+r" in stderr + assert_nothing_changed(project, before) + + +@needs_a_mode +def test_an_unopenable_workflow_is_refused_before_nothing_to_do( + project, scaffolded): + """EXIT 1 MUST NOT HIDE IT. `--to` the commit every fact already names is + `nothing to do` (exit 1), but the reads come before that decision: a tree + the next real bump cannot read is not one this run may call settled.""" + prepared(project) + pinned = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + + refused_while_unopenable( + project, scaffolded, pinned, "bump-leg-workflow-unreadable", + ".github/workflows/legs.yml") + + +# --- the directory ----------------------------------------------------------- + +@needs_a_mode +@MODES +def test_a_workflows_directory_that_cannot_be_listed_is_refused_naming_it( + project, scaffolded, advanced, extra): + """THE ISSUE'S ROW (e). `.github/workflows/` itself mode 000: git reads the + tree as clean (its `Permission denied` goes to stderr and `git status` + exits 0), the walk raised `PermissionError` out of `iterdir`, and the run + ended in a traceback. The refusal names the DIRECTORY, with a trailing + `/`, and says `chmod u+rx`.""" + prepared(project) + + stderr = refused_while_unopenable( + project, scaffolded, advanced, "bump-leg-workflow-unreadable", + ".github/workflows", extra=extra) + + assert (".github/workflows/: cannot be listed: Permission denied" + in stderr) + assert "chmod u+rx" in stderr + assert "cannot be opened" not in stderr, "no file was reached" + refused_again_fine(project, scaffolded, advanced, extra, + ".github/workflows/legs.yml") + + +@needs_a_mode +def test_a_workflows_directory_listable_but_not_searchable_is_refused( + project, scaffolded, advanced): + """A DIRECTORY WITH `r` AND NO `x`: its names list, and every `stat` of a + name in it is refused. The listing succeeds, so it is `is_file` that + raises, and it is a file-level refusal naming each: the same family, a + different frame.""" + prepared(project) + + stderr = refused_while_unopenable( + project, scaffolded, advanced, "bump-leg-workflow-unreadable", + ".github/workflows", mode=stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH) + + assert ".github/workflows/legs.yml: cannot be read: Permission denied" \ + in stderr + assert "chmod u+rx" in stderr + + +# --- the pin ----------------------------------------------------------------- + +@needs_a_mode +@MODES +def test_a_leg_pin_that_cannot_be_opened_is_refused_by_name( + project, scaffolded, advanced, extra): + """THE ISSUE'S ROW (c). The pin is read OUTSIDE `load_yaml`, so the + refusal #289 gave a pin that is not UTF-8 has to be given one this user + cannot open, under the same id. It names `contracts/spec-pin.yaml`.""" + prepared(project) + + stderr = refused_while_unopenable( + project, scaffolded, advanced, "bump-leg-pin-unreadable", + "contracts/spec-pin.yaml", extra=extra) + + assert ("contracts/spec-pin.yaml: cannot be read: Permission denied" + in stderr) + assert "chmod u+r" in stderr + refused_again_fine(project, scaffolded, advanced, extra, + "contracts/spec-pin.yaml") + + +@needs_a_mode +def test_a_contracts_directory_that_cannot_be_searched_names_the_pin( + project, scaffolded, advanced): + """`contracts/` mode 000: `is_file` on the pin is refused before the read + is reached. That is not an absent pin (`bump-leg-pin-missing`, whose + remedy is to restore it from history): the file is there and cannot be + reached, so it is the pin refusal, with the permission remedy.""" + prepared(project) + + stderr = refused_while_unopenable( + project, scaffolded, advanced, "bump-leg-pin-unreadable", "contracts") + + assert ("contracts/spec-pin.yaml: cannot be read: Permission denied" + in stderr) + assert "bump-leg-pin-missing" not in stderr + + +# --- an OSError that is not a permission (on every platform, root too) ------- + +def refusal_of(module, call, *args): + with pytest.raises(module.Refusal) as refused: + call(*args) + return refused.value + + +@pytest.mark.parametrize("failure", [ + FileNotFoundError(errno.ENOENT, "No such file or directory"), + IsADirectoryError(errno.EISDIR, "Is a directory"), + OSError(errno.EIO, "Input/output error"), +], ids=["vanished", "now-a-directory", "io-error"]) +def test_a_workflow_that_fails_for_another_reason_is_refused_sans_chmod( + bump_leg_module, project, advanced, monkeypatch, failure): + """THE `OSError` FAMILY, NOT `PermissionError` ALONE (the issue's second + point): a file that vanishes between the listing and the read, one that + stopped being a file, an I/O error. The same refusal, the system's own + reason on the line, and a remedy that does not tell the operator to + `chmod` what is not a permission. Called in-process with the read + replaced, so it runs on every platform and as root.""" + reference_both_legs(project) + real = Path.read_text + + def read_text(self, *args, **kwargs): + if self.name == "legs.yml": + raise type(failure)(failure.errno, failure.strerror, str(self)) + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", read_text) + refusal = refusal_of(bump_leg_module, bump_leg_module.plan_workflows, + project, f"{ORG}/{PROJECT}-spec", advanced) + + assert refusal.code == "bump-leg-workflow-unreadable" + assert (f".github/workflows/legs.yml: cannot be read: {failure.strerror}" + in refusal.detail) + assert "chmod" not in refusal.remediation + assert "Where the operating system gave another reason" in ( + refusal.remediation) + assert "(untracked or ignored)" not in refusal.detail, "it is tracked" + + +def test_a_pin_that_fails_for_another_reason_is_refused_by_name( + bump_leg_module, project, monkeypatch): + """The pin's own `OSError` that is no permission: the same id, the + system's reason, no `chmod`.""" + real = Path.read_text + + def read_text(self, *args, **kwargs): + if self.name == "spec-pin.yaml": + raise OSError(errno.EIO, "Input/output error", str(self)) + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", read_text) + refusal = refusal_of(bump_leg_module, bump_leg_module._leg_pin_text, + project, project / "contracts" / "spec-pin.yaml") + + assert refusal.code == "bump-leg-pin-unreadable" + assert refusal.detail == ("contracts/spec-pin.yaml: cannot be read: " + "Input/output error") + assert "chmod" not in refusal.remediation + assert "UTF-8" not in refusal.remediation + + +def test_a_listing_that_fails_for_another_reason_names_the_directory( + bump_leg_module, project, advanced, monkeypatch): + """`iterdir` failing with an I/O error: the directory refusal, with the + system's reason and no `chmod`. Both listings go through one helper, so + the first one `plan_workflows` makes is the one that is refused.""" + reference_both_legs(project) + real = Path.iterdir + + def iterdir(self): + if self.name == "workflows": + raise OSError(errno.EIO, "Input/output error", str(self)) + return real(self) + + monkeypatch.setattr(Path, "iterdir", iterdir) + refusal = refusal_of(bump_leg_module, bump_leg_module.plan_workflows, + project, f"{ORG}/{PROJECT}-spec", advanced) + + assert refusal.code == "bump-leg-workflow-unreadable" + assert refusal.detail == (".github/workflows/: cannot be listed: " + "Input/output error") + assert "chmod" not in refusal.remediation + + +def test_a_respelled_unopenable_workflow_is_refused_as_the_disk_spells_it( + bump_leg_module, project, advanced, monkeypatch): + """THE #344 COMPOSITION, through #344's own seam, so it runs on every + disk. A tracked `legs.yml` renamed outside git to `Legs.yml` that cannot + be opened. Every file is read before the loop asks `_tracked_spelling` + anything, so it is refused under the spelling the DISK gives it, and its + line is not marked untracked: it is the tracked file, under git's + spelling, which is what the question is asked.""" + reference_both_legs(project) + workflows = project / WORKFLOWS + respelled(workflows, "legs.yml", "Legs.yml") + monkeypatch.setattr(bump_leg_module, "_same_file", ignoring_case, + raising=False) + real = Path.read_text + + def read_text(self, *args, **kwargs): + if self.name == "Legs.yml": + raise PermissionError(errno.EACCES, "Permission denied", + str(self)) + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", read_text) + refusal = refusal_of(bump_leg_module, bump_leg_module.plan_workflows, + project, f"{ORG}/{PROJECT}-spec", advanced) + + assert refusal.code == "bump-leg-workflow-unreadable" + assert (".github/workflows/Legs.yml: cannot be read: Permission denied" + in refusal.detail) + assert ".github/workflows/legs.yml" not in refusal.detail + assert "(untracked or ignored)" not in refusal.detail + assert "chmod u+r" in refusal.remediation From e2c78ee0bc8620c3c339ca55ea989fb078e78bff Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:01:38 +0000 Subject: [PATCH 12/14] bump-leg.py: the refusal for a file that could not be opened says no commit is needed for it, so a mixed refusal that also says "Commit each file git tracks" for a file that is not UTF-8 no longer reads as telling the operator to commit the unreadable one too (#383) Reading the full text of a refusal that holds both groups, "Nothing is committed for such a file" followed the not-UTF-8 group's "Commit each file git tracks." and could be read either way. The sentence now says what is true of a file that could not be opened: made readable, it has no change, so no commit is needed. No behaviour changes; the three bump-leg test files pass (68). Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- scripts/bump-leg.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 28965662..8bbd4444 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -528,8 +528,8 @@ def _workflow_texts(root: Path, workflows: Path, tracked: set[str], "workflow file on disk too, whether or not it names this " "leg, so skipping one would only move the failure there. " + _cannot_open_advice([error for _, _, error in unopened]) - + "Nothing is committed for such a file, because one made " - "readable has no change to commit. ") + + "No commit is needed for a file that could not be opened: " + "made readable, it has no change to commit. ") if not all(held for _, held, _ in unopened): advice += ("A file marked (untracked or ignored) is not in " "the repository: it can also be moved out of " From 34162837790e34067a29dec7c0c4f39ee9b6768a Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:45:48 +0000 Subject: [PATCH 13/14] bump-leg.py: the test of the other leg's pin looks for the path in the spelling the validator prints, so tests-windows passes where the validator names `contracts\code-pin.yaml` (#289) `test_the_other_legs_pin_stays_the_validators_refusal` asserted the POSIX text `contracts/code-pin.yaml`. That file is named by the project's own `validate-pins.py`, whose `repo_shape.load_yaml` prints the Path it was given, absolute and in the platform's own spelling, and bump-leg echoes the validator's output under `bump-leg-validators-red`. On the Windows job it reads `D:\a\_temp\t\...\Atlas\contracts\code-pin.yaml`, so the assertion failed there (run 38266538839: 1 failed, 3635 passed). The behaviour was right: the validator refuses `yaml-unreadable`, the bump refuses `bump-leg-validators-red`, and every byte is rolled back. The test, not the tool, changes. This tool's own refusals name a file by its repository path, `as_posix()` (`_not_utf8`, `pin_rel`), and the rest of the module asserts that text; `load_yaml`'s native spelling is pinned by other tests as `f"REFUSED yaml-unreadable: {path}"`, and moving it would change a pinned template copy in every project. The assertion now looks for `str(Path("contracts") / "code-pin.yaml")`: the same text as before on POSIX, and `contracts\code-pin.yaml`, a substring of the validator's path, on Windows. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_bump_leg_unreadable.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_bump_leg_unreadable.py b/tests/test_bump_leg_unreadable.py index 0189af33..22773bb2 100644 --- a/tests/test_bump_leg_unreadable.py +++ b/tests/test_bump_leg_unreadable.py @@ -355,9 +355,15 @@ def test_the_other_legs_pin_stays_the_validators_refusal(project, scaffolded, commit_bytes(project, Path("contracts") / "code-pin.yaml", LATIN1_COMMENT, append=True) + # The file is named by the VALIDATOR, whose `load_yaml` prints the Path it + # was given, absolute and in the platform's own spelling: + # `...\Atlas\contracts\code-pin.yaml` on Windows. This tool's own refusals + # name a file by its repository path, `as_posix()`, and are asserted as + # such everywhere else in this module; this one is not its text. + pin = str(Path("contracts") / "code-pin.yaml") stderr = refused_and_untouched( project, scaffolded, advanced, "bump-leg-validators-red", - "yaml-unreadable", "contracts/code-pin.yaml") + "yaml-unreadable", pin) assert "bump-leg-pin-unreadable" not in stderr assert "bump-leg-workflow-unreadable" not in stderr From fab496fb11cedf41e01bbaee06e6b1cb262b0d7d Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:49:18 +0000 Subject: [PATCH 14/14] bump-leg.py: the leg's pin, the workflows directory and each workflow file are stat'ed directly, so on Python 3.14, where Path.is_file() and is_dir() answer False to any OSError, a refused stat is still refused by name and not read as a missing pin or a skipped file (#383) CI's Linux job at e2c78ee (Python 3.14.8) failed two of the new mode tests that pass on 3.12 here. Up to Python 3.13, pathlib's is_file() and is_dir() raised a refused stat (PermissionError) and answered False only for ENOENT, ENOTDIR, EBADF and ELOOP; from 3.14 they call os.path.isfile and isdir, which answer False to ANY OSError. The guards #383 added sat around is_file() and is_dir() and counted on that raise, so on 3.14: - a contracts/ this user cannot search read as an absent pin, `bump-leg-pin-missing`, whose remedy restores a file that is there; - every file of a .github/workflows/ this user can list but not search was skipped as no file, the bump went ahead without it, and validate-pins.py rolled it back as `bump-leg-validators-red`; - a .github/ this user cannot search read as no workflows directory, and `--dry-run` exited 0 saying no reference names the leg. `_is_a(path, kind)` asks `path.stat()` and answers False for exactly the errors pathlib ignored up to 3.13, raising every other one, so the three guards refuse by name with the permission remedy on every Python. It is the approach validate-pins.py already takes for its source directory. Tests: a real-mode `.github/` mode 000 test (bump and --dry-run), and in-process tests that refuse `Path.stat` for one name and make is_file, is_dir and exists answer as 3.14's do, so a tool leaning on the raise fails on every Python and platform and as root; plus the absent pin (ENOENT, ENOTDIR) staying `bump-leg-pin-missing`. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- scripts/bump-leg.py | 50 +++++++++- tests/test_bump_leg_unopenable.py | 149 +++++++++++++++++++++++++++++- 2 files changed, 191 insertions(+), 8 deletions(-) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 8bbd4444..191464ec 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -93,12 +93,15 @@ from __future__ import annotations import argparse +import errno import os import re +import stat import subprocess import sys from functools import partial from pathlib import Path +from typing import Callable SHAPE_ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(SHAPE_ROOT / "scripts")) @@ -296,6 +299,44 @@ def _not_utf8(root: Path, path: Path, exc: UnicodeDecodeError) -> str: f"{exc.start} ({exc.reason})") +#: The `OSError`s that `Path.is_file()` and `Path.is_dir()` answered False to +#: on Python 3.9 to 3.13 (pathlib's `_ignore_error`), because each means there +#: is nothing at the name to open: no such file, a part of the path that is +#: not a directory, a bad descriptor, a symlink loop, and Windows' "not +#: ready", "invalid name" and "cannot resolve". Every other one was raised. +_ABSENT_ERRNOS = (errno.ENOENT, errno.ENOTDIR, errno.EBADF, errno.ELOOP) +_ABSENT_WINERRORS = (21, 123, 1921) + + +def _is_a(path: Path, kind: Callable[[int], bool]) -> bool: + """Whether `path` is there and of `kind` (`stat.S_ISREG`, `stat.S_ISDIR`), + raising the `OSError` of a `stat` the operating system REFUSED (#383). + + NOT `Path.is_file()` OR `Path.is_dir()`, whose answer depends on the + Python. Up to 3.13 they raised a refused `stat` (`PermissionError`) and + answered False only when nothing was there; from 3.14 they call + `os.path.isfile` and `isdir`, which answer False to ANY `OSError`. On 3.14 + the pin inside a `contracts/` this user cannot search was "missing" + (`bump-leg-pin-missing`, whose remedy restores a file that is there), and + every file in a `.github/workflows/` this user can list but not search + was skipped as no file at all: the bump went ahead, `validate-pins.py` + found the workflow "pinned but absent", and the run was rolled back as + `bump-leg-validators-red`. Asked here, every Python answers as 3.9 to + 3.13 did, and a refused `stat` is a refusal that names the path. + """ + try: + mode = path.stat().st_mode + except OSError as exc: + if (exc.errno in _ABSENT_ERRNOS + or getattr(exc, "winerror", None) in _ABSENT_WINERRORS): + return False + raise + except ValueError: + # a name the operating system cannot take at all (a NUL in it) + return False + return kind(mode) + + def _os_reason(exc: OSError) -> str: """The operating system's own words for `exc` (`Permission denied`), or what Python made of it when there are none (an `OSError` raised with no @@ -465,7 +506,7 @@ def _workflow_texts(root: Path, workflows: Path, tracked: set[str], (#383). A file this user may not open raised `PermissionError`, an `OSError` that `main()` does not catch either: the same traceback, exit 1, `--dry-run` included. Any `OSError` of the read or of the `stat` before - it (`is_file`, which a directory this user can list but not search + it (`_is_a`, which a directory this user can list but not search refuses) is named the same way, as a line of its own with the system's reason, whether or not the file names the leg; a file that is not UTF-8 and one that cannot be opened meet in ONE refusal, each group with its own @@ -480,7 +521,8 @@ def _workflow_texts(root: Path, workflows: Path, tracked: set[str], unopened: list[tuple[str, bool, OSError]] = [] for path in _workflow_children(root, workflows): try: - if path.suffix not in (".yml", ".yaml") or not path.is_file(): + if (path.suffix not in (".yml", ".yaml") + or not _is_a(path, stat.S_ISREG)): continue texts.append((path, path.read_text(encoding="utf-8"))) except UnicodeDecodeError as exc: @@ -641,7 +683,7 @@ def plan_workflows(root: Path, repository: str, plans: list[tuple[Path, str, int]] = [] workflows = root / ".github" / "workflows" try: - if not workflows.is_dir(): + if not _is_a(workflows, stat.S_ISDIR): return plans except OSError as exc: # a directory above `workflows` this user cannot search (#383) @@ -1008,7 +1050,7 @@ def _leg_pin_path(root: Path, pin_rel: str) -> Path: """ pin_path = root / pin_rel try: - present = pin_path.is_file() + present = _is_a(pin_path, stat.S_ISREG) except OSError as exc: # `contracts/` this user cannot search: the `stat` is refused, which # is not an absent file (#383) diff --git a/tests/test_bump_leg_unopenable.py b/tests/test_bump_leg_unopenable.py index c3f42410..eb49862f 100644 --- a/tests/test_bump_leg_unopenable.py +++ b/tests/test_bump_leg_unopenable.py @@ -34,6 +34,14 @@ reason (a file gone between the listing and the read, an I/O error) do it with a stand-in for the call, so they run everywhere, root included. +A REFUSED `stat` IS ASKED OF THE OPERATING SYSTEM, NOT OF `Path.is_file()`. +Up to Python 3.13 `is_file` and `is_dir` raised a refused `stat`; from 3.14 +they answer False to ANY `OSError`, and CI's 3.14 turned two mode tests here +red at e2c78ee (an unsearchable `contracts/` read as a missing pin, and the +files of a workflows directory listable but not searchable were skipped). The +last section stands in for 3.14's answer with a refused `stat`, in-process, so +a tool that leans on `is_file` raising fails on every Python, root included. + EVERY MODE-000 TEST COMPARES THE WHOLE TREE, as `tests/test_bump_leg_rollback.py` does, from a snapshot taken BEFORE the mode is changed (the snapshot reads the files): the root's HEAD, the pin's and @@ -364,9 +372,12 @@ def test_a_workflows_directory_that_cannot_be_listed_is_refused_naming_it( def test_a_workflows_directory_listable_but_not_searchable_is_refused( project, scaffolded, advanced): """A DIRECTORY WITH `r` AND NO `x`: its names list, and every `stat` of a - name in it is refused. The listing succeeds, so it is `is_file` that + name in it is refused. The listing succeeds, so it is the `stat` that raises, and it is a file-level refusal naming each: the same family, a - different frame.""" + different frame. Asked through `Path.is_file()`, Python 3.14 answers "no + file" instead, every file is skipped, the bump goes ahead and + `validate-pins.py` rolls it back as `bump-leg-validators-red` (CI at + e2c78ee).""" prepared(project) stderr = refused_while_unopenable( @@ -378,6 +389,29 @@ def test_a_workflows_directory_listable_but_not_searchable_is_refused( assert "chmod u+rx" in stderr +@needs_a_mode +@MODES +def test_a_github_directory_that_cannot_be_searched_names_the_workflows( + project, scaffolded, advanced, extra): + """`.github/` mode 000, the directory ABOVE the workflows: the `stat` of + `.github/workflows` is refused before any listing, and it is the + directory refusal, whose `chmod u+rx` advice names every directory above + it inside the root. Asked through `Path.is_dir()`, Python 3.14 answers + "no directory", and the bump went on as if the project had no workflow + to move.""" + prepared(project) + + stderr = refused_while_unopenable( + project, scaffolded, advanced, "bump-leg-workflow-unreadable", + ".github", extra=extra) + + assert (".github/workflows/: cannot be listed: Permission denied" + in stderr) + assert "chmod u+rx" in stderr and "every directory above it" in stderr + refused_again_fine(project, scaffolded, advanced, extra, + ".github/workflows/legs.yml") + + # --- the pin ----------------------------------------------------------------- @needs_a_mode @@ -403,10 +437,12 @@ def test_a_leg_pin_that_cannot_be_opened_is_refused_by_name( @needs_a_mode def test_a_contracts_directory_that_cannot_be_searched_names_the_pin( project, scaffolded, advanced): - """`contracts/` mode 000: `is_file` on the pin is refused before the read + """`contracts/` mode 000: the `stat` of the pin is refused before the read is reached. That is not an absent pin (`bump-leg-pin-missing`, whose remedy is to restore it from history): the file is there and cannot be - reached, so it is the pin refusal, with the permission remedy.""" + reached, so it is the pin refusal, with the permission remedy. Python + 3.14's `Path.is_file()` answers "no file" here, which is how CI at e2c78ee + got `bump-leg-pin-missing`.""" prepared(project) stderr = refused_while_unopenable( @@ -535,3 +571,108 @@ def read_text(self, *args, **kwargs): assert ".github/workflows/legs.yml" not in refusal.detail assert "(untracked or ignored)" not in refusal.detail assert "chmod u+r" in refusal.remediation + + +# --- a refused `stat`, as Python 3.14 answers it (everywhere, root too) ----- + +def refused_stat(monkeypatch, *names: str) -> None: + """`Path.stat` refused (`Permission denied`) for each of `names`, as it is + for a name inside a directory this user may not search, and + `Path.is_file`, `is_dir` and `exists` answering as Python 3.14's do: + False to ANY `OSError` (they call `os.path.isfile` and the like), where + 3.9 to 3.13 raised a refused `stat`. + + CI's 3.14 is where the mode tests above first met that answer (e2c78ee): + `is_file` said "no file", and the tool took an unsearchable `contracts/` + for a missing pin and skipped every file of a workflows directory it could + list but not search. In-process, so this runs on every Python and every + platform and as root, and on a Python before 3.14 it still fails a tool + that asks `is_file` or `is_dir` and counts on the raise.""" + real = Path.stat + + def refusing(self, *args, **kwargs): + if self.name in names: + raise PermissionError(errno.EACCES, "Permission denied", str(self)) + return real(self, *args, **kwargs) + + def as_python_314(kind): + def answer(self, *args, **kwargs): + try: + return kind(self.stat().st_mode) + except (OSError, ValueError): + return False + return answer + + monkeypatch.setattr(Path, "stat", refusing) + monkeypatch.setattr(Path, "is_file", as_python_314(stat.S_ISREG)) + monkeypatch.setattr(Path, "is_dir", as_python_314(stat.S_ISDIR)) + monkeypatch.setattr(Path, "exists", as_python_314(lambda mode: True)) + + +def test_a_workflow_whose_stat_is_refused_is_named_not_skipped( + bump_leg_module, project, advanced, monkeypatch): + """A WORKFLOWS DIRECTORY LISTABLE BUT NOT SEARCHABLE, on every Python. The + listing names `legs.yml` and its `stat` is refused: the file refusal names + it, with the permission remedy. A file skipped as "not a file" is a bump + that goes ahead without it and is rolled back by `validate-pins.py`.""" + reference_both_legs(project) + refused_stat(monkeypatch, "legs.yml") + + refusal = refusal_of(bump_leg_module, bump_leg_module.plan_workflows, + project, f"{ORG}/{PROJECT}-spec", advanced) + + assert refusal.code == "bump-leg-workflow-unreadable" + assert (".github/workflows/legs.yml: cannot be read: Permission denied" + in refusal.detail) + assert "1 workflow file(s) cannot be opened" in refusal.detail + assert "chmod u+r" in refusal.remediation + + +def test_a_workflows_directory_whose_stat_is_refused_is_named( + bump_leg_module, project, advanced, monkeypatch): + """A `.github/` this user may not search, on every Python: the `stat` of + `.github/workflows` is refused, and it is the directory refusal. A + directory taken for "not there" is a bump that moves no workflow.""" + reference_both_legs(project) + refused_stat(monkeypatch, "workflows") + + refusal = refusal_of(bump_leg_module, bump_leg_module.plan_workflows, + project, f"{ORG}/{PROJECT}-spec", advanced) + + assert refusal.code == "bump-leg-workflow-unreadable" + assert refusal.detail == (".github/workflows/: cannot be listed: " + "Permission denied") + assert "chmod u+rx" in refusal.remediation + + +def test_a_pin_whose_stat_is_refused_is_not_called_missing( + bump_leg_module, project, monkeypatch): + """A `contracts/` this user may not search, on every Python: the pin is + there and cannot be reached, which is `bump-leg-pin-unreadable` with the + permission remedy, never `bump-leg-pin-missing`, whose remedy restores a + file nobody lost.""" + refused_stat(monkeypatch, "spec-pin.yaml") + + refusal = refusal_of(bump_leg_module, bump_leg_module._leg_pin_path, + project, "contracts/spec-pin.yaml") + + assert refusal.code == "bump-leg-pin-unreadable" + assert refusal.detail == ("contracts/spec-pin.yaml: cannot be read: " + "Permission denied") + assert "chmod u+r" in refusal.remediation + + +@pytest.mark.parametrize("missing", [ + "contracts/nowhere-pin.yaml", + "contracts/spec-pin.yaml/below", +], ids=["no-such-file", "a-file-on-the-path"]) +def test_a_pin_that_is_not_there_is_still_missing( + bump_leg_module, project, missing): + """THE OTHER HALF OF THE ANSWER: nothing at the name (`ENOENT`), or a part + of the path that is a file (`ENOTDIR`), is the absent pin it always was, + and not a file this user cannot open.""" + refusal = refusal_of(bump_leg_module, bump_leg_module._leg_pin_path, + project, missing) + + assert refusal.code == "bump-leg-pin-missing" + assert refusal.detail.startswith(f"{missing} does not exist")