Repository navigation
shape-doctor.py: every validator child starts with -S, so no .pth in the user's or a virtualenv's site-packages stands in for the repo_shape.py a root lost, and the manifest row is the gate's own ModuleNotFoundError (#342) - #636
Merged
Merged
Conversation
… in the user's or a virtualenv's site-packages stands in for the `repo_shape.py` a root lost, and the `manifest` row is the gate's own `ModuleNotFoundError` (#342) #306 took the operator's `PYTHONPATH` out of the child that runs a root's own validator, so a root that lost `scripts/repo_shape.py` is told what its gate says in CI. `PYTHONPATH` is one route onto `sys.path`, though, and `site` is another: at start-up it appends every directory a `.pth` file names, in the user's site-packages and in the interpreter's own. The child was `[sys.executable, script, ...]` with `site` on, so a `.pth` naming the standard's `scripts/` turned the lost-file root's `manifest` row into `ok ... passes`, and its `pins` and `family` rows into a `shape-copy-missing` finding, which those gates reach only by importing a `repo_shape` the root lacks. All three are rows CI's gate never gives. `run_validator` now starts every child as `[sys.executable, "-S", script, ...]`. Measured on main 8f453d0 with a scaffolded root and the doctor's argv changed one flag at a time: - no flag: a user-site `.pth` and a virtualenv `.pth` both give `ok`; - `-s`: the virtualenv `.pth` still gives `ok`; - `-I`: the virtualenv `.pth` still gives `ok`, the template fallback is CANNOT ANSWER because `-E` drops its `PYTHONPATH`, and the root's own copy writes `scripts/__pycache__` into the root because `-E` drops `PYTHONDONTWRITEBYTECODE`; - `-S`: the lost-file root is a crash FINDING on all four routes (clean, user site, virtualenv, exported `PYTHONPATH`). A root that keeps its `repo_shape.py` passes on all four, and the template fallback passes on all four with its `<shape>/scripts`. `naming` is ok throughout, and the `machine` row's reason and report are the same lines in all 48 runs. ONE RULE FOR EVERY CHILD, as the `env` block is: the own copy, the template copy, both naming rows and the machine's preflight. Each is a shipped file, or a root's pinned copy of one, that imports only the standard library and this standard's own modules (`test_shipped_code_imports_only_the_standard_ library`; PyYAML appears nowhere). The project's own validate.yml runs its gate on a fresh setup-python with no `pip install` ("Standard library only"), so nothing on a `site` path is anything the gate can import in CI either. Debian's `dist-packages` leave the child; the standard library stays (`/usr/lib/python3.12` and `lib-dynload`, measured). The preflight reaches `git` and `gh` as programs and the interpreter through `sys`. The printed NEXT is unchanged: it is the project's own command, with no `-S`. Under #306's decision, the row is about the gate, not the paste, and `run_validator`'s "WHICH AGREEMENT THE ROW KEEPS" paragraph now names site-packages beside the reader's `PYTHONPATH`. As measured, `-S` also stops coverage's `.pth` hook, which with `COVERAGE_PROCESS_START` exported left four `.coverage.*` files in the root on main. tests/test_doctor_validator_site.py holds the issue's two routes for the `manifest`, `pins` and `family` gates, a root that keeps its own `repo_shape.py` under both, and the child itself in process (`no_site`, no site-packages on its path, its own directory first, the template's `PYTHONPATH` kept) for each kind of caller. Closes #342 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer's GuideThe PR hardens doctor validation against false passes caused by Sequence diagram for isolated validator executionsequenceDiagram
participant Doctor
participant Child as Validator child
participant Root as Root validator
participant SitePackages as Site-packages
Doctor->>Child: subprocess.run(sys.executable, -S, script, args)
Note over Child,SitePackages: -S disables site initialization and .pth loading
Child->>Root: import repo_shape
alt Root copy exists
Root-->>Child: Import succeeds
Child-->>Doctor: Validator passes
else Root copy is missing
Root-->>Child: ModuleNotFoundError
Child-->>Doctor: Import finding
end
Flow diagram for validator outcomes after -Sflowchart TD
Start[Doctor starts validator child] --> Isolated[Start with python -S]
Isolated --> OwnCopy{Root has repo_shape.py?}
OwnCopy -->|Yes| Pass[Validator runs and passes]
OwnCopy -->|No| ImportFinding[ModuleNotFoundError finding]
Isolated --> Template[Template validator retains its PYTHONPATH]
Template --> TemplatePass[Template validator runs]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This was referenced Oct 16, 2026
|
brettheap
marked this pull request as ready for review
October 16, 2026 11:12
|
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 9 hours and 6 minutes by commenting |
brettheap
added a commit
that referenced
this pull request
Oct 16, 2026
…h, the `switch -c` remedy says to give it another name, or go back only if it holds work not merged yet, and the docs say `family/add-<Project>` (#607) (#637) ### Fix round 1 (rv607) Review rv607 of 6212c31 said CHANGES NEEDED: 0 blockers, 2 should, 7 nits. This round is one new commit, 66b1043, on top of 6212c31. No amend, and main is not merged in. Each point: - **S1, the sentence.** It sent a person back to the old branch first. That is right only where the branch holds work not merged yet. `BRANCH_LEFT` now puts another name first and going back second (the text is under "L2-46" below). It is word for word the same in `scripts/family.py`, in the pinned `validate-family.py` and in the pinned test constant, and the comments that described the old order are rewritten. Two new tests type the printed line in the states the review probed: `main` has moved since the earlier act, and a commit was made on a detached HEAD. - **S2, a guard for every `_switch_line` remedy.** A new test reads the validator's source with `ast` and fails any remedy that names `_switch_line` whose prose does not end with `BRANCH_LEFT`. On a merge of this head with #634 it fails at `_lockstep`, which is the point: whichever of #637 and #634 lands second adds the sentence there. #634's lines are not on this branch and are not touched. - **N1.** Under "Left out". - **N2.** The commit body of 6212c31 said neither tool can print the one line that works by asking first. That holds for the validator only. `family.py` could ask; it says the validator's words so the two tools keep one rule. Decision 1 says so, and the new commit's body corrects the old one. - **N3.** The merge table is refreshed: current origin/main, #620, #622, #626, #630 to #636, #638, #639 (lane 5's #611) and #634 (S2). - **N4.** Decision 1 no longer says the typed line works "in every shell". It says what a person types is "the printed line with one word changed (the branch name, or `-c` taken out)", so it keeps the printed quoting, and that the tests type both changes as printed. - **N5.** Decision 3 now says the case of the branch name "does not matter to `add`", and adds the note about case-insensitive disks. - **N6.** The `init` NEXT STEPS test read the lines with a regex whose `--family-root \S+` breaks on a root with a space in it (`C:\Users\Jane Doe`, or any `tmp_path` with a space). It now reads the words with `host_words`, as the NEXT STEPS test beside it does. Run with a space in the base temp directory, the old test fails with `AttributeError: 'NoneType' object has no attribute 'group'` and the new one passes. - **N7.** "Three existing tests take the new text" was wrong. It is two test functions plus `GITLINK_REMEDY`, a constant four cases share. Fixed under "Tests". The one new test that passes on BASE is disclosed there too. ### Fix round 2 (rv607b) The re-verify rv607b of 66b1043 said APPROVE WITH NITS: 0 blockers, 0 should, 5 nits. This round is one new commit, a277695, on top of 66b1043. No amend, and main is not merged in. It changes comments and docstrings only, so no behaviour moves and no mutation row changes. - **Nit 1.** Two comments called another name "right in every state". It is not: where the old branch holds work not merged yet, another name leaves that work behind. The comment above `BRANCH_LEFT` in `scripts/family.py` and the one in the pinned `validate-family.py` now say a new branch starts where the holder stands now, "which works in every state". The three `BRANCH_LEFT` texts are unchanged and still equal. - **Nit 3.** The guard's docstring said it finds each `_switch_line` call "however it is passed on". A switch line moved into a variable would defeat that. It now says "wherever the call stands inside the string expression that is passed on". - **Nit 4.** The N4 line above now quotes decision 1 as written. - **Nit 5, the docstring half.** The docstring of `test_another_name_works_where_main_has_moved_since` said going back to the old branch conflicts, but the test does not assert that. It now says the test shows the other name works, and that the conflict on the old branch is shown by rv607's probe and decision 2's table. No assertion is added. - **Nit 2, left as written.** The guard's control counts stay: each case fails loudly with a clear message. Checks on a277695: the diff to 66b1043 adds ten lines and removes seven, in three files, all comments and docstrings, and the `ast` of each file is equal once docstrings are blanked. The four files `test_family_switch_branch_exists.py`, `test_validate_family_branch_step.py`, `test_validate_family_unnamed_mounts.py` and `test_family_tracking_branch.py` give `96 passed in 51.34s`, and `test_repo_hygiene.py` gives `65 passed in 1.39s`. ### L2-46, the fix `family.py add`, `bump` and `remove` refuse the holder's tracking branch with `git -C <holder> switch -c family/<act>-<member>` (#487). The holder's `validate-family.py` names the same line before each `add` and `remove` it prints (#539, landed as 54041cc). If an earlier act left that branch, the line fails with `fatal: a branch named 'family/add-atlas' already exists` (exit 128), and neither tool said what to do next. Both tools now add one sentence, in the same words (`BRANCH_LEFT` in each file): > 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. - `scripts/family.py`: the sentence comes on its own line, right under the `switch -c` line, in the `family-on-tracking-branch` and `family-holder-detached` refusals (`_checked_branch`), and in the dry run's `branch` lines (`_branch_said`). - `templates/family-root/scripts/validate-family.py`: the sentence ends the remedy text of `member-gitlink-absent`, `member-duplicate-id` and `member-gitlink-unnamed`, the three remedies that print `_switch_line`. The command lines under them are unchanged. ### L2-45, rides along AGENTS.md, the holder's README, its AGENTS-shape.md and `init`'s NEXT STEPS spelled the branch `family/add-<project>`, right beside `--member <org>/<Project>`. All four now say `family/add-<Project>`, the one placeholder the validator already uses for both (`PROJECT_PLACEHOLDER`, #539). **1. What the remedy prints when the branch exists: one sentence that names the way out, not a second command line.** - *One line that works in both states* (`git switch X 2>/dev/null || git switch -c X`): rejected. On Windows, `family.py` quotes its lines for PowerShell (`shell_line` uses `os.name`; README.md:155-158 says so). Windows PowerShell 5.1 has no `||`; `&&` and `||` arrived in PowerShell 7, and `family.py`'s own `_print_next_steps` docstring says 5.1 rejects `&&`. PowerShell also has no `/dev/null`. And `2>/dev/null` would hide the real reason a `switch` fails, such as a changed file it would overwrite. - *The tool checks and prints the one line that works*: rejected for the validator. Its report is read in CI: the holder's `validate.yml` runs `python3 scripts/validate-family.py` (and `--no-members`) in a runner's checkout, which holds none of the person's branches. `family.py` could check, but then the two tools would follow two rules, and one rule in the same words was the aim. So `family.py` says the validator's words; it is not that it cannot ask. - *Two printed lines, one per state*: rejected. In the validator, each indented line is a step to run in order, and going back to the branch replaces step one; it is not another step. In `family.py`, a new `git switch` line would also be quoted for PowerShell on Windows. README.md:155-158 lists exactly which `family.py` lines are quoted that way (`NEXT`, `switch -c`, `add`, `git rm -r --cached`). That sentence would become false, and fixing it would mean editing README § "Starting a project in a new organisation". Seven handbook parts record that section's hash (`grep -c` gives 7). - *Chosen*: a sentence that names both ways out. It prints no new command, so nothing new needs quoting, and the text is the same in both tools. What a person types is the printed line with one word changed (the branch name, or `-c` taken out), so it keeps the printed quoting. The tests type both changes as printed (tests below); none of them runs a shell, and Windows and macOS are not checked (see "Left out"). **2. Another name first, going back second (S1).** The reviewer's probes, and mine, in real local holders (a holder `Orbit`, members `Atlas` and `Beacon`, local bare remotes, the `gh` stub first on PATH): | State of the old branch `family/add-atlas` | `git switch family/add-atlas` (going back) | `git switch -c family/add-atlas-2` (another name) | |---|---|---| | No commit on it, `main` has not moved (rv607) | works | works | | Holds a commit of its own, not merged yet (mine: `probe-atlas.sh`, `probe-b.sh`) | works; `add` then commits on top of it, and the earlier commit is the parent | works, but the earlier commit stays only on the old branch (`git branch --contains` names `family/add-atlas` alone) | | `main` has moved: a later member (`Beacon`) was squash-merged (mine: `probe-d.sh`; test `test_another_name_works_where_main_has_moved_since`) | `add` exits 0, but `git merge-tree --write-tree main family/add-atlas` exits 1 (`CONFLICT (add/add)` in `.gitmodules`, `CONFLICT (content)` in `family.yaml`), and an untracked `members/Beacon` is left (`warning: unable to rmdir 'members/Beacon'`) | `add` exits 0, `git merge-tree --write-tree main family/add-atlas-2` exits 0, and the validator says `2 member(s)` | | Merged, then the member removed and that merged too; the old branch is still local (rv607's probe, and mine: `probe-c.sh`) | `add` says `member-already-present`, whose remedy points at `bump` and `remove`: a dead end | `add` exits 0, once the object store `remove` leaves behind is deleted, as its NOTE says (either choice needs that) | | Detached HEAD with a commit made on it (mine: `probe-detached.sh`; test `test_another_name_keeps_a_commit_made_on_a_detached_head`) | `Warning: you are leaving 1 commit behind, not connected to any of your branches`, and `git branch --contains` names no branch | the commit stays: `git branch --contains` names the new branch | So going back is right only where the branch holds work not merged yet, and another name is right in every state but that one. The sentence says so, in that order. **3. The person's earlier work on that branch is never discarded.** `git switch <branch>` keeps the branch's commits. It refuses to switch when that would overwrite a changed file. The act then commits on top of what the branch holds; the tests check that the earlier commit is the parent of the act's commit. "Give the branch another name" leaves the old branch alone. No printed line names `switch -C` (which resets a branch), `branch -D`, `reset` or `--discard-changes`; a test checks that too. **4. The placeholder: `<Project>` everywhere.** - The validator already said `family/add-<Project>` beside `--member <org>/<Project>` (#539's fix round, pinned by `tests/test_validate_family_unnamed_mounts.py`). It is the only spelling that is one placeholder for both values. - The other option, a sentence saying the branch word is the name in lowercase, does not fit: AGENTS.md is at its cap, 492 of 492 lines (`tests/test_repo_hygiene.py:633`). This change keeps it at 492. - The case of the branch name does not matter to `add`. `_checked_branch` takes any branch but the tracking one, so `family/add-IRQS`, the doc line filled in as written, is a branch `add` commits on (test). Where the tools know the name, they still print it lowercased (`_suggested_branch`). On a case-insensitive disk (macOS, Windows) `family/add-Atlas` and `family/add-atlas` can be one loose ref there. That is untested: CI is Linux only. **5. Pinned copies.** This changes two pinned holder files: `templates/family-root/scripts/validate-family.py` and `templates/family-root/AGENTS-shape.md` (`scripts/shape_materialize.py` lists the pinned set). Each holder takes them as two `upstream-changed` rows on its next `update-shape.py` run. The holder's README is rendered once and is not pinned. No test pins those files' bytes; the tests that compare them read the template at run time. Fix round 1 changes only `validate-family.py` of the two. New file `tests/test_family_switch_branch_exists.py`, 25 tests (21 before the fix round). Four are new in the fix round: - `test_another_name_works_where_main_has_moved_since` (S1, state (d)): `family/add-irqs` is left at the `main` of its day; a later member (`IRXS`, scaffolded in the test) is added on a branch and squash-merged onto `main`, so `main` has one new commit; the refusal's `switch -c`, typed as printed, fails with `already exists` (exit 128); the same line with another name works; `add` exits 0, `git merge-tree --write-tree main <branch>` exits 0, and the validator passes. - `test_another_name_keeps_a_commit_made_on_a_detached_head` (S1): the refusal is `family-holder-detached`; after the printed line with another name, `git branch --contains <the commit>` names the new branch and no other. - `test_every_remedy_that_names_a_switch_line_ends_with_the_sentence` (S2, the guard) and `test_the_guard_sees_a_remedy_left_without_the_sentence` (its control: the source with the sentence taken out of any one remedy fails it, and so does a fourth remedy built by a helper as `_lockstep` builds its, until the sentence ends it). The other 21: - `test_the_refusal_typed_as_printed_puts_the_act_on_its_branch` [add, bump, remove] x [no-branch, branch-left]: the refusal, then its line typed as printed. With no branch, the `switch -c` makes one. With a branch an earlier act left, holding a commit of its own, the `switch -c` fails with `fatal: a branch named '<branch>' already exists` (exit 128) and changes nothing, and the same command with `switch` in place of `switch -c` goes back to that branch. The act then commits on top of the earlier commit, and NEXT pushes that branch. - `test_a_dry_run_on_the_tracking_branch_says_it_too` [add, bump, remove]. - `test_a_missing_gitlinks_remedy_typed_as_printed`, `test_a_duplicate_ids_remedy_typed_as_printed` (with the `remove` run as printed) and `test_an_unnamed_gitlinks_remedy_typed_as_printed`, each in both states. - `test_the_two_tools_say_the_same_words`. - `test_each_doc_spells_the_branch_as_its_member` [AGENTS.md, AGENTS-shape.md, holder README], `test_init_next_steps_spell_the_branch_as_its_member` (read with `host_words` now, N6), and `test_the_branch_filled_in_as_written_is_one_add_commits_on`. Two existing test functions take the new text: `test_init_next_steps_make_the_branch_before_the_add` (`<Project>`) and `test_a_missing_gitlink_remedy_makes_the_branch_and_names_the_holder` (the remedy line). `GITLINK_REMEDY` in `test_validate_family_unnamed_mounts.py`, a constant four cases share (`test_a_member_mounted_by_hand_is_two_findings` x 3 and `test_a_removed_row_is_named_by_its_gitlink_and_its_section`), ends with the sentence. The sentence is pinned once, as text, in `tests/test_validate_family_branch_step.py`. Before and after, with the four changed test files copied in (`run9.sh`, the `gh` stub first on PATH): - On BASE 396c0ff: `30 failed, 66 passed` in the four files. 24 of the 25 new-module tests fail. The one that passes, `test_the_branch_filled_in_as_written_is_one_add_commits_on`, is the evidence for decision 4: it shows behaviour this PR keeps. - On 6212c31: `23 failed, 56 passed` in the three files that pin the sentence. Both new S1 tests fail there, where they assert the new wording. The two guard tests pass there: they check where the sentence stands, not its words. - On the head 66b1043: the new module alone, `25 passed`. ### Targeted runs on the head 66b1043, BASE 396c0ff - Set A, 29 files: every `tests/test_family*.py` and `tests/test_validate_family*.py`, plus `test_bump_leg.py`, `test_bump_leg_dry_run_tracking.py`, `test_bump_leg_dry_run_truth.py`, `test_bump_leg_paste_lines.py` and `test_update_shape_branch_any_directory.py`. Result: `1016 passed, 20 skipped in 511.01s`. - From set B: `test_repo_hygiene.py`, `test_agents_md_codes.py`, `test_readme_sections.py` and `test_cli_reference.py`. Result: `210 passed in 15.73s`. The fix round changes no template other than `validate-family.py`'s text, so the rest of set B (the 27 files of the first round) is not re-run. - Every skip is a platform skip that was there before this change (a case-ignoring disk, a Windows default, powershell.exe), or a template-quoting skip in `test_family_init_values.py`. A real holder `Orbit`, made with `family.py init`, and a member `Atlas`, scaffolded with `scaffold-project.py --local-remote-dir`. - **On BASE 396c0ff** (first round): `add` refuses with `switch -c family/add-atlas`. Typed as printed, that gives `fatal: a branch named 'family/add-atlas' already exists`, exit 128, and the remedy says nothing more. The validator's `member-gitlink-absent` remedy prints the same line, with the same result. - **On the head, an unmerged act** (`probe-atlas.sh`, a branch holding a commit of its own): the refusal prints the new sentence. The same command with `switch` for `switch -c` gives `Switched to branch 'family/add-atlas'`, exit 0, at the earlier commit. `add` then exits 0, its NEXT pushes `family/add-atlas`, and its commit's parent is the earlier commit. The validator carries the same sentence. - **On the head, `main` moved** (`probe-d.sh`): the printed line fails (128). With another name: `add` exits 0, `git merge-tree --write-tree main family/add-atlas-2` exits 0, `family ok: Orbit (orbit), 2 member(s)`. Going back instead: `add` exits 0, and `git merge-tree --write-tree main family/add-atlas` exits 1 with the two conflicts in decision 2. - **On the head, a detached HEAD with a commit** (`probe-detached.sh`): the printed line fails (128). With another name, `git branch --contains` names `family/add-atlas-2`. Going back with `switch`: git warns it is leaving 1 commit behind, and no branch contains it. No row needs real GitHub. Each change below was made alone in a scratch clone of the head, then the new module was run (and, for the validator changes, the two other files that pin the sentence), and the file was put back. Every change that alters behaviour makes at least one new test fail: | Change | Result | |---|---| | `family.py` `BRANCH_LEFT` back to the first round's words | 12 failed: the six refusal cases, the three dry-run cases, both new S1 tests, `test_the_two_tools_say_the_same_words` | | `family.py` `BRANCH_LEFT` one clause dropped (`only if`) | 12 failed, the same tests | | `_checked_branch` remedy with no sentence | 8 failed: the six refusal cases and both new S1 tests | | sentence printed above the `switch -c` line, not under it | 8 failed: the same eight | | `_branch_said` (dry run) with no sentence | 3 failed: `test_a_dry_run_on_the_tracking_branch_says_it_too` [add, bump, remove] | | `_switch_command` prints `switch -C` | 11 failed, both new S1 tests among them: the printed line no longer fails `already exists` | | validator `BRANCH_LEFT` back to the first round's words | 12 failed in the three files, `test_the_two_tools_say_the_same_words` among them | | validator `member-gitlink-absent` with no sentence | 5 failed: its two typed-as-printed cases, the remedy test, the guard and its control | | validator `member-duplicate-id` with no sentence | 4 failed: its two typed-as-printed cases, the guard and its control | | validator `member-gitlink-unnamed` with no sentence | 8 failed: its two typed-as-printed cases, the four `GITLINK_REMEDY` cases, the guard and its control | | **S2:** a fourth remedy added that names `_switch_line` and does not end with the sentence (built by a helper, as #634's `_lockstep` does) | 2 failed: the guard and its control, and nothing else in the three files | | **S2:** the same fourth remedy ending with `{BRANCH_LEFT}` | 79 passed | | `init` NEXT STEPS branch back to `<project>`; its `--member` lowercased | 1 failed each: `test_init_next_steps_spell_the_branch_as_its_member` | | AGENTS.md, AGENTS-shape.md, holder README: branch back to `<project>` (and AGENTS.md's `--member`) | 1 failed each: `test_each_doc_spells_the_branch_as_its_member` for that doc | The other hunks change only docstrings and comments (`_checked_branch`, `_branch_said`, `_print_next_steps`, `_switch_line`, and the comments above the two constants). They change no behaviour, so no test can catch reverting them. | Ref | Result | |---|---| | origin/main f6376d7 (#614 landed; main has moved since BASE through #625, #583, #596) | clean, tree eb91b600. A committed merge of the head with it, e26dca8c, ran set A and #625's `test_family_bump_to_local_only.py`: `1024 passed, 20 skipped in 500.52s` | | #620 a2b4f89 | clean (it conflicted in `adopt-project.py` in the first round; it no longer does) | | #622 227bc3b (lane 2, #609, `scripts/family.py`) | clean | | #626 207d4e1 (lane 4, `scripts/family.py`) | clean | | #630 82f9403, #631 9733547, #632 86c6b9c, #633 f7f5de5, #635 2c0b157, #636 4aade9f | clean | | #638 64e6621 | clean | | #634 40a85e1 (lane 2, #610: `_lockstep` prints a fourth `switch -c`) | clean, and S2 fires on it. A committed merge, 4d1a7161: the new module, `test_validate_family_branch_step.py` and #634's `test_pinned_copy_text.py` give `2 failed, 86 passed`. The two failures are the guard, `assert [124] == []` (line 124 of the merged file is `_lockstep`'s return), and its control. With one line added to `_lockstep`, `f"moving one alone is what this refuses. {BRANCH_LEFT}"` in place of the plain string, the same merge gives `306 passed` over the nine files that read the validator or the lockstep rule (`test_validate_family*.py`, `test_family.py`, `test_family_tracking_branch.py`, `test_pinned_copy_text.py` and the new module). So the second PR to land needs that one line and nothing else. | | #639 6f938ed (lane 5, #611) | conflicts in the same one sentence, in the same two places: the end of the `member-gitlink-unnamed` remedy text in `validate-family.py` (`_check_gitlinks_named`), and the `GITLINK_REMEDY` constant in `tests/test_validate_family_unnamed_mounts.py`. #611 adds "and commit that removal, since `family.py add` refuses ..." and this PR appends `BRANCH_LEFT`. Resolved as in the first round, keeping #611's clause and appending ` {BRANCH_LEFT}` in the validator and `+ BRANCH_LEFT` in the test constant, in a committed merge (92602308): `scripts/family.py` merged cleanly, and the six test files that either PR changes or pins gave `157 passed`. Neither branch was resolved. | - No compound `||` line and no state check: see decision 1. - No README.md edit: no README sentence becomes false, because no new kind of line is printed. - The docs do not mention the exists case. AGENTS.md is at its cap, and the tools print the sentence where the failure happens. - The `switch -c` in `init`'s NEXT STEPS gets no sentence: a holder `init` just made has no branch an earlier act could have left. - **A branch checked out in another worktree (N1).** Where `family/add-atlas` is checked out in another worktree of the holder, `git switch family/add-atlas` gives `fatal: 'family/add-atlas' is already used by worktree at '<path>'` (exit 128), so the going-back half of the sentence does not work there. Git names the worktree, and another name works (`probe-n1.sh`: `switch -c family/add-atlas-2` exits 0). The sentence is not made longer for it. - I could not run Windows PowerShell or macOS here. The tests read `family.py`'s lines with `host_words`, which parses PowerShell's quoting on Windows. They make the earlier branch with `commit-tree` and `update-ref`, move `main` with the same two, write no file with `write_text` and compare nothing CRLF-sensitive. `test_another_name_works_where_main_has_moved_since` runs `git merge-tree --write-tree`, which needs git 2.38 or newer. No CI job checks Windows or macOS. - **bump-leg.py: the `git switch -c bump/<role>-<sha12>` remedy fails `already exists` when an earlier bump left that branch.** `_switch_command` (scripts/bump-leg.py:1124) prints the line for the two refusals and the dry run's closing hint. Like #607, it says nothing about a branch that is already there. - **setup-project.py: the family `git -C <holder> switch -c family/add-<project>` line has the same gap.** The NEXT lines a scaffold into a family folder ends with (setup-project.py:2358) print it with no word on an existing branch. It matters only when an earlier attempt left that branch. - L2-46 ([#459 comment](#459 (comment))) - L2-45 ([#459 comment](#459 (comment))) Review: rv607 found 0 blockers, 2 should-fix and 7 nits on 6212c31 (CHANGES NEEDED), fixed in 66b1043; the re-verify rv607b found 5 nits on 66b1043 (APPROVE WITH NITS), of which nits 1, 3 and 4 and the docstring half of nit 5 are fixed in a277695, and nit 2 is left. CI (Brett Heap's ruling 27, with his fast-refresh ruling of 2026-10-15T22:48:35Z): H0 a277695 passed tests (Linux), tests-git-bash and SonarCloud; the head 62263d2 is a clean update-branch merge of main a524542 (over e041867, a clean merge of 5b29ee1), with SonarCloud, tests-git-bash and package green on it, and the local gate passed on the head itself: the family and validator set, 1016 passed (20 skipped), and the hygiene, AGENTS.md-code, README-section and CLI-reference sets, 222 passed. Landed on Brett Heap's word of 2026-10-16T11:21:49Z, "Merge #637 when its turn comes". Closes #607 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
… verification row's colon, `run_validator`'s "costs no child an import" and the decoy test's `sitecustomize` claim say what main does (#666) (#730) Four comments and docstrings said something main no longer does. They now say what the code beneath them does. A fix round after review added two more of the same kind (see "Fix round"). Comments and docstrings only: no behaviour, no `--help` text, no pinned copy, so no README, handbook or `docs/cli.md` change. - **`adopt-project.py`, the module docstring.** It said the verification table reads `code: 0 of N source paths (seeded from template)`. The printed row has no colon (` {leg:<6} {count:>5} of {N} source paths{note}`). It now says the table's `code` row reads `0 of N source paths (seeded from template)`, the words `AGENTS.md` and `README.md` use since #505 and the docstring of the verification function uses too. `git grep 'code: 0 of'` finds nothing now. - **`shape-doctor.py`, `submodule_config`.** The last sentence of the docstring gave a reason that is true of the string and not decisive: "It is also what the row PRINTS, and a lone surrogate is a string no UTF-8 stream can write." The table goes through `ascii_text` and `--json` through `json.dumps`, so neither would raise on a lone surrogate. I dropped the sentence, the issue's first direction. The comparison with `origin_url`'s reading of a clone's remote is the reason, and the paragraph already gives it first. **This hunk carries the bytes of lane 3's #700**, which landed as `55c6ba7d`: the two removed lines and the one added line (` working clone.`) are the same. So the hunk is on main already and drops out of the squash (see "Evidence"). - **`shape-doctor.py`, `run_validator`.** "It costs no child an import" holds for an unedited copy only. A new paragraph says a root whose own copy of a validator gained an `import yaml` read `passes` while `site` was on, and now reads `crashed before reaching a verdict (exit 1): ModuleNotFoundError: No module named 'yaml'`, the crash its own CI gets, because `validate.yml` installs nothing. It calls that row a fair answer and says `-S` does not ask whether a copy was edited. No code change. - **`tests/test_shape_doctor.py`, the decoy test's two comments** (the block above `DECOY_REPO_SHAPE` and the inline comment before `reached`). `run_validator` starts every child as `[sys.executable, "-S", script, ...]`, so no child imports a `sitecustomize`. The comments now say that, say that replace-not-prepend and the #306 review S1 case are held by `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given`, and say that the decoy test holds the template row's real verdict under an exported `PYTHONPATH`. The review of the first two commits approved with nits. Two new commits take S1 and N1: `7f2bb36b` makes both edits, and `426877bf` rewrites the last sentence of the N1 comment, which did not parse ("...a future removal of `-S`, which `test_run_validator_starts_every_child_with_no_site` ... is the test that holds"). It now ends "...a future removal of `-S`; `test_run_validator_starts_every_child_with_no_site` (in `tests/test_doctor_validator_site.py`) is the test that holds `-S`." Both are comments and docstrings only. - **S1, a third stale sentence.** The docstring of `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given` said that for the callers which pass no `pythonpath`, "only a probe like this one, or (b)'s decoy, can see that they have none". That is false under `-S`: a `run_validator` that keeps the variable for those callers passes the decoy test in all three cases and fails the probe test twice (the second row of the table below). It now says only a probe like this one can see that they have none, and that the decoy of (b) could before `-S` (#342). It is the same defect as the issue's, in a place the issue did not name. "Could" holds for the `manifest` and `pins` cases: with `-S` removed, 2 of the 3 decoy cases fail, and the `family` case never caught it. - **N1, the inline comment overstated the loop.** The comment above `reached` said "What this loop still holds is that no row quotes the decoy or says it exited 86." Under `-S` the loop cannot fire. It now says so, says the loop is kept as a tripwire for a future removal of `-S`, and names `test_run_validator_starts_every_child_with_no_site` in `tests/test_doctor_validator_site.py` as the test that holds `-S`. That test is on main. - **N2, this body.** The merge checks and the notes on #630 were stale (they cited `de926142` and a conflict that is gone). They are rerun and rewritten below. **Only comments and docstrings changed.** `git diff 8062810 426877b` has 74 changed lines (44 added, 30 removed) in three files. A script checked each changed line against the file's comment tokens and docstring spans (both sides of the diff): 0 outside. A second check parsed each file at the base and at the head, blanked every docstring, and compared the ASTs: identical in all three files. For the two fix-round commits together, the AST of `tests/test_shape_doctor.py` with docstrings blanked is identical to the one at `a2113cff`, `git diff --check` is clean, and the longest added line is 75 columns (73 in the second commit). **What the squash holds.** `git merge-tree --write-tree` of this head with origin/main `20c8873b` gives the tree `6ead2ffc`. `git diff 20c8873 6ead2ffc` is 71 changed lines in three files, and `shape-doctor.py` has 8 of them: the new `run_validator` paragraph and nothing else. `git diff` of the two trees has no hunk that mentions `submodule_config`, `surrogate`, `PRINTS` or `working clone`. On main, `submodule_config`'s docstring has no "PRINTS" sentence and ends in `working clone.`. The same two checks on that tree: 0 lines outside comments and docstrings, ASTs identical. On main `2d988200` with the head `426877bf` the squash is the same: 71 changed lines, 8 of them in `shape-doctor.py`, 0 outside comments and docstrings, ASTs identical, and no hunk that mentions `submodule_config`. **`python3 scripts/render-cli-reference.py --check`**, at `426877bf`: `[ok] docs/cli.md is what the generator produces`. Only the first line of a module docstring is help text, and none of the first lines changed. **The decoy comments, measured.** I did not take the issue's reading on trust. In a throwaway clone at the base I put each #306 regression into `run_validator` and ran the two tests (`test_an_exported_pythonpath_does_not_reach_a_template_validator`, three cases, and `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given`, six): | mutant | with `-S` (main) | with `-S` removed | |---|---|---| | the standard's `scripts/` put in FRONT of the operator's value | decoy test: 3 of 3 pass. Probe test: 1 of 6 fails | decoy test: 3 of 3 fail | | the variable kept for the callers that pass none (S1) | decoy test: 3 of 3 pass. Probe test: 2 of 6 fail | decoy test: 2 of 3 fail | | the operator's directory AHEAD of the standard's | decoy test: 3 of 3 fail. Probe test: 1 of 6 fails | not run | So on main the decoy test catches neither #306 regression and the probe test catches both, as lane 4's R342 measured on #636's head. The third row is why the comment says the raising `repo_shape.py` "still bites": it kills an operator-first order. The review re-ran these mutants on `a2113cff` and got the same counts. **The `run_validator` paragraph, measured.** A root whose own `validate-manifest.py` is `import yaml` plus a print, run through `run_validator` from this tree: exit 1 and `ModuleNotFoundError: No module named 'yaml'`. The same call with `-S` taken out of the child's argv: exit 0, `manifest ok`. `yaml` is importable from the machine's site-packages and not without `site`. With the real `gh` unreachable (a stub first on PATH, a bogus token, an empty `GH_CONFIG_DIR`). On `a2113cff`, before the fix round: - `tests/test_cli_reference.py`, `tests/test_repo_hygiene.py`, `tests/test_agents_md_codes.py`, `tests/test_scaffold_leg_path_collisions.py`: 250 passed (57.5 s). - `tests/test_doctor_gitmodules_not_utf8.py`, `tests/test_doctor_validator_rows.py`, `tests/test_doctor_validator_site.py`, `tests/test_doctor_validator_verdicts.py`, `tests/test_adopt_spec_only.py`, `tests/test_adopt_entry_paths.py`, `tests/test_adopt_submodules.py` (the files that drive `run_validator`, `submodule_config` or the seeded-leg row): 334 passed (28 min 18 s). - `tests/test_shape_doctor.py`, in two halves run side by side (the first 204 test ids, then the last 204): 204 passed in 11 min 31 s, and 201 passed, 3 skipped in 4 min 58 s. That is all 408 tests. The 3 skips are the PowerShell-line tests, which need `powershell.exe` on a Windows host. - The review ran all of `tests/test_shape_doctor.py` there: 405 passed, 3 skipped (512 s). On the predicted merge with main `2fac9790` it ran `test_shape_doctor.py`, `test_doctor_validator_site.py`, `test_doctor_validator_rows.py`, `test_doctor_validator_verdicts.py`, `test_doctor_gitmodules_not_utf8.py`, `test_repo_hygiene.py`, `test_cli_reference.py` and `test_agents_md_codes.py`: 632 passed, 3 skipped (760 s). On `426877bf`, after the fix round, which changes comments and docstrings only, so the runs above stand: - The two tests the fix round touches, `test_an_exported_pythonpath_does_not_reach_a_template_validator` (3 cases) and `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given` (6 cases), run by node id: 9 passed (11.7 s). The same 9 also passed (16.1 s) at `7f2bb36b`. - `tests/test_repo_hygiene.py`: 77 passed (1.9 s). It also passed (2.0 s) at `7f2bb36b`. No test fails on the base, because nothing here changes behaviour. The AST comparison above is the proof in their place. `git merge-tree --write-tree` of this head against origin/main and against every ref on origin that could meet it. The full run was on 2026-10-16, with the head `7f2bb36b` (the one before the last comment commit) and main `20c8873b`: the 41 branches (`fix/*`, `l3-*`, `wip/*`, `docs/*` and `release/v1.0`) and the 55 PR heads from #630 up. That is 96 refs. I ran #731's head `ab55c29f` through the same list. After `426877bf` I re-ran main, now `2d988200`, and the six heads that edit `tests/test_shape_doctor.py`, the one file that commit touches (#705, #737, #740, `l3-669-members-row`, `l3-680-doctor-signals` and `l3-697-crash-cure-next`), and #630: all clean. - **origin/main: clean.** At `20c8873b`, with the head `7f2bb36b`, the predicted tree is `6ead2ffc`. At `2d988200` (three `adopt-project.py` commits later, with their tests and a change in `scripts/shape_materialize.py`), with the head `426877bf`, it is `7ee5caf4`, and the squash is the same three files (43 added, 28 removed). #731's head `ab55c29f` is clean with `2d988200` too. This head with #731's head: clean. - **#630's head `6fd33a93`: clean.** It conflicted with main in `docs/handbook.html` when this body was first drafted, and no longer does, with or without this change. - **#700 and #702 have landed** (`55c6ba7d` and `20c8873b`), and so has #701, and #699, #717 and #719 for `adopt-project.py`. Their heads still merge clean with this head. - **Every ref that edits `shape-doctor.py`, `tests/test_shape_doctor.py` or `adopt-project.py` merges clean with this head**, except the two old heads named in the bullet after next. They include the heads of PRs not on main yet (#705, #721, #737 and #742, and lane 3's `l3-680`, `l3-683`, `l3-687`, `l3-691` and `l3-697`, which were not on origin when this body was first drafted). #673 has no branch or PR head on origin. - **Four refs conflict with origin/main on their own**, and so with this head: `docs/release-notes-v1.0.0` and `release/v1.0`, and the pre-squash heads of the landed #637 and #641. Their conflicts do not come from this change. - **Two landed pre-squash heads conflict with this head only because they are old.** #632 conflicts with main on its own. #636 merges clean with main, but its old head conflicts with this head in `run_validator`'s docstring, next to the text #636 added; main has that text, and this head merges clean with main. - **Refs that conflict with main on their own are not made worse.** #682, #745, #747, `fix/648`, `fix/681` and `fix/704` conflict with main in `templates/family-root/scripts/validate-family.py`; #742 and `l3-683` in `shape-doctor.py`. Each merges clean with this head. - **Every other open head and branch: clean.** Nothing from the issue. Its four lines are done. The decoy test's `sitecustomize` machinery is dead code under `-S` and I left it, because the issue says comment only. The comment above `reached` now says it is kept as a tripwire. - **The decoy test's `sitecustomize.py` and exit-86 trap never run.** `DECOY_SITECUSTOMIZE`, `os._exit(86)` and the `(exit 86)` and `DECOY` clauses in `reached` can only fire for a child started without `-S`. They are kept as a tripwire and the comment says so. The other choice is to remove them; `test_run_validator_starts_every_child_with_no_site` already holds `-S`. Small, tests only. Review: rv666, lane 5's independent review, approved a2113cf with nits; 7f2bb36 and 426877b take its S1 and N1 (above), and codex found no major issue at a2113cf. CI (Brett Heap's ruling 27 and his fast-refresh ruling of 2026-10-15T22:48:35Z): H0 426877b passed tests, tests-git-bash and SonarCloud; the head 32722b3 is a clean update-branch merge of main bf1f099, with tests-git-bash and SonarCloud green; and the local gate passed on its predicted squash tree 55d5d3b over main dcf62ed: the shape_doctor, scaffold_leg_path_collisions, repo_hygiene, agents_md_codes, cli_reference sets, 686 passed, 3 skipped, 1 xfailed. Landed on Brett Heap's word of 2026-10-17T10:59:39Z, relayed by lane 3 on #473: "merge lane 5's approved v1.1.0 drafts". Closes #666 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 17, 2026
…d A3 (three SHOULDs, four nits), and notes entries for the seven pull requests landed since bf1f099, current to main 2000488 (#563, #683) Round A4, from lane 2's review (#630 issuecomment-6233033833, READY): - SHOULD 1: the doctor's exit-3 list names "a validator of its own that crashed or could not be run", so the case of one that cannot be given its `PYTHONPATH` (shape-doctor.py's EXIT CODES, #278) is listed too: README.md:1208-1222, its handbook twin (`doctor`) and the notes' docs line. The paragraph re-wraps to one more line: tests/readme-cap.d/683.txt is +2, and its text now says which causes the EXIT CODES block names and that #742 added the partial clone and #701 the naming policy. README.md is 1475 lines, its cap (1422 + 2 + 5 + 12 + 22 + 10 + 2). Both `Keeping a project's shape current` markers move to c728c970: `doctor` text and marker, `current` marker only. - SHOULD 2: #727's entry names the leg description, whose em dash stopped every `execute` against GitHub at the first leg. - SHOULD 3: #749's entry names `source-ambiguous`, for an `org/repo` that is also a directory here. - Nits: the doctor section's three near-duplicate pairs are merged (#636 with #737, #632 with #700 and #705, #586 with #736); #718 has its own bullet; #634's entry names `git_out`'s spelling for every caller and #744's the gate's `--link-source-label` and `--link-remedy`; the header comment says 2026-10-17 at main 2000488. Wave B1, `git log --first-parent bf1f099..2000488`: #706 (#664), #703 (#696), #670 (#646), #682 (#648), #726 (#667), #730 (#666) and #724 (#665), each from its squash commit; #726's and #682's from lane 5's lines (#630 issuecomment-6227129879), #682's with the unmerged `.gitmodules` its last round added, and #724's with lane 5's note that v1.0.0's test-hygiene line on the generator's `.pyc` files closes. The counts (106 pull requests; 116 of 135 milestone issues closed at 11:53Z), "current to main" and the open list (19) are at main 2000488, still placeholders. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 17, 2026
… in command scope and in a global config that includes the person's own, so a `copytree` of a fixture no longer meets a detached repack deleting its loose objects (#645) (#713) **What changed.** One session fixture in `tests/conftest.py`, `no_auto_maintenance` (autouse), and one new test file, `tests/test_no_auto_maintenance.py`. No shipped file and no other test file changes. Four commits: the fix and its tests (e2110f8), a comment (accc7d2), and the fix round below (8cfc8eb, and 979bb39, which only shares a test helper). The fixture sets `gc.auto=0` and `maintenance.auto=false` for every git the session starts, in two scopes: - **Command scope** (`GIT_CONFIG_COUNT`), appended after the entries the environment already holds (the Windows job holds two). Every git the suite starts, every git a tool under test starts, and every git a `git submodule` starts inherits it. It outranks every config file. - **A global config file** (`GIT_CONFIG_GLOBAL`, in the basetemp). It includes the global config git would read without it (the file `GIT_CONFIG_GLOBAL` already named, or `$XDG_CONFIG_HOME/git/config` and then `~/.gitconfig`) and sets the two keys after it. This one is for the `receive-pack` a push starts in a local remote: git starts it with `GIT_CONFIG_COUNT` and `GIT_CONFIG_PARAMETERS` removed, so command scope never reaches it. Where `HOME` is unset the `~/` paths are left out, and an empty `GIT_CONFIG_GLOBAL` includes nothing (fix round, below). A comment beside #440's note in `clone_just_pushed` says why, names both sites, and says what is left. **Why.** `shutil.copytree` reads a repository's `objects/` one file at a time. The suite copies a repository a fixture has just committed in, fetched into or pushed to in 139 places. Each `git commit`, `merge` and `fetch`, and the `receive-pack` in a remote after a push, runs `git maintenance run --auto` as it finishes. From git's source: - git 2.46: the auto run stays in the foreground. - git 2.47: it is started with `--detach`, takes `objects/maintenance.lock` and then detaches, so the command returns while it runs. - git 2.52 and 2.53: the default strategy is still `gc`, which does nothing below 6700 loose objects. - git 2.54: the default strategy is `geometric`. Its repack runs when packs are out of geometric order, or once its estimate, taken from `objects/17` alone, is above 256 loose objects, so two loose objects there are enough (`maintenance.geometric-repack.auto`, 100, rounded up). It deletes the loose objects it packed. So on a current git, a copy taken just after a commit can find a loose object gone. It happened twice on the Linux runner: - `holder` (`tests/test_family.py:129`), in `test_family_member_name.py` (lane 1's report on #473); - `the_standard_moved` (`tests/test_doctor_pin_restore_merges.py:99`), a copy of the session's `standard` with its `.git`, in `test_copies_neither_side_edited_are_taken_whole_with_the_pin` on #623's run 38895011713: `shutil.Error ... standard5/openRepoShape/.git/objects/1e ... [Errno 2]` (lane 2's #473 comment 6215422231). The same function copies `moved` at :102 just after a commit in it. Both are covered by the same session setting: the commits before each copy run under `os.environ` (`commit_all` passes `{**os.environ, ...}`). A listing of `.git/objects` taken before and after such a repack also differs: lane 3's `objects changed` in `tests/test_family_bump_remove_rollback.py` (#736's Linux run 38981191418), 37 loose objects packed into two. The same setting covers it (i645 on #645). **Decisions and evidence.** - *Option A (the suite's environment), not B or C.* - B (config in each copied repository) would have to reach repositories the tools under test create themselves, such as the bare remotes `scaffold-project.py --local-remote-dir` makes and pushes into before a test sees them. A config written after that first push is too late. - C (a copy that survives a repack) cannot be made safe. A copy can list `objects/pack/` before the new pack lands, and then a loose-object directory after it was emptied. That copy succeeds and is missing objects. - A covers all 139 places from one place. - *Two scopes, not one.* Probe on git 2.43, pushing into a local bare remote with `receive.denyDeletes=true` given each way: - with `GIT_CONFIG_COUNT`, the branch was deleted; - with `-c`, deleted; - through a `file://` URL with `GIT_CONFIG_COUNT`, deleted; - with `GIT_CONFIG_GLOBAL`, `remote: error: denying ref deletion` (refused). So command scope alone misses the remotes, and ten copies are of bare remotes. A global file alone would be dropped wherever a test gives git a global file of its own (14 test files set `GIT_CONFIG_GLOBAL`, five more through `excluding()`). No test file replaces both scopes and also copies a repository. rv645, the reviewer, repeated the `receive.denyDeletes` probe and got the same answers, and instrumented the git calls of the batches it finished before the checkpoint: the only receive-packs that ran without both settings were in `test_family_add_excludes.py` and `test_adopt_clone_template.py`, neither of which copies a remote. - *Which verbs start auto maintenance* (git 2.43 trace2): `commit`, `fetch`, `pull` and `submodule update --remote` (its fetch in the submodule) start `git maintenance run --auto`. A push's `receive-pack` starts `git gc --auto --quiet`. `clone`, `submodule add` and `submodule update --init` start none. - *Behaviour no test checks is unchanged.* The person's own global config is still read: it is included, and it comes first. A test of that is part of the new file. The tests that read or set a global config (`test_git_helper_identity.py`, `test_adopt_git_reads.py`, `test_tracking_branch_ref.py` and others) set their own, which replaces the session's. **Fix round (8cfc8eb), on rv645's review** (no MUST; two SHOULD, five NIT). - **SHOULD 1: with `HOME` unset every git was fatal.** The file's include of `~/.config/git/config` and `~/.gitconfig` made git exit 128, `error: could not expand include path '~/.config/git/config'`, in every command the session started. The estate runs with `HOME` set, so nothing failed here; an `env -i` or hermetic runner would have lost the whole suite. - `session_global_config` now writes a `~/` path only where `home_is_known()` (`HOME` is set, or `os.name == "nt"`, where Git for Windows finds a home itself), and writes no `[include]` header when nothing is included. An `XDG_CONFIG_HOME` that is set is still included, because it needs no `HOME`. - The two `test_a_persons_own_global_config...` cases skip where `HOME` was unset. They skip on the environment and not on the file's bytes, so a fixture that dropped its includes still fails them (probe M5 below). - A new test, `test_where_home_is_unset_the_file_is_one_git_can_read` (with and without `XDG_CONFIG_HOME`), reads the file back through `git config --global --includes`. It is skipped on Windows. It and the empty-value test share one helper, `probe_read_from_the_session_file`, so the new tests do not repeat a body for SonarCloud's duplication gate. - Measured under `env -u HOME -u XDG_CONFIG_HOME`, `run9.sh`, git 2.43.0: | file | before (accc7d2) | after (8cfc8eb) | |---|---|---| | `tests/test_clone_just_pushed.py` | 1 passed, 6 errors | 7 passed | | `tests/test_no_auto_maintenance.py` | 3 failed, 2 passed, 2 errors (7 tests) | 9 passed, 2 skipped (11 tests) | | `tests/test_family_member_name.py` | not run | 13 passed | The 2 skips are the two `test_a_persons_own_global_config...` cases. - Where `HOME` is set and the file is written as before, nothing changes: across 27 combinations of `HOME` (set, unset, empty), `XDG_CONFIG_HOME` (set, unset, empty) and `GIT_CONFIG_GLOBAL` (unset, a file, empty), the file's bytes are the same as accc7d2's in the 12 where `GIT_CONFIG_GLOBAL` names a file or `HOME` is set with `GIT_CONFIG_GLOBAL` unset, and differ in the 15 where `HOME` is not set or `GIT_CONFIG_GLOBAL` is empty. - **SHOULD 2: nothing guarded the fixture's scope.** At function scope all seven tests passed (rv645's probe), and the session fixtures `scaffolded` and `family` would have been built before the setting existed. A new test, `test_the_settings_are_in_place_before_a_session_fixture_builds_a_repo`, takes a session fixture as its FIRST argument, which reads `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_COUNT` as that fixture sees them, and compares them with what the test sees and with the fixture's file. Pytest builds the higher scope first, and then an autouse fixture, so the fixture is only in place early enough if it is session scope and autouse. - **NIT 1: the receive-pack test failed under a person's global `core.hooksPath`**, which hid the remote's own `post-receive` hook (`FileNotFoundError` on `read-by-receive-pack`). The test now sets `core.hooksPath` to the remote's own `hooks` directory in the remote's local config, which outranks a global one. Measured with `GIT_CONFIG_GLOBAL` naming a file that holds `core.hooksPath`: before, `1 failed, 4 passed, 2 skipped`; after, `9 passed, 2 skipped` (11 tests). - **NIT 2: an empty `GIT_CONFIG_GLOBAL=` was read as unset.** Git 2.43 reads it as "no global config" (checked here: with `HOME` holding a `.gitconfig` probe, `GIT_CONFIG_GLOBAL=` does not read it). The fixture now includes nothing for an empty value: `if named is not None: included = [...] if named else []`. A new test, `test_an_empty_GIT_CONFIG_GLOBAL_is_no_global_config_and_includes_nothing`, fails on the old `if named:`. - **NITs 3 to 5: no code change.** - NIT 3: `test_family_dry_run.py`'s own `NO_MAINTENANCE` is redundant now. It lives in another test file that this PR leaves alone, and it is harmless. - NIT 4: a `git config --global` read without `--includes` sees only the session's two keys. The only reader is `setup-project.py:1105`, behind `os.name == "nt"`, and no test reads it. `_credential_helper_configured` uses `--get-all` with no scope, so it still sees the person's config. The conftest comment already says so. - NIT 5: the version facts in the conftest comment ("2.47 detaches", "2.54 default geometric") are from git's source tags and are not run here: only git 2.43.0 is installed. The comment is unchanged. **Tests** (`run9.sh`: gh stub first on PATH, identity blanked; git 2.43.0, Python 3.12, pytest 9.1.1). - On the fix round (8cfc8eb; 979bb39 differs from it in the new test file alone, and the batches that read that file were run on it): - `tests/test_no_auto_maintenance.py`: 11 passed (7 from the first commit, 4 new: the `HOME` test twice, the empty value, the scope). - `tests/test_no_auto_maintenance.py`, `tests/test_clone_just_pushed.py`, `tests/test_repo_hygiene.py` and `tests/test_git_helper_identity.py` together, on 979bb39: 105 passed. With `tests/test_family_dry_run.py` added, on 8cfc8eb: 166 passed. - `tests/test_adopt_git_reads.py`, `tests/test_tracking_branch_ref.py`, `tests/test_adopt_late_failure.py`, `tests/test_adopt_github_exits.py` and `tests/test_family_add_excludes.py` (the tests that read or set a global config or a hooks path): 314 passed. - On the first two commits (e2110f8 for the first four, accc7d2 for the rest; the second commit changes comment lines only). The fix round changes nothing where `HOME` is set and `GIT_CONFIG_GLOBAL` is not empty (the byte comparison above), so these were not run again: - Family, `tests/test_family*.py` (20 files): 823 passed, 19 skipped. - Adopt, `tests/test_adopt*.py` (46 files): 3463 passed, 2 skipped. - Doctor, `tests/test_shape_doctor.py` and `tests/test_doctor*.py` (12 files): 741 passed, 3 skipped. - Scaffold, `tests/test_scaffold*.py`: 634 passed. - Update-shape and bump-leg, `tests/test_update_shape*.py` and `tests/test_bump_leg*.py` (24 files): 857 passed, 4 skipped. - The rest of the files that copy a repository (`test_bootstrap_after_bump.py`, `test_non_list_project_fields.py`, `test_pin_source_env_collisions.py`, `test_pin_source_unreadable.py`, `test_shape_pin_files_not_a_list.py`, `test_validator_child_encoding.py`, `test_siblings_family_name.py`, `test_bootstrap_spelling.py`, `test_non_utf8_git_reads.py`), with `test_repo_hygiene.py`, `test_git_helper_identity.py`, `test_clone_just_pushed.py` and the new file: 924 passed. - On a merge of accc7d2 with main c6122e8: `test_no_auto_maintenance.py`, `test_git_helper_identity.py`, `test_repo_hygiene.py`, `test_clone_just_pushed.py` and `test_family_member_name.py`, 114 passed. **Reproductions.** - *The mechanism, on git 2.43.* A repository of 300 files plus two loose objects named `17…`, committed under `gc.auto=1`, then copied at once with `shutil.copytree`. This is what git 2.54 does to such a repository by default. With the base's settings, 10 of 10 copies failed, then 9 of 10 (`[Errno 2]` on a loose object directory or file). With the fix's two entries appended after it, 0 of 10 failed. - *The emulation of the race* (rv645's recipe, run on 8cfc8eb: a global `gc.auto=1` that includes the person's `~/.gitconfig`, plus a `core.hooksPath` pre-commit hook that leaves two `17…` objects and drops `gc.log`, so git 2.43 repacks in the background after each commit; `GIT_TRACE2_EVENT` over the whole run). Base figures are rv645's, on a524542; head figures are the fix round's own, on 8cfc8eb: | file | git | auto-maintenance children | repack runs | result | |---|---|---|---|---| | `test_doctor_pin_restore_merges.py` | base | 78 `maintenance run --auto`, 77 `gc --auto` | 65 | 1 failed, 14 passed, 1 error | | | head | 4 `gc --auto` (from `receive-pack`, inert under `gc.auto=0`) | 0 | 16 passed | | `test_family_member_name.py` | base | 19 `maintenance run --auto`, 29 `gc --auto` | 15 | 13 passed | | | head | 14 `gc --auto` (inert) | 0 | 13 passed | On the base, rv645 reproduced the symptom itself: `shutil.Error ... moved/openRepoShape/.git/objects/pack/tmp_rev_...`, at `the_standard_moved`'s copy of `moved` (`tests/test_doctor_pin_restore_merges.py:102`). On head, 0 maintenance runs and 0 repacks in both files. - *The first writer's runs on the issue's file* (`test_family_member_name.py`, base then head, 19 maintenance and 15 repacks against 0 and 0) agree. **Mutation probes.** Each reverts one hunk of the fix in a throwaway clone; the new file then fails. First round (against the first seven tests): | probe | fails | |---|---| | fixture sets nothing | 6 of 7 (all but the append test) | | no command scope | `test_a_commit_a_fetch_and_a_merge_start_no_maintenance[a-tests-own-global]` | | no global file | the receive-pack test, both person's-config cases | | count overwritten, not appended | the append test | | global file without the includes | both person's-config cases | | the two keys before the includes | both person's-config cases | | no `maintenance.auto` | 5 tests | | no `gc.auto` | the receive-pack test, the append test | Fix round (against the eleven tests): | probe | result | what fails | |---|---|---| | S1, `no_auto_maintenance` at function scope | 1 failed, 10 passed | the scope test, alone (the first seven passed in rv645's run of the same mutant) | | S2, not autouse | 4 failed, 7 passed | the two commit/fetch/merge cases, the tool test, the receive-pack test | | H1, `home_is_known()` always true | 2 failed | `test_where_home_is_unset_the_file_is_one_git_can_read`, both variants | | H2, the XDG include needs `HOME` | 1 failed | the same test, the `xdg` variant | | E1, an empty value is read as unset (`if named:`) | 1 failed | the empty-value test | | M5, the file includes nothing (the first round's probe, again) | 3 failed, 8 passed | both `test_a_persons_own_global_config...` cases, and the `xdg` variant of the `HOME` test | | K1, the receive-pack test without its own `core.hooksPath`, under a global `core.hooksPath` | 1 failed, 8 passed, 2 skipped | the receive-pack test | S2 is caught by tests that ask for the setting by itself; the scope test does not catch it, because earlier tests in the file build the fixture on request. K1 only fails where a global `core.hooksPath` is set, as NIT 1 says. On the base itself the new file does not import (`cannot import name 'NO_AUTO_MAINTENANCE'`). **Merge checks** (`git merge-tree --write-tree` on 979bb39): clean against origin/main 2fac979 and against lane 5's `fix/662-insteadof-helper` (4149cbb; #662 is stacked on accc7d2 and also edits `tests/conftest.py`: the fix round's hunks are in `session_global_config`, its helper `home_is_known`, and the comment above them). Earlier, on e2110f8 and accc7d2: clean against origin/main d4a7fab and c6122e8, #621, #622, #630, #631, #634, #635, #636, #637, #638 and #639, and the sibling branches for #646, #647, #648 and #649. **What it leaves.** - A git whose environment drops both scopes (a `GIT_CONFIG_COUNT` overwritten, not appended to, together with a test's own `GIT_CONFIG_GLOBAL`, or an environment built from nothing), and a push made under a test's own global file. No repository the suite copies is made or pushed to that way today. - A git older than 2.32 ignores `GIT_CONFIG_GLOBAL`. Its `receive-pack` runs `gc --auto`, which does not detach below 6700 loose objects. - `clone_just_pushed` keeps reading through git. A test with its own global file still needs it. - `test_family_dry_run.py`'s own `NO_MAINTENANCE` is now redundant, and is left alone (NIT 3). - Not run: a real Windows job (none runs the full suite since #597, and the `HOME` test is skipped on Windows), and a git newer than 2.43. **Copies found** (`git grep -n 'shutil.copytree(' -- tests`, 152 calls): - 127 copy a repository the suite built (non-bare, many holding `.git/modules`): `conftest.py` 1, `test_family.py` 18, `test_shape_doctor.py` 32, `test_doctor_legs_members_next.py` 8, the `test_update_shape*` files 30, and 38 more in 28 files. Four of them copy a subtree that holds no store: `standard / name` twice, and a member's work tree twice. - 10 copy a bare remote, or a folder of them: `test_bump_leg_paste_lines.py:95`, `test_doctor_unfinished_merge.py:166`, `test_family_add_member_id.py:239`, `test_family_add_refusals.py:487`, `:788` and `:1983`, `test_family_dry_run.py:1446`, `test_family_tracking_branch.py:279`, `test_update_shape_landing.py:156` and `test_update_shape_next_quoting.py:116`. - 2 copy a `.git` directory: `test_adopt_clone_template.py:190` and `test_scaffold_mount_ignored.py:381`. - 13 hold no repository: a template, or this checkout with `.git` ignored. - `test_adopt_plan.py:449` copies a clone named by `SHAPE_MEDXEHR_CLONE`. It is outside the suite and skipped without the variable. No shipped script copies a repository. **Checklist candidates** (rv645's; the coordinator places them): - The import-time git probe at `tests/test_family_refuse_up_front.py:75-96` runs in a throwaway directory before any session setting. It is harmless today. - Tests that overwrite `GIT_CONFIG_COUNT` (14 files) lose command scope and rely on the global file alone. That holds today, and nothing guards it. Review: rv645, lane 5's independent review, found no MUST, two SHOULDs and five NITs on accc7d2; the fix rounds above take them, and its re-verification approved 979bb39. CI (Brett Heap's ruling 27): H0 979bb39 passed tests, tests-git-bash and SonarCloud; the head a9f36ce is a clean update-branch merge of main bf1f099, with tests, tests-git-bash and SonarCloud green; and the local gate passed on its predicted squash tree 760892f over main 2000488: every test file that #706, #703 and lane 5's six round-1 PRs change, run under #713's conftest, with the two copytree-race files and the hygiene trio, 1332 passed, 3 skipped, 1 xfailed. Landed on Brett Heap's word of 2026-10-17T10:59:39Z, relayed by lane 3 on #473: "merge lane 5's approved v1.1.0 drafts". Closes #645 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 18, 2026
brettheap
added a commit
that referenced
this pull request
Oct 19, 2026
…under `-S` cannot start: the machine row's preflight must report its git, Python and bootstrap checks `[ok]` and the closing line of the exit it chose, and over a root `scaffold-project --local-remote-dir` makes on the runner's disk the naming, manifest and pins children must each reach a verdict (#894) (#945) `shape-doctor.py` starts every child as `python -S` (#342, #636), and the install check is the one run of the real doctor on macOS and Windows. It ran the doctor over an empty repository and asserted the exit, 2, and the machine row's `git-filter-repo` path, which the doctor finds itself. The row's child, `setup-project.py --preflight`, could die on an import under `-S` and the check still passed: it exits 1 then, as on a runner with no `gh` login. Measured on Linux at 5017830 with the release's own wheel built and installed the workflow's way: a `setup-project.py` importing `git_filter_repo`, which only `site` puts on the path, passes every step, `openRepoShape --preflight runs` included, and so does a template `validate-manifest.py` importing it. A module that does not exist at all already fails at `openRepoShape --preflight runs`, which runs the child without `-S`. - The empty repository's step prints the preflight child's exit and every line of its report, then requires the report to hold the closing line `preflight_only` prints for that exit: `[ok] this machine is ready.` for 0, `[!!] this machine is not ready yet (see above).` for 1. It holds with or without a `gh` login and fails for a child that reported no line, stopped part way, closed with the other exit's line, or refused. It also requires a line opening `[ok] git `, `[ok] python ` and `[ok] bootstrap runs as`, the three checks every runner passes, so one that failed quietly into `[!!]` under `-S`, leaving the exit at a runner's own 1, fails the step. Its problems are gathered and printed together, the `git-filter-repo` one included. - A new step makes a root with the installed `scaffold-project --org releasecheck --project Sample --local-remote-dir --work-dir`: bare repositories on the runner's disk, no network, no `gh`, no token (0 `gh` calls measured). The doctor over its working tree starts the naming script, `validate-manifest.py` and `validate-pins.py` under `-S`; each of the three rows must have run a child, and no row that ran one may carry `detail.crashed` or read "crashed before reaching a verdict". Every such row and the verdict are printed; compliance is not asserted, since a finding is a verdict the child reached. A `root.json` that is not JSON fails the step with one `::error::` naming the doctor's exit. Linux: 3 s to scaffold, 1 s for the doctor. tests/test_release_install_check_children.py runs each step's own Python over doctor reports in the shapes measured (twelve cases for the preflight, six for the root, three root reports that are no JSON), over this checkout's doctor's own report for a clean standard and for one whose `setup-project.py` dies on an import, and runs the step's own `scaffold-project` line through `scaffold-project.py`, then the doctor and the step, for a fresh root and for one whose own `validate-manifest.py` dies on an import. It holds both steps' place, their commands, the closing lines and the three `[ok]` openings against `setup-project.py`'s own, and that the new step names no token and runs no `gh`. Round 1 took RV945's four NITs (#945 issuecomment-6252748418): the three `[ok]` openings, the doctor's exit named for a `root.json` that is not JSON, and the body's notes. The rehearsal, dispatched by lane 3 on `27903fb4`: release run 39161129613 passed `build and verify` and all four install-check legs (ubuntu, macOS and Windows on Python 3.x, ubuntu on 3.9), with both publish jobs skipped. neg-b, one commit on it whose `setup-project.py` imports `git_filter_repo` and whose `release.yml` lacks only the build job's tests gate (`6b05273b`, branch `rehearsal/894-neg-b`): run 39162043099 passed `build and verify` and failed every install-check leg, as designed. Lane 3's RV945 round 2 found #945 READY at `27903fb4` (2026-10-19T11:56:09Z), MUST 1 met by that rehearsal. Closes #894 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: #342 (comment)
What changed and why
The defect. #306 took the operator's
PYTHONPATHout of the child that runs a root's own validator. Since then, a root that lostscripts/repo_shape.pyis told what its gate says in CI:ModuleNotFoundError, a FINDING. ButPYTHONPATHis one route ontosys.path, andsiteis another. At start-upsiteappends every directory a.pthfile names, in the user's site-packages and in the interpreter's own. The child was[sys.executable, script, ...]withsiteon. So a.pthnaming the standard'sscripts/stood in for the lost file:manifestrow saidok ... passes;pinsandfamilyrows quotedshape-copy-missing, a finding those gates reach only by importing arepo_shapethe root does not have.In CI, all three gates fail on the import instead. I measured main's three rows by running the new tests against main's argv (mutant M1 below); the issue had read the
familyrow, not run it.The fix is one argv token in
run_validator: every child now starts as[sys.executable, "-S", script, ...]. It keeps torun_validator, its docstring, and one sentence incheck_machine's docstring, the only caller whose docstring describes how its child is started.The options, measured. On main
8f453d0, I scaffolded a root against local bare remotes and ran the doctor withrun_validator's argv changed one flag at a time (--json, all 48 combinations below). There were three roots:repo_shape.pyis gone;There were four routes:
.pth:PYTHONUSERBASE;.pth: the doctor started by apython -m venv --without-pipinterpreter;PYTHONPATH: the standard'sscripts/exported.The
manifestrow, by flag:.pth.pthPYTHONPATHok ... passesok ... passes-sok ... passes-Iok ... passesscripts/__pycache__into the root.pthstands in-S-Iis-splus-E.-Edrops the template copy'sPYTHONPATHand thePYTHONDONTWRITEBYTECODEthe doctor sets so that it writes nothing.repo_shapefound outside the root, would answer forrepo_shapealone. It would leavesiterunning in every child, and it would be a new branch invalidator_rowrather than one token.-Sis the smallest change that makes the row true in every case. Thenamingrow is ok in all 48 runs, and themachinerow's reason anddetail.reportare the same lines under every flag.Why
-Scosts no child an import.run_validatorstarts is a shipped file, or a root's pinned copy of one: the three validators,validate-repository-naming.pyandsetup-project.py --preflight. Each imports only the standard library andrepo_shape, by an AST walk of each, andtest_shipped_code_imports_only_the_standard_libraryalready holds the shipped files to that. PyYAML appears nowhere;repo_shape.pyhas its own reader.siteinstalls (exit,quitand the like), and none starts a Python of its own.validate.ymlruns its gate on a freshsetup-pythonwith nopip install, under the comment "Standard library only. If this ever needs apip install, the change is wrong". So nothing on asitepath is anything the gate can import in CI either.-Skeeps the child's own directory, the standard library andPYTHONPATH. On this Debian hostsys.pathkeeps/usr/lib/python3.12andlib-dynloadand drops bothdist-packages, which hold onlya1_coverage.pthanddistutils-precedence.pth.gitandghas programs and the interpreter throughsys.One rule for every child, as the
envblock above it is: the own copy, the template copy, both naming rows and the preflight. That is the issue's decision 2, "every validator child for one rule". The flag does not ask which script it starts.The printed NEXT is unchanged (decision 3). It is the project's own command, with no
-S. Under #306's decision the row is about the gate, not the paste, andrun_validator's "WHICH AGREEMENT THE ROW KEEPS" paragraph now names a.pthin the reader's site-packages beside theirPYTHONPATH. Printing-Swould change the command a person is told to run, which needs a ruling.validator_command,docs/cli.mdand--helpare untouched.Also closed by the same flag, measured. With
COVERAGE_PROCESS_STARTexported, coverage'sa1_coverage.pthon this host made four of the doctor's children write.coverage.*files into the root on main. Under-Snone was written. This is the doctor's "writes nothing" promise; there is no test for it, because CI installs no coverage.Tests
The run (
-q -rs, test identities, frvenv first, one basetemp, 2026-10-15T23:11:34Z to 23:37:48Z): 1420 passed, 0 failed, 4 skipped in 1572.7 s, exit 0, overshape-doctor.py0f9a9868…(this commit's) andtests/test_doctor_validator_site.py0658af58…. That file then changed in one docstring only, to say what main'sfamilyrow read. The new file andtest_repo_hygiene.pywere run again on this commit'sedb87124…: 80 passed. The run covered 25 files:test_doctor_validator_site.py(new);run_validator:test_shape_doctor.py,test_doctor_gitlink_repairs.py,test_doctor_legs_members_next.py,test_doctor_machine_row.py,test_doctor_next_is_a_command.py,test_doctor_pin_restore_merges.py,test_doctor_unfinished_merge.py,test_doctor_validator_rows.py,test_doctor_validator_verdicts.py,test_family.py,test_gh_guard.py,test_openreposhape_command.py,test_pin_source_env_collisions.py,test_refusal_default_remediation.py,test_shape_advisory.py,test_shape_pin_files_not_a_list.py,test_stdout_encoding.py,test_update_shape_merged_away.py,test_update_shape_next_quoting.py,test_validate_pins_unreadable_workflow.py,test_validator_child_encoding.py;test_repo_hygiene.py,test_cli_reference.pyandtest_agents_md_codes.py.The four skips are Windows-only:
test_shape_doctor.py:9965, three times ("the PowerShell line is printed, and powershell.exe exists, only on a Windows host"), andtest_stdout_encoding.py:286("the default this is about is Windows' own"). Nothing in the new file skipped here: this interpreter reads a user site-packages, and the virtualenv was made.New,
tests/test_doctor_validator_site.py(15 tests, ontest_shape_doctor.py's fixtures). Each case is the issue's own input, a one-line.pthnaming the standard'sscripts/:.pth, for themanifest,pinsandfamilygates: a root with a valid manifest and its ownrepo_shape.pyremoved, and the doctor run withPYTHONUSERBASEpointing at it. Each row is a FINDING,own_copy, exit 1,crashed, quotingModuleNotFoundError: No module named 'repo_shape', with nopasses. The fixture first proves this interpreter importsrepo_shapethrough that.pth, and skips, with a reason, where it reads no user site-packages (a virtualenv runner)..pth, for the same three:python -m venv --without-pip, the.pthin itspurelib, and the doctor started by that interpreter.run_scriptstartssys.executable, so it starts a three-line hand-off that starts the venv interpreter with the environmentrun_scriptbuilt. The same assertions apply.repo_shape.py, three gates under each route: the row isthe project's own <gate> passes, exit 0.pythonpath), template copy (pythonpathgiven), and the machine's preflight (reach_network=True). The probe reportssys.flags.no_site == 1, nosite-packagesordist-packagesentry onsys.path, its own directory first, the standard library importable, and the template'sPYTHONPATHkept.The #306
PYTHONPATHcase keeps its own tests intest_shape_doctor.py, and they are in the run.Mutants, each one change to this head's
shape-doctor.py, run in a scratch clone over the new file, and for M3 and M4 also over namedtest_shape_doctor.pytests. All four are killed.-s: 6 failed, 9 passed. The three virtualenv tests and the three in-process tests fail.-I: 10 failed, 9 passed. The same six fail, plustest_the_doctor_writes_nothingandtest_a_template_validators_next_command_runs_when_it_is_pasted[family|manifest|pins].PYTHONPATHstill in the child's environment, so for an operator who exports one that reaches arepo_shape.pythemanifestrow of a root that lost its ownscripts/repo_shape.pysaysok ... passesagain, the false pass #238 removed, where the project's own gate in CI fails onModuleNotFoundError: No module named 'repo_shape'#306's prepend put back under-S(the template child'sPYTHONPATHset to the given directory followed by the operator's): 1 failed, 8 passed. The failure istest_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given[/operator/a:/operator/b-scripts-dir].test_an_exported_pythonpath_does_not_reach_a_template_validator[*]now passes under this mutant, because its decoy is asitecustomize.py, which-Snever imports. shape-doctor.py: after #238 the doctor runs a root's OWN validator with the operator'sPYTHONPATHstill in the child's environment, so for an operator who exports one that reaches arepo_shape.pythemanifestrow of a root that lost its ownscripts/repo_shape.pysaysok ... passesagain, the false pass #238 removed, where the project's own gate in CI fails onModuleNotFoundError: No module named 'repo_shape'#306's replace-not-prepend rule is therefore held by the in-process probe test alone (see below).What it cannot tell
.pthsees the pasted line pass where the row says FINDING, as shape-doctor.py: after #238 the doctor runs a root's OWN validator with the operator'sPYTHONPATHstill in the child's environment, so for an operator who exports one that reaches arepo_shape.pythemanifestrow of a root that lost its ownscripts/repo_shape.pysaysok ... passesagain, the false pass #238 removed, where the project's own gate in CI fails onModuleNotFoundError: No module named 'repo_shape'#306 accepted forPYTHONPATH. The row is about the gate.locally-modifieddrift thepinsrow already reports, and the project's own workflow installs nothing, so its CI fails it too.-Sexists on every supported version. Reasoned rather than measured: the standard library's entries onsys.pathcome from the interpreter's path calculation, not fromsite, on macOS and Windows alike, and a virtualenv finds its standard library throughpyvenv.cfg'shome.test_shape_doctor.py'sDECOY_SITECUSTOMIZEcomment says the decoysitecustomize.py"is what does the work". Under-Sno child imports one, so that test no longer tells shape-doctor.py: after #238 the doctor runs a root's OWN validator with the operator'sPYTHONPATHstill in the child's environment, so for an operator who exports one that reaches arepo_shape.pythemanifestrow of a root that lost its ownscripts/repo_shape.pysaysok ... passesagain, the false pass #238 removed, where the project's own gate in CI fails onModuleNotFoundError: No module named 'repo_shape'#306's replace from a prepend (M4). The in-processPYTHONPATHprobe still does. That file is not this PR's to edit.Shared files
shape-doctor.pyis the shared file. Heads were read withls-remoteat 2026-10-15T23:38:45Z, and everygit merge-tree --write-treeagainst head4aade9f4is clean (exit 0):bf7b19ba(two commits past this branch's base8f453d0e, neither touching these files): treedb5bf03d.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,fix/525-adopt-answers-and-submodule-rowsat0ea4fc5f, which changesshape-doctor.py(+281/-15): treee5339c23.fix/432-doctor-gitmodules-bytesat86c6b9c5(+33/-3 inshape-doctor.py): tree2587f33c.Neither branch adds a
run_validatorcall or anothersys.executablechild. #617'svalidate-manifest.pystill imports onlyrepo_shapeoutside the standard library, so-Scosts it nothing.Closes #342
🤖 Generated with Claude Code
Summary by Sourcery
Run all shape-doctor validator children with site initialization disabled so missing repository validators consistently produce the same findings as CI.
Bug Fixes:
Enhancements:
Tests: