Skip to content

bump-leg.py: a workflow file, the workflows directory or the leg's pin that this user cannot open is refused by name with a remedy, so the run ends in exit 2 and not a PermissionError traceback (#383) - #403

Merged
brettheap merged 20 commits into
mainfrom
fix/bump-leg-unreadable-oserror
Oct 12, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openreposhape-2 (openRepoShape-2)
Claim: #383 (comment)
#352 landed as 53bee5b; this branch merged its final head (dfc998a) and main, and now reads as a change on main alone.

What

bump-leg.py read the leg's pin, listed .github/workflows/ and read every .yml/.yaml file in it with no handler, and main() catches Refusal and CommandFailed only. A file or directory this user cannot open raised PermissionError, and the run ended in a traceback, exit 1, --dry-run included. It is now a refusal, exit 2, before anything moves, under the two ids #352 introduced (now on main):

  • bump-leg-workflow-unreadable, for a workflow file (<path>: cannot be read: Permission denied) and for the directory (.github/workflows/: cannot be listed: Permission denied);
  • bump-leg-pin-unreadable, for the leg's pin.

The OSError clause sits beside #352's UnicodeDecodeError in _leg_pin_text and _workflow_texts. Both iterdir() calls go through one guarded helper, _workflow_children. The three existence checks (the workflows directory in plan_workflows, each workflow file, and the pin in _leg_pin_path) ask _is_a, which calls stat() itself, so a refused stat is refused by name on every Python (see "CI fix").

Why

Reproduced on the base this branch was built on (797c96b, #352's head then) with local bare remotes and a stub gh. Every unreadable row of the issue's table exits 1 with a PermissionError traceback, for the bump and for --dry-run (tools/repro-base.log). On this head each row exits 2 with the named refusal, the tree clean and the root's and leg's HEAD unmoved (tools/repro-head.log).

Git reads these trees as clean whenever it does not re-read the file: with core.trustctime=false, with --assume-unchanged, or for a directory it cannot list, which git status reports on stderr only. So _ensure_root_clean lets the run through to the read.

CI fix (5bc6b47)

At e2c78ee, CI's Linux and macOS jobs each failed two of the new mode tests: test_a_workflows_directory_listable_but_not_searchable_is_refused was refused bump-leg-validators-red, and test_a_contracts_directory_that_cannot_be_searched_names_the_pin was refused bump-leg-pin-missing. Both pass on Python 3.12 locally.

The cause is the Python, not the runner. CI's python-version: '3.x' is 3.14 on all three OSes.

  • Up to 3.13, pathlib's Path.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.

A probe on 3.10.21, 3.12.3, 3.13.15 and 3.14.7 asked about a file in a mode-r-- directory and one in a mode-000 directory. is_file/exists/is_dir raise PermissionError (errno 13) on 3.10 to 3.13 and answer False on 3.14. stat() raises on all four.

The guards were written around is_file()/is_dir() and counted on that raise, so on 3.14:

  • the pin inside a contracts/ this user cannot search was "missing";
  • every file of 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 validate.yml "pinned but absent", and the run was rolled back;
  • a .github/ this user cannot search read as no workflows directory, and --dry-run exited 0 saying no @<sha> reference names the leg.

Reproduced on 3.14.7 locally: at e2c78ee, tests/test_bump_leg_unopenable.py gives 2 failed and 23 passed, the same two tests and refusals as CI (tools/x403-py314-e2c78ee-unopenable.log).

The fix (fab496f). _is_a(path, kind) asks path.stat(). It answers False for exactly the errors pathlib ignored up to 3.13 (the four errnos, and Windows' 21, 123 and 1921), and raises every other one. The three guards now use it, so they give the same answer on every Python. validate-pins.py already handles its source directory the same way.

New tests (7) in tests/test_bump_leg_unopenable.py:

  • a .github/ made mode 000, run as a bump and as a --dry-run. It is refused as the directory, with chmod u+rx "and on every directory above it";
  • in-process tests that refuse Path.stat for one name and make is_file, is_dir and exists answer as 3.14's do: one for a workflow file, one for the workflows directory and one for the pin. They run on every Python and platform and as root, and on 3.12 they still fail a tool that leans on the raise;
  • an absent pin (ENOENT) and a file on the pin's path (ENOTDIR), which stay bump-leg-pin-missing.

Proof.

  • The new tests against e2c78ee's code fail 3 on 3.12 (the stand-ins) and 7 on 3.14 (tools/x403-py312-newtests-on-e2c78ee.log, tools/x403-py314-newtests-on-e2c78ee.log).
  • Four mutants of _is_a are each killed (tools/x403-mutants.log):
    • every OSError absent: 7 tests fail;
    • ENOTDIR not absent: 1 fails;
    • the pin guard back on is_file: 1 fails;
    • the directory guard back on is_dir: 1 fails.
  • The last two pass every real-mode test on 3.12 and are caught only by the stand-in.

Merged #352's 3416283 (5bc6b47). It is the Windows fix for #352's own test_the_other_legs_pin_stays_the_validators_refusal, which was tests-windows' only failure at e2c78ee. The merge was clean.

#352 landed: merged its final head and main (5b1da66)

#352 landed as the squash 53bee5b, and GitHub retargeted this PR to main and read it CONFLICTING: the branch held #352's original commits, and main held only their squash.

Tests

tests/test_bump_leg_unopenable.py has 32 tests. They cover:

Each mode test:

  • makes git read the tree as clean (--assume-unchanged, cleared on restore);
  • asserts git status --porcelain is empty while the file is unreadable;
  • compares the whole tree afterwards;
  • runs the same command again with the mode restored.

Mode tests skip on Windows and as root; the mode is restored in a finally.

Composes with

git merge-tree --write-tree of 5b1da66 is rc 0, with no conflict, against:

Four are rc 1, each in a file this change does not touch, and each conflicts the same way with main 53bee5b alone: #358 (2745890) and #402 (36321c4) in shape-doctor.py, #394 (563ea16) in tests/test_manifest_validator.py, and #435 (3efcc7a) in adopt-project.py.

Left out

Closes #383

🤖 Generated with Claude Code

Brett Heap and others added 14 commits October 11, 2026 22:20
…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 <noreply@anthropic.com>
…m, 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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
…e 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 <leg> 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 <noreply@anthropic.com>
…r 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 <noreply@anthropic.com>
…d 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/<role>-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/<role>-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 <noreply@anthropic.com>
… 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 <noreply@anthropic.com>
…le-files

The one conflicted file is scripts/bump-leg.py, in three hunks in
`plan_workflows`, all against #222's tracked-files change (#207): the helper
block keeps main's `_tracked_workflows` and this branch's `_not_utf8`,
`_leg_pin_text` and `_workflow_texts`; the docstring keeps both paragraphs;
the loop head reads `tracked`, `untracked`, then
`for path, text in _workflow_texts(root, workflows)`, main's own
`text = path.read_text(...)` line going with the walk it belonged to. The
net diff against main is scripts/bump-leg.py and
tests/test_bump_leg_unreadable.py, nothing else.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…344 landed in bump-leg.py

Lane 3's #344 landed on main as a2cb7d3 (closes #332) and changed
scripts/bump-leg.py in the same two places as this branch, so the merge
conflicted in that one file, in two add/add regions. No other file conflicted.
Both sides are kept in each, because the two changes add different things and
neither replaces the other:

1. The helpers before `plan_workflows`. This branch adds `_not_utf8`,
   `_leg_pin_text` and `_workflow_texts` (#289); main adds `_same_file` and
   `_tracked_spelling` (#332). All five stay: ours first, then theirs.

2. The last paragraph of `plan_workflows`'s docstring. Main's "A TRACKED FILE
   THE DISK SPELLS ANOTHER WAY IS STILL TRACKED (#332)" paragraph stays whole,
   and this branch's one sentence on a file that is not UTF-8 follows it.

Git merged the body of `plan_workflows` with no conflict, and the composition
is the one both changes need: the loop reads
`for path, text in _workflow_texts(root, workflows)`, so every workflow is read
as UTF-8 before anything is written and a file that is not UTF-8 is refused by
name whatever its spelling on disk; each row is then spelled through
`_tracked_spelling(root, path, tracked, listed)`, so a tracked file respelled
by case is still planned under git's spelling.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ine 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 <noreply@anthropic.com>
…ow 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 <noreply@anthropic.com>
…n 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 `<path relative to the root>: cannot be read: <the operating
  system's reason>` (`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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 2 days and 2 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR hardens bump-leg.py against unreadable or otherwise failing filesystem paths by routing all relevant OSError cases through named, pre-write refusals with accurate diagnostics and remediation, while documenting the behavior and adding broad regression coverage for normal, dry-run, mixed-cause, tracking, and cross-platform scenarios.

Sequence diagram for filesystem refusal during a leg bump

sequenceDiagram
    participant User
    participant BumpLeg as bump-leg.py
    participant FS as Filesystem
    participant Main as main()

    User->>BumpLeg: bump command or --dry-run
    BumpLeg->>FS: Read pin or list/read workflows
    alt OSError on pin
        FS-->>BumpLeg: OSError
        BumpLeg-->>Main: Refusal bump-leg-pin-unreadable
    else OSError on workflow directory or file
        FS-->>BumpLeg: OSError
        BumpLeg-->>Main: Refusal bump-leg-workflow-unreadable
    end
    Main-->>User: Named path, system reason, remediation, exit 2
    Note over BumpLeg,Main: No working-tree, index, or checkout writes
Loading

Flow diagram for unreadable path refusal in bump-leg.py

flowchart TD
    A[bump-leg.py starts] --> B[Check leg pin and workflow paths]
    B --> C{Filesystem operation succeeds?}
    C -->|Yes| D[Read UTF-8 content and plan workflows]
    C -->|No| E{Path involved}
    E -->|Leg pin| F[bump-leg-pin-unreadable]
    E -->|Workflow directory| G[bump-leg-workflow-unreadable]
    E -->|Workflow file| G
    F --> H[Print path and operating-system reason]
    G --> H
    H --> I[Print cause-specific remediation]
    I --> J[Exit 2 before writes]
    D --> K[Continue bump or dry-run]
Loading

Flow diagram for mixed workflow readability causes

flowchart TD
    A[_workflow_texts] --> B[_workflow_children]
    B -->|OSError| C[Refuse directory as cannot be listed]
    B -->|Children returned| D[Inspect every .yml and .yaml]
    D --> E{Read/stat succeeds?}
    E -->|No UTF-8| F[Collect non-UTF-8 diagnostic]
    E -->|OSError| G[Collect cannot-open diagnostic]
    E -->|Yes| H[Collect workflow text]
    F --> I{Any refusals collected?}
    G --> I
    H --> I
    I -->|Yes| J[One workflow refusal with separate cause groups and advice]
    I -->|No| K[Return texts for planning]
Loading

File-Level Changes

Change Details Files
Convert filesystem read, listing, and reachability failures into named refusals with cause-specific remediation instead of uncaught tracebacks.
  • Catch the full OSError family for leg-pin reads, workflow reads, directory listings, and is_file checks.
  • Add workflow-file, workflow-directory, and leg-pin refusal details using repository-relative paths and OS-provided reasons.
  • Provide chmod/ownership guidance for permission failures and restoration/I/O guidance for other OSErrors, while preserving UTF-8 refusal advice.
  • Keep refusals before writes and apply the same behavior to dry runs and files that do not name the leg.
scripts/bump-leg.py
Combine UTF-8 and operating-system read failures into one workflow refusal while preserving per-cause grouping and tracking semantics.
  • Report non-UTF-8 and unopened workflow files under separate headers in a single refusal.
  • Retain tracked versus untracked-or-ignored annotations and avoid instructing operators to commit unreadable files.
  • Handle multiple unreadable files and mixed failure causes in one run.
scripts/bump-leg.py
Document the new unreadable-file and directory refusal behavior for users and AI assistants.
  • Describe the new refusal scope, identifiers, and readable-file remediation in the handbook.
  • Update the bump-leg operational contract and refusal guarantees.
docs/handbook.html
scripts/bump-leg.py
Add comprehensive regression coverage for permission and general OSError failures across bump and dry-run flows.
  • Cover unreadable workflow files, workflow directories, leg pins, untracked/ignored files, mixed UTF-8 failures, and case-respelled paths.
  • Verify exit 2, refusal identifiers, diagnostics, cause-specific advice, clean trees, unchanged state, and successful reruns after remediation.
  • Use platform-independent fault injection for vanished files, type changes, I/O errors, and listing failures.
tests/test_bump_leg_unopenable.py

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

brettheap and others added 3 commits October 12, 2026 16:45
…e 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 <noreply@anthropic.com>
… 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 <noreply@anthropic.com>
…mp-leg-unreadable-oserror, so this stacked branch carries #352's Windows test fix

#352 moved from 797c96b to 3416283, one test-only commit: the test of
the other leg's pin in tests/test_bump_leg_unreadable.py looks for the
path in the spelling the validator prints, which is
`contracts\code-pin.yaml` on Windows. That inherited test was the only
tests-windows failure at e2c78ee. No file of this branch's own change is
touched, so the merge is clean.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-12T17:13:38.712098Z 5bc6b47 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 5bc6b47eb6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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

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

This was referenced Oct 12, 2026
@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-2, 2026-10-12T17:33Z)

This lane reviewed its own PR #403 at 5bc6b47 (the increment over #352's 3416283), read-only in its own detached worktrees. Nothing was authored.

MERGEABLE WITH NITS. Land #352 first and retarget this PR to main. Land it once tests-windows is green; tests and tests-macos already pass at 5bc6b47.

There is no P1 or P2. Every row of the issue's table is refused by name, exit 2, with the tree and both HEADs unmoved, on Python 3.12.3 and 3.14.7. The same holds for an r-- workflows directory, an unsearchable contracts/ or .github/, and a workflow symlinked into a mode-000 directory. _is_a's absent errnos and winerrors (scripts/bump-leg.py:307-308) equal pathlib's on 3.10 to 3.13, and it raises a refused stat on 3.10 through 3.15rc2. The 3.14 stand-in tests copy genericpath.

Findings:

  1. P3, follow-up issue. Three other existence guards still give a different answer on each Python, on main too. With scripts/ at mode r--, run_validator (:853) behaves like this:

    • on 3.12 it refuses bump-leg-rewrite-failed and rolls back;
    • on 3.14 it prints "absent; SKIPPED" for both validators and commits the bump.

    _initialized_submodule (:1009) and Project.open (:159) have the same kind of guard.

  2. Nit, tests. Nothing pins that ELOOP stays "absent" (:307). A mutant dropping it survives.

  3. Nit, wording. When the directory above refuses the stat, the file's line still says "cannot be read" (:354).

Tests:

  • Head, the unopenable and unreadable files: 51 passed on 3.12 and 51 passed on 3.14.
  • Head's new file on 3416283: 30 failed for the issue's reasons on each Python, and the 2 absent-pin tests passed.
  • Merged with main 92e29d9 (main is now f0a8bf6, family.py only, rc 0): 464 passed and 2 skipped on 3.12 (10 files); 236 passed and 1 skipped on 3.14 (5 files).
  • render-cli-reference --check is ok.
  • Mutants: 3 of 4 new ones are killed (15 of 16 with the implementer's).

Merges: git merge-tree is rc 0 against main and every open head. The exception is #358, rc 1 in shape-doctor.py, the same against main alone.

The report is in opensoft/brett-wip at handoffs/xFactory/attachments/lane-openRepoShape-2/review403-5bc6b47/final-report.md.

brettheap and others added 2 commits October 12, 2026 18:47
… so the branch holds the head that squash-landed as 53bee5b

#352 landed on main as the squash 53bee5b. This branch was stacked on #352
and still holds #352's original commits (up to 3416283), while main holds
only their squash, so GitHub reads the retargeted #403 as CONFLICTING.

dfc998a is #352's final head: 3416283 plus a merge of main 45e01f4. It is
the first of the two merges that clear this: the merge is clean, since the
branch already holds 3416283, and it brings in main 45e01f4's own changes
(scripts/bump-leg.py auto-merged with no conflict).

The second merge, of main 53bee5b, follows.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…or, keeping this branch's own bump-leg.py, docs/handbook.html and tests/test_bump_leg_unreadable.py

#352 landed as the squash 53bee5b. This branch holds #352's original
commits, merged up to its final head dfc998a in the previous commit, so
git saw the squash and those commits as two edits of the same lines and
conflicted in scripts/bump-leg.py and docs/handbook.html.

Both took this branch's side (`git checkout --ours`). That is safe because
`git diff dfc998a 53bee5b -- scripts/bump-leg.py docs/handbook.html
tests/test_bump_leg_unreadable.py` is empty: the squash's content of #352's
three files is dfc998a's, which this branch holds with #403's own change on
top. tests/test_bump_leg_unreadable.py merged with no conflict. No other
file conflicted. The three files are byte for byte what they were before
this merge.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@brettheap
brettheap merged commit dba5fb0 into main Oct 12, 2026
5 checks passed
@brettheap
brettheap deleted the fix/bump-leg-unreadable-oserror branch October 12, 2026 19:52
brettheap pushed a commit that referenced this pull request Oct 12, 2026
…landed: `_commit_the_split` keeps #445's reported steps and reads `follow_ups:` through `_plan_follow_ups`

Main moved from dd8d0ea to 580d7df: decff87 (#267), dba5fb0 (#403),
245d775 (#446), 75052cb (#445), ec8e09a (#406) and 580d7df (#416).
Only #445 touches adopt-project.py, and it conflicted in one hunk of
`_commit_the_split`.

#445 wraps that function's body in a `try` that names each step
(cloning the source for the split, mounting the legs, committing the
split, pushing the split branch), so a failure once both legs are pushed
flushes stdout, prints `REFUSED <step>.`, what failed and one notice of
what the run left. This branch reads `follow_ups:` there through
`_plan_follow_ups`, the one read of it (#293). The resolution keeps
#445's `try`, steps and reports whole, and carries this branch's read
into its "committing the split" step: git had kept main's
`[str(f) for f in plan.get("follow_ups", [])]` inside the `try`.

The two contracts meet without overlap. `plan-bad-follow-ups` and
`election-date-malformed` are findings of `_entry_findings`, which
`execute` refuses through `_refuse_what_check_finds`. That runs before
#445's `_refuse_an_occupied_work_dir`, `_refuse_a_split_branch_left_behind`
and `_create_leg_remotes`, so these are refusals before any leg exists:
exit 2, `REFUSED <code>:` and the remediation that says no leg
repository was created and nothing was pushed, as #445's own refusals
before the legs say, and no notice. #445's report covers the failures
once a leg exists, which these values no longer reach. The guard in
`_manifest_values` is unchanged; #445 does not touch it.

#445's comment over `_template_values` in `cmd_execute` named "an
`elected_on` that is not a date" as a refusal the table can raise. With
#294's finding, `_refuse_what_check_finds` refuses such a date first, so
the comment now says the date no longer reaches the table. #445's test
of a table refusal stubs `_template_values` and is unchanged.

tests/test_adopt_execute_refusals.py: the two end-to-end runs through
`main` (#293, #294) now also assert that the refusal says no leg was
created and that #445's notice ("NOTHING has been rolled back") does not
follow it.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 12, 2026
…eport of a failure after the legs carries the materializer's refusals, and its two early refusals follow this branch's `core.symlinks` probe

origin/main moved dd8d0ea -> 75052cb (four commits), merged by sha:
- decff87 (#237 via #267), shape-doctor.py and update-shape.py;
- dba5fb0 (#383 via #403), scripts/bump-leg.py and docs/handbook.html;
- 245d775 (#412 via #446), the template's scripts/validate-pins.py;
- 75052cb (#171, #188 and #177 via #445), adopt-project.py and AGENTS.md:
  every failure once a leg repository exists is reported at its step as
  `REFUSED <step>.`, what failed, and `_nothing_rolled_back`'s notice, exit
  2; `execute` refuses `adopt-work-dir-occupied` and `adopt-branch-exists`
  before it creates anything.

tests/test_adopt_ignored_shape_files.py and every other file merge with no
conflict. adopt-project.py conflicts in five hunks, each resolved by keeping
both sides:
1. The materializer's import list: this branch's names (BOOTSTRAP,
   SYMLINK_HOPS, TOO_MANY_SYMLINKS, template_files), without RULESET_HINT,
   which #188 replaced by its own ADOPT_RULESET_HINT and nothing here uses.
2-4. `Source`: this branch's `work_root`, where a climb above the root
   leads (#183), and #188's `cloned`, read by `_cloned_here`. Both are
   keyword-defaulted, so the four-argument constructions in the tests and
   #188's `cloned=True` read as before, and `open` hands both on.
5. `_commit_the_split`: main's steps inside #188's report, with this
   branch's `spell=_spelled`, and its comment, on the materializer call.

How #188 meets this branch's code:
- `execute`, all before `_create_leg_remotes`: `_refuse_what_check_finds`
  (with `plan-root-symlink`), this branch's `core.symlinks` probe
  (`adopt-symlinks-off`), `_refuse_unconsented_seeding`, then #188's
  `adopt-work-dir-occupied` and `adopt-branch-exists`, then the
  confirmation and `_template_values`. A plan's own faults come first, then
  whether `git` here can hold what the plan keeps, then what an earlier run
  left, whose way out is "run `execute` again". The probe's throwaway
  repository (`symlinks-probe-*` in the work directory) is removed before
  `_refuse_an_occupied_work_dir` reads the work directory, and it is none of
  the three directories that function asks about.
- The materializer's refusals (`materialize-outside`,
  `materialize-symlink-loop`, `materialize-blocked`) are raised inside the
  step "committing the split", which #188 wraps. They are `Refusal`s, so
  `_failed_loudly` prints `REFUSED committing the split. <code>: <detail>`
  on one line, still spelled by `_spelled`, then their remediation, then the
  notice that both legs are pushed; `execute` exits 2, as it did.
- The two end-to-end tests of that path expected the old one-line
  `REFUSED <code>:`. The next commit makes them expect #188's report.

Checked on the merged tree:
- AST (fin183g/names.py, base dd8d0ea): no top-level name is bound twice
  in adopt-project.py, scaffold-project.py or scripts/repo_shape.py, and
  none is loaded but bound nowhere; scripts/shape_materialize.py binds
  `YamlError` and `parse_yaml` twice, as main does since 3ef059c. Main
  changed the signatures of `Source.__init__` (`cloned=False`),
  `_create_leg_remotes` (`work_root`, `source=None`) and `_verify` (`urls`,
  `branch`); every call of the last two is main's, and every construction
  of `Source` passes four arguments or `cloned=` by keyword.
- Both sides intact (fin183g/sides_intact.py): the only lines that differ
  from either side's own change are those of the five hunks.
- A rehearsal of this merge (tree 59eca4e, the same as this one), with the
  next commit's test change, passed tests/test_adopt_root_symlinks.py,
  tests/test_adopt_late_failure.py, tests/test_adopt_filter_repo_failure.py
  and tests/test_adopt_ignored_shape_files.py: `231 passed`.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 12, 2026
… lines and clones nothing, and render-cli-reference.py refuses a tree that is not a checkout, so asking a download for help is no longer a clone of GitHub (#341) (#452)

From anywhere but a checkout of the standard (a downloaded `setup.sh`, `curl … | bash -s -- --help`, a `git archive` extraction), `setup.sh --help` went down section 0 like any other run. It cloned the public upstream before printing a word, died with git's exit 128 offline, and otherwise printed the CLONE's `setup-project.py` usage. `scripts/render-cli-reference.py` inherited all of it in such a tree: every rendering, `--check` included, reached GitHub and then documented upstream's flags under that tree's `setup.sh` heading. The clone attempts were reproduced on 87d2eb7, offline (`GIT_ALLOW_PROTOCOL=file`); what the generator renders when that clone succeeds is read from the code, not reproduced:

| | base 87d2eb7 | this branch |
|---|---|---|
| lone `setup.sh`, `bash setup.sh --help` | `(0) self-bootstrap`, `git clone --quiet --depth 1 -- https://github.com/opensoft/openRepoShape.git <tmp>`, `fatal: transport … not allowed`, exit 128 | the shim's own text, exit 0, no clone (`GIT_TRACE`), no temporary directory |
| same, `OPENREPOSHAPE_REPO=<checkout>` | a local clone, then the clone's `usage: setup-project.py …` | the shim's own text, no clone |
| `bash -s -- -h < setup.sh` | the clone, exit 128 | the shim's own text, exit 0 |
| `render-cli-reference.py --check` in a copy with no `.git` | tried the clone (stopped only by `GIT_ALLOW_PROTOCOL`, surfacing as `setup.sh --help exited 128`) | refuses, naming the tree, exit 2, no clone |

**What changed.**

**`setup.sh`**
- Section 0's one scan of the command line now also notes `-h` and `--help` before the first `--`.
- The shim answers them first: before the `$OPENREPOSHAPE_REPO` guard, before `mktemp`, before `git clone`. That is where `setup-project.py` answers them: before it looks at anything.
- The text says:
  - this is the bash front door to `setup-project.py`;
  - any other run from here first clones the standard into a temporary directory, from `$OPENREPOSHAPE_REPO` or, when that is unset or empty, `https://github.com/opensoft/openRepoShape.git`;
  - the one flag list is `python3 setup-project.py --help`, which answers with no network from a checkout or from that one file downloaded on its own, and `docs/cli.md` in the repository.
- It lists no flag, so #50's one `--help` still holds.
- The default is now one constant, `DEFAULT_SHAPE_REPO` (the name `setup-project.py` gives it), read by both the clone and the help. The help never prints the variable's value.
- Inside a checkout nothing changes: the shim still execs `setup-project.py` before section 0 reads a word.
- The header comment no longer says the shim "parses nothing". It now says the shim parses nothing of the RUN, and names the three things section 0 reads. The comment above the scan no longer says the scan never decides what the run does: the first two reads only decide whether a temporary directory is removed and whether a ref is checked out, and the third decides whether there is a run at all.

**`scripts/render-cli-reference.py`**
- `require_a_checkout()` asks `setup.sh`'s own `is_shape_checkout` question: `git -C <tree> rev-parse --show-toplevel`, and `scaffold-project.py` plus `contracts/repository-naming.yaml` at that top level.
- It also asks that the top level IS the tree (`os.path.samefile`). A copy nested inside another checkout passes the shim's probe, and its `setup.sh --help` would print that checkout's usage.
- It is called in `main()`, before the no-bash logic, and at the top of `render()`, which `tests/test_cli_reference.py` calls directly. The refusal names the tree and says to run it in a `git clone`, exit 2.
- It reads git's answer the way the shim's `$(...)` does: `os.fsdecode(...).rstrip("\r\n")`, so a checkout whose path ends in a space, or is not UTF-8, is a checkout here too.
- Every child now runs under `GIT_ALLOW_PROTOCOL=file`, so a regression fails closed.
- Inside a checkout neither measure changes a byte of a rendered help block. `--check` was `[ok]` before, and `[ok]` after regenerating.

**Docs**
- `README.md`: "`python3 setup-project.py --help` and, from a checkout, `setup.sh --help` list every flag, this one included. From a download, `setup.sh --help` clones nothing and lists none: it names `setup-project.py --help` … and `docs/cli.md`". That is +3 lines against main; the README cap in `tests/test_repo_hygiene.py` goes 1419 -> 1422, with its dated paragraph. `openRepoShape --help` is not the subject any more, because it names eight of `setup-project.py`'s fourteen flags besides `--help`. § What setup.sh does no longer gives the shim a line count ("about seventy lines of code"; it was 93 before this change and is 122 now), here, in the handbook and in `setup-project.py`'s docstring.
- `docs/handbook.html`: the page's copy of that sentence and of the shim's description, the footer's history clause, and the new README blob (`5465d135…`).
- `openRepoShape --help`: its one sentence "`./setup.sh --help` prints setup-project.py's usage, which lists them all" was false from a download. It now names `python3 setup-project.py --help`, and `./setup.sh --help` "in a checkout of this standard".
- `docs/cli.md` is regenerated: a note under `setup.sh` saying what `--help` does outside a checkout, and that `openRepoShape` sentence.
- `tests/test_setup_sh.py`: `test_no_test_here_depends_on_gh_being_authenticated`'s docstring is made true.

**Decisions** (the coordinator's; Brett may reverse).

1. **The shim's answer outside a checkout is option (a), with no flag list.** It is a short text of its own, exit 0, with no clone, no network and no temporary directory. Inside a checkout `--help` is still `setup-project.py`'s.
   - `-h` is treated like `--help`, because `setup-project.py` reads both.
   - Only arguments before `--` count. `setup.sh <Project> -- --help` is still a run, whose scaffold is asked for ITS help.
   - **Known false positives** (written into the shim's comment): runs `setup-project.py` would refuse anyway, such as `--org --help` or `--wat --help`, get the text and exit 0 instead of exit 2. Nothing is created either way.
   - **One known false negative**: `--visibility -- --help`, where `--` is a value, still clones as before. Fixing it would need the value-flag table in bash, which is the second parser #50 removed.
2. **Whose help.** The shim's text is its own, so it names no clone URL or ref. No path that prints a usage is left unattributed. `setup.sh <P> -- --help` prints the clone's scaffold usage under the `(0) self-bootstrap` clone line and `checkout: <tmp>`, which already say whose it is.
3. **The generator gets both measures**: the checkout refusal and `GIT_ALLOW_PROTOCOL=file`.
4. **Docs**: README, the handbook's reading of it, the shim's header, the test docstring and `openRepoShape --help`'s sentence, with `docs/cli.md` regenerated.
5. **The #50 guard is changed on purpose.** `tests/test_setup_project_py.py::test_setup_sh_parses_no_flags_of_its_own` refused any `--help` in the shim's code. It now excuses the spelling in exactly two places: the lines of `shim_help()`, each of which must be one `say "..."` with no `$(`, `${` or backtick in it, and section 0's one `-h|--help)` label. Outside those two, neither `--help` nor a bare `-h` may occur in the code. It still refuses `-y)`, `-h)`, `--yes)` and a second `--help)` label, and since fix round 1 it also refuses a quoted `"--help")`, a `[[ $1 == --help ]]` reader anywhere else (the developer path, the scan), and a reader hidden in a `say` line.

**Evidence.** The runs and the merge-tree below were taken at e2df9a8, the branch's head before the last merge of main (the last paragraph).

- **Targeted runs on e2df9a8** (d15a888, the fix round, merged with `origin/main` 53bee5b; the branch's first commit 968db85 is the writer's 16d1639 re-authored with the same tree 0d3243f), with a stub `gh` first on PATH, `GH_TOKEN` bogus, an empty `GH_CONFIG_DIR`, `GIT_ALLOW_PROTOCOL=file` and a blanked git identity:
  - `tests/test_setup_sh.py` and `tests/test_setup_project_py.py`: **132 passed**, 0 skipped;
  - `tests/test_cli_reference.py`, `tests/test_repo_hygiene.py`, `tests/test_agents_md_codes.py` and `tests/test_openreposhape_command.py`: **164 passed**, 0 skipped;
  - all six files together: **296 passed**, 0 skipped, 0 failed. Before the merge, on a throwaway merge of d15a888 and main 5678f68, the same six files gave 296 passed as well. review341 ran them on 968db85 merged with main 3ef059c: 132 + 162 passed.
- **The new tests against base 87d2eb7's code**: 15 failed. These are every new behaviour test (the shim's help ×4 spellings, its text, no echo, `bash -s`), the five generator refusal tests, `GIT_ALLOW_PROTOCOL` and the shim-probe tie. The 9 that pass are the "unchanged" guards (`-h`/`--help` inside a checkout, `-- --help` passthrough ×2) and pre-existing tests.
- **Mutation proof**: 18 single-hunk mutations in a throwaway worktree, each caught by a named test. They cover:
  - the help exit;
  - `-h`;
  - the `--` stop;
  - help after the env guard;
  - the default not named;
  - a flag line added;
  - the default not shared;
  - a second `--help)` label;
  - `main`'s guard, `render`'s guard, and both;
  - the top-level-is-the-tree check;
  - the files check;
  - `GIT_ALLOW_PROTOCOL`;
  - the files drifting from the shim;
  - the Tool note;
  - the `openRepoShape` sentence;
  - the README sentence.
- **The new tests**:
  - `tests/test_setup_sh.py` (`WINDOWS_SKIP` with the rest of the file):
    - `--help`, `-h`, after a run's arguments, and beside `--keep-shape-checkout --shape-ref`: exit 0, the shim's text, no `(0) self-bootstrap`, no usage, no `git clone` in `GIT_TRACE`, nothing in `TMPDIR`;
    - the text names the variable, its default, `setup-project.py --help` and `docs/cli.md`, and no flag but `--help`;
    - a forged `$OPENREPOSHAPE_REPO` is never echoed;
    - `bash -s -- --help`;
    - `<Project> -- --help` and `-- -h` still clone (locally) and reach the scaffold's usage. This is also the positive control that the trace sees a clone;
    - inside a checkout, `-h` and `--help` are `setup-project.py --help` byte for byte, on stdout and on stderr (stderr was not compared before the fix round; by probe it was already identical).
  - `tests/test_cli_reference.py`:
    - a `git ls-files` copy with no `.git`, one inside an unrelated work tree, and one inside a work tree that looks like a checkout of the standard: `--check` and a plain write both exit 2, naming the tree, with nothing written and no clone;
    - `render()` itself refuses;
    - the checkout refusal comes before the no-bash one;
    - `child_env()` pins `GIT_ALLOW_PROTOCOL=file`;
    - the generator's files are the ones the shim's probe reads;
    - (fix round 1) `--check` in a real `git init` copy whose path ends in a space, and in one whose path is not UTF-8, exits 0 with `[ok]`; each is skipped where the file system cannot hold the name (both on Windows; the second wherever creating the directory fails, which is expected on macOS, where no runner was available here).

**Merge-tree, as of e2df9a8 (re-run after merging `origin/main` 53bee5b into the branch).**

- At e2df9a8, `origin/main` (53bee5b) is an ancestor of the head: the branch carries a merge commit, so it was mergeable with main by construction. `render-cli-reference.py --check` was `[ok]` on it.
- **#352 landed as 53bee5b (#289) and conflicted in `docs/handbook.html`'s footer, as predicted.** The merge keeps both "added (#n)" clauses in landing order after "(#288)": (#289), then (#341), and keeps this branch's `README.md blob 5465d13`, which is `git hash-object README.md` again: main does not touch README.md and its own blob is `e8da9381…`.
- #314 has landed (3ef059c). The other PRs then open that share a file with this branch, re-checked against d15a888 before the merge (heads fetched at that time) and #403 again against e2df9a8:
  - clean against this branch: #394 (`tests/test_repo_hygiene.py`, not the README cap; it conflicts with main alone in `tests/test_manifest_validator.py`, a file this branch does not touch), #395 (one-line `visibility` assertion edits in `tests/test_setup_sh.py` and `tests/test_setup_project_py.py`, independent of this branch; its hunks in `docs/cli.md` are at :246-279), #397, #400 and #401 (each regenerates other sections of `docs/cli.md`);
  - **#403 (head 5b1da66) conflicted in the same footer hunk**: it merged clean with main alone and conflicted with this branch. It has since landed as dba5fb0 (#383, #403), and the merge of main into this branch (the last paragraph) keeps every clause in landing order after "(#289)", that is (#383) and then (#341), and names the blob of the README that lands, `5465d135ca7263010ee354424041f90c4553d2e7`. Main does not change README.md, so that is the blob this branch already named.
- `docs/cli.md`: #395, #397, #400 and #401 all merged clean with d15a888 plus main 5678f68, and the generator's `--check` was `[ok]` on each merged tree (#395's head 1ef3310, #397's abb27e6, #400's 7062a47, #401's 409f10b). Should one ever conflict, regenerating resolves it.

**Left out** (candidate follow-ups, not filed here).

- `setup.sh --preflight` from a download still clones before checking the machine, though `setup-project.py` answers `--preflight` before its own self-bootstrap. The preflight is Python, so the shim cannot answer it itself; it needs its own decision.
- From a download, `setup.sh --help` still needs Python 3.9 and git first: the interpreter and git checks run before the checkout probe, as they always did.
- Windows does not run the shim. The generator refusal tests do run there: they reach the refusal before any bash. The `tests-windows` job was their first run on that platform.

**Fix round 1 (review341's CHANGES NEEDED 0/1/5).**

One commit (d15a888) on top of 968db85, then a merge of `origin/main` 53bee5b (e2df9a8), no force:
- **S1 (should).** The #50 guard excused the spelling `--help` everywhere in the shim, so a second reader in any form but an unquoted label passed it. It now excuses the spelling in `shim_help()`'s `say "..."` lines and the one `-h|--help)` label, and nowhere else (decision 5). Mutation proof in a throwaway worktree: the guard passes on the unmutated shim and fails on each of review341's three variants (a quoted `"--help")` label, a `[[ "${1:-}" == --help ]]` reader in the developer path, a `[[ "$arg" == --help ]]` reader inside the scan) and on four more (a reader as its own line in `shim_help()`, one chained after a `say`, one substituted into a `say`, a bare `-h` reader). The first four and the bare `-h` reader fail only the guard, and the help tests around it stay green; the chained and substituted `say` variants also break the help's own output.
- **N1.** The inside-a-checkout test compares stderr as well as stdout. With the developer-path reader above, it now fails and `test_help_exits_zero` still passes.
- **N2.** The generator reads git's top level with `os.fsdecode(...).rstrip("\r\n")`, so a checkout at a path ending in a space, or not UTF-8, is accepted again, as the shim accepts it. A test for each; on 968db85's generator both fail with exit 2 "not a checkout".
- **N3.** README and the handbook name `python3 setup-project.py --help` as the command that lists every flag; `openRepoShape --help` names eight of fourteen. The handbook's copy, its footer clause and its README blob follow. README stays 1422 lines, so its cap does not move.
- **N4.** "About seventy lines of code" is dropped from README, the handbook and `setup-project.py`'s docstring: the shim is 122 lines of code, 93 before this change, and a count in prose goes stale again.
- **N5.** The comment above section 0's scan says the third read decides whether there is a run at all.
- `docs/cli.md` is unchanged: no `--help` text moved, and `--check` is `[ok]`.

Reviewed adversarially at 968db85, the branch's first commit (CHANGES NEEDED: 0 blockers, 1 should-fix, 5 nits). The shim and the generator behaved as this description says. The should-fix was the #50 guard, which a second `--help` reader slipped past; the nits were a test that compared stdout and not stderr, two real checkouts the generator refused (a path ending in a space, a path that is not UTF-8), and three pieces of stale doc or comment text (the README and handbook sentence that said `openRepoShape --help` lists every flag, the shim's line count, and the shim's own comment). Fix round 1 (d15a888, merged with main 53bee5b at e2df9a8) resolved them, and its re-verification of e2df9a8 read APPROVE WITH NITS (0 blockers, 0 should-fixes, 1 nit: § What setup.sh does does not say that the shim answers `--help` itself outside a checkout, left as it was because README says it a few lines above). Main 75052cb was then merged in at 4c28890: the one conflict, `docs/handbook.html`'s footer, keeps both "added" clauses (#383's, then #341's) and names the merged README's blob (5465d13), and the merged `docs/cli.md` matches its generator. CI is green on Linux, macOS and Windows, with SonarCloud passing, at 4c28890 (main 75052cb merged in).

Closes #341

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant