Repository navigation
bootstrap.py and validate-manifest.py: a branch holding a NUL or a lone surrogate is a finding and step (a) does not hand it to git, so tracking_branch: "a\ud83db" or a leg's branch: "a\x00b" no longer ends make bootstrap in a subprocess traceback under a manifest ok (#455) - #644
Merged
Merged
Conversation
…ne surrogate is a finding and step (a) does not hand it to git, so `tracking_branch: "a\ud83db"` or a leg's `branch: "a\x00b"` no longer ends `make bootstrap` in a subprocess traceback under a `manifest ok` (#455) `templates/assembly-root/scripts/bootstrap.py` step (a) hands each leg's branch to git as an argument: `--branch`, else `project.yaml`'s `tracking_branch`, else `main`, and a leg's own `branch:` over all three (`_checkout_all_tracking_branches`). `subprocess` cannot give a command a NUL (`ValueError: embedded null byte`) or a lone surrogate the system cannot encode (`UnicodeEncodeError`), and `_try_git` catches `Refusal` and nothing else. So each of the issue's four rows (`tracking_branch` of either, a leg's `branch:` of either) ended `make bootstrap` (exit 2) in a traceback from `subprocess.py`, with `(a) legs on tracking branches` printed and nothing after it. `validate-manifest.py` read neither key and said `manifest ok`, exit 0, so no tool named the value. It is #391's sibling: that value ended the run in a `print`, this one in an argument, and in both the run ended one step before the validator that could name the problem. Both halves of the issue's expected outcome, in one change, because each alone leaves a gap: a finding alone is never reached by `make bootstrap`, which still died in step (a); a step (a) that survives alone reaches a validator that says `manifest ok`. `validate-manifest.py`: `_tracking_branch_finding` and `_leg_branch_findings` (both over `_branch_finding`) give a `tracking_branch` and a leg's `branch` that holds a NUL or ANY surrogate (U+D800 to U+DFFF) a finding in the style of #280's `manifest-name`: `manifest-tracking-branch` and `manifest-leg-branch`, naming the key (the leg by role), the value as `repr` spells it, the code point and why. Any surrogate, as `name` is: no UTF-8 file or console holds one. Not asked: whether the value is a name git will create (`a..b`, `x.lock`), which is `git check-ref-format`'s and what #283 asks of `--tracking-branch` at scaffold time, and a value that is not text, which `bootstrap.py` already turns into text. `bootstrap.py`: `_argument_fault(branch)` is asked in `checkout_tracking_branch` before any command is built from the branch. A leg whose branch holds a NUL, or a lone surrogate that has no byte, is not touched and says so: [spec] spec: could not create branch a\ud83db: it holds U+D83D, a lone surrogate, which no command can be given as an argument; the leg is left as it is the branch is the leg's `branch:`, else `--branch`, else `tracking_branch`. Give that one a name git can create and run bootstrap again. The run then reaches step (b), the manifest validator names the key, and the run ends `bootstrap FAILED: manifest (exit 1)` as it ends for any other finding; `make bootstrap` exits 2, make's own code. Under `--skip-validators` the line is the whole story and the exit is 0, as it is for a branch git refuses. Decisions the issue left open: - ASKED FIRST, not read off an exception. The issue's second half proposed `_try_git` catching `(Refusal, UnicodeError, ValueError)`. A `UnicodeDecodeError`, which `git_out` raises over output that is not UTF-8, is a `ValueError` too, so that catch would read a leg whose HEAD is on such a branch as detached. And on Windows `CreateProcess` takes wide characters, so a lone surrogate raises nothing and git would be handed it mangled. The question is whether the VALUE can be an argument, so it is asked of the value. - A surrogate is refused unless it is ONE BYTE of a name. POSIX spells a name that is not UTF-8 with U+DC80 to U+DCFF and `os.fsencode` writes each back as the byte it stands for, so `--branch` with such a byte still creates its branch, as before. Where `os.fsencode` writes three bytes for every surrogate (`surrogatepass`, Windows) none is a byte and every one is refused. The answer is `os.fsencode`'s own, not a list kept here. The validator's finding does not depend on it. - The leg's line keeps `could not create branch ...` for the git-refused branch and the one no command can be given, and says why, and that the leg is left as it is, not `leaving detached`: it asks where HEAD is of nothing. - No `--branch` finding: the flag is not in the manifest, and the line step (a) prints is the only place it is named. `family-root/scripts/bootstrap.py` reads no branch, and `siblings.py` already refuses a `tracking_branch` that does not print (#543). `shape-doctor.py` and `bump-leg.py` only compare and print it. No sentence of the docs is made false; `docs/cli.md` is unchanged (no `--help` text moved). Pinned copies: `templates/assembly-root/scripts/bootstrap.py` and `templates/assembly-root/scripts/validate-manifest.py` are shape copies a project pins by digest, so both show as `upstream-changed` to a project's `update-shape.py check`, which is how a project takes the fix. `scripts/repo_shape.py` is not touched. Tests: `tests/test_bootstrap_branch_bytes.py` (54): the four rows of the issue through the validator, `bootstrap.py`, `--skip-validators` and `make bootstrap`; both keys at once; the values the question must leave alone; a git first on PATH that logs every command, with `main` as its control; `--branch` with the byte 0xff on a disk that holds it; and `_argument_fault` over every kind of character, with Windows' `os.fsencode` emulated. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
#617 (59e5a07) changed `templates/assembly-root/scripts/validate-manifest.py` in other hunks than this branch's (#455 adds `_branch_finding`, `_tracking_branch_finding` and `_leg_branch_findings` before `_shape_findings`, one bullet in the docstring and two `out.extend` lines in `_findings`). The merge was clean: no conflict, no hand resolution. Main also brought #583, #596, #614, #618, #619 and #594, none of which touches `bootstrap.py`, `validate-manifest.py` or this branch's test file. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Reviewer's GuideThe pull request hardens assembly bootstrap against NUL-containing and unencodable surrogate branch values by validating them explicitly, preventing unsafe subprocess calls, and adding extensive regression coverage while preserving existing behavior for valid and byte-carrying branch names. Flow diagram for branch manifest findingsflowchart TD
A[Manifest branch value] --> B{String?}
B -- No --> C[No branch finding]
B -- Yes --> D{Contains NUL or surrogate?}
D -- No --> C
D -- Yes --> E[_branch_finding]
E --> F[FINDING manifest-tracking-branch]
E --> G[FINDING manifest-leg-branch]
F --> H[Bootstrap ends with manifest failure]
G --> H
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
brettheap
marked this pull request as ready for review
October 16, 2026 11:00
|
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 10 hours and 52 minutes by commenting |
This was referenced Oct 16, 2026
brettheap
pushed a commit
that referenced
this pull request
Oct 16, 2026
main gained #563's own #590 (fe237ec) and #599 (32c22b0), whose trees this branch already held through #599's 750366b (#599's final head, eaa0cda, is 750366b plus a merge of main), and #626, #633, #642, #641, #643, #644, #632, #636, #631, #638, #635, #640, #623, #637 and #622. No conflict: the merged tree is 9be0d0d, the tree `git merge-tree --write-tree` predicted. AGENTS.md, docs/cli.md and tests/test_repo_hygiene.py merged as text; README.md and docs/handbook.html are unchanged by the merge. README.md is 1461 lines, its cap (1449 + 12, tests/readme-cap.d/563.txt), and AGENTS.md 494, its cap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 16, 2026
… tab shows the one-line install for Git Bash, held to the other copies by a test, and the notes are current to main d4a7fab with the fifteen pull requests since 4cf5fb0, the environments, the tag ruleset and the rehearsal (#563, #461) The handbook (#461's line "The handbook's step 3 has no Git Bash path", which lane 3 told lane 5 on #473 that #630 settles): step 3's install block was `data-platform="linux mac wsl"`, so a reader on the Windows tab never saw the one line the Windows panel says Git Bash runs. The Windows tab now has a lead, "In Git Bash, the bash that Git for Windows installs, the one-line install works as on Linux:", the Linux tab's block copied byte for byte as `data-lang="sh"`, and the panel's caveat: the preflight it ends with cannot take a typed `yes` in Git Bash. README.md is unchanged: no test asked for it, and it is at its cap. tests/test_repo_hygiene.py: the test above it finds one copy of the one line per file; `test_every_handbook_copy_of_the_one_line_install_is_the_same` holds every handbook copy (the masthead's and step 3's two) to ONE_LINE_INSTALL, so a copy edited on one tab alone fails. A drifted Windows copy fails it; the page as committed passes. The release notes: - "What changed since v1.0.1" gains one entry per pull request that landed after 4cf5fb0 and is not #563's own, from `git log --first-parent 4cf5fb0..origin/main` at d4a7fab: under update-shape.py and bump-leg.py, #644, #633 and #631; under scaffold, family and the materializer, #622, #643, #640, #641, #623, #638, #635, #626, #637 and #642; under shape-doctor.py, #636 and #632. Each from its squash commit and the issues GitHub says it closes. - The list's placeholder says it is current to main d4a7fab and stays a placeholder for what lands after it; the counts are refreshed inside their placeholder (66 pull requests, 75 of 97 milestone issues closed). - #590 and #599 are cited beside #563, and "Before tagging" says they landed (fe237ec, 32c22b0) and lists the 22 open milestone issues. - "How this release is cut": Brett Heap's word at 2026-10-16T12:06:40Z, the environments testpypi (12:07:31Z) and pypi (12:07:39Z), the tag ruleset 25237399 (12:08:14Z), and the rehearsal, run 38898735926 (12:10:53Z-12:12:00Z, success, both uploads skipped), each read back from the API. The trusted publishers' date stays a placeholder. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 16, 2026
brettheap
added a commit
that referenced
this pull request
Oct 16, 2026
… it what to do where an earlier act left that branch, so `already exists` is no dead end (#607's sentence, in the two tools it left) (#657) #607 (#637) gave `family.py` and the holder's `validate-family.py` one sentence for a `switch -c` that fails `fatal: a branch named '<branch>' already exists` (exit 128): > If `git switch -c` says that branch already exists, an earlier act left it: give the branch another name, which starts it where the holder stands, or, only if that branch holds work not merged yet, the same command with `switch` in place of `switch -c` goes back to it and keeps every commit on it. Two tools still printed a `switch -c` and said nothing more. Both now say the same sentence under that line, each as its own constant, `BRANCH_LEFT`, as #637 does: - **`scripts/bump-leg.py`**, with "the project" where the others say "the holder". It follows the command in all three places `_switch_command` is named: - the `bump-leg-on-tracking-branch` remedy: after `and re-run.`; - the `bump-leg-root-detached` remedy: on the line under it; - the dry run's closing hint (#477): the new last line, under the hint. - **`setup-project.py`**: the same sentence, word for word (the holder), as one more comment line under the family block's `switch -c` line. The family block's two comment lines stay, and the line comes after them. No command line changes, and none is quoted differently. The sentence is words, not a second command, so no PowerShell quoting is added (#532) and no README sentence about quoting becomes false. Pinned files: none. This changes no file under `templates/`. 1. **Where the sentence goes in bump-leg's tracking-branch remedy: after `and re-run.`, not between the command and `and re-run.`** The remedy reads `Run:`, the command on its own line, `and re-run.` With the sentence between, `and re-run.` would hang under a full sentence. After it, the text before the sentence is byte for byte what it was, so the command line and `and re-run.` keep their places. The sentence is still under the `switch -c` line. 2. **setup's sentence is one comment line, unbroken (about 300 columns).** The brief asked for one comment line, word for word. `family.py`'s dry run prints it unbroken too. The block's other comments are hand-wrapped at 78 columns. A wrapped block would be a small change if the coordinator prefers it; the test would then join the comment lines. 3. **Every printed command line is unchanged**, and the tests prove it: bump-leg's line is read back by `shlex.split` and compared to the argv it always meant (`switch_argv`); setup's by `host_words`, on a folder with a space and one with a space beside an apostrophe. The block still has exactly two command lines. 4. **One rule, four copies.** #637's equality test, now `test_the_tools_say_the_same_words`, also asserts that `setup-project.py`'s constant equals `family.py`'s and the validator's, and that bump-leg's equals it with "the project" for "the holder". It was renamed from `test_the_two_tools_say_the_same_words`, because "two" is no longer true. The project's wording is pinned as text once, as `BRANCH_LEFT_PROJECT` in `tests/test_bump_leg_paste_lines.py`, beside the lines it follows; the holder's is #637's pinned copy in `tests/test_validate_family_branch_step.py`. `test_the_two_pinned_texts_differ_in_the_one_word_only` ties the two pinned copies together. 5. **Existing tests that pinned the old last lines take the sentence.** Seven tests in three files (listed under Tests). Each read the last line of an output, or the exact end of a remedy; none changes what it checks. 6. **The new module imports #637's typing helpers** (`leave_branch`, `refused_as_left`, `type_other_name`, `typed`, `without_c`, `head`, `on_branch`) instead of copying them, so the duplication gate sees one body. New module `tests/test_branch_left_bump_leg_setup.py`, 19 tests: - bump-leg, each of three places (`tracking`, `detached`, `dry-run`): the command is the one it always was and what stands under it is the sentence (3); the printed line makes the branch where none is left (3); where an earlier bump left `bump/spec-<id>` holding a commit of its own, the printed line fails `already exists` (exit 128) and changes nothing, then **another name** works (the new branch starts where the root stands, the bump goes through on it, its commit sits on the commit the root stood on, and `HEAD:spec` is the new commit) (3), and **going back** with `switch` for `switch -c` works (back on the branch at its own commit, the bump goes through there on top of it) (3). - setup, the family block: the line, the two comments, the sentence, the `add` line, two command lines (2, one per folder name); a real `setup-project.py` run prints the sentence once, under the right comment (1); in a real local holder (the `family` fixture's, on its tracking branch), with a real member: the printed line makes the branch where none is left, and where an earlier try left `family/add-<project>` with a commit of its own, **another name** works and `family.py add` goes through on it, and **going back** works and `add` goes through on top of the earlier commit (3). - `test_the_two_pinned_texts_differ_in_the_one_word_only` (1). Edited tests: seven cases in six test functions of the three bump-leg files, `test_bump_leg_dry_run_tracking.py` (4: the dry run's report, the difference from a dry run on a branch, the hint pasted for a root with a space, the real refusal byte for byte), `test_bump_leg_paste_lines.py` (1: the exact remedies) and `test_bump_leg_dry_run_truth.py` (2: the hint's branch name, `[local-remote-dir]` and `[origin]`); and, beside them, the extended `test_the_tools_say_the_same_words` in `test_family_switch_branch_exists.py`. Runs, on be1bff9 (BASE a524542 plus main 95780dd), through `run9.sh`, `gh` stub first on PATH: - New module with #637's module: `44 passed in 61.90s`. - Set 1, 21 files: every `tests/test_bump_leg*.py` (7), the new module, `test_bootstrap_after_bump`, `test_stdout_encoding`, `test_non_utf8_git_reads`, `test_agents_md_codes`, `test_commit_trailers`, `test_update_shape_paste_lines`, `test_validate_pins_lockstep_lines`, `test_pin_rewrite_layout`, `test_validator_child_encoding`, `test_validate_pins_unreadable_workflows_dir`, `test_template_text_placeholders`, `test_repo_hygiene`, `test_cli_reference`: `1040 passed, 2 skipped in 459.92s`. Both skips were there before: a case-ignoring disk (`test_bump_leg.py:693`) and a Windows default (`test_stdout_encoding.py:286`). - Set 2, 10 files: every file `git grep -l 'switch -c' tests/` names outside set 1 (`test_dry_run_asks_remotes`, `test_family_printed_quoting`, `test_family_refuse_up_front`, `test_family_switch_branch_exists`, `test_family_tracking_branch`, `test_validate_family_branch_step`, `test_validate_family_unnamed_mounts`), plus `test_setup_by_hand_commands`, `test_setup_project_py`, `test_setup_sh`: `592 passed in 506.74s`. - Before #637 landed, on 6ac545f: set 1 `1042 passed, 2 skipped`, set 2 (9 files) `441 passed`. **Fails on BASE.** The same five test files from the head, run on main 95780dd (no source change): `26 failed, 80 passed in 168.33s`. 18 of the 19 new tests fail, `test_the_tools_say_the_same_words` fails (no `BRANCH_LEFT` in either new place), and the seven edited tests fail. The one new test that passes on BASE is `test_the_two_pinned_texts_differ_in_the_one_word_only`: it reads text only. On the first commit alone, against a524542, the old module copy gave `20 failed, 1 passed`. A real project (`scaffold-project.py` into local remotes, a recursive clone, the spec leg advanced on a branch of its remote) and a real holder (`family.py init`, then a real `setup-project.py Sample --yes --family Orbit` run). An earlier act left the branch with a commit of its own. - **On BASE a524542:** `bump-leg.py` on the tracking branch ends `Run:` / the `switch -c` line / `and re-run.` and nothing more. The line typed as printed gives `fatal: a branch named 'bump/spec-712e4ce5d766' already exists`, exit 128. The detached refusal and the dry run's hint end on the command the same way. setup's block is the `switch -c`, `# a branch first: add refuses the` and `# holder's tracking branch`, then the `add`. Typed, `fatal: a branch named 'family/add-sample' already exists`, exit 128. - **On the head:** each of the four prints the sentence under the line, and the same line gives the same 128. Then: - another name (`<line>-2`): exit 0; `bump-leg.py` goes through on it, on top of the commit the root stood on; `family.py add` goes through on it; - going back (`switch` for `switch -c`): exit 0, at the earlier commit; `bump-leg.py` and `family.py add` go through, their commit on top of it. - Both choices already worked on BASE: the sentence tells a person so. No row needs real GitHub. - **A real earlier bump, not a made-up branch.** I ran a real bump on `bump/spec-<id>`, then went back to `main`. There the root shows ` M spec` (the leg is still at the bump's commit), and the next `bump-leg.py` is refused `bump-leg-root-dirty` before it reaches the `switch -c` line. After `git submodule update` puts the leg back, the line appears, fails `already exists`, and the other name works. See the checklist candidates. Each hunk was reverted or broken alone in a scratch clone of the head, and the new module plus `test_the_tools_say_the_same_words` was run. Every mutation makes new tests fail: | Mutation | Result | |---|---| | bump-leg `BRANCH_LEFT` says "the holder" | 13 failed: the equality test, and the three places in each of four tests | | bump-leg `BRANCH_LEFT` drops `only if` | 13 failed, the same | | `bump-leg-root-detached` remedy without the sentence | 4 failed (the `detached` case of each of four tests) | | `bump-leg-on-tracking-branch` remedy without the sentence | 4 failed (the `tracking` case of each) | | the same remedy with the sentence above `and re-run.` | 4 failed, the same | | dry run's hint without the sentence | 4 failed (the `dry-run` case of each) | | `_switch_command` prints `switch -C` | 9 failed (the "another name" and "going back" cases, and the three "command is the one it was" cases); the control, where no branch is left, still passes, as `-C` makes a new branch too | | setup `BRANCH_LEFT` says "the project" | 7 failed (the equality test and six) | | setup block without the sentence | 6 failed | | setup sentence above the block's two comments | 6 failed | | setup `BRANCH_LEFT` drops `only if` | 7 failed | The other hunks change only docstrings and comments (`_switch_command`, `_branch_to_bump_from`, `family_next_command`, the comment above the dry run's hint). They change no behaviour. | Ref | Result | |---|---| | origin/main d4a7fab (one commit past my merge of 95780dd; touches `scripts/family.py`, `scaffold-project.py`, `docs/cli.md`) | clean | | #637 (landed as 95780dd) | merged into this branch once, clean | | #623 (lane 2, `setup-project.py` steps (3) and (4); landed as abb956e) | merged in with #637, clean: its hunks are far from `family_next_command` | | #639 08b6245 (lane 5, #611) | clean | | #641 e87fcb0 (touches `bump-leg.py` and `family.py`) | conflicts in `scripts/family.py`, and the same conflict arises against main alone: not from this branch. `bump-leg.py` merges clean. | | #615 6ad2303 (touches `bump-leg.py`) | clean | | #631, #636 (lane 4) | clean | | #642, #643, #644 | clean | | lane-5 siblings fix/645 e2110f8, fix/646 2216e1c, fix/648 852c5c4, fix/649 d91b314 | clean | | #561 (lane 4's `fix/bump-leg-tag-dry-run`) | its work is on main already (7d9bec9, `asked = commit`); its stale pull head conflicts with main alone, so it is not an open PR to check | - No README, AGENTS.md or handbook edit. I checked every sentence about these lines: AGENTS.md:337-351 and 374 (the dry run "prints the `git switch -c`"), README.md:157 and 1011, `docs/cli.md` (generated; no `--help` text changes) and the handbook. None becomes false. - Windows and macOS are not checked here, and no CI job checks them since #597. The tests type argvs, run no shell, compare no file that `write_text` wrote, and read setup's lines with `host_words`. - A branch checked out in another worktree makes `git switch <branch>` fail (#637's "Left out", N1); the sentence is not made longer for it. - I did not tick or mark the lines on #458 and #459. - **Review rv647 (APPROVE WITH NITS, 0 blockers, 0 should-fix, 5 nits), and what is left of it:** - Commit 6ac545f's message says "Four existing tests … take the sentence". It is six test functions (seven cases) in three files, as the Tests section says. The squash message is this body. - `scripts/bump-leg.py`'s comment near :1164 still says "the three tools follow one rule"; there are four copies of the sentence now, and `setup-project.py`'s comment names two. Left for the `bump-leg.py` issue filed from #458's lines, which lands after this and edits that file. - No AST guard holds a fourth `_switch_command` caller to the sentence, as #637's `test_every_remedy_that_names_a_switch_line_ends_with_the_sentence` holds the validator's. - Setup's sentence prints as one comment line of about 317 columns in a block wrapped at 78. It is unbroken as `family.py`'s `_branch_said` prints it, which is what lets the byte-for-byte equality tests hold (decision 1). - **bump-leg.py: after a real bump on a branch and a switch back to an older branch, the root is dirty (` M spec`) and `bump-leg-root-dirty`'s remedy, "commit or stash them first", is wrong advice.** The leg is left at the bump's commit and `git switch` does not move it (README says the leg is left detached at the new commit). `git submodule update` (or `make bootstrap`) puts it back and clears the refusal. The remedy could say so where the one dirty path is a leg's mount. It also means the `switch -c` sentence is reached only after that step in this flow. - **The same flow's "go back" choice meets that refusal too** (rv647): `git switch` to the branch that holds the earlier bump leaves the leg behind, so the re-run is refused `bump-leg-root-dirty`; after `git submodule update` it says `nothing to do`, and only the push is left. - **update-shape.py:5279 prints `NEXT git -C <root> checkout -b shape/update-<12> && git … commit`** (rv647, read only): it fails `already exists` the same way, and the `&&` chain then commits nothing. It is `checkout -b`, outside #647. - **family.py, after a real earlier `add` and a switch back:** another name works for the switch, but `family.py add` is then refused `member-path-taken`, because git left the populated `members/<Project>` checkout behind (rv647; identical on the base). It bears on #637's "another name works". Review: rv647 found 0 blockers, 0 should-fix and 5 nits on be1bff9 (APPROVE WITH NITS); nits 1 and 2 are answered in this body, nit 3 (the stale "three tools" comment in `scripts/bump-leg.py`) rides with #672, and nits 4 and 5 are noted above. CI (Brett Heap's ruling 27, with his fast-refresh ruling of 2026-10-15T22:48:35Z): H0 be1bff9 passed tests (Linux), tests-git-bash and SonarCloud; the head ac8a810 is a clean update-branch merge of main 8062810, with SonarCloud, tests-git-bash and package green on it, and the local gate passed on the head itself: the bump-leg set, 1040 passed (2 skipped), and the `switch -c` and setup set, 592 passed. Landed on Brett Heap's word of 2026-10-16T13:07:34Z, "merge #656 and #657 when green". Closes #647 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 16, 2026
…-letter `$OPENREPOSHAPE_REPO` is a path, a Windows-spelled `$OPENREPOSHAPE_BIN_DIR` is respelled or refused, and `tests-git-bash` runs #598's installer cases (#649) (#656) #596's Git Bash follow-ups, the five lines of #649. Two commits on main a524542: **d91b3144**, the fix, and **e85573ac**, fix round 1 from rv649's review. Together they change `openRepoShape`, `.github/workflows/tests.yml` and `tests/test_openreposhape_command.py`, and nothing else. `git diff --numstat`, d91b314: `openRepoShape` +42/-4, `tests.yml` +6/-1, the test file +182/-7. e85573a: +9/-6 (comments only), +3/-2 (a comment only), +56/-27. README, the handbook and `docs/cli.md` are unchanged: every sentence about these blocks stays true (below). ### Fix round 1 (rv649) rv649 found the launcher's code correct and asked for changes: 0 blockers, 1 should, 6 nits. e85573a answers each point. It changes no line of the launcher's code and no command of the workflow: three comments and the tests. - **SHOULD-1, the `D:/…` arm had no test.** Both BIN_DIR tests now run over `D:\tools\bin` and `D:/tools/bin`, and `fake_cygpath` takes the typed value. On Windows they use `str(...)` and `.as_posix()` of a path under `tmp_path`. With the arm dropped the suite stayed green (91 passed); now 5 cases fail. - **NIT-1, the refusal's drive-answer arm had no test.** A third refusal case: a `cygpath` that answers `D:/tools/bin` with exit 0. With that arm dropped the suite stayed green; now 2 cases fail. - **NIT-2, a `python` that is Python 2, or older than 3.9, reads as present.** Named as a cost in decision 1 and in the comment above `machine_report`. No code change: `--preflight` still refuses those machines. - **NIT-3, the refusal's reason was false in Git Bash with `cygpath` off `PATH`.** Git Bash does read `D:\…` as a drive. The comment in `install_commands` and decision 3 now give the true reason. - **NIT-4, the job's opening comment named two files.** It names the three. - **NIT-5, on Windows the refusal test's value was a real `D:` drive path.** It is now a path under `tmp_path`, and the assert names that value, so a refusal that regressed would place the command where the test looks. - **NIT-6, four claims.** Fixed in this body: "rows 1-3 are checked through the tests" (now says what runs), the clone URL for each slash (`C:\src\…` gave `https://github.com/C:\src\openRepoShape.git`), the Store stub clause (dropped, see decision 1) and the report's diffstat totals (the exact `--numstat` counts are at the top). d91b314's own commit message keeps its `....` typo and its one-URL sentence in history; the squash message is this body. - **Composition and merge table.** Refreshed against origin/main 879d804 (below). #630 no longer conflicts. - **Checklist candidate 3**, from rv649, is added below. ### The fix **1. The `machine:` row for Python** (`machine_report`). - Before: the block asked `command -v python3` and nothing else. Where Python came from python.org, Git Bash has `python` and no `python3`, so the row said `python3 MISSING` and printed the line pointing at `--preflight`, which then passed with `python`. - Now: it asks for `python3`, then `python`, in `find_python`'s order. When only `python` answers, the row says `python3 present, as python`, and nothing points at `--preflight` for it. With neither, it says `MISSING`, as before. With `python3`, it says `present`, byte for byte as before. **2. A drive-letter `$OPENREPOSHAPE_REPO`** (`shape_clone_url`). - Before: `C:/src/openRepoShape` and `C:\src\openRepoShape` were read as an owner/name, so setup.sh's clone and `--doctor`'s were handed `https://github.com/C:/src/openRepoShape.git` for the first and `https://github.com/C:\src\openRepoShape.git` for the second. - Now: a letter, a colon and then `/` or `\` is a path, and goes through as it is, as `/…` and `.…` do. No owner/name holds a colon. The package's own `clone_url` (`src/openreposhape/standard.py`, `DRIVE_RE`) already read a drive letter this way. **3. A Windows-spelled `$OPENREPOSHAPE_BIN_DIR`** (`install_commands`). - Before: `--install` named the directory as typed. With `D:\tools\bin` it printed `installed at D:\tools\bin/openRepoShape` and `export PATH="D:\tools\bin:$PATH"`, which the drive's colon splits in two. It also never found the directory on a `$PATH` that holds `/d/tools/bin`, so it printed that line every time. - Now: a drive-letter value, in either slash, is spelled again with `cygpath -u`, which Git Bash has, before anything is checked or placed. Every line then names `/d/tools/bin`, and the PATH note appears only when that directory is not on `$PATH`. - Where no `cygpath` answers (none on PATH, it fails, or it answers a drive spelling again), the value is refused before anything is placed: "--install would place this command in 'D:\tools\bin', which is spelled for Windows, and no cygpath here spells it the way bash does, so the directory could not go on $PATH: the drive's colon splits it. Nothing was placed. Set $OPENREPOSHAPE_BIN_DIR to the directory as this shell spells it, then run this again." **4. `tests-git-bash` runs `tests/test_install_sh_filter_repo.py`.** That file holds #598's uv install, the two readinesses and the pasted next steps, which ran on Linux only, while README says Git Bash runs step 3's one line "tested in CI". It is the third path on the job's pytest line, with a comment saying the installer's suite is two files. The job's other lines, the header's "the launcher's and the installer's suites, and nothing else" and #630's release-notes sentence of the same meaning stay true. **5. The stale docstring.** `test_a_ref_the_doctor_would_check_out_is_validated` said the refusal "has to arrive before any clone does". The command clones first, then checks the ref, and refuses before `git checkout` is handed it. The docstring now says that. The order is unchanged. - **Line 1: `command -v` for both names, and no version check.** - The block promises to execute nothing but `gh auth status`, and `test_the_machine_report_runs_no_installer` holds it to that: every fake on its PATH logs a run. Running `find_python`'s check would break that promise. - What answers is therefore never asked its version, and that has a cost. The Microsoft Store's `python3` stub reads as present, as macOS's Command Line Tools stubs for `python3` and `git` already do (setup.sh's comment at :41-47). That is not new: the base asked `command -v python3` too, and the Store installs its `python.exe` and `python3.exe` aliases together, so `python3` answers first. - What is new is a `python` that is Python 2, or older than 3.9, where there is no `python3`. On the base such a machine said `python3 MISSING` and printed the `--preflight` pointer. Now it says `present, as python` and prints no pointer. It is rare (CentOS 7, macOS 10.14 or older, Python 2.7 from python.org), and `--preflight` still refuses it with "no Python 3.9 or newer on this machine (tried python3, python)". The comment above `machine_report` says so. - The row keeps its name `python3`, and the state carries the name that answered, in the form `gh`'s row already uses (`present, logged in`). So README's sentence ("`git`, `gh` and whether it is logged in, `python3`, and `git-filter-repo` … each `present` or `MISSING`") and the handbook's stay true, and README is not edited. Labelling the row `python` would have made both false in Git Bash. - **Line 3: respell, and refuse only where respelling is not possible.** Respelling keeps a Git Bash user's value working: `cp`, `mkdir` and the record took the Windows spelling already, and only the printed lines were wrong. Where no `cygpath` answers, nothing in the command can spell the value as bash's `$PATH` needs it: in Git Bash with `cygpath` off `PATH` the drive is real but its colon still splits `$PATH`, and on Linux `D:\tools\bin` is a relative directory with a colon in its name. No `$PATH` line could work there, so it refuses. A `cygpath` that answers a drive spelling again is refused alike. - **The tests' `install_with_machine` now names its directory with `bash_path`.** Its `$PATH` holds only the programs `--install` runs, and no `cygpath`. On Windows the old `str(bin_dir)` is a drive path, which would now be refused. Off Windows, `bash_path` changes nothing. Other tests that pass `str(...)` keep the full PATH, so on Windows they now go through Git Bash's own `cygpath`. - **`run_cmd` takes a `cwd`.** On the base, and under a mutation, the new BIN_DIR tests' `D:\tools\bin` is a relative directory. With `cwd=tmp_path` it lands in the test's own directory, never in the checkout. - **Both BIN_DIR tests run over both slashes** (`BIN_DIR_SPELLINGS`, `typed_bin_dir`). Off Windows the typed value is the fixed `D:\tools\bin` or `D:/tools/bin`. On Windows it is a path under `tmp_path`, so neither test can place a command on the machine's own `D:` drive. New, in `tests/test_openreposhape_command.py`, 27 cases: - `test_the_machine_report_finds_a_python_by_either_name`: `python` alone, both, neither (3). - `test_a_windows_spelled_bin_dir_is_spelled_the_way_bash_spells_it`: not on `$PATH`, on `$PATH`, each in both slashes (4). On Windows it is a real drive path (`str(bin_dir)` or its `as_posix()`) and Git Bash's own `cygpath`; elsewhere a fake `cygpath` answers as that one would. - `test_a_windows_spelled_bin_dir_is_refused_where_no_cygpath_spells_it`: no `cygpath`, a `cygpath` that fails, a `cygpath` that answers `D:/tools/bin`, each in both slashes (6), on the machine report's own `$PATH`. - `test_a_clone_is_handed_a_path_or_url_as_it_is`: owner/name, URL, scp, POSIX path, `C:/…`, `C:\…` and `d:\…`, each through setup.sh (a setup.sh that prints `$OPENREPOSHAPE_REPO`) and through `--doctor` (a `git` that prints the URL it is handed and fails) (14). Nothing is cloned or fetched. **On the base a524542** (this head's test file over the base's command): **17 failed, 80 passed** in 62.32s. The failures are exactly the new cases that need the fix: `python-alone`, the four respelling cases, the six refusal cases and the six drive-letter cases. The other 10 new cases pass there too, as they should. **On the head e85573a** (Linux, `dirty=0`): - `tests/test_openreposhape_command.py`, `tests/test_install_sh.py`, `tests/test_install_sh_filter_repo.py`, `tests/test_openreposhape_package.py`: **220 passed** in 175.18s. The package suite imports 13 of the launcher's cases and swaps `run_cmd`, so it is in the set. - `tests/test_repo_hygiene.py`, `tests/test_workflow_concurrency.py`, `tests/test_release_workflow.py`: **138 passed** in 14.46s. - The head merged with main 879d804 (a probe commit, b6cfafe3): the launcher's, the two installer files, the package suite and `tests/test_repo_hygiene.py`, **297 passed** in 188.95s. - **The three files `tests-git-bash` runs, with Windows' `write_text` emulated**: a plugin made every `Path.write_text` in the test process write CRLF, as it does on `windows-latest`. **162 passed** in 121.06s (97 + 47 + 18). A probe test shows the plugin takes effect (it passes with the plugin and fails without it). - **actionlint 1.7.12**, with shellcheck on PATH, on `tests.yml` alone and on the four files the CI job names: exit 0. d91b314's own `tests-git-bash` run was green on `windows-latest`. It ran the first round's 21 new cases and `tests/test_install_sh_filter_repo.py` there, so Git Bash's own `cygpath` answered the backslash respelling on a real drive path, and the five existing tests that pass `str(...)` and now go through it passed. What was checked, and what was not: - **Checked, from #596's Windows runs (113 passed on 5f001de, and its probes):** a fake that Python writes with `#!/bin/sh` runs under Git Bash with no mode bits, and `command -v` finds it. A script piped in text mode reaches Git Bash's bash as CRLF, and bash drops the CR. A `$OPENREPOSHAPE_BIN_DIR` set from Python reaches bash unconverted: #596's probe 4 printed `D:\a\_temp\…\bin/openRepoShape`, which is why the respelling now runs in those tests. - **Checked on Linux:** LF bytes (the CRLF emulation above). No assert in the test file compares a path bash printed with a Python path. - **Not checked, until e85573a's own run:** on Windows, the slash spelling (`as_posix()`) through Git Bash's real `cygpath`, the refusal's values under `tmp_path` on the machine report's `$PATH`, and the new drive-answer case. If one fails there, its log shows the typed value and the directory. One script ran each row against the base's command and the head's, offline, with a `gh` stub first on PATH. Row 3's slash lines ran again for this round. | Row | Base a524542 | Head e85573a | |---|---|---| | 1. PATH with `git`, a logged-in `gh`, `python`, `git-filter-repo`, no `python3` | `python3 MISSING`, then `run: openRepoShape --preflight …`; the log holds only `gh auth status` | `python3 present, as python`, no `run:` line; the log holds only `gh auth status` | | 2. `OPENREPOSHAPE_REPO=C:/src/openRepoShape` | setup.sh clones `https://github.com/C:/src/openRepoShape.git`; the doctor's `git clone` too | `C:/src/openRepoShape`, both | | 2. `OPENREPOSHAPE_REPO=C:\src\openRepoShape` | `https://github.com/C:\src\openRepoShape.git`, both | `C:\src\openRepoShape`, both | | 2. `OPENREPOSHAPE_REPO=acme/openRepoShape` | `https://github.com/acme/openRepoShape.git` | the same | | 3. `OPENREPOSHAPE_BIN_DIR='D:\tools\bin'`, no `cygpath` | exit 0, `installed at D:\tools\bin/openRepoShape`, `export PATH="D:\tools\bin:$PATH"` | exit 2, the refusal above, nothing placed | | 3. `OPENREPOSHAPE_BIN_DIR='D:/tools/bin'`, no `cygpath` | exit 0, `installed at D:/tools/bin/openRepoShape`, `export PATH="D:/tools/bin:$PATH"` (on Linux, a relative directory named `D:` in the working directory) | exit 2, the same refusal with `'D:/tools/bin'`, nothing placed | | 3. the same, a fake `cygpath -u` answering `<dir>/d/tools/bin` | the same as with none: `cygpath` is never asked | exit 0, `installed at <dir>/d/tools/bin/openRepoShape`, `export PATH="<dir>/d/tools/bin:$PATH"` | | 4. `tests-git-bash`'s pytest line | two files | three files | | 5. the docstring | "the refusal has to arrive before any clone does" | "clones this checkout first, then checks the ref" | Rows 1-3 need Windows to be real: there, `python` is a real python.org install, and `cygpath` and the drive letter are Git Bash's own. On Linux they are fakes. The first Windows run runs rows 1-3's tests under Git Bash: row 3 with Git Bash's own `cygpath` on a real drive path, rows 1 and 2 with a fake `python` and a fake `git`, so a real python.org install and git's own reading of `C:/…` stay unchecked. Each row reverts one hunk of the launcher, or one part of the BIN_DIR hunk, in a throwaway clone, then runs the 27 new cases. | Hunk reverted | Result | Failing cases | |---|---|---| | none (the head) | 27 passed | — | | `shape_clone_url`'s drive-letter arm | 6 failed | the six drive-letter cases, setup.sh and doctor | | `machine_report`'s `python` arm | 1 failed | `python-alone` | | the BIN_DIR block, whole | 10 failed | the four respelling cases and the six refusal cases | | the `cygpath -u` call (always refuse) | 4 failed | the four respelling cases | | the refusal (keep the value as typed) | 6 failed | the six refusal cases | | `\|\| spelled=""` after the `cygpath` call | 2 failed | `cygpath-fails`, both slashes: `set -e` exits with `cygpath`'s status, 1, not the refusal's 2 | | **the `D:/…` arm of the drive case (SHOULD-1)** | 5 failed | the two slash respelling cases and the three slash refusal cases (was green) | | the `D:\…` arm of the drive case | 5 failed | the same five, backslash | | **the refusal's drive-answer arm, `[A-Za-z]:*` (NIT-1)** | 2 failed | `cygpath-answers-a-drive`, both slashes (was green) | | the refusal's empty-answer arm, `""` | 4 failed | `no-cygpath` and `cygpath-fails`, both slashes | | `--` dropped from `cygpath -u -- "$dir"` | 4 failed | the four respelling cases: the fake answers only `-u -- <value>` | | the respelling after the placement, not before | 4 failed | the four respelling cases | Not probed by a test: the `tests.yml` line (CI configuration, checked by actionlint and by this PR's own Windows run), the docstring, and `install_with_machine`'s `bash_path` (identity off Windows; on Windows it is what keeps four machine-report tests from the new refusal). | Against | Result | |---|---| | origin/main 879d804 (d4a7fab, then 879d804, since the last table's 95780dd) | **clean**. Of this PR's files, the three suites', README and the handbook, only `tests/conftest.py` has moved since the base a524542 (new helpers for setup-project's tests), and nothing since the last table's 95780dd. The head merged with main (a probe commit, b6cfafe3) passes the suites below. | | lane 3's #630 61b3907 | **clean**. It conflicted at ca5c49f, at the `tests-git-bash` pytest line, because its base was older than main's #590; its head now holds main d4a7fab, and it does not change `tests.yml`. It changes `AGENTS.md`, README, `adopt-project.py`, the handbook, `docs/cli.md`, the release notes and four test or render files, none of which this PR touches. | | #620 9f2b67c, #621 10dda17, #622 55666f3, #623 fc08a28, #627 83ca2db, #628 d64252a, #631 21bdac8, #634 b7472df, #636 30a52c0, #637 62263d2, #638 b393e1f, #639 08b6245, #642 1a42df4, #643 1fe3764, #644 733f853 | **clean** (#623 changes `tests/conftest.py`; #627 changes README and the handbook) | | #641 e87fcb0 | conflicts in `scripts/family.py`, and so does #641 against main alone: its own conflict, not an overlap with this PR. | | lane 5's `fix/645-fixture-copy-maintenance-race` accc7d2, `fix/646-add-member-variant-remedy` 79754b9, `fix/647-branch-left-bump-leg-setup` be1bff9 (#657), `fix/648-bootstrap-and-gate-order` 852c5c4 | **clean**, all four | - **The Store stub check.** See the decision on line 1. `--preflight` is where a stub is told apart. - **`install.sh`'s own lines.** It names a Windows-spelled `$OPENREPOSHAPE_BIN_DIR` as typed in its `preflight:` and `done:` lines, though the command it installs now prints `/d/…`. #649 names `--install`'s PATH note, and `install.sh` prints no PATH line. Checklist candidate 1. - **`--help`.** Its `$OPENREPOSHAPE_REPO` and `$OPENREPOSHAPE_BIN_DIR` lines stay true, so `docs/cli.md` is not regenerated. - **`D:`, `D:tools` and UNC values.** Checklist candidate 3. 1. **`install.sh` echoes a Windows-spelled `$OPENREPOSHAPE_BIN_DIR` as typed.** It builds `target="$bin_dir/openRepoShape"` from the raw value, so in Git Bash with `D:\tools\bin` its `preflight:` and `done:` lines say `D:\tools\bin/openRepoShape`, while the command's own lines say `/d/tools/bin/openRepoShape`. Both spellings work in Git Bash, so nothing breaks. On a machine with no `cygpath`, the command's `--install` now refuses the value, and `install.sh` stops with "the <version> openRepoShape's --install refused (exit 2); its own refusal above says why", which is right but could refuse earlier, before the fetch. 2. **The PATH note is not quoted for every directory.** `--install` prints `export PATH="$dir:$PATH"` with the directory between double quotes as it is. A directory holding `"`, `$`, a backquote or `\` prints a line that does not do what it says, and one holding `:` cannot be on `$PATH` at all. A drive letter was the one case a person in Git Bash meets by default; the rest need a deliberately odd directory name. 3. **Two kinds of value slip past both new arms** (rv649). `$OPENREPOSHAPE_BIN_DIR` set to `D:` or `D:tools` matches neither respelling arm, so it is kept as typed and prints `export PATH="D:tools:$PATH"`, as on the base. A UNC `\\server\share\…`, as `$OPENREPOSHAPE_BIN_DIR` or `$OPENREPOSHAPE_REPO`, is never respelled (the PATH note's `\\` collapses) or becomes a github.com URL. The package's `clone_url` passes a UNC path through on Windows (`os.path.isabs`). Review: rv649 found 0 blockers, 1 should-fix and 6 nits on d91b314 (CHANGES NEEDED); the fix round e85573a answers all seven, so both mutations that survived now fail a test, and lane 5's coordinator checked it. CI (Brett Heap's ruling 27, with his fast-refresh ruling of 2026-10-15T22:48:35Z): H0 e85573a passed tests (Linux), tests-git-bash (windows-latest) and SonarCloud; the head 022bd50 is a clean update-branch merge of main 8062810, with SonarCloud, tests-git-bash and package green on it, and the local gate passed on the head itself: the launcher, installer and package set, 220 passed, and the hygiene, workflow and README-section set, 214 passed. Landed on Brett Heap's word of 2026-10-16T13:07:34Z, "merge #656 and #657 when green". Closes #649 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 16, 2026
brettheap
added a commit
that referenced
this pull request
Oct 17, 2026
… and names a dangling link to move aside first, a NUL or lone surrogate in `project:` or url is a finding, `.gitmodules` is read as git reads it, the warning's prose is spelled, and a tag-only branch is named (#690) (#805) Six lines of `templates/family-root/scripts/siblings.py` (`make siblings`, a pinned copy in every family holder, which takes this as one `upstream-changed` row), each re-measured on main 4ce9045 (then merged with bf1f099) with a holder from `family.py init --local-remote-dir`: - `parent_folder_warning` printed its staging name whatever held it. With `<t>/InkRouter.holder` a folder already, the three printed lines all exited 0 and left the holder at `<t>/InkRouter/InkRouter/InkRouter`, one level too deep; `<Family>.folder` alike on a disk that ignores case. `staging_path` now gives the first of `<Family>.holder`, `<Family>.holder.1`, `.holder.2` ... at which `os.path.lexists` finds nothing, a link to nothing included. - A link to nothing named `<Family>` passed `not wanted.exists()`, so the staged move was printed, its first `mv` ran, and `mkdir -p` failed `File exists` with the holder half moved. The warning now says, before the moves, `First move <path> aside: it is a link to nothing, named after the family, and in the way of the move. Then:`, and the moves under it are the ones that run once it is gone. - `place` handed a `project:` holding a lone surrogate or a NUL to `git clone` as the clone's path: `UnicodeEncodeError` or `ValueError: embedded null byte`, exit 1, and the members after it never placed. The url `clone_url` builds from a `repository:` holding a NUL ended the same way. `argument_fault`, #455's rule as #644 wrote it for `make bootstrap` (a NUL, or a surrogate `os.fsencode` cannot write as one byte), is now asked of both before anything: the member is `NOT AN ARGUMENT`, a finding (`<project>: its `project:` in family.yaml holds U+D83D, a lone surrogate, which no command can be given as an argument, so nothing was cloned or fetched for this member: git was never run with it.`), the dry run says the same, and the other members are placed. - The warning's prose printed the holder, the parent folder and the family raw on stderr (an ESC in a folder name reached the terminal). Each is now `spelled`; the first line keeps its `repr`, and the `mv` lines stay as typed (#486's question). - `submodule_urls` read `git config --list` as text, split by `str.splitlines` and at the first `=`: a url holding NEL, U+2028, U+2029, VT, FF or U+001C to U+001E was cut there, and `[submodule "members/a=b"]` (or `"a=b"`) gave no url, so its member was cloned from the `repository:` guess; a CR in a section's name cut its key in two, so the section was read as none. It now reads `git config -z --list`, records cut at NUL and each key from its value at the first newline, a section's name being all of its key between `submodule.` and the last dot: the reading `validate-family.py` makes and lane 3's #668 gives the doctor (#146). - A `tracking_branch:` naming only a tag of the member's remote was cloned DETACHED by `git clone --branch`, under a line saying "on <name>, its tracking branch", exit 0. A line after the clone now says `<name> is not a branch of <url> but a tag, which git clone checks out DETACHED: this clone is on no branch`. The exit is unchanged, as the issue asks. `tests/test_siblings_inputs.py` covers each line with the issue's inputs, runs the printed moves as typed in a POSIX shell, and holds the parity of the doctor's and `make siblings`' readings over one corpus (#668's names and breaks, its Latin-1 name, and lane 3's bare `a=b` and CR names; #668 is on main as #700, so the parity runs). A section holding a CR and `update = bogus` reads as the same section without the CR; what a read does with a value git refuses is #733's. No doc sentence is made false, and no `--help` changes. The holder's header spelling U664 pins is untouched. Closes #690 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 17, 2026
… YAML reader refuses for opening with an indicator is named by its line and says to double-quote it or reword it, `plan --org @x` names `--org` again under the reader's own remedy, and `check` names one bad `tracking_branch` once (#708) (#817) Three refusals named the value to change on main or in v1.0.1, and a v1.1.0 change took that away (lane 4's L4-151, L4-152 and L4-127). L4-151, from #635 (#392, #393). The reader refuses a plain value that opens with an indicator, so an adoption plan whose hand-written `resolution:` opens with a backtick, which AGENTS.md's step 2 makes likely (`` `adopt-project.py` reads it ``), ended `check` in `REFUSED yaml-unparsable: <plan>: <words>`, exit 2, with no line among the hundreds a plan holds and no remedy, where it checked clean before. - `YamlError` carries a `line` and a `remediation`. `_scalar` and `_flow` read text and know no line, so the block reader records it: the new `_block_value` reads a block mapping's value and a block sequence's scalar item (`_parse_mapping`'s lone `-` refusal moves with it), and sets the line on a refusal raised there that carries a remedy. - `load_yaml` names it, `REFUSED yaml-unparsable: <path>: line N: <words>`, and passes the refusal's remedy on. The words of `parse_yaml` are unchanged: the tools that ask the reader about ONE rendered line quote them in a sentence of their own, where `line 1:` would be noise. - `_indicator_refusal` carries the remedy: write the value in double quotes, spelled by `double_quoted` (so the remedy's spelling reads back as the value), or reword it. That is #393's block-value openers, and #392's flow-sequence items and flow-mapping values. The other refusals carry none (#370): the fix for an alias or an unclosed quote is no one edit. - A flow-mapping entry whose key opens with an indicator (`{@A: b}`, ``{`a`: b}``, `{: a}`, `{? a: b}`, `{#a: 1}`) carries no remedy, and so no line: `_split_key_flow` asks `_refuse_entry_indicator(text, remedy=False)`, because the whole entry double-quoted (`{"@A: b"}`) reads as an entry without a key and is refused again (lane 1's review of #817, finding 1). - Only a refusal with that remedy names its line. A line on every value's refusal (an alias, an unclosed flow, an empty flow entry) was measured: it changed words that the naming gate's `declared-unverified` reason and the root's `validate-pins.py` print whole, pinned by tests/test_pinned_copy_text.py, tests/test_naming_policy.py and tests/test_refusal_default_remediation.py, which is beyond this issue. - The project's own gates read their files with this copy: an indicator in a pin file is now the quoting remedy under `validate-pins.py`, where it printed the lockstep one, which is not a syntax error's remedy. L4-152, from #635. The reader refuses `repository: @x/Northwind`, so `plan --org @x` (a typo of `@opensoft`: GitHub owners never begin with `@`) stopped reaching the validator, whose `manifest-leg-repository` named `--org` (`FINDING_PLACEHOLDER`). It said "the manifest this run would write is a file the project's own YAML reader refuses", named no value, and its remedy said "the words after each value are the validator's own". - `_unread_sentences` renders each template line that carries a value of the caller's, by its own placeholder or by one `CARRIED_BY` names (each leg's `repository:` carries `ORG`, as the validator's finding about it does), reads it alone as `_misread_rows` reads one, and names the value where the reader refuses its line: `--org is '@x', which the project's own YAML reader refuses: <the reader's words for each line>`, main's sentence with the reader in the validator's place. The validator's own words cannot be had: it never reads a file the reader refuses. Where no such line is refused, the sentence is the reader's refusal of the file, as before. - `MANIFEST_UNREADABLE_REMEDIATION` is that case's remedy: choose another value for each one named, and where none is named, the text the reader's words quote holds it; the words are the reader's. `manifest_value_problems` gives the case the kind `KIND_UNREADABLE` (the kinds are named once: `KIND_MISREAD`, `KIND_INVALID`, `KIND_UNREADABLE`), and `refuse_manifest_values` (`plan`, the scaffold), which takes each kind's remedy from `KIND_REMEDIATIONS`, and `check`'s `_manifest_findings` (and so `execute`) give it that remedy, under the code they gave it before, `-manifest-invalid`. `MANIFEST_INVALID_REMEDIATION` is not reworded, so `scripts/family.py`'s copy of it does not move, and `family.py init --org @x` stays refused. L4-127, from #644 (#455). The validator finds a NUL or a lone surrogate in `tracking_branch`, and `check` asks it, so `tracking_branch: "a\x00b"` was `unsafe-value`, then `plan-manifest-invalid` in the validator's words, which name no plan key. No such value is in `checked_value`'s alphabet, so `unsafe-value` names every one. `_validator_ignores` leaves the validator's `manifest-tracking-branch` out where `checked_value` refuses the branch, as `LEG_PATH_CODES` leaves the leg paths to `_leg_findings`, and `_manifest_findings` and `_validator_judged` ask it alike. It asks `checked_value` alone, which starts no process, as the validator's question must not (test_the_question_runs_no_process_at_all). The issue's other option, a `FINDING_PLACEHOLDER` row, was measured: it names the key and still prints two findings. Pinned copy. `scripts/repo_shape.py` is the one pinned shape copy this changes. No digest of it is recorded in this tree; each root takes it through `update-shape.py` as an `upstream-changed` row, with v1.1.0's other bumps. Tests: tests/test_reader_refusals_named.py (new), with lane 1's test_a_flow_mapping_key_is_not_told_to_quote_the_whole_entry. In tests/test_yaml_indicator_openers.py, two assertions that pinned an opener's refusal without its line expect `line N:` on its row; in tests/test_adopt_plan_yaml_quoting.py, one that pinned the validator's remedy under the reader's refusal of a whole manifest expects the reader's. Closes #708 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 17, 2026
…ame it is given, so a branch whose upstream is not UTF-8 is moved back, and a good branch this system's encoding cannot write gets a line of its own, not a traceback (#710) (#824) `_try_git` read every answer through `repo_shape.git_out`'s strict UTF-8 decode. It now reads bytes and decodes them so that a name git prints goes back to git as the same bytes: - on POSIX with `os.fsdecode`, as #745 (#704) made `_head_of` and `_in_progress` read theirs, the inverse of the `os.fsencode` that `subprocess` applies to each argument; - on Windows as UTF-8, since `subprocess` hands git its arguments through `CreateProcessW`. Measured on main 24a977c before this change: - lane 5's site on #710: a tracking branch past the pin with no commit of its own, its name and upstream not UTF-8, and the leg detached at the pin, ended in `UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff`, exit 1, from `_upstream_holding`. It is now moved back. - under `LC_ALL=C PYTHONUTF8=0`, `main` tracking `origin/日本` past the pin ended in `UnicodeEncodeError` once the name, read as UTF-8 text, was handed back to `rev-parse`. It is now moved back. - the bytes are stripped and not the text, because `str.strip` took a trailing U+00A0 off `refs/remotes/origin/a<U+00A0>`, and the branch was then not moved back. `git_out` is not changed, as #710's ruling took Option 2. The readers of `origin`'s URL in `shape-doctor.py` and in the advisory it reads take its `UnicodeDecodeError` to mean the URL names nothing (#385). `adopt-project.py`'s `_default_branch` decodes locally since #717 (#425). L4-124, as Brett Heap ruled at 2026-10-17T13:01:07Z ("go with (a) for L4-124"): `_argument_fault` also faults a character `os.fsencode` cannot write (`UNWRITABLE`). The leg is left as it is, under a line that says `did not try branch`. - The fault is asked last. A name that also holds a surrogate is faulted for the surrogate, which `validate-manifest.py` names too (#280). - The second line calls the name good only where `git check-ref-format --branch`, asked in the root, takes the name's UTF-8 bytes; in a leg whose `.git` file names a git dir that is gone it exits 128. The line then names the encoding and gives `LC_ALL=C.UTF-8` and `PYTHONUTF8=1`. - Where git refuses the name (`feature/日本..x`, `日本.lock`, `日本 x`), the line gives git's words and #455's way out. L4-125's own route (a leg on `a<ff>b`, bootstrapped twice) and L4-126 (the manifest's `"a\udcffb"` followed back to `main`) already exit 0 on main through #745's `_head_of`. L4-126 is answered by option (b), the root cause, as R455 recommended. The new tests pin both. No pinned copy or recorded digest names `bootstrap.py` in the repository's tests, as #644 and #745 found when they changed it, so nothing is re-pinned. `tests/test_bootstrap_non_utf8_branch.py` runs each case on the harness of `tests/test_bootstrap_after_bump.py`. It holds the rule on every platform, under an emulated `ascii` `os.fsencode` and a legacy-mode Windows `os` stand-in, and skips where pytest itself is not under UTF-8. `tests/test_bootstrap_branch_bytes.py`'s no-fault test skips the rows this system's encoding cannot write. RV824 (lane 3) reviewed it, and found it READY after two fix rounds. Closes #710 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Lane: openreposhape-4 (openRepoShape-4)
Claim: #455 (comment)
What changed and why
Step (a) of
templates/assembly-root/scripts/bootstrap.pyhands each leg's branch to git as an argument:--branch, elseproject.yaml'stracking_branch, elsemain, and a leg's ownbranch:over all three.subprocesscannot give a command a NUL (ValueError: embedded null byte) or a lone surrogate the system cannot encode (UnicodeEncodeError), and_try_gitcatchesRefusaland nothing else. So each of the issue's four rows (tracking_branchof either, a leg'sbranch:of either) endedmake bootstrap(exit 2) in a traceback fromsubprocess.py, with(a) legs on tracking branchesprinted and nothing after it.validate-manifest.pyread neither key and saidmanifest ok, exit 0, so no tool named the value. It is #391's sibling: that value ended the run in aprint, this one in an argument.Both halves of the issue's expected outcome, in one change, because each alone leaves a gap: a finding alone is never reached by
make bootstrap, which still died in step (a); a step (a) that survives alone reaches a validator that saysmanifest ok.validate-manifest.py._tracking_branch_findingand_leg_branch_findings(both over_branch_finding) give atracking_branchand a leg'sbranchthat holds a NUL or any surrogate (U+D800 to U+DFFF) a finding in the style of #280'smanifest-name. Measured on main8f453d0efirst: both saidmanifest ok, exit 0, for all four rows.bootstrap.py._argument_fault(branch)is asked incheckout_tracking_branch, after theNOT INITIALIZEDtest and before any command is built from the branch. A leg whose branch holds a NUL, or a lone surrogate that has no byte, is not touched and says so:The run then reaches step (b), the manifest validator names the key, and the run ends
bootstrap FAILED: manifest (exit 1), as it ends for any other finding.make bootstrapexits 2, make's own code. Under--skip-validatorsthe line is the whole story and the exit is 0, as it is for a branch git refuses. A plainmainprints exactly what it printed (the golden oftest_bootstrap_spelling.pypasses unchanged).Decisions the issue left open.
_try_gitcatching(Refusal, UnicodeError, ValueError). AUnicodeDecodeError, whichgit_outraises over output that is not UTF-8, is aValueErrortoo, so that catch would read a leg whose HEAD is on such a branch as detached. And on WindowsCreateProcesstakes wide characters, so a lone surrogate raises nothing and git would be handed it mangled. The question is whether the VALUE can be an argument, so the value is asked.os.fsencodewrites each back as the byte it stands for, so--branchwith such a byte still creates its branch, as on main. Whereos.fsencodewrites three bytes for every surrogate (surrogatepass, Windows) none is a byte and all are refused. The answer isos.fsencode's own, not a list kept here.namedoes (validate-manifest.py accepts a display name holding a lone surrogate (name: "a\ud83db") and then raisesUnicodeEncodeErrorprinting it, somake validateand shape-doctor.py fail on a traceback instead of a finding while bump-leg.py and update-shape.pyapplypass the same project once #254 lands #280). A UTF-8 file holds none.a..b,x.lock), which isgit check-ref-format's and what scaffold-project.py holds--tracking-branchonly tochecked_value's alphabet, so a name git refuses as a branch (~,a..b,x.lock, a trailing/) passes--dry-runand, against GitHub, dies at the firstgit init -baftergh repo createhas made all three repositories #283 asks of--tracking-branchat scaffold time; and a value that is not text (2026,true), whichbootstrap.pyalready turns into text.--branchfinding: the flag is not in the manifest, and step (a)'s line is the only place it is named.Pinned shape copies.
templates/assembly-root/scripts/bootstrap.pyandtemplates/assembly-root/scripts/validate-manifest.pyare copies a project pins by digest; both show asupstream-changedto a project'supdate-shape.py check, which is how a project takes the fix.scripts/repo_shape.py, the other pinned copies,templates/family-root/and every doc are untouched: no sentence of README, AGENTS.md, the handbook ordocs/cli.mdis made false, and no--helptext moved. No recorded digest or blob in this repository changes (tests/test_repo_hygiene.pypasses). The family root'sbootstrap.pyreads no branch, andsiblings.pyalready refuses atracking_branchthat does not print (#543).Tests
New
tests/test_bootstrap_branch_bytes.py, 54 tests ontest_bootstrap_spelling.py's harness (theprojectfixture, itsrunandedit):validate-manifest.pyalone (exit 1, the finding spelled out in full, nothing on stdout, no traceback),bootstrap.py(the line on each leg, the other leg still placed, the legs' HEAD and branches unchanged,bootstrap FAILED: manifest (exit 1)),--skip-validators(exit 0, nothing moved) andmake bootstrap(exit 2, no traceback), fortracking_branchand for each leg'sbranch:;release/1.2,feature/日本, a zero-width joiner,é, the\Uescape of an emoji, the numbers2026andtrue;gitfirst on PATH that logs every command:mainis its control, and a branch no command can be given shows no command that names a branch (POSIX only);--branchwith the byte 0xff still creates its branch (skipped on a disk that takes no such name);_argument_faultover a NUL, each kind of surrogate, the first fault named, and everything that is not one, with Windows'os.fsencodeemulated.Runs, over the exact bytes of the pushed head (
733f8530):4cf5fb07: 24 files, 2461 passed, 11 skipped, exit 0, in 21 min: this file,test_bootstrap_spelling,test_bootstrap_after_bump,test_scaffold_e2e,test_manifest_validator,test_validator_child_encoding,test_naming_policy,test_load_yaml_refusals,test_doctor_validator_rows,test_doctor_validator_verdicts,test_shape_doctor,test_doctor_recorded_answers,test_doctor_next_is_a_command,test_leg_path_yaml_scalars, the threetest_template_*,test_scaffold_manifest_values,test_scaffold_leg_path_collisions,test_adopt_manifest_values,test_tracking_branch_ref,test_repo_hygiene,test_cli_reference,test_agents_md_codes. The 11 skips are the PowerShell line (Windows host only, 3) and 8 intest_leg_path_yaml_scalars(empty parameter sets, and rules with nothing to refuse because the templates quote).64e0dcee: 45 files (every file namingtracking_branchor--tracking-branch, plus the files above), 3710 passed, 31 skipped, exit 0, in 34 min. The 31 skips: a case-insensitive disk (1), a template that quotes the placeholder (19 intest_family_init_values), the 8 intest_leg_path_yaml_scalarsabove, and the Windows PowerShell line (3). None is in the new file.executerecords the plan's answers underanswered_paths:and a moved submodule names its row, so an answeredexamples/is not asked again and a deliberate move names no plan (#525) #617's head (acebd025) before it landed: this file,test_manifest_validator,test_validator_child_encoding,test_bootstrap_spelling: 416 passed.Mutants, each one change to the pushed code, new file only, each run then restored byte-for-byte:
--branch, which main already carried)checkout_tracking_branchremoved_tracking_branch_findingnot called_leg_branch_findingsnot calledos.fsencodecannot encode (Windows' three bytes pass)What it cannot tell
os.fsencodewrites three bytes for each) is reasoned and held by a test that emulatessurrogatepass; whatCreateProcessdoes with a surrogate is untested here.日本as a branch under a Latin-1 locale), still ends step (a) inUnicodeEncodeError. It is not what the issue names.adopt-project.py checkof a plan whosetracking_branch:holds either value was not measured:scaffold-project.pyrefuses such a--tracking-branchfirst (scaffold-project.py holds--tracking-branchonly tochecked_value's alphabet, so a name git refuses as a branch (~,a..b,x.lock, a trailing/) passes--dry-runand, against GitHub, dies at the firstgit init -baftergh repo createhas made all three repositories #283), and the new finding has no row inshape_materialize.FINDING_PLACEHOLDER, so it would be worded as "the project's own validate-manifest.py refuses" without a flag.update-shape.py. Until then its own validator saysmanifest okfor the value, and its own bootstrap ends in the traceback.Shared files
Read with
git ls-remotejust before the push, merge-tree of the pushed head (733f8530, which already contains main4cf5fb07and so lane 1's #617, landed as59e5a07):4cf5fb07: clean, exit 0 (tree55079882). The branch's diff against main is this commit's three files only.neutral_product_pins:or a leg'snaming.referent_chain:that is not a list isvalidate-manifest.py's finding for that field in every gate that reads it, so5no longer endsmake validatein aTypeErrorand a scalaropenXdoxis no longer read as its characters (#443) #631fix/443-validators-non-list,97335473: clean, exit 0 (tree51e1dde5). It changes_declared_pinsinvalidate-manifest.py; this change adds functions before_shape_findings, a docstring bullet and twoout.extendlines in_findings.pipx install openreposhapeis the second way in and the first on native Windows, git-filter-repo is installed in install.sh's order and never with a barepip install, and a first draft of the v1.1.0 release notes (#563) #630l3-563-docs,82f9403d: conflicts in.github/workflows/tests.yml,README.mdanddocs/handbook.html; the same three conflict between main4cf5fb07and README, AGENTS.md, docs/cli.md and the handbook:pipx install openreposhapeis the second way in and the first on native Windows, git-filter-repo is installed in install.sh's order and never with a barepip install, and a first draft of the v1.1.0 release notes (#563) #630 alone, and it changes neither of this branch's two templates against its merge base. Not this branch's.executerecords the plan's answers underanswered_paths:and a moved submodule names its row, so an answeredexamples/is not asked again and a deliberate move names no plan (#525) #617acebd025, before it landed: clean; this branch merged4cf5fb07with--no-ff, no conflict, no hand resolution.Closes #455
🤖 Generated with Claude Code
Summary by Sourcery
Handle branch values that cannot be passed to commands safely and report them through manifest validation instead of allowing bootstrap to fail with a traceback.
Bug Fixes:
tracking_branchand leg-levelbranchvalues so validation identifies the offending key and character.Enhancements:
Tests: