Repository navigation
tests: every test that clones a just-pushed local remote by its plain path now clones it with clone_just_pushed, so the macOS failed to copy file race of #440 cannot reach it; load_without_bytecode keeps the doctor's and the Windows-path tests' loads of the family root's scripts from writing .pyc into the templates; the not-a-list doctor cases hold INVALID, exit 1; and GOOD_KEYS holds th - #554
Merged
Merged
Conversation
… path now clones it with `clone_just_pushed`, so the macOS `failed to copy file` race of #440 cannot reach it; `load_without_bytecode` keeps the doctor's and the Windows-path tests' loads of the family root's scripts from writing `.pyc` into the templates; the not-a-list doctor cases hold INVALID, exit 1; and `GOOD_KEYS` holds that a bare or absent `source:` and a bare `legs:` raise no `plan-bad-*` (#544) What: - `clone_just_pushed` (tests/conftest.py, #467) takes `recurse`, `bare`, `config` and `env` for the tests whose subject is the clone itself, decodes its output as UTF-8, and `cloned_just_pushed` is the same call that fails the test where the clone fails. About fifty plain-path clones of a bare remote a scaffold, `family.py` or `adopt-project.py execute` pushed into move onto it: the `scaffolded`-remote clones, the `advanced` fixtures of the bump-leg tests, the family member copies, the adopted source at its split branch, and the leg remotes the adopt tests read. Each keeps its test's meaning: the non-recursive clones stay non-recursive (`recurse=False`), the bare copies stay bare, the autocrlf, template-free and command-scope environments pass `config` or `env`. - Left as they are: clones of the product checkout (`REPO`), clones that BUILD a remote out of a working repository (`clone --bare <work>`), clones of a repository nothing was pushed into, the plain clones standing for `_commit_the_split`'s and `gh repo clone`, and the tests whose subject is git's own plain clone. - `load_without_bytecode` (tests/conftest.py) is #456's `load()` of test_bootstrap_spelling.py, moved; it replaces the bare `exec_module` of `bootstrap.py` and `siblings.py` in the `siblings` fixtures of test_shape_doctor.py and test_windows_paths.py. - test_shape_pin_files_not_a_list.py asserts the doctor's exit 1 and its INVALID verdict for a refused root pin, and its docstring says #267 has moved it there. - `GOOD_KEYS` of test_adopt_execute_refusals.py gains `source-bare`, `source-absent` (`REFUSED plan-no-source`) and `legs-bare` (`FINDING naming-role-mismatch`); dropping either `is None` arm of `_plan_key_findings` fails them. - tests/test_clone_just_pushed.py holds the new options. Why: about a dozen fixtures still cloned the remote a scaffold had just pushed into by its plain path, which is git's local clone and walks the remote's `objects/` while a maintenance run can pack and delete them (the #440 flake, three reruns of the 3-OS CI). The two riding corrections and the control rows are the backlog lines #461 marked for #544. Not here: templates/family-root/scripts/siblings.py:455 (#543), and #415 (waits on #532). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 2 days and 6 hours by commenting |
Reviewer's GuideThis tests-only PR removes the macOS local-clone race from tests that clone remotes immediately after pushing by routing them through a pack-reading clone helper, adds coverage for the helper’s supported modes, prevents fixture imports from polluting templates with 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 14, 2026
…is proved to reach git; the by-env case appends to `GIT_CONFIG_COUNT` and every clone case asks the store; `family.py siblings` runs with `NO_BYTECODE`, so it leaves no `.pyc` in the templates; and the control-table test says what it holds (#544) What: - tests/test_clone_just_pushed.py: `test_a_branch_the_remote_lacks_fails_the_clone` (removing `-b` from the helper fails it; the `main` case passes without it). The branch, `config` and `env` cases also ask `assert_store_read_through_git`, so the docstring is true of each case that makes a clone. The `env` case appends its entry at `GIT_CONFIG_COUNT`'s index, as `command_scope` does, and no longer drops the Windows job's two entries. - tests/conftest.py: `NO_BYTECODE`, the child-process counterpart of `load_without_bytecode`. `test_family_py_siblings_runs_the_holders_own_file` and the parent-folder test of test_family_printed_quoting.py run `family.py siblings`, which `runpy`s the standard's `siblings.py`; it imports `bootstrap` from beside it and wrote `templates/family-root/scripts/__pycache__/bootstrap.cpython-312.pyc`. Both pass `NO_BYTECODE`, and test_family.py and test_family_printed_quoting.py leave no `.pyc` there. `test_family_py_siblings_refuses_a_path_that_is_not_a_holder` refuses before `runpy` and wrote none. - tests/test_adopt_execute_refusals.py: the test over `GOOD_KEYS` is renamed `..._are_not_plan_bad_findings`, because `legs-bare` is a FINDING (`naming-role-mismatch`) and only not a `plan-bad-*` one. Why: the independent review of 7135da6, nits 1 to 3, 5 and 6. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This was referenced Oct 14, 2026
brettheap
pushed a commit
that referenced
this pull request
Oct 14, 2026
…5 cites #518, section 6 drops #181's line, which #518 closed, adds the three limits lane 4's reviews posted on #458 and #460 after the triage and extends two #544 lines with what #461's body now says, and section 7 no longer says #181 is outside the milestone (#472) Refresh round 3c, the last before the v1.0.0 tag. Main is b8b7062 (#518, merged 2026-10-14T18:50:03Z, closing #181), the only merge since round 3b's 5ed3e5d. All 227 merge commits are ancestors of it. The notes describe nothing on an unlanded pull request. Section 5: one adopt-project.py bullet, "Keywords as typed", written from #518's squash message with its body beside it. Read in main's code: adopt-project.py `_PLAIN_RE` (:580), `loader_reading` (:583), `_reads_back_as_text` (:603), `y` (:625-637), `MANIFEST_MISREAD_REMEDIATION` (:2216), `_misread_findings` (:2587, `plan-manifest-misread`), `_refuse_misread_values` (:2939, `adopt-manifest-misread`), `_entry_findings`' return (:5354), `_refuse_what_check_finds` (:7576, called :8206); scripts/shape_materialize.py `leg_path_problem` (:3492, a null leg path is "not text"); templates/assembly-root/project.yaml quotes each placeholder the gate asks about (:31, :42, :46, :47, :62, :67, :159, :163). Run offline, with a stub `gh` that was never called, at b8b7062 and at 5ed3e5d: `--spec-path null` mounts the leg at `null/` (at 5ed3e5d, at `spec`), `--id null` writes `id: "null"` (`"northwind"`), `--tracking-branch true` on a source with that branch puts both legs on `true` (refused `plan-tracking-branch-missing`), a hand-edited `spec_path: null` is `plan-bad-leg-path` (`plan ok`), and the default flags adopt as before. The gate's "names none today" rests on the templates' quotes and on lane 1's fuzz (286 values, 0 rows), not on a run. Section 6: #181's line is dropped (a source with files named `null` and `false` now plans them quoted, asks the ordinary `plan-unresolved` question, and adopts once it is answered; probed). #457 has no new comment and an unchanged body since 16:09:31Z. L1-J1, J2, J3, K1 and K8 hold on main (`check` halves probed; K8 read at adopt-project.py:5861), K3 (`id: 007` written as `"7"`) and K7 (two findings for a tab in a pin's product) probed again. K4 and K6 stay out: nothing a person meets. Added from lane 4's 2026-10-14T18:32Z posts, which are untriaged and so carry no number: on #458 a quoted or spaced `files:` or `shape:` key (read at update-shape.py:1378 and :1502, and run in process for the pin; `check`'s exit and the manifest half rest on R558's measurement) and `bump-leg.py --dry-run` over a pin its validators refuse (read at bump-leg.py:1475 against :1388-1396; the tab-line example rests on R561); on #460 an unparsable standard policy file (shape-doctor.py:1723 read; the exit 2 rests on R557). Left out: #458's two R556 lines (a wording imprecision; and a refusal that is by design on main, bootstrap.py:275), #459's two R557 lines (each repeats a line the notes already hold, #443's and the `UnicodeEncodeError` one), and #461's optional hygiene-test line. #461's other two new lines extend the two #544 lines: four `git submodule update --init` runs in two test files and test_adopt_plan_yaml_quoting.py:2036 (read), and tests/test_cli_reference.py's `--help` children (render-cli-reference.py's `child_env` sets no PYTHONDONTWRITEBYTECODE; the `.pyc` itself rests on #554's review). Section 7: the sentence about #181 is gone. The v1.0.0 milestone stands at 1 open (#472) and 46 closed. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…e `execute` has just pushed the split into, which #518 added, is a `cloned_just_pushed`, so no plain-path clone of a just-pushed remote is left that main brought in (#544) What: after `execute --local-remote-dir` the test cloned the source at the split branch by its plain path with `git -c protocol.file.allow=always clone --recurse-submodules -b`. It is now `cloned_just_pushed(source, tmp_path / "clone", SPLIT_BRANCH)`, the same options: recursive, `-b`, the legs read through the `file://` rewrite (they sit in `tmp_path / "remotes"`, beside the source). `FILE_PROTOCOL` is no longer imported there; nothing else in the file used it. Why: the landing condition of #554, which said to move #518's clone at the pre-landing merge of main. A re-grep of every `"clone"` call in tests/ finds no other new plain-path clone in what the merge brought in (test_adopt_leg_paths.py, test_adopt_manifest_values.py, test_template_text_placeholders.py, test_adopt_plan_yaml_quoting.py). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
This was referenced Oct 14, 2026
Contributor
Author
Contributor
Author
|
(lane openRepoShape-3, 2026-10-14T23:45:28Z) To lane openRepoShape-5: thanks. #554 landed first, as 2247dbf at 23:44Z. So #578 is the second to land, and per your note it does both fixes: it keeps both lines at tests/test_family_printed_quoting.py:375-376, and it rewords the |
This was referenced Oct 14, 2026
brettheap
pushed a commit
that referenced
this pull request
Oct 15, 2026
…uling 27, with no hand resolution Main moved from 5ed3e5d to 2247dbf while this PR waited. #554 changed tests/test_bump_leg.py, one of this PR's files, so ruling 27's condition 4 needs this merge of main. It is clean: git merge-tree --write-tree of the pre-merge head 434a668 with 2247dbf exits 0 and prints tree fa48b52, and this commit's tree equals it. No file was resolved by hand. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 15, 2026
…merge for ruling 27, no hand resolution Merges origin/main 2247dbf (#554's squash, which made about 50 test files clone just-pushed local remotes with `clone_just_pushed`, and added it to tests/conftest.py) into the PR's start head fabdd7e, which was green on all three OSes and SonarCloud. The merge was CLEAN: git reported no conflict, tests/test_update_shape.py (a file this PR and #554 both change) was auto-merged in different hunks, and nothing was resolved by hand. The merge tree equals what `git merge-tree --write-tree fabdd7e origin/main` predicted, 6dd5f4d. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 15, 2026
…main, for ruling 27, with no hand resolution (#533) Main moved from 5ed3e5d to bbcfd34 (#473, #551, #552, #554, #553, #561, #562) while this PR waited. It changed two of this PR's files, scripts/bump-leg.py (#561) and tests/test_bootstrap_after_bump.py (#554), and both merged without a conflict, so no file was resolved by hand and this merge is not a new H0 under ruling 27. The merged tree equals the one `git merge-tree --write-tree e5da3c5 bbcfd34` predicted: f4c0ed1. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 15, 2026
…w and its both-ways-in line, `clone_just_pushed`) so the PR's CI merge ref and this fix round run on current main The brief named main 2247dbf; the shared store's origin/main had advanced to b4c1f50 (2247dbf plus #553), so this merges that. No conflicts: the PR's four files (scaffold-project.py, scripts/shape_materialize.py, tests/test_reuse_visibility.py, tests/test_scaffold_reuse_seeded_legs.py) are not among those main changed since bfed143, apart from the test files below. #554's `clone_just_pushed` is kept as main has it. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 15, 2026
…ust_pushed`) so the PR's CI merge ref and this fix round run on current main CI runs on the merge of this PR with main, and it failed on all three OSes: tests/test_scaffold_e2e.py::test_the_next_steps_clone_under_local_remotes_reads_the_legs raised `NameError: name 'FILE_PROTOCOL' is not defined`. #554 rewrote the file's first conftest import without `FILE_PROTOCOL`, and this PR's test still uses it, so the textual merge was clean and the merged file was broken. Resolution: #554's import line stays as it is, and `FILE_PROTOCOL` is imported again on the file's second conftest import line (`from conftest import FILE_PROTOCOL, indented_commands, pasted, read_through_git`). No other change; the merge had no textual conflicts. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 15, 2026
…ust_pushed`, #553, #561) into fix/540-pin-source-unread This merges origin/main 7d9bec9 as the shared store held it at 2026-10-15T11:02Z: 5e59e3b (#551), 3102027 (#552, shape-doctor's machine row), 2247dbf (#554's `clone_just_pushed`), b4c1f50 (#553) and 7d9bec9 (#561). It is done so that the PR's CI merge ref and this fix round run on current main. The merge was clean. `tests/test_adopt_pins.py` keeps #554's `cloned_just_pushed` call sites and its import line. This PR adds no test that clones a just-pushed remote by its plain path. `docs/cli.md` merged without a conflict, and `scripts/render-cli-reference.py --check` passes. Main: 7d9bec9. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 15, 2026
…lone_just_pushed`) so the PR's CI merge ref and this fix round run on current main The brief named 2247dbf; origin/main in the shared store is b4c1f50, which is 2247dbf plus #553 (shape-doctor.py only), so that is the tip merged. Conflicts, both on import lines only (rv537): tests/test_scaffold_e2e.py and tests/test_scaffold_manifest_values.py. #554 moved those files' clones to `cloned_just_pushed` and dropped `FILE_PROTOCOL`; this PR adds `scaffold_dry_run_on_github` to the same import. Both are kept, and `FILE_PROTOCOL` stays out because neither file uses it after #554. No test this PR adds clones a just-pushed local remote by its plain path. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 15, 2026
…ust_pushed`, #553's adoption line, #561) so the PR's CI merge ref and this fix round run on current main Merges origin/main 7d9bec9 into the #528 branch at 60a258f. Since the branch's base bfed143 main gained #551 (the handbook's per-section README hashes), #552 (shape-doctor.py's machine row), #554 (`clone_just_pushed` at every test that clones a just-pushed local remote), #553 (shape-doctor.py's adoption and scaffold lines) and #561 (bump-leg.py's dry run). The fix round's design leans on shape-doctor.py's placement row, which #553 moved, so it is read and tested on this tree. No conflicts. tests/test_adopt_single_repository_record.py clones no just-pushed remote (it reads the bare remotes with `git ls-tree`), so no call site moves to `clone_just_pushed`. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 15, 2026
… tests/test_family_printed_quoting.py keeps #554's `env=NO_BYTECODE` call line and this branch's exit-1 assert, and tests/conftest.py's `NO_BYTECODE` comment no longer says `family.py siblings` runs the template's copy through `runpy` (#543) Main moved from bfed143 to 9f80939: #551, #552, #554, #553, #561, #562, #579 and #567. One hunk conflicted. tests/test_family_printed_quoting.py, in test_the_siblings_mv_lines_paste_whole_and_move_the_holder: #554 (2247dbf) added `env=NO_BYTECODE` to the `run_script(FAMILY, "siblings", ...)` call, and this branch changed the next line's `returncode == 0` to `== 1`, because a misplaced holder now stops before cloning. The two lines are adjacent. Resolved to #554's call line and this branch's assert, both kept whole: result = run_script(FAMILY, "siblings", "--family-root", str(holder), "--dry-run", env=NO_BYTECODE) assert result.returncode == 1, result.stdout + result.stderr # #543 tests/test_family.py merged by itself: its `env={**ALLOW_FILE_PROTOCOL, **NO_BYTECODE}` call sits under this branch's corrected docstring. tests/conftest.py: #554's comment above `NO_BYTECODE` said `family.py siblings` runs the standard's `templates/family-root/scripts/siblings.py` through `runpy`. On this branch it runs the holder's own copy in a child process standing in the holder, so that sentence became false in this merge. The comment now says what the constant is for, what it used to guard, and why it stays on those two calls. Lane 3 agreed to this wording on #554. `NO_BYTECODE` stays at both call sites; removing it is a later cleanup. The merged tree differs from 9f80939 only in this branch's nine files and that comment. 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 15, 2026
…e-doctor adoption line and `clone_just_pushed`) so the PR's CI merge ref and this fix round run on current main The brief named main 2247dbf; the shared store's origin/main now reads b4c1f50, which is 2247dbf plus #553 (shape-doctor.py and its tests only). The merge is clean: no file of this PR is touched by either side. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 15, 2026
…split branch with `clone_just_pushed`, so the macOS `failed to copy file` race of #440 cannot reach it (#522) `execute` pushes the split branch into the source, and the fixture then cloned the source by its plain path: git's local clone walks `objects/` while the maintenance the push left running can repack it. #554 moved every such clone onto `conftest.clone_just_pushed` and no guard enforces it for a test added after, so this PR's own fixture is moved too. `FILE_PROTOCOL`, which only that clone used, leaves the import. Review of PR #572, N7. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 15, 2026
…PR's CI merge ref and this fix round run on current main Main moved from bfed143 to f6eaab7 since this PR's head 93b68f6: the doctor's machine row and #553's adoption line, #554's `clone_just_pushed`, bump-leg.py's `--dry-run`, update-shape.py's accept-local check and the README setup commands. The merge was clean; no conflict to resolve. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 15, 2026
…nnotated tag's own id as `add --at` does, and says to push first a commit the member does not hold after its fetch (#541) (#568) `family.py bump --to` skipped two checks `add` makes, and failed in git's own words on a commit nobody had pushed. This PR fixes the three lines of #541 in `scripts/family.py`. Both runs, the dry run and the real one, now give the same answer for each line. The head is 8024f9c. It has two commits on bfed143: eeadc65 is the fix, and 8024f9c answers review rv541 (see "Second commit" below). - [x] **`bump --to` never asked `add`'s duplicate-id question.** If the member's `project.yaml` at `--to` recorded another row's `id`, `bump` committed it with exit 0. The holder's own `validate-family.py` then refused that commit as `member-duplicate-id`. Now `bump` refuses with `member-duplicate-id` before anything changes, and the holder stays byte-identical. - [x] **`bump --to` an unpushed commit failed as git's `reference is not a tree`.** Now both runs refuse with `member-to-unknown`. The refusal names the commit, says no branch of the member's remote reaches it, and says to push it first. Once the commit is pushed, the same command pins it. - [x] **Riding: `add --at <tag id>` peels it, `bump --to` refused the same id, and the two worded a tag of a tree differently.** Brett Heap ruled on 2026-10-14 that `bump --to` peels an annotated tag's own id, as `add --at` does. - `bump` now pins the commit the tag names, in both runs, and prints one line saying so. - A tag of a tree is now named in `add`'s words, "names a tree", through one shared function. - Both verbs' `--help` say that an annotated tag's own id is peeled. - One input still differs between the two verbs: a tag whose commit is on no branch (see "Left out"). - **`_checked_bumped_id`** (new) asks `add`'s question (#330) of the row `bump` writes. The member's own row is left out. Every other row is asked the way the validator asks it, so `bump` refuses what the gate would refuse and nothing it would pass. It is asked in three places: - by the dry run, before it prints the row; - by the real run, before its checkout (`_checked_bump_target`), so the refusal comes before the first write; - by the real run again, after its checkout (`_bump_and_pin`), of the id that checkout read. This catches a `project.yaml` that is a symbolic link, which only a checkout reads through. That refusal is put back by `PinRollback` (#365), like any refusal after the first write. - **`_row_recording`** (new) finds the row that records an id. `add`'s `_checked_member_id` now uses it too. `add`'s words do not change. - **`_checked_to_names_a_commit`** now returns the commit to pin: - a commit: itself; - an annotated tag, or a tag of a tag: the commit it names (`^{}`); - a tree, a blob or a tag of either: refused with `member-to-not-a-commit`, in `add --at`'s words (`_names_no_commit`, shared with `_probe_commit`); - an id the member does not hold after the fetch: refused with `member-to-unknown`. If the fetch failed, the refusal says so and does not claim the remote lacks the commit. - **`cmd_bump`** goes on with the returned commit in both runs. When a tag was peeled, both runs print this as their first line: `--to <tag12> is an annotated tag in <repo>; the commit it names, <commit12>, is what would be pinned`. The words "would be" are deliberate: the dry run prints the line, and so does a run refused by a question asked after the peel, and neither pins anything. - **`--help`.** - `add --at` now reads "pin this 40-hex commit instead of the member's current tip (an annotated tag's own 40-hex id is peeled to the commit it names)". - `bump --to` ends "(an annotated tag's own 40-hex id is peeled to the commit it names, as `add --at` peels one)". - `docs/cli.md` is regenerated by `scripts/render-cli-reference.py`. - **Docstrings.** - The sentence that said `bump` refuses a tag "as `bump-leg.py` refuses the same `--to`" is gone (lane 4's note on #541), and so is "left to the checkout" for an id the member does not hold. Both files now compare only with `family.py add --at`, so they stay true whichever of #561 and this PR lands first. - `_checked_to_names_a_commit` now names the tag whose commit is on no branch. - **S1.** The peel line said "is what is pinned" in the dry run and above a refusal. It now says "is what would be pinned". A new test puts a duplicate id behind a tag: both runs print the line, refuse `member-duplicate-id` in the same two lines, exit 2, and pin nothing. - **N1.** A new test covers the branch hint with a tag's id on the tracking branch. The dry run's hint and the real run's refusal both name `family/bump-irrs-<the typed id's first twelve>`. The reviewer's mutant, in which the dry run takes its hint from the peeled commit, fails that test. - **N2.** The tag whose commit is on no branch is named in the docstring, in "Left out" and as a checklist candidate. Its behaviour is unchanged. - **N3.** The `--help` lines above, with `docs/cli.md` regenerated. - **N4.** The merge-check paragraph below is redone at 8024f9c. - **N5.** The ripple numbers in "Left out" and the checklist candidate are now the reviewer's measured ones. 1. **The wording for a tag of a tree.** `bump` now uses `add --at`'s words: "names a tree" and "names a blob", with `add`'s remediation sentence. Both verbs build the refusal from one function, `_names_no_commit`, and each keeps its own code (`member-at-unknown` and `member-to-not-a-commit`). I followed the brief's "exactly as `add --at` does". - #561 keeps "is an annotated tag of a tree" for `bump-leg.py`, under its own code and in its own frame. - Changing `add` to that wording would change `add`'s tested output, so it is not done here. - `add`'s tests pass unchanged (`test_family_add_refusals.py`, inside the 670 below). 2. **A new code, `member-to-unknown`.** It mirrors `add`'s `member-at-unknown` for the same input. The fetch's outcome selects one of two wordings, so a member that could not be fetched is never called unpushed. 3. **The peel is announced.** `add --at` peels a tag without saying so. `bump` prints one line in both runs, because the `old -> new` line the person reads then shows an id they did not type. The line says "would be pinned", the same words #561 uses for `bump-leg.py`. 4. **The suggested branch keeps the typed id.** The real run refuses the tracking branch before the fetch, so before any peel. The dry run's hint names the same branch, and a test now holds that. #561 does the same with `asked`. 5. **A linked `project.yaml` is still left to the real run by the dry run**, as `add --dry-run` leaves one (#326). The real run refuses the duplicate id after its checkout and puts the holder back. A test holds that. New: `tests/test_family_bump_to_checks.py`, with 6 tests: - the duplicate id read from the member's objects, refused alike by both runs; - the duplicate id read through a linked `project.yaml` (POSIX only), refused and put back; - a duplicate id behind a tag: both runs print the peel line, refuse, and pin nothing; - the branch hint on the tracking branch with a tag's id, the same in both runs; - an unpushed commit in a working clone: refused, then pinned once pushed; - a commit missing after a failed fetch, not called unpushed. Changed: - `tests/test_family_bump_to_a_tag.py` now holds the ruling: the peel in both runs, the peel after the fetch, and the tree, blob and tag-of-one wording. Its `both_runs` helper also takes the stdout a run prints before its refusal. Its clone lines are untouched, so it merges cleanly with main, where #554 has landed. - `tests/test_family_dry_run.py`: its test of a commit the member lacks now expects `member-to-unknown` from both runs. Results at 8024f9c, through the lane's runner (`gh` stubbed, a bogus token, an empty `GH_CONFIG_DIR`): - `test_family_bump_to_checks.py`, `test_family_bump_to_a_tag.py`, `test_family_dry_run.py`, `test_family_bump_remove_rollback.py`, `test_family_add_member_id.py`, `test_cli_reference.py`: **139 passed in 149.62s** - `test_family.py`, `test_family_tracking_branch.py`, `test_family_add_refusals.py`, `test_family_add_excludes.py`, `test_family_member_path.py`, `test_family_printed_quoting.py`, `test_cli_reference.py`, `test_repo_hygiene.py`, `test_refusal_default_remediation.py`, `test_commit_trailers.py`, `test_agents_md_codes.py`, `test_stdout_encoding.py`: **670 passed, 1 skipped in 300.71s**. The skip is `test_stdout_encoding.py:286`, which runs on Windows only. - `scripts/render-cli-reference.py --check`: `[ok] docs/cli.md is what the generator produces`. The new and changed tests at bfed143, with the test files copied in: - The 12 from the first commit: **12 failed, 60 passed**. Each fails for its line's reason: - on a duplicate id, the real bump moved the member; - on an unpushed commit, git's own words; - the tag refused; - the old tree wording. - The 2 added in 8024f9c: **2 failed**. bfed143 refuses the tag, so no peel line is printed and the dry run exits 2. The script scaffolds Atlas, Borg and Cello into bare remotes, makes a holder with Atlas and Borg, then runs each line. At bfed143, then at eeadc65; 8024f9c changes only the peel line's last words: - **Line 1.** Atlas's `project.yaml` is changed to record `id: "borg"`, pushed, and Atlas is bumped to it. - bfed143: the dry run exits 0 with `id: borg`, and the real run commits (exit 0). `validate-family.py` then says `FINDING member-duplicate-id: Borg and Atlas both record id 'borg'`, exit 1. - eeadc65: both runs say `REFUSED member-duplicate-id: testorg/Atlas at <sha12> is already a member of Sonata under another name: ...`, exit 2. The holder's state hash is the same before and after, and the validator says `family ok`. - **Line 2.** A commit is made in the working clone `work/Atlas` and not pushed. - bfed143: the dry run says `REFUSED resolving ... fatal: Needed a single revision`. The real run says `REFUSED checking members/Atlas out ... fatal: reference is not a tree`. Both exit 2. - eeadc65: both runs say `REFUSED member-to-unknown: --to <sha> is not a commit any branch of testorg/Atlas reaches: ...` and `Remediation: push it first, ...`. After `git push`, the same command exits 0 and `family ok`. - With the member's origin gone, eeadc65 says the fetch failed and does not claim the remote lacks the commit. - **Riding line.** Atlas gets a commit and an annotated tag `v2`, both pushed. - bfed143: both runs refuse with `member-to-not-a-commit ... is an annotated tag`. - eeadc65: both runs print the peel line. The dry run shows `commit <old> -> <commit12>`. The real run pins the commit in `pin.commit` and in the gitlink alike, and the validator says `family ok`. - `add --at <Cello's tag id> --dry-run` pins the tagged commit at both shas. - **Tag of a tree.** - bfed143: `bump` says "is an annotated tag of a tree" and `add` says "names a tree". - eeadc65: both say "names a tree". No line needs real GitHub. Each behavioural hunk was reverted or broken on its own in a throwaway worktree (M1 to M10 and R1 at eeadc65, M11 to M13 at 8024f9c), and the tests were run: | Probe | Result | |---|---| | M1, the dry run's duplicate-id check removed | `test_bump_to_a_commit_recording_another_rows_id_is_refused_alike` fails | | M2, the real run's pre-checkout duplicate-id check removed | the same test fails: the refusal comes after the checkout, worded differently from the dry run's | | M3, the post-checkout duplicate-id check removed | `test_a_duplicate_id_read_through_a_linked_manifest_is_put_back` fails | | M4, an id the member lacks goes on as before | both `member-to-unknown` tests fail | | M5, a failed fetch worded as a successful one | the failed-fetch test fails | | M6, the peel turned back into a refusal | both `pins_the_commit_it_tags` cases and the release-tag test fail (3) | | M7, the old tree, blob and tag-of-one wording | the 4 `refused_by_name` cases fail | | M8, the peel line not printed | the same 3 peel tests fail | | M9, the returned commit ignored | the same 3 peel tests fail | | M10, the member's own row asked against itself | every ordinary bump is refused: 4 of 4 selected tests fail, including `test_bump_dry_run_prints_the_move_and_a_real_bump_writes_exactly_that` | | R1, `add`'s refusal written inline again (a refactor, no change in output) | `add`'s tag, tree and blob tests pass: 7 passed | | M11, the dry run takes its branch hint from the peeled commit (the reviewer's D4 mutant) | `test_both_runs_name_the_branch_for_the_tag_id_as_typed` fails; 1 failed, 29 passed | | M12, the peel line says "is what is pinned" again | 5 fail: the duplicate-id-behind-a-tag test, the branch test, and the 3 peel tests | | M13, `bump --to`'s help without the peel | 5 `test_cli_reference.py` tests fail, `test_the_committed_reference_is_what_the_generator_produces` among them | The merge is clean (rc=0) against each of these: - origin/main 7d9bec9, the current tip. Lane 3's #554 (2247dbf) and lane 4's #561 (7d9bec9) have both landed since this branch was cut; - lane 5's siblings on origin: - `fix/family-siblings-tracking-branch` at 43d32eb; - `fix/family-members-case-variants` at 40eda84; - `fix/family-remove-gitmodules-missing` at b1120c3; - `fix/link-source-chain-defaults` at 04c9d77; - lane 2's #573 (`fix/537-dry-run-asks-remotes`) at 0c81423. On the tree merged with main 7d9bec9, `scripts/family.py` and `scripts/bump-leg.py` parse, and `render-cli-reference.py --check` says `[ok] docs/cli.md is what the generator produces`. The review checked the same on the merges with `fix/link-source-chain-defaults` and lane 2's #573. **#561 landed first, so this PR fixes the cross-reference** (lane 4's note on #541, issuecomment-6175160194). On main, #561 made two sentences false: `scripts/family.py:3991`, "REFUSED, NOT PEELED, as `bump-leg.py` refuses the same `--to`", and the module docstring at `tests/test_family_bump_to_a_tag.py:17`. This PR rewrites both to compare with `family.py add --at` only. The comparisons with `bump-leg.py` that remain still hold on main 7d9bec9: a tag whose commit is on no branch is still `bump-leg-commit-not-on-remote` there, in both runs, and the `nothing to do` sentence and the `trailers` lines are unchanged. Main was not merged into this branch. - **A commit only the member's mount holds is still pinned.** This is a commit made inside `members/<P>` and never pushed. The object is present, so `bump` proceeds. - #541 is about the commit that fails. - `bump-leg.py` refuses this input as `bump-leg-commit-local-only`. Refusing it here needs a ruling on offline bumps, because today a bump whose fetch failed goes on from the objects the member already has. - The reviewer measured the ripple with bump-leg's reachability rule applied: at least 12 cases in 5 files fail, and only 6 of them make their commit through `commit_in_mount`. The twelfth is this PR's own peel-line test. - It is the first checklist candidate. - **A tag whose commit is on no branch.** `add --at` pins its id, but `bump --to` refuses the tag and its commit as `member-to-unknown`. - `bump`'s fetch brings every branch and, by git's own rule, only the tags that point into the history those branches bring. `add`'s bare clone brings every tag. A `git push origin v2` without the branch makes one, and the refusal's push to a branch is what lets `bump` pin it. - `bump-leg.py` refuses its twin the same way, as `bump-leg-commit-not-on-remote`. Fetching every tag would write refs the person's member did not have. - The behaviour is unchanged and named in `_checked_to_names_a_commit`'s docstring. It is the second checklist candidate. - **`docs/release-notes/v1.0.0.md`** describes v1.0.0's behaviour (its lines 259-260, 461-462 and 487) and stays true of v1.0.0. The v1.1.0 notes should say what changed. - **No README, AGENTS.md or handbook sentence became false**, so none is edited. - **`family.py bump --to` a commit only the member's mount holds pins it, where `bump-leg.py` refuses `bump-leg-commit-local-only`.** - A commit made inside `members/<P>` and never pushed passes `bump`'s object check, because the object is in the mount's store. It is pinned with exit 0, and the holder then cannot be bootstrapped anywhere else. - `bump-leg.py` asks `for-each-ref --contains` against `refs/remotes/origin/` and refuses the same input. - A fix needs a ruling on offline bumps. - Under bump-leg's rule, at least 12 cases in 5 test files fail: 11 existing ones (review rv541's `c1.log` and `c2.log`) and this PR's own peel-line test. Only 6 of the 12 make their commit through `commit_in_mount`. - **`family.py bump --to` refuses a tag whose commit is on no branch (`member-to-unknown`), where `add --at` pins the same id.** - The remedy it prints, pushing the commit to a branch, works. But a person who pushed only the tag is told to push. - Telling that case apart (for example with a `git ls-remote --tags` check, or a fetch of that one tag) is a change of its own, and so is any rule that brings it in line with `add`. Review: rv541 found 1 should and 5 nits on eeadc65; the fix round 8024f9c was re-verified APPROVE WITH NITS, and its one nit (this message's merge paragraph and ripple count) was fixed in the body. CI (Brett Heap's ruling 27): H0 8024f9c passed tests, tests-macos, tests-windows and SonarCloud; the head 51ec8ff is a clean update-branch merge of main a2243f1, with tests and SonarCloud green. Landed on Brett Heap's word of 2026-10-15T13:32:47Z, "All five, once green". Closes #541 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 15, 2026
…er before it clones, clones each member on its tracking branch, and `family.py siblings` runs the holder's own copy (#543) (#578) Three lines of #543, in one change and one re-pin, because `scripts/siblings.py` is a pinned copy in every family holder. Head: `8d3d1cceb30dfc93c7ceec130131038dc4f89e40`. The branch's own commits: the change (43d32eb, on bfed143), a merge of main at 9f80939 (8b4fc0b), the review round (2156a01), round 3 (5ce5b68, a family folder reached through a link; 0112dcb, a test skipped on Windows), and round 4 (8d3d1cc, a Windows junction counts as a link, and rv543b's nits). **1. The family folder is asked first** (`templates/family-root/scripts/siblings.py`, `_run_placement`, `parent_folder_warning`). - Before: in a holder whose parent folder is not named after the family, `make siblings` cloned every member into that folder, then printed the warning and the `mv` lines, and exited 0. After the move, the re-run cloned every member a second time beside the moved holder. The first clones stayed behind. - Now: the check runs before any `git` command runs or any member is cloned. The run prints the same WARNING and `mv` lines, says `NOTHING WAS CLONED` and why, ends with `siblings: stopped before cloning: the holder is not in a folder named <Family>. Nothing was cloned, fetched or moved.`, and exits 1 (`make siblings` exits 2, make's own code). `--dry-run` stops the same way. After the move, one run clones each member once. - Why a stop and not a later warning: no flag or documented answer lets a person place siblings outside the family folder. AGENTS.md, README.md, the handbook and the holder's own README and AGENTS-shape.md all describe a warning plus a printed `mv`, and the warning itself told the person to move the holder and re-run. A stop gives the same outcome without the stray clones. Exit 1 is the file's "finding a human can act on" (exit 2 is kept for "the question could not be asked"). - **The printed move runs as typed on every disk** (review round, S3). The name test stays exact on Linux, macOS and Windows. So the remedy has to work on each: - On a disk that ignores case (macOS, Windows), a holder in `inkrouter/` for the family `InkRouter` used to get `mkdir -p <p>/InkRouter` and `mv <p>/inkrouter/InkRouter <p>/InkRouter/InkRouter`. There `<p>/InkRouter` IS `<p>/inkrouter`, so that moved the holder onto itself and failed. Now, when `<p>/InkRouter` exists and `os.path.samefile` says it is this folder, the FOLDER is renamed through a staging name: `mv <p>/inkrouter <p>/InkRouter.folder`, then `mv <p>/InkRouter.folder <p>/InkRouter`. - At a filesystem or drive root (`/InkRouter`, `C:\InkRouter`), `<root>/InkRouter` is the holder itself, and the old lines were `mv X X/X`. Now the holder moves through `<root>/InkRouter.holder`, the staged move the common case already used. The warning names the root (`'/'`) where it used to say `''`. - Linux is unchanged: with nothing at `<p>/InkRouter` the holder moves there through `InkRouter.holder`; with a separate `<p>/InkRouter` folder it moves into it. - A link named `InkRouter` beside the folder and leading to it passes the check: the family folder, reached through a link (see "Round 3"). - **The header says `parent folder` until the check passes** (C3). It used to print `family folder <parent>` right above a warning saying the parent is not the family folder. Through a link it names the folder the clones go into, and the link. **2. auF-13: the tracking branch** (`siblings.py`, `tracking_branch()`, used in `place()`). - Before: a plain `git clone`, so a sibling landed on its remote's default branch. - Now: the clone passes `--branch=<branch>`, where `<branch>` is the `tracking_branch:` of the member's own `project.yaml` in the pinned copy under `members/`. That copy is the holder's record of the member, and the same field the member's own bootstrap reads for its legs. - Not read: `.gitmodules` (both templates say it carries no `branch=`, and nothing writes one) and `family.yaml`'s `tracking_branch:` (the holder's own branch: `family.py add`, `bump` and `remove` refuse to commit on it). - No record: when `members/<Project>` is not checked out, or its `project.yaml` names no branch, the clone keeps today's behaviour (the remote's default branch) and its line says why. - **A value that is no branch name is not used** (review round, N1). A number or a list (`5`, `[main]`) now reads `declares tracking_branch 5, which is not a branch name`, where it used to say it declares none. A string holding a character that does not print (an ESC sequence, a newline) is not handed to git and is shown in its `repr`, for example `declares tracking_branch '\x1b[2Jcleared', which holds a character that does not print, so it is not used as a branch`. It used to reach stdout raw. The member's own bootstrap escapes the same field (#391). Each such clone keeps the remote's default branch. No import was added to the pinned file. - A declared branch the remote does not have is git's own refusal: a `CLONE FAILED` finding naming it, exit 1, and no directory left behind. **3. auF-14: the holder's pinned copy** (`scripts/family.py`, `cmd_siblings`). - Before: `family.py siblings` used `runpy` to run this checkout's `templates/family-root/scripts/siblings.py`, inside family.py's own process. A holder pinned at another shape ran different code than `make siblings`. With no holder copy at all it still ran, and its `siblings-script-missing` remedy said it "runs the holder's own copy". - Now: it runs `<holder>/scripts/siblings.py` with the same interpreter, in its own process, from the holder, as the Makefile's `siblings` target does, and returns that exit code unchanged. The command is `scripts/siblings.py --root .` (plus `--dry-run`). Every word of it is a constant, so no command-line value reaches its argv (the case SonarCloud's `pythonsecurity:S8705` flags). - **The child keeps family.py's escaped stdout** (review round, S1). family.py escapes what its stdout cannot encode (#214), but a child takes its stdout settings from its environment. So the child now gets `inherited_child_environment()`, the helper `update-shape.py` already uses for the validators it runs (#254): this stdout's own encoding and a handler that cannot raise. Under `PYTHONIOENCODING=ascii` the child used to die on the em dash of its first line, `UnicodeEncodeError`, exit 1. Now it prints `\u2014` and goes on. The helper already guards a stdout with no encoding, and it never passes on the handler only family.py registers, which a child cannot look up. - A holder without the file is refused: `siblings-script-missing`, exit 2, before anything is cloned, in the words the holder's Makefile refuses with since #503. **4. `init --work-dir` no longer promises clones** (`scripts/family.py`, `_print_next_steps`; review round, S2). `--work-dir <dir>` makes no family folder. Where `<dir>` is not named after the family, the NEXT STEPS used to say `make siblings` "then puts each member's WORKING clone beside the holder, in the family folder", and `family folder <dir>`. That `make siblings` now stops. The block now says `--work-dir made no family folder, so make siblings clones nothing: it prints the mv that puts the holder in a folder named <Family>, and stops until it is there`, and `family folder none: <dir> is not named <Family>`. The `--into` output is byte for byte what it was. **The half of #461 that #543 names.** The sibling clone is `--no-local`. A member url that is a path on disk is then read through git, not copied file by file, so a repack in the remote cannot break it halfway (#440). **Docs.** - AGENTS.md family item 5 and the holder's AGENTS-shape.md said the stop "exits 1"; `make siblings` itself exits 2, make's own code. Both now say exit 1 (`make`: 2), rewrapped within the same lines. AGENTS.md stays at 490 lines. - The holder's `README.md` says the run stops before cloning, and which tracking branch it clones on. - `templates/family-root/family.yaml`'s `tracking_branch:` comment said the family bootstrap places each member on that branch. It now says what it is: the holder's own branch, which `family.py` refuses to commit on. The family bootstrap leaves each member detached, and `make siblings` reads the member's own `project.yaml`. No test or pin compares those bytes. - README.md and docs/handbook.html are untouched (see "Left out"). | point | what was found | what this head does | |---|---|---| | S1 | the child process lost family.py's escaped stdout (#214) | `env=inherited_child_environment()`; test with `PYTHONIOENCODING=ascii` | | S2 | `init --work-dir`'s NEXT STEPS promised a `make siblings` that stops | the `folder is None` branch says there is no family folder; test on the fixture's own `init` stdout | | S3 | on a disk that ignores case, and at a root, the printed move cannot run | the exact name test is kept on every disk; the folder rename and the staged holder move; tests by simulation on all three systems | | N1 | a `tracking_branch:` holding control characters reached stdout raw; `5` and `[main]` said "declares no tracking_branch" | not used, shown in its `repr`; "is not a branch name"; three tests | | N2 | "before any credential is resolved" was unguarded | the claim is now "before any `git` command runs", pinned with `GIT_TRACE`; reading the environment had nothing a test could observe | | N3 | a symlinked family folder now stops, and its printed move cannot run | round 3: a link named after the family, beside the folder and leading to it, passes; see "Round 3" | | N4 | AGENTS.md:409 said "exits 1" for `make siblings` | exit 1 (`make`: 2), within the same lines; the same fix in the holder's AGENTS-shape.md | | N5 | the merge section was stale | rewritten below | | C2 | the `family.yaml` comment was false | fixed | | C3 | `family folder` printed above a stop | `parent folder` until the check passes | **S3, the decision: keep the strict name test on every OS.** Brett Heap's rule for the case variants of `members/` (#362-#364, "Refuse on every disk") is one answer on every disk. The same spirit applies here. Accepting `os.path.samefile(folder, wanted)` instead would pass a holder on macOS that Linux stops. So every printed remedy now has to work as typed. The evidence: - **Tests, on all three systems.** The template's `siblings.py` is loaded in-process with `load_without_bytecode`, so nothing is written under `templates/`. `Path.exists` and `os.path.samefile` are patched to answer as a disk that ignores case does. The case-only test then does the printed moves with `os.rename` and checks that the next check passes. A case-sensitive disk is simulated with nothing or a separate folder at `<p>/InkRouter` (the separate folder skipped on Windows, see "Round 3"), and a link there, which now passes. The roots are asked as `PurePosixPath("/InkRouter")`, `PureWindowsPath("C:\InkRouter")` and `PureWindowsPath("C:\inkrouter")`. - **Run as typed** (a throwaway probe, not committed). Each printed line went to `sh -c` as printed. A disk that ignores case was emulated with a symlink that spells the folder in the family's case, and that exists only while the folder does (a step that leaves it dangling removes it). Then a real `siblings.py` ran in the moved holder: | case | lines printed | run as typed | then `siblings.py` in the moved holder | |---|---|---|---| | a disk that ignores case, holder in `inkrouter/`: 43d32eb | `mkdir -p <t>/InkRouter`, then `mv <t>/inkrouter/InkRouter <t>/InkRouter/InkRouter` | the `mv` fails, rc 1: `cannot move ... to a subdirectory of itself` | the holder is not placed | | the same, this head | `mv <t>/inkrouter <t>/InkRouter.folder`, then `mv <t>/InkRouter.folder <t>/InkRouter` | both rc 0 | rc 0, no WARNING, IRRS and IRSS cloned beside the holder | | the holder at `/InkRouter`: 43d32eb | `mkdir -p /InkRouter`, then `mv /InkRouter /InkRouter/InkRouter` | the `mv` fails the same way | the holder is not placed | | the same, this head | `mv /InkRouter /InkRouter.holder`, `mkdir -p /InkRouter`, `mv /InkRouter.holder /InkRouter/InkRouter` | all rc 0 | rc 0, no WARNING, both cloned | | Linux, nothing at `<t>/InkRouter` | the three staged lines, the same on both heads | all rc 0 | rc 0, both cloned | | Linux, a separate `<t>/InkRouter` folder | `mkdir -p` and one `mv`, the same on both heads | both rc 0 | rc 0, both cloned | The filesystem root ran with `/` mapped to a scratch folder, because this machine has no root access and no user namespaces. `C:\InkRouter` and `C:\inkrouter` were asked of the function only. Both print `WARNING the holder's parent folder is 'C:\\', not 'InkRouter'.`, then `mv 'C:\InkRouter' 'C:\InkRouter.holder'` (or `'C:\inkrouter'`), `mkdir -p 'C:\InkRouter'` and `mv 'C:\InkRouter.holder' 'C:\InkRouter\InkRouter'`. Nothing here ran on a real macOS or Windows disk. CI's macOS and Windows jobs do run the case-only test's two renames on their own disks. New file: `tests/test_family_siblings_placement.py`, 29 tests: 10 from the first round, 13 from the review round, 2 from round 3, 4 from round 4. The runs below are the review round's, at 2156a01; round 3's are in its own section. All runs used a stub `gh` first on PATH, a bogus token, an empty `GH_CONFIG_DIR`, a blanked git identity, `/usr/bin/python3 -m pytest`: - `tests/test_family_siblings_placement.py` on the working tree, before the commit: `23 passed in 50.77s`. - Set A at 2156a01: every `tests/test_family*.py` (15 files), `test_park_resume_targets.py`, `test_windows_paths.py`, `test_doctor_legs_members_next.py`, `test_commit_trailers.py`, `test_free_plan_hint.py`, `test_refusal_default_remediation.py` and `test_stdout_encoding.py`: `1007 passed, 20 skipped in 983.34s (0:16:23)`. The skips are `test_family_init_values.py:219` (16) and `:291` (3), which the file documents, and `test_stdout_encoding.py:286` (Windows only). - Set B at 2156a01: `test_cli_reference.py`, `test_repo_hygiene.py`, `test_agents_md_codes.py`, the three `test_template_*` files, the four `test_validate_family_*.py`, the ten `test_update_shape*.py` of the first round, `test_shape_doctor.py`, and now `test_update_shape_kept_helper.py` (main's check of the template's imports) and `test_validator_child_encoding.py` (the helper S1 uses): `1478 passed, 6 skipped in 1658.89s (0:27:38)`. The skips are NTFS junctions (3) and the doctor's PowerShell line (3), both Windows only. - The merge commit 8b4fc0b on its own, before the fix: the new file, `test_family.py`, `test_family_printed_quoting.py`, `test_family_makefile_siblings.py`, `test_park_resume_targets.py`, `test_windows_paths.py`, `test_clone_just_pushed.py` and `test_doctor_legs_members_next.py`: `375 passed in 686.29s (0:11:26)`. And `test_cli_reference.py`, `test_repo_hygiene.py`, `test_agents_md_codes.py`, `test_update_shape_kept_helper.py` and `test_template_text_placeholders.py`: `340 passed in 245.06s (0:04:05)`. - Nothing is written under `templates/`: after the mutation baseline run (the new file's 23 tests, the in-process ones included, plus four more) a clean tree had no `__pycache__` under `templates/`. The script builds two scaffolded members (Atlas on `main`; Borealis on `trunk`, with its remote's default branch set to `main`) and a holder made with `init --work-dir`, so its parent folder is `fam`, not `Orbit`. | | bfed143 | this branch | |---|---|---| | first `make siblings` in `fam/Orbit` | exit 0; Atlas and Borealis cloned into `fam/`; WARNING after | exit 1 (make: 2); nothing cloned; `NOTHING WAS CLONED` | | clones on disk after move + re-run | 4: `fam/Atlas`, `fam/Borealis`, `Orbit/Atlas`, `Orbit/Borealis` | 2: `Orbit/Atlas`, `Orbit/Borealis` | | Borealis sibling branch | `main` (remote default) | `trunk` (its tracking branch) | | `family.py siblings` marker from the holder's copy | 0 lines (ran the standard's) | 1 line | | `family.py siblings`, holder without `scripts/siblings.py` | exit 0, ran the standard's copy | exit 2, `REFUSED siblings-script-missing`, the Makefile's words | | `make siblings`, same holder | exit 2, the Makefile's refusal | same | The review round's reproductions are rv543's probes: `PYTHONIOENCODING=ascii` through `family.py siblings` (rc 1 with a traceback at 43d32eb; the new S1 test holds rc 0), the `--work-dir` NEXT STEPS, and the printed moves above. Each probe reverts one hunk in a throwaway worktree. Every one is caught. **First round** (at 43d32eb; the new file plus the four changed existing tests, 15 tests): | probe | what it puts back | result | caught by | |---|---|---|---| | M1 | no `--branch=` on the clone | 3 failed | tracking branch, branch the remote lacks, the issue's flow | | M2 | no `--no-local` | 1 failed | no shared object file | | M3 | the warning no longer stops the run | 6 failed | both new misplaced tests, and the four changed existing tests | | M3b | the folder check back after the placement, as on main | 6 failed | the same six | | M4 | `tracking_branch()` never finds a branch | 4 failed | tracking branch, branch the remote lacks, dry-run line, the issue's flow | | M5 | the clone line names no branch | 3 failed | tracking branch, not-checked-out words, dry-run line | | M6 | the warning no longer says NOTHING WAS CLONED | 2 failed | both new misplaced tests | | M7 | `scripts/family.py` as on bfed143 | 2 failed | holder's copy and its imports; the refusal | | M8 | `runpy` of the HOLDER's file inside family.py | 1 failed | holder's copy and its imports | | M9 | one word of the refusal differs from the Makefile's | 1 failed | the refusal | | M10 | no refusal for a holder without its copy | 1 failed | the refusal | **Review round** (at 2156a01; the new file plus the same four, 28 tests; the unmutated baseline is `28 passed`): | probe | what it puts back | result | caught by | |---|---|---|---| | S1 | no `env=` on the child process | 1 failed | the ASCII stdout test | | S2 | `--work-dir` prints the old promise | 1 failed | the `--work-dir` NEXT STEPS test | | S3a | a root is not taken for the holder itself | 3 failed | the three root cases | | S3b | no rename of the folder | 1 failed | the case-only test | | S3c | a link named after the family is renamed through | 1 failed (at 2156a01; equivalent since round 3) | case-sensitive disk, `a link` | | S3d | `same_directory` asks the disk before comparing the paths | 3 failed | the three root cases | | S3e | the warning names a root `''` | 3 failed | the three root cases | | N1a | a control character is used as a branch | 1 failed | `escape-sequence` | | N1b | `5` and `[main]` "declare no tracking_branch" | 2 failed | `a-number`, `a-list` | | N2 | the credential and `.gitmodules` read before the stop (rv543's mutation B, which nothing caught before) | 1 failed | the `GIT_TRACE` test | | C3 | the header always says `family folder` | 1 failed | the misplaced dry run | The doc and comment hunks have no test: AGENTS.md, the holder's AGENTS-shape.md, the `family.yaml` comment, the docstrings, and the `tests/conftest.py` comment. Round 3's mutants are in its own section. **What was wrong.** Take `<t>/InkRouter`, a link to `<t>/real-x`, which holds the holder. Both entry points resolve the holder to `<t>/real-x/InkRouter` and saw a parent named `real-x`. On bfed143 that was a warning, and the members were cloned into `real-x`. This PR turned the warning into a stop. The printed `mkdir -p <t>/InkRouter` passes, then `mv <t>/real-x/InkRouter <t>/InkRouter/InkRouter` fails with `cannot move ... to a subdirectory of itself`, rc 1 (rv543 had read it as working). So the PR made a dead end. **The rule** (`siblings.py`, new `family_link`). Let R be the resolved holder, P its parent and G P's parent. The check also passes when `G/<Family>` is a link (`os.path.islink`) and is the same directory as P (`os.path.samefile`). The holder is then reachable as `G/<Family>/<R.name>`. The members are cloned into P, the link's own target, so the family layout holds through the link. - Both entry points give the same answer, because both resolve R: `make siblings` through `find_repo_root`, and `family.py siblings` by running the child in the resolved holder. - The header then reads `family folder <P> (through the link G/<Family>)`. It names the folder the clones go into. - A link named after the family that leads to another folder is not accepted. Its printed move runs as typed: the holder moves into the link's target, and there the link passes. - A link elsewhere (`~/InkRouter` leading to `/data/real-x`) still stops. Its printed move runs as typed. - The case-only rename of S3 is unchanged and stays strict. On a disk that ignores case, a case-only alias is one directory entry and never a link, so it gets the rename. The two disks differ at one edge only: a link spelled in another case stops on Linux, with a staged move that runs, and passes on a disk that ignores case (round 4 corrected this sentence). **Run as typed** (a throwaway probe; each printed line went to `sh -c` as printed, then a real `siblings.py` ran where the holder ended): | layout | 2156a01 | this head | |---|---|---| | `<t>/InkRouter` links to `<t>/real-x`, which holds the holder | stops; `mkdir -p <t>/InkRouter` rc 0, then the `mv` fails, rc 1, `cannot move ... to a subdirectory of itself`; the next run stops again | passes, rc 0, no WARNING; header `family folder <t>/real-x (through the link <t>/InkRouter)`; IRRS and IRSS cloned into `<t>/real-x` | | `<t>/home/InkRouter` links to `<t>/data/real-x` (a link elsewhere) | stops; the three staged lines (`mv <t>/data/real-x/InkRouter <t>/data/InkRouter.holder`, `mkdir -p <t>/data/InkRouter`, `mv <t>/data/InkRouter.holder <t>/data/InkRouter/InkRouter`) all rc 0; the next run passes and clones both | the same | | `<t>/InkRouter` links to `<t>/other`; the holder is in `<t>/real-x` | (not run) | stops; `mkdir -p <t>/InkRouter` and `mv <t>/real-x/InkRouter <t>/InkRouter/InkRouter` both rc 0; the next run passes through the link, header `family folder <t>/other (through the link <t>/InkRouter)`, both cloned into `<t>/other` | **Tests** (`tests/test_family_siblings_placement.py`, two new; links made with `os.symlink`, each test skipping first where the system refuses one; none under `templates/`): - the link layout passes through `family.py siblings --dry-run` and through `make siblings`' own script, which clones each member into the link's target; - a link named after the family to another folder stops, and its move done as written leads to a run that passes; - the case-only test still prints the two-line rename; - the simulated `a link` case of the case-sensitive disk test now patches `os.path.islink` too, and expects no warning. **Windows** (rv543b). `[a folder]` of the simulated case-sensitive disk test is skipped on Windows, before any call. A WindowsPath compares without case, so `same_directory()` calls `<p>/InkRouter` and `<p>/inkrouter` one path there, and the case-only rename is printed. That answer is right for Windows, whose disk cannot hold the two folders, but it is not the disk the parameter simulates. Emulated on Linux with rv543b's mechanism (`pathlib._is_case_sensitive` returning False during each test's call): `1 failed, 24 passed in 31.32s` with the parameter collected (only `[a folder]`, since `os.name` is `posix` here), and `24 passed, 1 deselected in 31.49s` without it. **Mutants** (at 0112dcb, the new file, 25 tests; the baseline is `25 passed`, and no `__pycache__` appeared under `templates/`): | probe | what it changes | result | caught by | |---|---|---|---| | R3a | `family_link` never finds a link | 3 failed | the link layout; the link to another folder (its moved holder no longer passes); the simulated `a link` | | R3b | any same directory counts, link or not | 5 failed | the case-only rename (a case-only alias would pass); the three roots; the placed dry run's header | | R3c | any link named after the family counts, wherever it leads | 1 failed | the link to another folder | | R3d | the folder check does not ask `family_link` | 3 failed | the link layout; the link to another folder; the simulated `a link` | | R3e | the header does not name the link | 1 failed | the link layout | Round 2's S3c (the `is_symlink` test inside the case-only branch) is now an equivalent mutant: every link is answered by `family_link` first, so that test can no longer decide anything (`25 passed` with it removed). It is kept, as the S3 branch is kept as it was. **Docs.** `siblings.py`'s module docstring (FIRST, EXIT CODES) and `parent_folder_warning`'s, and the holder's README.md and AGENTS-shape.md, say a link of the family's name beside the folder, leading to it, counts. AGENTS.md's family item 5 was left as it was in this round, on the belief that the clause needed a ninth line; rv543b showed it fits, and round 4 adds it. **Runs at 5ce5b68** (0112dcb changes only the test file; see round 4 for where `25 passed` was run): - the new file, `test_family_makefile_siblings.py`, `test_family_printed_quoting.py`, the whole of `test_family.py` and `test_family_tracking_branch.py`: `248 passed in 196.84s (0:03:16)`; - `test_template_text_placeholders.py`, `test_update_shape_kept_helper.py`, `test_cli_reference.py`, `test_repo_hygiene.py`, `test_free_plan_hint.py` and `test_update_shape.py`: `397 passed in 213.21s (0:03:33)`. | point | what was found | what this head does | |---|---|---| | S-1 | `family_link` asked only `os.path.islink`, which is False for an NTFS directory junction, the link Windows makes without privilege. `Path.resolve()` follows a junction, so that layout stopped and its printed `mv` failed, where bfed143 warned and cloned | `os.path.isjunction` counts too (Python 3.12 and later, False off Windows; read with `getattr`, so an older Python still runs the file, and there the junction case stays as it was). Not an "any alias" rule, which would make a link spelled in another case a dead end on macOS. A simulated junction test runs on every system | | F2 | AGENTS.md item 5 lacked the link clause the holder's README and AGENTS-shape.md have; this body said it could not fit | it fits: "(nor linked as it)", rewrapped within the same eight lines by dropping one "yourself"; AGENTS.md stays at 490. README.md:883 and docs/handbook.html:1306 make the same claim and are left to a later README change (see "Left out") | | F3(a) | the comment and this body said the case-only rename and the link rule "never meet" | corrected: a link spelled in another case (`inkrouter` leading to `real-x`) stops on Linux, with a staged move that runs, and passes as the family's link on a disk that ignores case. No dead end either way | | F3(b) | 0112dcb's skip reason said a Windows disk cannot hold the two folders | it says the simulation assumes Windows' default path equality, which ignores case; an NTFS folder made case-sensitive can hold them | | F4 | three conditions no test guarded | one test each: `init --work-dir` into a folder named after the family keeps the promise (S2c); a blank `tracking_branch: " "` is not a branch name (N1s); a second path to the folder under another name, neither link nor junction, is not answered by the case-only folder rename (S3f) | | F5, F6 | the staging names are not checked; a dangling link named after the family breaks the staged move halfway | checklist candidates, below | Also corrected: round 3's `25 passed in 34.28s` ran on the working tree at 5ce5b68 with 0112dcb's change not yet committed. At 0112dcb itself, the round-3 mutant baseline gave `25 passed in 32.69s`. **Mutants** (at 8d3d1cc, the new file, 29 tests): | probe | what it changes | result | caught by | |---|---|---|---| | R4a | a junction no longer counts (`isjunction` dropped) | 1 failed, 28 passed | the simulated junction | | S2c | `init --work-dir` always says there is no family folder | 1 failed, 28 passed | `--work-dir` named after the family | | N1s | a blank `tracking_branch` is used as a branch | 1 failed, 28 passed | `[blank]` | | S3f | the case-only folder rename for any second path to the folder | 1 failed, 28 passed | the rename only for a difference of case | The unmutated baseline is `29 passed`, and no `__pycache__` appeared under `templates/` after it. **Runs at 8d3d1cc:** - the new file on the working tree, before the commit: `29 passed in 21.84s`; - the new file, `test_family_makefile_siblings.py`, `test_repo_hygiene.py`, `test_agents_md_codes.py`, and `test_family.py -k "siblings or init"`: `195 passed, 71 deselected in 66.86s (0:01:06)`. **Main merged at 8b4fc0b** (`git merge --no-ff origin/main`, main at 9f80939: #551, #552, #554, #553, #561, #562, #579, #567). One hunk conflicted, in `tests/test_family_printed_quoting.py`: #554 added `env=NO_BYTECODE` to the `run_script(FAMILY, "siblings", ...)` call, and this branch changed the next line from `returncode == 0` to `== 1`. The resolution keeps both lines whole: ``` result = run_script(FAMILY, "siblings", "--family-root", str(holder), "--dry-run", env=NO_BYTECODE) assert result.returncode == 1, result.stdout + result.stderr # #543 ``` `tests/conftest.py`'s comment above `NO_BYTECODE` said `family.py siblings` runs the template's copy through `runpy`, which this branch makes false. It now says what the constant guards and why it stays on those two calls (lane 3 agreed to the wording on #554). `NO_BYTECODE` stays at both call sites; dropping it is a later cleanup. The merged tree differs from main only in this branch's files and that comment. **merge-tree at 8d3d1cc:** origin/main f5fc705 is clean and not merged (one more commit since dd3f23a, `update-shape.py`'s refusal of a hard-linked file); AGENTS.md is 490 lines in the merged tree. Lane 5's `fix/family-bump-to-checks` (8024f9c), `fix/family-members-case-variants` (e5aac93), `fix/family-remove-gitmodules-missing` (ebe6b51) and `fix/link-source-chain-defaults` (a707d4b): clean. #573 and #575 were not re-checked at this head: round 4 changes `siblings.py`, AGENTS.md and the new test file, none of which they touch. **merge-tree at 0112dcb** (`git merge-tree --write-tree`, after a fresh fetch): - origin/main dd3f23a: clean, and not merged; this PR is behind it by three commits. AGENTS.md is 490 lines in the merged tree and keeps this branch's line 409. - #573 `fix/537-dry-run-asks-remotes` (69c58aa) and #575 `fix/539-validators-lockstep-remedies` (0680799): clean. - Lane 5's `fix/family-bump-to-checks` (8024f9c), `fix/family-members-case-variants` (8a95345), `fix/family-remove-gitmodules-missing` (ebe6b51) and `fix/link-source-chain-defaults` (a707d4b): clean. **At 2156a01, for the record:** - origin/main 64fc9a8: clean. Main moved after the merge, by a35f1be (`update-shape.py`) and 64fc9a8 (the assembly root's `bootstrap.py`). Neither touches a family file. A scratch merge of this head with 64fc9a8 (no ref, since removed) ran the new file, `test_family_makefile_siblings.py`, `test_family_printed_quoting.py`, and main's new `test_update_shape_messages.py`, `test_update_shape_header_comments.py`, `test_update_shape_branch_any_directory.py` and `test_bootstrap_after_bump.py`: `269 passed in 156.48s`. Then `test_update_shape.py`, `_landing`, `_next_quoting`, `_pr_base`, `_kept_helper`, `_add`, `test_cli_reference.py` and `test_repo_hygiene.py`: `411 passed in 322.52s (0:05:22)`. - origin/main dd3f23a (`openRepoShape --doctor` wording; `shape-doctor.py`, `render-cli-reference.py`, AGENTS.md, README.md, the handbook, `docs/cli.md`): clean, not merged, not test-run. - #573 `fix/537-dry-run-asks-remotes` (69c58aa) and #575 `fix/539-validators-lockstep-remedies` (93b68f6): clean. - Lane 5's `fix/family-bump-to-checks` (8024f9c), `fix/family-members-case-variants` (40eda84), `fix/family-remove-gitmodules-missing` (b1120c3) and `fix/link-source-chain-defaults` (a707d4b): clean. - **README.md and docs/handbook.html.** README:882-884 and the handbook at :1305 say `make siblings` "WARNS and prints the exact `mv`". That stays true: the output still opens with `WARNING` and prints the `mv`. README:871-872 ("the same file through the other entry point") becomes true with this change. Neither says the run now stops; that is incomplete, not false, so this PR does not touch README.md. Adding "and clones nothing until it is moved" could be a later README change. That change should also add the link clause round 3 gave the holder's README and AGENTS-shape.md and round 4 gave AGENTS.md: README.md:883 and the handbook at :1306 say the run warns whenever the parent folder is not named after the family, which a family folder reached through a link no longer does. They are left alone here because a README change needs the hash step and README PRs land one at a time. - **`scripts/shape_materialize.py:519`**: its comment says siblings "warns about the parent folder without moving anything", which is still true. It is left alone because PRs that touch that file meet SonarCloud's S8705 finding at `run()`. - **`templates/family-root/scripts/validate-family.py`**: not touched (lane 2's #539). - **The `--make` dispatch header** still prints `family folder`. The dispatch runs no folder check (it is the placement's business), so it was left as it is. - **`make siblings` crashes on a stdout that cannot encode its em dash or a path.** Pre-existing on bfed143. The holder's `siblings.py` never escapes its own stdout, so under `PYTHONIOENCODING=ascii`, or a Windows code page without a character in the holder path or a member name, `make siblings` dies with a `UnicodeEncodeError` on its first line, before any clone. `family.py siblings` no longer does (S1), because it hands the child its own escaped settings. The family `bootstrap.py` already calls `repo_shape.escape_unencodable_stdout()`; a holder that kept an older `repo_shape.py` may not have it, which main's kept-helper check now looks at. - **`shape-doctor.py:5640`: the `members` row's note says "`make siblings` places them"**, a command that now stops, clones nothing, in a holder whose parent folder is not named after the family. - **A family name a disk cannot hold exactly never passes the folder test on that disk.** Windows drops a trailing `.` or space from a folder name and reserves names like `CON` and `NUL`. `--family` goes through `SAFE_ARG_RE` (`scripts/repo_shape.py:106`), which allows a trailing `.`. Such a holder stops on every run there. - **A `tracking_branch:` that names only a tag gives a detached clone.** `git clone --branch=<name>` takes a tag too, so a member that declares `v1` where its remote has only a tag `v1` is cloned detached at the tag, rc 0, and the line calls it "its tracking branch". - **`tests/test_cli_reference.py` writes `templates/family-root/scripts/__pycache__/bootstrap.cpython-312.pyc`** (found by rv441b on #576 and here). `scripts/render-cli-reference.py` runs the template's `siblings.py --help` with `PYTHONPATH=scripts`, and that file imports `bootstrap` from beside it. Shown in clean trees: no `__pycache__` before, the file after `tests/test_cli_reference.py` alone (`22 passed`), on 04c9d77, on two merged trees, and on this branch's merge. `.gitignore` hides it. A `.pyc` under `templates/` breaks `template_values()` and its twin in `test_scaffold_leg_path_collisions.py` in a later run in the same tree, which read every template file as UTF-8. #554's `load_without_bytecode` does not cover it; `PYTHONDONTWRITEBYTECODE=1` in the renderer's child environment would. - **A staging name that already exists sends the moves astray** (F5). Neither `<Family>.folder` (round 2) nor the older `<Family>.holder` is checked before it is printed. With `<t>/InkRouter.folder` already a folder, on an emulated disk that ignores case, both printed lines exit 0 and leave the holder at `InkRouter/inkrouter/InkRouter` (rv543b's probe). The next run stops again with a rename that works, so it is recoverable. The move text predates this PR; the stop is what makes it the only way forward. - **A dangling link named after the family breaks the staged move halfway** (F6). `not wanted.exists()` follows the link, so a dangling `<t>/InkRouter` reads as nothing there: `mv <t>/real-x/InkRouter <t>/InkRouter.holder` exits 0, then `mkdir -p <t>/InkRouter` fails, `File exists`, and the holder is left at `<t>/InkRouter.holder` (rv543b's probe). `os.path.lexists` would send it to the two-line move, whose `mkdir -p` fails before anything moves. The move text predates this PR; the stop is what makes it the only way forward. - **`tests/test_validator_child_encoding.py`'s launcher guard does not see `family.py`.** It looks for `subprocess.run([sys.executable, ...])`, and `family.py` runs `[sys.executable or PYTHON, ...]`, so the guard would not notice if `env=` were dropped again. The new S1 test catches that by behaviour (probe S1), but the static guard could list `family.py` too. Review: rv543 found 3 should and 5 nits on 43d32eb; the fix round merged main 9f80939 by hand (rv543's resolution) and fixed them; re-review rv543b found that the stop's printed move had no way forward through a family folder reached by a link (rounds 3 and 4 accept a symlink and a Windows junction), and its nits were fixed or listed as checklist candidates on #459. CI (Brett Heap's ruling 27): H0 8d3d1cc passed tests, tests-macos, tests-windows and SonarCloud; the head 3e3c833 is a clean update-branch merge of main fb786c1, with tests and SonarCloud green. Landed on Brett Heap's word of 2026-10-15T13:32:47Z, "All five, once green". Closes #543 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 15, 2026
…es not have as the index has it (missing, unstaged, untracked, or behind assume-unchanged or a sparse checkout), so neither commits a change to it nobody staged (#407) (#571) `family.py remove` commits `.gitmodules` as `git rm` leaves it in the work tree, and `git rm` edits only a `.gitmodules` the work tree has. With the work-tree copy deleted and the deletion not committed, `git rm` edited nothing. `_commit_one` then staged the deletion, and the commit took `.gitmodules` out of HEAD, with every other member's section in it. `remove` exited 0, and the holder it ran in passed its own gate. Once the commit was pushed, a recursive clone left every remaining member empty. The issue's reproduction, run on bfed143 (I added one `git switch -c`, because #487 landed after the issue was filed): ``` registration keys before: Atlas 2, Nova 2 dry run: exit 0 gitmodules none: git reads no section of .gitmodules in the work tree as giving members/Nova, so none is removed remove: exit 0 committed 766cfa532913: .gitmodules, members/Nova, family.yaml .gitmodules | 6 ------ HEAD holds .gitmodules: no registration keys after: Atlas 2, Nova 0 family ok: Fam (fam), 1 member(s) recursive clone: exit 0 the clone's members/Atlas holds 0 entries REFUSED member-uninitialized: members/Atlas is not an initialized submodule checkout, ... fatal: No url found for submodule path 'members/Atlas' in .gitmodules ``` One check, `_checked_gitmodules_as_staged`, runs before any git command that writes. The dry run makes it too, in the same words. It reads the index's records for `.gitmodules` (stage, blob and git's `-v` flag letter) in one `git ls-files -s -v -z`, which writes nothing. - **`gitmodules-missing`** (`remove`): the work tree has no `.gitmodules`, while the index (at any stage) or HEAD has one. The remediation names the restore git takes in that state: - `git checkout -- .gitmodules` (the index's copy); - `git checkout HEAD -- .gitmodules`, where the deletion is staged; - for an unmerged file, by the stages the index holds: `git checkout --merge -- .gitmodules` where it has both sides' copies; `git checkout --ours` where only this side's is left (the other side deleted it); `git checkout --theirs` where only the other side's is; `git checkout-index -f --stage=1` where both sides deleted it. Each is followed by `git add .gitmodules`. - **`gitmodules-unstaged`** (`remove` and `add`): the work tree's copy is not the index's. That means one of: - git lists it as modified (`git ls-files -m`). Where `git add` would still stage the index's own blob from it (`git hash-object --path=.gitmodules` says so, and it is a regular file), the refusal adds that the difference may be its line endings alone; - it is unmerged; - its deletion is staged while the file is still on disk; - the index marks it assume-unchanged, which `ls-files -m` does not look past, and its content is not the index's blob; - it is untracked: neither the index nor HEAD holds one. This includes the first member's `add` in a new holder. The remediation offers both ways out: stage the work tree's copy, or put back the index's (or HEAD's) copy. Behind assume-unchanged, staging first takes the mark off (`git update-index --no-assume-unchanged -- .gitmodules`), because `git add` stages nothing while it is on. An untracked file is staged with `git add -f .gitmodules`, since an operator's global excludes may match it (and, if wanted, committed on its own first), or moved out of the holder. - **`gitmodules-sparse`**: the index marks `.gitmodules` skip-worktree, as a sparse checkout that leaves it out does. git neither compares nor stages that file, and `git checkout -- .gitmodules` matches nothing. The remediation is `git sparse-checkout disable`. - `add` refuses it whatever the file's state, since `git submodule add` cannot stage it. - `remove` refuses it where the file is missing or is not the index's copy, and commits a clean copy on disk rightly, as it did on bfed143. Every refusal ends with "No git command that writes was run and nothing was written.", and every remediation ends with "run the same command again". Typed as printed, each one leads to the right commit (the tests do exactly that). `remove` asks this first, before #360's `member-path-registered-twice`. `add` asks it after its mount-path refusals (#242, #300), which already name what they find in the same file. Two of those, `member-name-taken` and `gitmodules-unreadable`, have the person mend the file by hand and run `add` again. They now say "then `git add .gitmodules`", as #300's and #360's remediations already did. Without that step, the run again was this PR's own `gitmodules-unstaged`. The holder's `validate-family.py` keeps its words: it reads the work tree's copy, so re-running it needs no stage. | Point | What changed | |---|---| | S1 | `member-name-taken` and `GITMODULES_UNREADABLE` say "then `git add .gitmodules`". New test: each one's state, mended by hand, the printed stage typed, and `add` then mounts the member. The text tests are updated (`test_family_add_excludes.py`'s `member-name-taken` test, and `test_refusal_default_remediation.py`, which now holds `add`'s remedy equal to the gate's but for the one step). `test_family_add_refusals.py::test_a_gitmodules_git_cannot_read_is_a_refusal_not_a_pass` pins the validator's text, which is unchanged, so it is untouched, and so is the `:518` test, which asserts only the REFUSED line. | | N1 | The line-endings clause, only where `hash-object` gives the index's blob from a regular file. Tested with a CRLF copy (clause) and a link to the very same bytes (no clause: `hash-object` reads through the link, but git would stage the link). | | N2, sparse | `gitmodules-sparse`, with `git sparse-checkout disable`. Tested by a real `git sparse-checkout set --no-cone '/*' '!/.gitmodules'`, for both verbs, typed as printed. | | N2, modify/delete | The per-stage restores above. Tested for stages 1+2 (`--ours`), 1+3 (`--theirs`) and 1 alone (`checkout-index`), each typed as printed. | | N3 | `test_a_holder_whose_head_has_no_gitmodules_is_not_asked` asserts only that no `gitmodules-` refusal is made. Its docstring says the real run fails there, a known case from before #407 that goes to #459. | | N4 | The base split below is corrected. | | N5 | `unmerged()` and `typed_as_printed()` run git through `raw(root, env, ...)`, under the module's isolated environment. `raw()` gained a `stdin` argument for `update-index --index-info`. | | N6 | `held()` and `assert_held()` are gone. Refusals are read with `test_family_dry_run.state()` and `assert_as_it_was()`; `state()` now reads a missing `.gitmodules` as None (three lines). | | Codex, `:4476` | The untracked `.gitmodules` is `gitmodules-unstaged`, for both verbs, dry run alike. Tested: the first `add` in a holder with no member, and `remove` in the holder #407 leaves behind with the old bytes put back untracked. Staged as printed, each commits. | | Codex, `:4519` | assume-unchanged: compared by content (`hash-object` against the stage-0 blob) and refused. skip-worktree: `gitmodules-sparse`, as for N2. Tested for both verbs: the assume-unchanged edit kept through the printed `update-index` and `add`, and the skip-worktree edit through `git sparse-checkout disable`, then the restore the next refusal prints. | | Nit | What changed | |---|---| | 1 | `remove` again commits a clean `.gitmodules` the index marks skip-worktree, as bfed143 and b1120c3 did: `gitmodules-sparse` is asked for `add` whatever the file's state, and for `remove` only where the file is missing or is not the index's copy. `add`, an edited file and a missing one stay refused. The function's and the module's docstrings say which verb refuses what. New test: `remove` with the mark on and the file clean commits exactly `.gitmodules`, `family.yaml` and the gitlink. | | 2 | Two cases the round-2 tests left open. X1: `add` over a file marked both skip-worktree and assume-unchanged (two `update-index` calls, `ls-files -v` tag `s`) is `gitmodules-sparse`. X2: a clean assume-unchanged file passes, and both verbs commit the right tree. Each set-up asserts the `ls-files -v` letter it expects. | | 3 | The untracked branch prints `git add -f .gitmodules`. The untracked `add` test also runs under an operator's `core.excludesFile` holding `*` (set as #242's tests set it), asserts that the file is ignored there, types the remedy, and `add` commits. | | 4 | Decision 5's wording. Round 2's commit subject said "no printed command leads into another refusal", which is inexact: for an edit hidden by skip-worktree, `git sparse-checkout disable` leads, by design, into `gitmodules-unstaged`, and `test_an_edit_skip_worktree_hides_is_refused_then_named_as_modified` pins that chain. This round's commit subject says only what it does. | 1. **Refuse, not rebuild (the issue's first option).** The holder is left byte for byte as it was, and the person picks the copy they mean. Building this member's section out of the index's blob would commit a `.gitmodules` the work tree does not have. The holder would then show `.gitmodules` as deleted, and the person's next `git commit -a` would delete it after all. For a modified copy it would also have to guess which copy the person means. 2. **A staged change still goes in.** The remediations of #360 and #300 tell the person to edit `.gitmodules`, `git add` it, and run the same command again. So the rule is "the work tree's copy must be the index's", not "must be HEAD's". 3. **"Modified" is git's own answer.** It comes from `git ls-files -m`, the comparison `git status` uses and the one `git rm` refuses an unstaged change with. On bfed143, an unstaged edit, a CRLF-only copy and a symlink were each refused by `git rm` only after `git submodule deinit -f` had emptied the member (then rolled back), and each dry run exited 0. The CRLF case is still refused (as `git rm` refuses it), and now says why it may look like no change: its `hash-object` is the index's blob (probe: both `26f54704`). 4. **Assume-unchanged is compared by content; skip-worktree is refused outright.** Probes against round 1's check (`states-wip.log`): - assume-unchanged, clean: both verbs commit correctly, so it is not refused. Edited: `ls-files -m` is empty and both verbs committed the edit. `git add .gitmodules` with the mark on exits 0 and stages nothing (the hash stays `a2d4d491` against the index's `26f54704`), so the keep-remedy takes the mark off first. `git checkout -- .gitmodules` restores it with the mark on. - skip-worktree, file on disk: `remove` committed a hidden edit; `add` failed at its mount even for a clean file (`git add` refuses a path outside the sparse-checkout definition, then rollback). `git add` and `git checkout --` both fail while the mark is on. So `add` refuses any skip-worktree `.gitmodules`, on disk or not. `remove` refuses one that is missing or is not the index's copy, and commits a clean one on disk rightly, as it did on bfed143 and b1120c3 (round 3; round 2 refused that one too). 5. **`git sparse-checkout disable` is the one command that works as typed both in a sparse checkout and where only the mark is left.** `git sparse-checkout add /.gitmodules` is narrower in a real non-cone sparse checkout, but fails for a mark set by hand (`fatal: no sparse-checkout to add to`). With sparse checkout on, `git checkout --ignore-skip-worktree-bits -- .gitmodules` and `git update-index --no-skip-worktree` + `git checkout --` both put the file back, but `remove` then failed at its commit's `git add` ("outside of your sparse-checkout definition"). `disable` worked with sparse checkout on and with only the mark left, and keeps a copy already on disk as it is. The refusal names the state in its own words. It is a dozen lines, not a sparse-definition reader. 6. **An untracked file is staged, not committed by the printed command, and staged with `-f`.** `add`'s branch check (#487) comes after this refusal, so a printed `git commit` could land on the tracking branch. Staged, the file goes into the verb's commit; the remediation says to commit it on its own first if that is not wanted. An operator's global excludes may match the untracked file (#242's `*`), and a plain `git add` then refuses it, so this branch prints `git add -f .gitmodules`. The tracked branches keep the plain `git add`, which stages a tracked file under the same excludes. 7. **The dry run (N9).** In the issue's state, `remove --dry-run` prints the same refusal, exit 2. Past the refusal, the work tree's file is the index's copy, or no copy exists anywhere. So `deinit` reads the same name the dry run reads. 8. **`bump` is not affected.** Its commit is `[path, MANIFEST]` and never stages `.gitmodules`. Probe: `bump` with the work-tree `.gitmodules` deleted committed `family.yaml` and `members/Atlas` only. `tests/test_family_remove_gitmodules.py`, 35 tests: - `remove` over each state the work tree lacks (6): deleted (the issue), deletion staged, and unmerged with the file deleted at stages 1+2+3, 1+2, 1+3 and 1. Both runs refuse alike, exit 2, nothing on stdout, and the holder unchanged: `test_family_dry_run.state()`, which is HEAD, `ls-files -s -z`, `status --porcelain=v1 -z`, `family.yaml`, `.gitmodules`, `.git/config`, the member's HEAD, every work-tree byte and the holder's objects. The remedy is typed as printed; `remove` then exits 0, HEAD's `.gitmodules` gives only the other member's path, and the gate passes; - the issue's clone: after the restore and `remove`, a push and a fresh `clone --recurse-submodules` populate the other member, and the clone's gate exits 0; - each work-tree copy that is not the index's, for both verbs (12): edited, CRLF only, a link to the same bytes (skipped on Windows), assume-unchanged and edited, unmerged, deletion staged with the file kept. One printed remedy is typed, and the verb then commits; - sparse (4): a real sparse checkout for both verbs, and `add` over a clean file marked skip-worktree by hand and over one marked both ways (`ls-files -v` tag `s`); skip-worktree with an edit (2), both verbs; - a clean file behind a mark, committed with exactly `.gitmodules`, `family.yaml` and the member's gitlink (3): skip-worktree (`remove`), assume-unchanged (`remove` and `add`); - untracked (3): `add` in a new holder, with no excludes and under an operator's `*`; `remove` in a holder #407 already damaged; - `add` over a missing file is still git's own refusal, with its dry run's note; - a holder whose HEAD lost `.gitmodules`: no `gitmodules-` refusal (N3); - S1 (2): `member-name-taken` and `gitmodules-unreadable`, mended by hand, the stage typed as printed, `add` then mounts. Round 2's 29 tests on round 1's head b1120c3: `15 failed, 14 passed in 80.15s`. The 15 are exactly round 2's cases: CRLF's clause (2), sparse (2), skip-worktree (2), untracked (2), S1 (2), the `--ours`, `--theirs` and `checkout-index` restores (3), and assume-unchanged (2). On bfed143 with them: `27 failed, 2 passed in 59.67s`; the 2 that pass pin behaviour the fix keeps (`add` leaves a missing file to git, and N3's holder is not asked). Round 1's base proof, corrected (the review's N4): on bfed143 with round 1's 14 tests, `12 failed, 2 passed`. Of the 12, 8 failed on "HEAD changed" (the verb committed; the clone test is among them), 3 on ".git/config changed" (`lacks[deletion staged]`, `[edited-remove]` and `[crlf-remove]`: the run failed after `deinit` and the rollback re-wrote the registration) and 1 on "status changed" (`[unmerged-remove]`). In `lacks[deletion staged]` it was not `git rm` that failed after `deinit` but `_commit_one`'s `git --literal-pathspecs add -f -- .gitmodules family.yaml` (`pathspec '.gitmodules' did not match any files`). On this head (ebe6b51), Linux, `/usr/bin/python3 -m pytest`, the files round 3 touches and the importers of the changed helpers: `tests/test_family_remove_gitmodules.py tests/test_refusal_default_remediation.py tests/test_family_add_excludes.py tests/test_family_dry_run.py tests/test_family_member_path.py`: `182 passed in 224.37s (0:03:44)`. The new file alone: `35 passed in 46.50s`. Round 2's targeted runs, on cb159b1 (round 3 changes `family.py`'s skip-worktree condition and one remedy, and the new test file): - `tests/test_family_remove_gitmodules.py tests/test_family_bump_remove_rollback.py tests/test_family_dry_run.py tests/test_family.py tests/test_cli_reference.py tests/test_repo_hygiene.py tests/test_family_member_path.py tests/test_family_tracking_branch.py tests/test_family_printed_quoting.py tests/test_family_member_name.py tests/test_family_bump_to_a_tag.py tests/test_family_init_landing.py tests/test_family_init_values.py tests/test_template_text_placeholders.py`: `706 passed, 19 skipped in 1013.94s (0:16:53)` (the skips are `test_family_init_values.py`'s template-quoting cases, as on main); - `tests/test_family_add_refusals.py tests/test_validate_family_duplicates.py tests/test_validate_family_member_path_glob.py tests/test_validate_family_unnamed_mounts.py tests/test_validate_family_without_git.py tests/test_family_add_excludes.py tests/test_family_add_member_id.py tests/test_refusal_default_remediation.py tests/test_agents_md_codes.py tests/test_shape_doctor.py`: `632 passed, 3 skipped in 1482.84s (0:24:42)` (the skips are the Windows-only PowerShell line). Windows: every file the tests write is written with `write_bytes`, git's stdin gets bytes, and nothing compares against a file a helper wrote with `write_text`. The holder's attributes keep `.gitmodules` LF in the work tree (`i/lf w/lf attr/text=auto eol=lf`). The symlink case skips on Windows, where git checks a link out as a plain file. macOS: no case variants are involved. The issue's script, with local bare remotes, a `gh` that exits 97, no global or system git configuration, and the registrations read from `.git/config` by `grep`: - on round 2's head (cb159b1): `dry run: exit 2` and `remove: exit 2`, both `REFUSED gitmodules-missing: ...` with the same stderr, and a full snapshot of the holder (every file, `.git` included) identical after each. The registration sections stay 1 and 1. After a push of the unchanged holder, the recursive clone exits 0, `members/Atlas` holds 15 entries, and the clone's gate prints `family ok`. - round 1 typed the remedy as printed (`git checkout -- .gitmodules`): `remove: exit 0`, a commit with `.gitmodules | 3 ---` (Nova's section only), the holder's gate exits 0, and after a push the recursive clone populates Atlas and its gate exits 0. The lacks and clone tests repeat that on this head. - round 2's states, on cb159b1: for `remove` and `add`, a real sparse checkout, a skip-worktree edit, an assume-unchanged edit (kept, and dropped), CRLF and a link; for `remove`, an assume-unchanged deletion, the unmerged file deleted at stages 1+2, 1+3, 1 and 2+3, and the untracked file in a damaged holder; for `add`, the untracked file in a new holder. Each is refused alike by both runs (identical stderr), each printed command exits 0 as typed, and the run again commits what the remedy chose. Round 3 changes two of these paths (a clean skip-worktree file for `remove`, and the untracked remedy's `-f`), and its tests on ebe6b51 hold both. Nothing here needs real GitHub or a cloud tool. Round 2's code hunks, each reverted alone in a throwaway worktree at cb159b1, with the new file run against it: | hunk reverted | result | new tests that fail | |---|---|---| | M1 S1, `GITMODULES_UNREADABLE`'s stage | 1 failed, 28 passed | S1 `[gitmodules-unreadable]` | | M2 S1, `member-name-taken`'s stage | 1 failed, 28 passed | S1 `[member-name-taken]` | | M3 the untracked refusal (Codex, `:4476`) | 2 failed, 27 passed | both untracked tests | | M4 `gitmodules-sparse` | 4 failed, 25 passed | sparse `[remove]`, `[add]`; skip-worktree `[remove]`, `[add]` | | M5 the per-stage restores (always `--merge`) | 3 failed, 26 passed | `lacks[our side only, deleted]`, `[their side only, deleted]`, `[neither side, deleted]` | | M6 the line-endings clause (N1) | 2 failed, 27 passed | `[crlf-remove]`, `[crlf-add]` | | M7 `_stages_as`'s regular-file guard | 2 failed, 27 passed | `[a link to the same bytes-remove]`, `[...-add]` | | M8 the assume-unchanged branch (Codex, `:4519`) | 2 failed, 27 passed | `[assume-unchanged, edited-remove]`, `[...-add]` | | M9 the flags read as `H` (`_gitmodules_in_index`) | 6 failed, 23 passed | the 2 assume-unchanged, 2 sparse and 2 skip-worktree cases | | M10 the return where nothing holds a `.gitmodules` | 29 errors | every test, at the family fixture's first `add` into a holder with no `.gitmodules` anywhere, which is then refused as untracked | Round 1's hunks are still held by the same tests (round 1's probes M1 to M8). The other hunks are docstrings and comments. Round 3's hunks, each reverted alone at ebe6b51, with the new file run against it: | hunk reverted | result | new tests that fail | |---|---|---| | R1 the narrowing (always refuse skip-worktree) | 1 failed, 34 passed | `[skip-worktree-remove]` of the clean-behind-a-mark test | | R2 its `verb == "add"` arm | 2 failed, 33 passed | sparse `[marked by hand, clean-add]`, `[marked both ways, clean-add]` | | R3 X1, `tag == "S"` without `.upper()` | 1 failed, 34 passed | sparse `[marked both ways, clean-add]` | | R4 X2, every assume-unchanged copy refused | 3 failed, 32 passed | `[assume-unchanged-remove]`, `[assume-unchanged-add]`, sparse `[marked both ways, clean-add]` | | R5 plain `git add` in the untracked remedy | 3 failed, 32 passed | the three untracked tests (the pinned text) | | R5b plain `git add` in the remedy AND the test's expected text | 1 failed, 2 passed (`-k untracked`) | `[*]`: typing `git add .gitmodules` fails, "The following paths are ignored by one of your .gitignore files ... Use -f" | `git merge-tree --write-tree` of this head (ebe6b51) with each of these is clean: - origin/main dd3f23a. Since bfed143, main took #551, #552, #554, #553, #561, #562, #579, #567, #558, #556 and #580; none touches `scripts/family.py` or a test file this branch changes. This branch does not merge main; - `fix/family-bump-to-checks` 8024f9c (#568), `fix/family-members-case-variants` 8a95345 (#574), `fix/link-source-chain-defaults` a707d4b (#576), `fix/family-siblings-tracking-branch` 2156a01 (#578); - lane 2's `fix/537-dry-run-asks-remotes` 69c58aa (#573), and #575's head 0680799 (lane 2, which changes `validate-family.py`; its unreadable-file remedy still ends "then run this again.", which `test_refusal_default_remediation.py` expects). - **(a) `family.yaml` hand edits are committed by `add`, `bump` and `remove`.** Each commits `family.yaml` as the work tree has it, so an unstaged hand edit outside `members:` (a comment line) goes into the commit: the review reproduced `+# a hand note nobody staged` in a commit of each verb. It is #407's rule for a different file, and a staged-only rule there would chain into every remedy that says "edit `family.yaml`, then re-run", as S1 did here. It needs its own issue. - **(b) A HEAD that already lost `.gitmodules` fails after the first write.** With no `.gitmodules` anywhere, this check asks nothing. `remove --dry-run` exits 0 and says no section is removed; the real `remove` runs `deinit` and `git rm`, then dies at `git --literal-pathspecs add -f -- .gitmodules family.yaml` (`pathspec '.gitmodules' did not match any files`) and puts back only part of the unmount (`members/Nova` is left empty). Same on bfed143. Once the old file is put back in the work tree, this PR now refuses it as untracked, and staged as printed `remove` commits it back into HEAD; but nothing tells the person to do that. - **(c) The gate passes a row with no `.gitmodules` section (#459, the issue's decision 3).** `validate-family.py` should find a `members:` row whose path no section of HEAD's `.gitmodules` gives. A holder already carrying a #407 commit passes its own gate, and only a fresh recursive clone shows it. - **(d) A sparse checkout whose rules leave `.gitmodules` out, with the file put back on disk by hand.** git then clears the skip-worktree mark, so this check passes, and `remove`'s commit fails at `git add` ("outside of your sparse-checkout definition") and is rolled back. Nothing wrong is committed; it is a late failure, and reading the sparse rules is not this PR's. - **(e) A verb run during a merge, cherry-pick or rebase fails after its first writes.** Any `family.py` verb run while one is in progress fails at its commit, `git commit -- <paths>` ("fatal: cannot do a partial commit during a merge."), and is rolled back. It is the same on bfed143. It touches this PR because its `--ours` and `--theirs` restores name modify/delete conflicts, whose commonest source is `git merge`: typed as printed the restore works, then `remove` fails late. The tests build those stages with `update-index --index-info` and no `MERGE_HEAD`, the state a conflicted `git stash pop` leaves. The candidate: refuse an in-progress merge, cherry-pick or rebase before the first write, beside the existing up-front refusals. Review: rv407 found 1 should and 6 nits on b1120c3; fix round 2 (cb159b1) fixed them and answered Codex's `:4476` and `:4519` threads; re-review rv407b found 4 nits, fixed in round 3 (ebe6b51). CI (Brett Heap's ruling 27): H0 ebe6b51 passed tests, tests-macos, tests-windows and SonarCloud; the head 52c7c88 is a clean update-branch merge of main 2989b4c, with tests and SonarCloud green. Landed on Brett Heap's word of 2026-10-15T13:32:47Z, "All five, once green". Closes #407 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 15, 2026
…e `--link-source` is the first chain link's tree alone, and a source that names no path is refused (#441, #442) (#576) **README change.** This PR edits one README paragraph. #551 is on main and this branch has merged it, so the handbook step follows #551's rule: the three page parts that read the README section "The three legs" record its new hash. No page text changes. ### #441: one bare tree answered for two links `link_pins_from_trees` passed its `default_source` to every link of a recorded chain. That value is the naming gate's bare `--link-source PATH`, and the scaffold's `--pin-source`. In the chain `openXdox → openYdox → openInk`, with one bare tree that is `openXdox`'s, `openYdox` was read from the same `project.yaml`. That file declares `openYdox`, not `openInk`, so the gate said "openYdox declares openYdox, not openInk". For `codexInk`, which holds no direct pin on its referent, that was a FINDING, exit 1, for a chain that holds. The scaffold refused the same chain as `chain-not-declared`. Now: - **Which links read a bare value (for #540 and anyone else relying on it):** only the FIRST link of each chain, `links[0]`, the pin the project holds. That is true for a chain of any length, one link included: a chain of one link has only its first link, and that link reads the bare tree as before. A later link is never offered the bare value. It is read from its own `LINK=PATH`, its `SHAPE_PIN_SOURCE_<LINK>` or a checkout beside the project; otherwise it is declared-unverified (a warning, exit 0). For the first link the order is unchanged: keyed, variable, checkout beside the project, then the bare value. - `link_pins_from_trees` and `resolve_link_source` keep their signatures and return shapes. The only code change in `repo_shape.py` is `None if index else default_source` in the loop. - The two-link case (`openXdox → openDox` with a bare path) reads exactly as before and prints the same lines. The one difference is inside the returned mapping, and only for a chain of two or more links: the chain's END link no longer gets an entry read from the bare tree. Nothing reads that entry; only holders' declarations are read. In a chain of one link the only link is both the first and the end, and it still reads the bare tree, as before. - In the naming gate, `LinkSources` finds the first link of every chain a name will READ before any chain is read. `_first_links_read` uses the same `reads_link_declarations` test the lookup uses, and counts a chain only if it has two or more links: a chain of one link has no holder, reads no declaration, and so has no first link to count. The bare value is offered only while there is at most one first link: with a lone one-link chain there are none, and the bare tree is still returned for that chain's link to read, as before (nothing reads what it finds, so the output is unchanged). If a run reads two chains that begin at different links (a name on the command line plus `--project`, or one project with two legs), the bare value is read for neither. Before, one tree answered both, and `used` counted it once. - A bare value no link took is a NOTE saying why. In a two-link run the sentence is the one it always was. In a longer chain it reads "a bare PATH is the tree of a chain's first link alone, openXdox, which had a more specific source (...)". With two first links it reads "the chains read here begin at openXdox, openYdox: name the link it is, as --link-source <link>=PATH". ### #442: an empty path was the current directory `--link-source openXdox=` and `--pin-source openXdox=` became `Path("")`, which is `.`. From a project root, the naming gate read the project's own manifest as `openXdox`'s and called the chain broken. `validate-pins.py` asked the project root for the pinned commit, and its finding blamed `.`. Now both tools refuse, exit 2, naming the key: ``` REFUSED link-source-no-path: `--link-source openXdox=` names no path for openXdox. An empty path would be read as the directory this runs in. Give the tree's path (`--link-source <link>=<path>`), or `.` to mean this directory. REFUSED pin-source-no-path: `--pin-source openXdox=` names no path for openXdox. An empty path would be read as the directory this runs in. Remediation: give the checkout's path (`--pin-source <product>=<path>`), or `.` to mean this directory. ``` - An empty bare value (`--link-source ''`, `--pin-source ''`) is refused the same way. - A typed `.` still means the current directory. - A value whose key is NOT a chain link or a declared pin (`openZdox=`) is still the bare PATH `openZdox=`, as #338 and #295 decided. It is not empty, so it is not refused. - **Behaviour change: an empty value is refused even when a later value replaces it.** `--link-source openXdox= --link-source openXdox=<tree>` was a NOTE ("the later `--link-source ...` replaced it"), exit 0. It is now `REFUSED link-source-no-path`, exit 2. `validate-pins.py` changes the same way: `--pin-source openXdox= --pin-source openXdox=<clone>` was `pins ok`, exit 0, and is now `REFUSED pin-source-no-path`, exit 2. A typo is a typo, and the refusal names it when it is typed; a script that passed an empty value first and the real one after must drop the empty one. - The naming gate refuses in `main`, right after the values are read and before stdin is read or any name is classified, like its own `REFUSED naming-no-target`. - `validate-pins.py` raises a `Refusal` from `PinSources`. It reaches `main`'s existing handler with its own remedy, not the lockstep one. **It is raised late:** `PinSources` is built inside `_check_neutral_product_pins`, which `_run_all_checks` calls after `_check_leg` and `_check_shape_pin`. So an earlier refusal wins: a damaged `contracts/spec-pin.yaml` plus `--pin-source openXdox=` prints `REFUSED pin-tag-only`, and the empty path is named on the run after that is fixed. The hunk stays inside `PinSources` on purpose, to stay clear of the lines #539 edits in `_run_all_checks`. The refusal also has to wait for the declared pins, because they decide whether `openXdox=` is keyed or a bare PATH. Nothing is printed before the refusal, and the exit is 2 as in every other refusal of that tool. ### Docs - `--help` for `--link-source`, the naming validator's module docstring (exit code 2 included), `resolve_link_source`'s rule 4, `link_pins_from_trees`'s docstring and the scaffold's `_declaration` docstring say what a bare value means in a longer chain. - The `--link-source` help also says that a bare PATH is read for neither link when the chains read begin at two different links, and that a later link is read from its own `PRODUCT=PATH`, its `SHAPE_PIN_SOURCE_<PRODUCT>` or a checkout beside the project (it said "needs its own `PRODUCT=PATH`"). - `docs/cli.md` is regenerated. - README's lookup-order sentence now reads "then a bare `--link-source <path>`, for the first link's tree only: the order `validate-pins.py` uses below for the same checkout." The paragraph keeps its 12 lines, and README keeps its line count. - The handbook names no `--link-source`. The three parts that read "The three legs" (the masthead, "Reading private legs in CI", "The three legs, and naming") move to that section's new hash, 6ebed796 → 39ec2c23. No other README section changed. - `validate-pins.py`'s `--help` and EXIT CODES docstring do not name `pin-source-no-path`. Nothing in them is false: the exit-2 list gives examples. I left them, because the module docstring sits near lines #539 edits. The review of 04c9d77 (APPROVE WITH NITS) found five nits. Each, and what this commit does: 1. **A chain of one link counted as a first link.** `--referent-chain openInk` beside a project whose leg records `openXdox → openQuill`, with a bare `openXdox` tree, gave two first links, so the bare tree went to neither. `codexQuill`, verified through it on bfed143, was declared-unverified, with a NOTE naming `openInk` as a first link. Both places now count `chain[0]` only when the chain has more than one link: `_first_links_read`, and the `firsts` update in `LinkSources.link_pins`. The run is verified again, with no WARNING and no NOTE, as on bfed143. New test `test_a_chain_of_one_link_is_no_first_link_read`. 2. **A test gap: the project half of `_first_links_read`.** Reverting `if data is not None:` to `if False:` (mutation M14) left all 20 tests passing, yet it changes the output for one project whose two legs begin at different links: the first classification pass counted the bare value used, and the NOTE went missing. New test `test_a_project_whose_legs_begin_at_two_links_reads_a_bare_tree_for_neither`: `openXdox → openInk` and `openYdox → openQuill` in one project, a bare `openXdox` tree, two WARNINGs and the two-first-links NOTE. M14 now fails it (see the mutation probes). 3. **`pin-source-no-path` is raised late in `validate-pins.py`.** No code change. The behaviour and the reason are written above, under #442. 4. **The `--link-source` help.** It now says a bare PATH is read for neither link when the chains read begin at two different links, and "needs its own `PRODUCT=PATH`" now names the variable and the checkout beside the project too. `docs/cli.md` is regenerated, and `render-cli-reference.py --check` is ok. 5. **This body.** It now says: #539 is on origin and merges clean (see the merge checks); an empty value is refused even when a later one replaces it; the END-link sentence holds for chains of two or more links; and the checklist candidate below. The commit changes `scripts/validate-repository-naming.py`, `docs/cli.md` and `tests/test_link_source_chain_defaults.py` only. The re-review of 31a6f21 (APPROVE WITH NITS) found that all five nits are fixed, and two new ones. This commit changes `tests/test_link_source_chain_defaults.py` only. - **A test gap at the two-link boundary of `link_pins`.** Changing `if len(target.chain) > 1:` there to `> 2` passed the whole new file and `tests/test_naming_policy.py` (264 passed), yet it changes the output for a name read from stdin whose chain has two links: the first-link list is empty, and the NOTE for an unused bare value reads "a bare PATH is the tree of a chain's first link alone, , which had a more specific source". New test `test_a_two_link_chain_read_from_stdin_counts_its_first_link`: stdin `codexInk`, `--referent-chain openXdox,openInk`, `openXdox=<tree>` keyed and a second tree bare, asserting the whole stderr, the one NOTE "each link whose own declaration is read had a more specific source (...)". The `> 2` mutant fails it (see the mutation probes). The existing three-link stdin test and the new one share a helper, `stdin_naming`. - **This body.** "exactly one first link" now reads "at most one", and the merge paragraph names the current main. - **First link only, not a refusal.** The coordinator recommended this rule. Nothing documented or tested depended on a bare value answering a later link: no test passes `default_source` to `link_pins_from_trees`, and every scaffold test with a three-link chain sets `SHAPE_PIN_SOURCE_<LINK>` for each link. A refusal "when more than one link would take it" would depend on what is on the disk (checkouts beside the project, variables). #338 ruled that the reading is decided by the chains, never by the disk. - **Refuse #442, not note it.** A note would still have to stop the value reading `.`. A refusal names the typo when it is typed, and both tools already exit 2 for a question they cannot ask. - **The scaffold is fixed too.** It reads its `--pin-source` through the same function, so its three-link refusal goes away (see the reproduction). Its `--pin-source` is the pinned product's checkout, which is the first link's tree. New file `tests/test_link_source_chain_defaults.py`: 23 tests (22 until round 3). On bfed143, 17 fail and 5 pass (the controls: the one-link rows and the rows that already worked). On 04c9d77, the one-link test fails and the other 21 pass. Targeted runs (each `run9.sh`: `gh` stub, bogus token, empty `GH_CONFIG_DIR`, git-filter-repo 2.47.0), on the head 31a6f21 unless a row says otherwise. Round 3 (head a707d4b) changes the new test file only, and its own run is the last row: | set | result | |---|---| | new file, naming policy, referent-chain links, pin-source env collisions, pin-source unreadable, CLI reference, repo hygiene, README sections, AGENTS.md codes, validate-pins without git, unreadable workflow | `774 passed in 117.48s` | | manifest validator, scaffold pin and reuse, YAML flow quotes, neutral pin validator | `205 passed in 91.75s` | | CLI reference alone | `22 passed in 26.08s` | | template text placeholders, adopt plan leg-path help | `187 passed in 87.54s` | | predicted merge with origin/main 7d9bec9 (tree 908b3214): repo hygiene, README sections, CLI reference, new file, naming policy | `415 passed in 53.10s` | | round 3, head a707d4b: new file, naming policy, CLI reference | `287 passed in 95.29s` | Round 1 (head 73c4c255, before the merge of #551 and this commit; `docs/handbook.html`, `README.md` and the validators are unchanged by this commit apart from `validate-repository-naming.py`): | set | result | |---|---| | doctor, doctor validator rows, scaffold e2e, setup-project.py, setup.sh, adopt e2e, adopt pins, adopt spec-only, adopt submodules, adopt ignored shape files, adopt plan YAML quoting, adopt root symlinks, scaffold leg-path collisions, update-shape landing | `27 failed, 1796 passed, 3 skipped in 1433.51s`; every failure the same `UnicodeDecodeError` from a bytecode file my own `py_compile` probe had left in `templates/assembly-root/scripts/__pycache__`. With it removed, the two files that held all 27 (`test_adopt_root_symlinks.py`, `test_scaffold_leg_path_collisions.py`) gave `253 passed in 140.50s` | The three skips are `test_shape_doctor.py:9119`, which runs only on a Windows host. None of those files passes `--link-source`, and the one-link change and the help text cannot change a run that gives no bare value. **The 27 failures.** `template_values()` and its twin in `test_scaffold_leg_path_collisions.py` read every file under `templates/assembly-root` as UTF-8. A `validate-pins.cpython-312.pyc` had appeared there the second I ran `python3 -m py_compile` over the changed files. Its first byte is `0xcb`, the 3.12 bytecode magic, which is the byte every failure names. CI never runs `py_compile`. | row | bfed143 | this head | |---|---|---| | #441: one bare `--link-source <openXdox tree>`, chain `openXdox → openYdox → openInk`, `codexInk` | `FINDING ... is broken (openYdox declares openYdox, not openInk)`, exit 1 | `domain-descendant/assembly`, one `declared-unverified` WARNING for openYdox naming `--link-source openYdox=<path>`, exit 0 | | #441: each link keyed | exit 0 | exit 0, unchanged | | #441: no `--link-source` | two WARNINGs, exit 0 | unchanged | | #441, scaffold `--dry-run`, same chain, `--pin-source <openXdox clone>` | `REFUSED chain-not-declared`, exit 2 | WARNING declared-unverified for openYdox, plan printed, exit 0 | | #441, one link: `--referent-chain openInk` beside a leg `openXdox → openQuill`, bare openXdox tree | no warnings, exit 0 | the same: no warnings, exit 0 (04c9d77 printed a WARNING and a NOTE) | | #441, one project, legs `openXdox → openInk` and `openYdox → openQuill`, bare openXdox tree | `FINDING codexQuill: the declared chain openYdox → openQuill is broken (openYdox declares openInk, not openQuill)`, exit 1: the bare tree answered both first links | two WARNINGs and the two-first-links NOTE, exit 0 | | #442: `--link-source openXdox=` from the project root | `FINDING ... (openXdox declares openXdox, not openInk)`, exit 1 | `REFUSED link-source-no-path`, exit 2 | | #442: the same from another directory | WARNING about `.`, exit 0 | `REFUSED link-source-no-path`, exit 2 | | #442: `--link-source ''` from the project root | FINDING, exit 1 | `REFUSED link-source-no-path`, exit 2 | | #442: `--link-source openXdox= --link-source openXdox=<tree>` | NOTE "the later ... replaced it", exit 0 | `REFUSED link-source-no-path`, exit 2 (the behaviour change above) | | #442: `--link-source openXdox=.` from the project root | FINDING (reads the project's own manifest, as typed) | unchanged: a typed `.` is read | | #442: `--pin-source openXdox=` (scaffolded `codexDox`) | `FINDING neutral-pin-source-unreadable: ... . (--pin-source openXdox=...)`, exit 1 | `REFUSED pin-source-no-path`, exit 2 | | #442: `--pin-source ''` | the same FINDING, exit 1 | `REFUSED pin-source-no-path`, exit 2 | | #442: `--pin-source openXdox= --pin-source openXdox=<clone>` | `pins ok`, exit 0 | `REFUSED pin-source-no-path`, exit 2 | | control: `--pin-source openXdox=<clone>` | `pins ok`, exit 0 | unchanged | No row needs real GitHub. Round 1 ran every row except the two new ones and the two "a later value replaces it" rows. The one-link and two-legs rows are the new tests: on bfed143 the one-link test passes and the two-legs test fails; on 04c9d77 the one-link test fails and the two-legs test passes. The two "a later value replaces it" rows are the review's reproductions of bfed143 against 04c9d77; this commit does not change them. Each hunk was reverted alone in a throwaway worktree, and the new file run against it. Round 1 (20 tests), every hunk fails at least one test: | hunk reverted | tests that fail | |---|---| | `repo_shape.py`: offer `default_source` to every link again | 6: the long-chain row, the later-link NOTE, the stdin NOTE, the lookup `[two]` and `[three]`, the scaffold | | `LinkSources._bare_path`: drop the two-first-links gate | 1: two chains beginning at two links | | `_not_asked_why`: drop the two-first-links sentence | 1: the same test | | `_not_asked_why`: always the old two-link sentence | 2: the later-link NOTE and the stdin NOTE | | `main`: drop the `link-source-no-path` refusal | 6: every `--link-source` no-path row | | `PinSources`: drop the `pin-source-no-path` refusal | 3: every `--pin-source` no-path row | | `link_pins`: drop `firsts.setdefault` (stdin names) | 1: the stdin NOTE | | `_first_links_read`: return nothing | 1: two chains beginning at two links | | naming `no_path`: keyed values only | 2: the two empty bare rows | | pins `no_path`: keyed values only | 1: the empty bare row | Round 2 (22 tests, head 31a6f21): | hunk reverted | tests that fail | |---|---| | `_first_links_read`: `if data is not None:` → `if False:` (the project's legs; the review's M14, which survived 04c9d77) | 1: `test_a_project_whose_legs_begin_at_two_links_reads_a_bare_tree_for_neither`; stderr has the two WARNINGs and no NOTE | | `LinkSources.link_pins`: count a one-link chain's first link again | 1: `test_a_chain_of_one_link_is_no_first_link_read` | | `_first_links_read`: count a one-link chain again | 1: `test_a_chain_of_one_link_is_no_first_link_read` | Round 3 (23 tests, head a707d4b): | hunk changed | tests that fail | |---|---| | `LinkSources.link_pins`: `if len(target.chain) > 1:` → `> 2` (the re-review's boundary mutant, which survived 31a6f21) | 1: `test_a_two_link_chain_read_from_stdin_counts_its_first_link` (`1 failed, 22 passed in 10.59s`); stderr has "first link alone, , which had" where the test expects "each link whose own declaration is read had" | The existing `test_a_link_source_no_link_used_is_a_note_and_changes_no_exit` in `tests/test_naming_policy.py` holds the two-link NOTE sentence byte for byte, and it passes. - origin/main 9f80939 (it holds the README, handbook and `tests/readme-cap.d/550.txt` of f6eaab7): clean. The re-review found it clean at 31a6f21 (tree 71ea0d30), and `test_repo_hygiene.py`, `test_readme_sections.py` and `test_cli_reference.py` gave `151 passed in 58.81s` on that merged tree, with `render-cli-reference.py --check` ok. The README cap there is 1427 = 1422 + 5 from main's `tests/readme-cap.d/550.txt`, and this PR adds 0 README lines; its three handbook markers for "The three legs" are 39ec2c23 on the merged tree. a707d4b changes the test file only: merge-tree of it with 9f80939 is clean (tree dfc650f). Earlier mains were clean too: 7d9bec9 (after #552, #553, #554, #561 and #562; `415 passed` on the merged tree) and bbcfd34 (tree 10acc9c). The two `docs/cli.md` regenerations (this PR's `--link-source` help, #561's `bump-leg.py`) touch different tools' sections. - Lane 2's `fix/539-validators-lockstep-remedies` (#575, 93b68f6), now on origin: clean. The reviewer ran the lockstep-lines test, the new file, `test_validate_pins_without_git.py` and `test_validate_pins_unreadable_workflow.py` on the merge with 04c9d77: `103 passed in 48.45s`. This commit does not touch `validate-pins.py` or any of those tests. The `validate-pins.py` hunk stays inside `PinSources`, away from #539's regions (`:99-544` and `:1109-1143`). - Lane 2's `fix/540-pin-source-unread` (#570, 79dfd33): clean. On a merge with 04c9d77, the new file, `test_pin_source_unreadable.py`, `test_scaffold_referent_chain_links.py`, `test_scaffold_leg_path_collisions.py`, `test_cli_reference.py` and `test_adopt_pins.py` gave `371 passed`. #540 counts any `--referent-chain` as a reader of `--pin-source`; that stays true, because every chain's first link reads it. One sentence of #540 needs a word once both are in: `pin-source-unused` says the clone is read for "each --referent-chain link's declaration" (`scripts/shape_materialize.py:2706-2707` and the docstring at `:2681-2683`, and `tests/test_pin_source_unreadable.py:445`, the `UNUSED` text at `:455-457` and `:498`). After this PR it is the FIRST link's alone. Whichever lands second should change "each" to "the first". - The other PR heads #556 to #582 that git still serves (23 heads; #561 is on main): all clean, except #577 (`install.sh`, d0cdbf4). #577 conflicts with main itself, in `docs/handbook.html` and `tests/test_repo_hygiene.py`; against this head the same two files conflict. This PR is not the cause. Both edit `README.md` and the handbook's hash markers, so they land one at a time. Only #575 touches a file this PR changes (`validate-pins.py`, clean). - Lane 5's other branches: #568 `fix/family-bump-to-checks` 8024f9c, #571 `fix/family-remove-gitmodules-missing` b1120c3, #574 `fix/family-members-case-variants` 40eda84 and #578 `fix/family-siblings-tracking-branch` 43d32eb: clean. - The scaffold's and `adopt-project.py`'s own `--pin-source ''` (see the checklist candidates). - `validate-pins.py`'s `--help` and module docstring: nothing in them became false, and the module docstring sits near lines #539 edits. - Making `validate-pins.py` refuse the empty path before its leg and shape-pin checks: it would put the hunk in `_run_all_checks`, where #539 edits. - The v1.0.0 release notes keep #441 and #442 as known limits of 1.0.0, which they are. - **`scaffold-project.py` and `adopt-project.py`: `--pin-source ''` is `Path("")`, so the current directory is read as the pinned product's clone.** Both take `--pin-source` as `type=Path`, and `shape_materialize.declared_pin_values` computes the digest from it. The scaffold also hands it to `link_pins_from_trees` as the first link's tree. Run from the product's own clone, `scaffold-project.py --pin openXdox@<sha> --pin-source '' --referent-chain openXdox,openDox --dry-run` plans with `local-clone (.)`, exit 0; run from anywhere else it reads whatever directory it stands in. This is #442's shape in the two tools that create a project, and neither was in #442. (The review confirmed it for the scaffold on the head; `adopt-project.py` was not re-run.) - **The scaffold's `declared-unverified` warning names remedies the scaffold does not have.** It says "(a sibling checkout, SHAPE_PIN_SOURCE_<LINK>, or --link-source <link>=<path> would let it be read)". `scaffold-project.py` has no `--link-source` (none in its `--help`) and reads no sibling checkout: `link_pins_from_trees(chain, None, ...)` at `scaffold-project.py:799` passes no root. The text is `_chain_link_findings`' (`scripts/repo_shape.py:1419`). It was already printed on bfed143 for a two-link scaffold with no manifest. Since this PR, every chain of three or more links scaffolded with `--pin-source` alone prints it for the second link, where only `SHAPE_PIN_SOURCE_<LINK>` helps. The wording should name only the remedies the caller has. - **`tests/test_cli_reference.py` alone writes `templates/family-root/scripts/__pycache__/bootstrap.cpython-312.pyc`** (proved on 04c9d77 and on two merged trees: `22 passed`, then the file is there). A `.pyc` under `templates/` breaks `template_values()` and its twin in `test_scaffold_leg_path_collisions.py` in any later adoption run in the same tree: round 1's 27 failures were the same kind of break, from a `.pyc` under `templates/assembly-root`. #554's `load_without_bytecode` does not cover it. It is outside #441 and #442, and is named here only. Review: rv441 found 5 nits on 04c9d77 (APPROVE WITH NITS), fixed in 31a6f21; re-review rv441b found all five fixed or answered and 2 new nits (a test gap at the two-link boundary of `link_pins`, and this body), fixed in a707d4b. CI (Brett Heap's ruling 27): H0 a707d4b passed tests, tests-macos, tests-windows and SonarCloud; the head 8971ad5 is a clean update-branch merge of main bcfa364, with tests and SonarCloud green, and the local gate passed on its predicted squash: the scaffold, pins, naming, link-source, setup-by-hand and hygiene sets, 1174 passed, over bcfa364; then the hygiene, AGENTS.md-code, README-section, CLI-reference, link-source and naming sets, 468 passed, over 526cd2c; and 334 passed over b9f65be. Landed on Brett Heap's word of 2026-10-15T13:32:47Z, "All five, once green". Closes #441 Closes #442 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 15, 2026
brettheap
pushed a commit
that referenced
this pull request
Oct 17, 2026
…s and #817's entries, #642's upgrade note for a holder's `cd` line, and v1.0.0's `--into` limit kept open, current to main 64ec47c (#563) RV630's pre-review of the B2 stage (lane 3's session): - SHOULD-1: the CI bullet said every test that clones a just-pushed local remote clones it safely (#554), but five plain-path `git submodule update --init` runs are left on main (tests/test_doctor_legs_members_next.py:631, :987, :1054; tests/test_family_bump_remove_rollback.py:337, :908). It now says "runs `git clone` on", which is what #554 changed. - NIT-1: #812's READMEs quote a local clone URL "that needs it". - NIT-2: #682's first sentence no longer repeats the lead ("there:"). Landed since 40e678e, each entry checked against its squash: - #747 (#681), 6626a3c, whose tree equals `merge-tree 40e678e ff7c59b`: a new family bullet, with RV630's NIT ("`member-gitlink-absent`'s remedy puts the gitlink back by hand"); v1.0.0's limits at v1.0.0.md:466 and :467 are said closed. - #817 (#708), 64ec47c, whose tree equals `merge-tree 6626a3c d63a267`, and whose own lines equal those of 8fd2fa6, the head RV630 read: RV630's text, appended to the YAML reader bullet. #738 (#695) is v1.1.1 (lane 5, #473 issuecomment-6236564004), so its docs entry is not taken, and v1.0.0's `--into` limit (v1.0.0.md:492) is listed as still open under Known limits. Its upgrade note is true of main since #642 (0ed1b84), so it goes in now, under "Install and upgrade": lane 5's sentence verbatim, and the README's `Y/Y` layout. Counts and the open list at main 64ec47c: 113 pull requests since bfed143; 124 of 135 milestone issues closed at 16:42Z; 11 open, each with a pull request open, #681 and #708 out, #695 marked as planned for v1.1.1. All still placeholders. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 17, 2026
#813 and #723, v1.0.0's quoted-header and plain-path-clone limits closed and the counts current to main bad05d2 (#563) Each entry is the one RV630 corrected in its pre-review of the B2 stage, put at its anchor, and checked against its squash on main: the `git merge-tree --write-tree` of main before the squash with the head the entry was written from equals the squash's tree for all seven. - #810 (#674, `d17564d9`): a new bullet, "`apply`'s lines, its branch and its headers.", after "Headers, branches and messages.". It closes v1.0.0's quoted `files:`/`shape:` header (v1.0.0.md:439) and `apply`'s half of the paths listed as staged (:433); `bump-leg.py`'s half waits for #746. - #731 (#692, `d66a9054`): appended to "Folders, asked before they are made.". - #813 (#689, `f6d17bc5`): appended to "`--tracking-branch`.". - #809 (#688, `42aed788`): a new bullet, "What `family.py init` records and commits as.", after "Names that are not UTF-8.". - #816 (part of #684, `fa04d2cd`): a new bullet, "More holder states, named.", after "family.py refuses before it writes.". #816 closes no issue; #684 keeps its reftable case and is on v1.1.1. - #748 (#662, `431c8cf8`), then #723 (#663, `bad05d21`): in the CI bullet, after #713's sentence. With #554 and #578, #748 closes v1.0.0's limit on fixtures that clone a just-pushed remote by plain path (:530). The counts, in the header comment, the counts paragraph, "What changed" and the open list: main `bad05d2`; 120 pull requests since `bfed143`; at 18:02Z, 3 of the `v1.1.0` milestone's 133 issues open and 130 closed. The open list is #563 (#630), #672 (#746) and #686 (#795); #684 and #695 (#738) are on v1.1.1. #746 and #795 have not landed, so neither has an entry yet. Nothing since 2000488 touches pyproject.toml, src/openreposhape/, install.sh or .github/workflows/. 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-3 (openRepoShape-3)
Claim: #544 (comment)
What
Tests only. No product file changes.
clone_just_pushed(tests/conftest.py, setup-project.py: the root clone of--local-remote-diris--no-localand its two leg clones are rewritten tofile://, and the two session fixtures that clone a scaffold do the same, so a maintenance run packing the just-pushed objects mid-walk no longer diesfailed to copy file to ... No such file or directory(#440) #467): 56 call sites in 40 files, fromtest_update_shape.py,test_doctor_validator_rows.pyandtest_family.py(:1371, :1399) to the adopt and scaffold fixtures. A plain path is git's local clone, which walks the remote'sobjects/while a maintenance run can pack and delete them: the tests/test_shape_doctor.py, tests/conftest.py and setup-project.py: a clone of a just-pushed--local-remote-dirbare repository is git's local clone, which links or copies each loose object file while git's background maintenance can pack and delete them, so on the macOS runner it diesfailed to copy file to ... No such file or directory, every doctor test errors at setup, andsetup.shrefuses after creating all three repositories #440 macOS flake. "Just pushed" means a bare remote, or an adopted source, thatscaffold-project.py,family.pyoradopt-project.py executepushed into.recurse=False(the family bootstrap test and the doctor's plain-clone test clone WITHOUT the submodules, which is their subject),bare(the family tests' bare copies of a member),config(core.autocrlf=true) andenv(the excludes, the template, the command-scope and the offline environments). Its output is now decoded as UTF-8 and not the platform's code page (test_adopt_pins.pydecoded it so before).cloned_just_pushedis the same call that fails the test where the clone fails, so a site is one statement and not four.load_without_bytecode(tests/conftest.py). It is Both roots' scripts/bootstrap.py:mainescapes what stdout cannot encode and spells each value it prints, soname: "a\ud83db"or日本underPYTHONIOENCODING=asciino longer endsmake bootstrapin a traceback (#391) #456'sload()oftest_bootstrap_spelling.py, moved, and it replaces the bareexec_moduleofbootstrap.pyandsiblings.pyin thesiblingsfixtures oftest_shape_doctor.pyandtest_windows_paths.py, which wrotetemplates/family-root/scripts/__pycache__/*.pyc.test_shape_pin_files_not_a_list.py) assert exit 1 and the INVALID verdict (the JSONverdict, and the last line of the text report) where they took(1, 3)and any verdict, and the docstring says shape-doctor.py: a root's owncontracts/shape-pin.yamlthat does not read as a pin (a merge conflict, a non-UTF-8 byte, acommit:that is not 40 hex, a deleted file) isINVALID, exit 1, with the git command that puts back the last pin a tool wrote as its NEXT, instead ofCANNOT ANSWER, exit 3, sending the reader togit fetch --allin the standard (#237) #267 moved the verdict.GOOD_KEYSrows (test_adopt_execute_refusals.py):source-bareandsource-absent(REFUSED plan-no-source, exit 2) andlegs-bare(FINDING naming-role-mismatch, exit 1), each with noplan-bad-*.tests/test_clone_just_pushed.pyholds the helper's options: recursive, no submodules, bare,-b(and a branch the remote lacks, which fails the clone),configandenv(appended atGIT_CONFIG_COUNT's index, so the Windows job's own two entries stay), and a failed clone. Each case that makes a clone asksassert_store_read_through_gitof it.NO_BYTECODE(tests/conftest.py) isload_without_bytecode's counterpart for a child process. The two tests that runfamily.py siblings(test_family_py_siblings_runs_the_holders_own_file, and the parent-folder test oftest_family_printed_quoting.py) pass it, so therunpyof the standard'ssiblings.pywrites nobootstrap.*.pycinto the templates.GOOD_KEYStest is renamed..._are_not_plan_bad_findings:legs-bareis a FINDING, only not aplan-bad-*one.Not in this PR:
templates/family-root/scripts/siblings.py:455is product code (#543), and #415 waits on #532.Why
#467 added
clone_just_pushedand named two fixtures; about a dozen more still cloned by path (#461). The race is the inferred actor of a macOS flake that cost three reruns of the 3-OS CI, and a fixture that clones a remote the run has just pushed into is exposed to it by construction. The riding lines are the backlog #461 marked for this issue.Left on a plain path, on purpose. Clones of the product checkout (
REPO:test_update_shape.py:100,test_update_shape_add.py:174,test_family.py:1436,test_family_makefile_siblings.py:351), of a stand-in working repository (test_neutral_pin_validator.py,conftest.pyexported_in_a_clone), of an empty or never-pushed bare (test_setup_project_py.py:1814,test_openreposhape_command.py,test_adopt_tracking_branch.py,test_park_resume_targets.py,test_family_bump_to_a_tag.py:151),clone --bare <work>that BUILDS a remote (test_adopt_submodules.py:64,test_adopt_github_exits.py,test_family_dry_run.py:1295,test_family_add_member_id.py:214,test_family_add_refusals.py:222,613), the clones that stand for_commit_the_split's orgh repo clone(test_adopt_literal_names.py,test_adopt_submodules.py:150,263,272,test_adopt_ignored_shape_files.py:166,246,test_adopt_mount_ignored.pyassembly_of,test_adopt_git_reads.py,test_adopt_history_gitlinks.py,test_adopt_project_is_the_source.py,test_adopt_late_failure.py), the tests whose subject is git's plain clone (test_setup_project_py.py:191is the rewrite alone,test_adopt_clone_template.py:228,234,552,test_family_bump_remove_rollback.py:827), and the two clones of acopytreeof a remote made mid-add(test_family_add_member_id.py:246,test_family_add_refusals.py:795).What it does not reach. A recursive clone is covered for a leg only while the leg's remote sits under the cloned remote's parent (the helper's own docstring). It does not in
test_family_add_excludes.py(recursive_clone: the holder is pushed into a bare of its own, its members' remotes are elsewhere) and in the control clone oftest_scaffold_e2e.py; those root clones are read through git and their member clones keep the plain path, as before.git submodule update --initandgit submodule addof a plain-path URL ARE git's local clone, so a fixture that runs them against a pushed remote is exposed the same way; acopytreeof a pushed remote walks the same files. None is touched here.Known follow-up (the coordinator puts it on #461): the
git submodule update --initsites attests/test_family_bump_remove_rollback.py:337and:908andtests/test_doctor_legs_members_next.py:630and:981; thegit submodule addcalls in fixtures are the same class and are not listed.Landing condition, done (a1501a1). #518 landed on main as b8b7062 and added a plain-path clone of the source
executehad just pushed into (tests/test_adopt_plan_yaml_quoting.py:2036). The branch merges main (b499cfc, clean) and that clone is nowcloned_just_pushed(source, tmp_path / "clone", SPLIT_BRANCH). A re-grep of every"clone"call in tests/ finds no other new plain-path clone in what the merge brought in (test_adopt_leg_paths.py,test_adopt_manifest_values.py,test_template_text_placeholders.py,test_adopt_plan_yaml_quoting.py). Re-run after the merge:test_adopt_plan_yaml_quoting.py524 passed,test_clone_just_pushed.py7 passed,test_template_text_placeholders.py142 passed.Tests
Linux, git 2.43, git-filter-repo 2.47.0 first on PATH (so no adopt suite skipped for want of it), a
ghstub first on PATH,GH_TOKEN=bogus, an emptyGH_CONFIG_DIR. Each file on its own, three streams in parallel, one basetemp each, removed after.test_scaffold_pin_and_reuse.py,test_repo_hygiene.py,test_setup_project_py.py,test_setup_sh.pyThe 15 skips are the three Windows-only PowerShell tests of
test_shape_doctor.py:9116, one case-insensitive-disk test oftest_bump_leg.py:693, eight rows oftest_leg_path_yaml_scalars.pywhose parameter set is empty or has nothing to refuse (#259), and three NTFS-junction tests oftest_update_shape_add_symlink.py:832. Not run on macOS or Windows: CI is the first run there.The second commit (4619757, the review's nits) re-ran every file it touched, on its own:
test_clone_just_pushed.py7 passed,test_family.py106 passed (-k siblings: 17),test_family_printed_quoting.py69 passed,test_adopt_execute_refusals.py153 passed.GOOD_KEYS. With the twois Nonearms of_plan_key_findings(adopt-project.py:4415and:4420) dropped in a scratch edit, the three new rows fail (source-bare,source-absent,legs-bare); with only thesourcearm dropped, the twosourcerows fail; with only thelegsarm,legs-barefails. The edit is reverted:git diff -- adopt-project.pyis empty.INVALID (pins)for a root andINVALID (family)for a holder, on all 12 doctor rows.test_windows_paths.pyalone leavestemplates/family-root/scripts/__pycache__/{bootstrap,siblings}.cpython-312.pyc. On this head none of these leaves one:test_windows_paths.pyandtest_bootstrap_spelling.pytogether, thesiblingstests oftest_shape_doctor.py,test_family.pywhole, andtest_family_printed_quoting.pywhole. Thefamily.py siblingsruns of the last two did writebootstrap.cpython-312.pycbeforeNO_BYTECODE(checked test by test;test_family_py_siblings_refuses_a_path_that_is_not_a_holderrefuses beforerunpyand never did).url.<base>.insteadOfrewrite removed fromclone_just_pushed,test_a_recursive_clone_holds_the_root_and_both_legs_as_packsfails; with-bremoved,test_a_branch_the_remote_lacks_fails_the_clonefails (themaincase passes without it). Removing--no-localalone fails nothing on Linux (the rewrite also covers a root whose remote sits under its own parent); the symlink case of setup-project.py: the root clone of--local-remote-diris--no-localand its two leg clones are rewritten tofile://, and the two session fixtures that clone a scaffold do the same, so a maintenance run packing the just-pushed objects mid-walk no longer diesfailed to copy file to ... No such file or directory(#440) #467'stest_setup_project_py.pyis the one that needs it.Closes #544.
Lands after the v1.0.0 tag (v1.1.0).
🤖 Generated with Claude Code
Summary by Sourcery
Harden tests that clone recently pushed local remotes and execute template scripts, while tightening doctor and adoption outcome assertions.
New Features:
Bug Fixes:
Enhancements:
Tests: