Skip to content

update-shape.py: apply refuses to keep an --accept-local copy of a helper that lacks a name a copy it writes imports from it, before anything is written, so a kept scripts/repo_shape.py no longer leaves make bootstrap dying on an ImportError after the validators passed (#536) - #562

Merged
brettheap merged 2 commits into
mainfrom
fix/update-shape-kept-helper
Oct 15, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openreposhape-4 (openRepoShape-4)
Claim: #536 (comment)

What changed and why

Since #456, both roots' bootstrap.py import spelled and escape_unencodable_stdout from scripts/repo_shape.py. A project or holder whose repo_shape.py was both could keep its own copy with apply --accept-local scripts/repo_shape.py while the same run copied a bootstrap.py that imports a name only the upstream's repo_shape.py defines. The validators passed (neither imports bootstrap.py), check then read every row unchanged, and the first make bootstrap, make siblings or bootstrap.py --help died on an ImportError. #507 took the one other live case out of siblings.py. Nothing guarded the class.

cmd_apply gets one new line, _refuse_kept_copies_missing_names(root, rows, taking, accepted, target, kind), immediately before confirm(...). That is after update-conflict, update-local-drift and update-rewrite-symlink, so every both or locally-modified row left is one --accept-local named. It is also before any write, so nothing is rolled back and nobody is asked for a yes. The new refusal is update-accept-local-missing-name, exit 2.

  • Kept: the both and locally-modified rows in the --accept-local set, read from disk.
  • Written: each upstream-changed row's target bytes and each --add file's, where the path ends .py. A row the run leaves alone is not asked about.
  • Resolution: from M import ... in <dir>/<file>.py is the pinned copy <dir>/M.py, the way every shipped importer puts its own directory first on sys.path; a dotted M names subdirectories. One code path covers repo_shape.py, and any helper kept beside its importer, a holder's bootstrap.py under siblings.py included.
  • Readers (ast, nothing executed): both parse BYTES, as Python's own import does, so a BOM or a PEP 263 coding cookie is read rather than taken for unparsable. _names_imported takes every absolute from M import in the written bytes, function-level ones included. An import inside a try counts unless the first handler that catches ImportError binds that name itself (its asname, else the name); one under if TYPE_CHECKING: or typing.TYPE_CHECKING does not count. _names_defined takes what the kept copy binds at module scope: def, async def, class, the targets of =, annotated = and augmented = (unpacking included), import and from … import aliases, and the same under top-level if/try/with/for/while; also := targets, match captures, type X = aliases, and a global declared in a function or class the module defines.
  • The refusal: the detail names each kept copy, each written copy that imports from it and each name it lacks. It adds "though the validators, which never import it, pass" only when no importer it names is one of the kind's validators: where a validator is the importer, the validators would go red and roll back, so that clause would be false. The remediation names --accept-local in prose and gives the human's two exits. Either merge the upstream's version in by hand, commit, and re-run with the same --accept-local. Or drop the edit by taking the upstream's bytes (git show <target>:<source>, spelled out), which for a both copy is update-conflict's merge again. No NEXT line is printed.
  • The code: update-accept-local-missing-name sits beside update-accept-local-unknown, the other refusal of what that flag names. It says what is wrong (a name the kept copy lacks), which is true whether the kept copy predates the name or the project's edit removed it; "stale" would be false in the second case.
  • check is unchanged. No docs change: AGENTS.md rules 3-4 stay true, and README's update section still reads true.

Measured with the call line removed (mutant 1), the issue's case applies with exit 0: pins ok, manifest ok, and check afterwards says unchanged 11 / nothing to do. Then scripts/bootstrap.py --help in that tree prints:

  File ".../Atlas/scripts/bootstrap.py", line 66, in <module>
    from repo_shape import kept_helper_added_upstream  # noqa: E402
ImportError: cannot import name 'kept_helper_added_upstream' from 'repo_shape' (.../Atlas/scripts/repo_shape.py)

Tests

New file tests/test_update_shape_kept_helper.py (76 tests after the review's fix round), on test_update_shape.py's scaffold. The standard's next commits are branches of a clone of this repository.

  • The issue's case: repo_shape.py gains a function and bootstrap.py imports it. The project's committed edit makes the row both, and apply --accept-local scripts/repo_shape.py gives exit 2 naming scripts/bootstrap.py and the name. Every file, ref and HEAD is unchanged, git status --porcelain is empty, there is no copied line and no NEXT line. The premise is measured too: the target's bootstrap.py beside the kept repo_shape.py dies on that import at --help.
  • With --branch: the same refusal, and no branch is made.
  • Controls:
    • A change that adds no name applies, exit 0, and the written bootstrap.py --help starts.
    • Without --accept-local, the row is update-conflict as on main.
  • An --add file that imports the new name is refused the same way and not written.
  • A family holder: siblings.py importing a new name from a kept repo_shape.py is refused.
  • The readers over the real templates on this checkout:
    • Every name the seven shipped importers take from repo_shape (read as text, independently of ast) is what _names_imported reads, and is found defined in scripts/repo_shape.py.
    • All three of validate-pins.py's statements are read.
    • What siblings.py takes from the holder's bootstrap.py is defined there.
  • In process: every binding form, a function-level import, and the "cannot tell" cases below. Also the scope rules (another directory, a non-.py copy, a row the run leaves alone, an unaccepted row, a module no kept copy is) and the message's lines and order.

Run (Linux, py-bench) at 8bacf61d: all 11 tests/test_update_shape*.py, tests/test_repo_hygiene.py, tests/test_agents_md_codes.py and tests/test_cli_reference.py gave 574 passed, 3 skipped (NTFS junction cases), 0 failed. The independent verification's run of the 11 update-shape files gave 452 passed, 3 skipped.

Mutants, each run against the new file:

mutant result
the call line removed 4 failed: the issue's case, --branch, --add, family holder (each exits 0)
only the first ImportFrom collected 14 failed, including test_the_reader_finds_all_three_of_validate_pins_statements and every shipped-importer case
only the first ImportFrom per module 2 failed: test_the_reader_finds_all_three_of_validate_pins_statements and the validate-pins.py shipped-importer case
only top-level defs counted as definitions 16 failed, including test_every_binding_at_module_scope_is_a_name_the_kept_copy_defines, every shipped-importer case and the no-new-name control (falsely refused)
the validator clause dropped (fix round) killed, 3 failed
the validator clause always printed killed, 4 failed
typing.TYPE_CHECKING not recognised killed, 1 failed
the first round's blanket guard exemption killed, 7 failed
the kept copy decoded as text, not parsed as bytes killed, 2 failed
the guard walker's scope reset removed killed, 1 failed
a try's handlers, else and finally treated as guarded survives (the code refuses an import there, as Python requires; no test pins it yet)

What it cannot tell

It does not refuse in these cases:

  • A star import of the kept module, or import M with attribute use.
  • Bytes on either side that Python could not parse either. This includes the RecursionError/MemoryError CPython raises for a source too deep to build.
  • A kept copy with its own star import or a module __getattr__.

There the validators answer after the writes, as before.

An import inside a try is a name the copy needs unless the first handler that catches ImportError binds that name itself, as a fallback. A handler that exits, re-raises or passes, or a def inside the try, does not answer it. An import under if TYPE_CHECKING: (or typing.TYPE_CHECKING) is not a name the copy needs.

Known limits:

Shared files

update-shape.py is also edited by #558 (fix/update-shape-small-fixes: imports, Row, checked_branch, header finders, merge_by_hand, _land and its call) and by #560 (fix/update-shape-rollback-links: module docstring, Rollback, a block after _refuse_linked_copies, one line in cmd_check, one line after _refuse_linked_copies(rows) in cmd_apply). This PR adds functions between _nothing_to_do and _write_copies and one line before confirm(...), four unchanged lines below #560's.

git merge-tree --write-tree <other> 8bacf61d gave a tree with exit 0 and no conflict for each of the three:

other head tree
origin/main 7d9bec9 f8e08d5dbfe82427759784c3fac041c79108a81e
#558 origin/fix/update-shape-small-fixes 6b73e70 1c265ebd0073431a93482ef270bfbc8a9cbc49ed
#560 origin/fix/update-shape-rollback-links e0c7185 b61d6fe98d7b13a938d2f184b751118ae7b597cc

Review

  • An independent review at ae3bd339 asked for one MUST (the validator clause was false where a validator is the importer) and two SHOULDs (a BOM or coding cookie defeated the check; the guard exemption also passed a handler that exits). Fix round 8bacf61d made all seven changes it listed.
  • Its verification at 8bacf61d: APPROVE WITH NITS. 46 in-process cases each match Python, apart from accepted safe-direction ones. 12 of 13 mutants are killed. SonarCloud's gate is OK, with 0 open issues. One NIT is left: the else/finally test gap above.

Milestone v1.1.0. v1.0.0 was tagged on 2026-10-14; this lands in the order kept on #473.

Closes #536

🤖 Generated with Claude Code

…a helper that lacks a name a copy it writes imports from it, before anything is written, so a kept `scripts/repo_shape.py` no longer leaves `make bootstrap` dying on an ImportError after the validators passed (#536)

Since #456 moved `spelled` and `escape_unencodable_stdout` into
`scripts/repo_shape.py`, both roots' `bootstrap.py` import them from it. A
project or holder whose `repo_shape.py` was `both` could keep its own copy
with `apply --accept-local scripts/repo_shape.py` while the same run
copied a `bootstrap.py` importing a name only the upstream's
`repo_shape.py` defines. The validators passed, since neither imports
bootstrap.py; `check` then read every row `unchanged`; and the first
`make bootstrap`, `make siblings` or `bootstrap.py --help` died on an
ImportError. #507 took the one other live case out of `siblings.py`;
nothing guarded the class.

`cmd_apply` now calls `_refuse_kept_copies_missing_names` after
`update-conflict`, `update-local-drift` and `update-rewrite-symlink` and
immediately before `confirm`: every `both` or `locally-modified` row left
is one `--accept-local` named, nothing has been written, so nothing is
rolled back, and the human is not asked for a yes. It refuses as
`update-accept-local-missing-name`, exit 2.

- KEPT: the `both` and `locally-modified` rows in the `--accept-local`
  set, read from disk. WRITTEN: each `upstream-changed` row's target
  bytes and each `--add` file's, where the path ends `.py`. A row the run
  leaves alone is not asked about.
- `from M import ...` in `<dir>/<file>.py` resolves to the pinned copy
  `<dir>/M.py`, as every shipped importer puts its own directory first on
  `sys.path` (a dotted M names subdirectories). One code path covers
  `repo_shape.py` and any helper kept beside its importer, a holder's
  `bootstrap.py` under `siblings.py` included.
- `_names_imported` reads every absolute `from M import` in the written
  bytes with `ast`, function-level ones included. `_names_defined` reads
  what the kept copy binds at module scope: def, async def, class, the
  targets of `=`, annotated `=` and augmented `=` (unpacking included),
  import and from-import aliases, and the same under top-level if, try,
  with, for and while.
- The detail names each kept copy, each written copy that imports from it
  and each name it lacks. The remediation names --accept-local in prose
  and gives the human's two exits: merge the upstream's version in by
  hand, commit, and re-run with the same --accept-local; or drop the edit
  by taking the upstream's bytes, which for a `both` copy is
  `update-conflict`'s merge again. It names `git show <target>:<source>`
  for those bytes and prints no NEXT line.

What it cannot tell, it does not refuse: a star import, `import M` and an
attribute, bytes that do not decode as UTF-8 or parse (with the
RecursionError and MemoryError CPython raises for a source too deep to
build), and a kept copy with a star import or a module `__getattr__`.
An import under a `try` whose handler catches ImportError, or under `if
TYPE_CHECKING:`, is not a name the copy needs. `check` is unchanged: it
does not know which paths a later `apply` will accept. The code is
`update-accept-local-missing-name` beside `update-accept-local-unknown`,
the other refusal of what that flag names, and it says what is wrong (a
name the kept copy lacks), which holds whether the copy predates the name
or the project's edit removed it.

Tests, in a new tests/test_update_shape_kept_helper.py on
test_update_shape.py's scaffold, with the standard's next commits as
branches of a clone of this repository: the issue's case end to end
(refused by name; files, refs and HEAD unchanged; no `copied` line; and
the premise measured, the target's bootstrap.py beside the kept
repo_shape.py dying on that import at --help); the same with --branch,
no branch made; the controls, a change adding no name applies and its
bootstrap.py starts, and without --accept-local the row is
`update-conflict` as before; an --add file; a family holder's
siblings.py; both readers held to every shipped importer on this
checkout, validate-pins.py's three statements included; and what the
check cannot tell.

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

sourcery-ai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR prevents apply --accept-local from producing a validated but unstartable tree by statically checking written Python imports against names defined by accepted local helper copies, refusing safely before confirmation or any write, with comprehensive regression coverage.

Flow diagram for static validation of written Python imports

flowchart TD
    A[Written Python copy] --> B[_names_imported]
    B --> C{Absolute from-import resolves beside importer?}
    C -- No --> D[Skip check]
    C -- Yes --> E[Accepted local helper copy]
    E --> F[_names_defined]
    F --> G{Imported names all defined?}
    G -- Yes --> H[Continue apply]
    G -- No --> I[Raise update-accept-local-missing-name]
    I --> J[No confirmation and no writes]
Loading

File-Level Changes

Change Details Files
Add a pre-write refusal for accepted local helper copies that lack names imported by files the apply run would write.
  • Detect kept both and locally-modified copies selected by --accept-local.
  • Inspect written Python and --add files with AST-based import and module-scope definition readers.
  • Resolve absolute imports to helper copies beside each importer, including dotted modules and family-holder helpers.
  • Report missing names with remediation and return update-accept-local-missing-name without prompting, writing, or creating a branch.
update-shape.py
Add end-to-end, reader, scope, and limitation coverage for the new refusal behavior.
  • Cover the reported bootstrap failure, --branch, --add, family-holder, controls, and no-write guarantees.
  • Test import and definition extraction across shipped templates, binding forms, function-level imports, and unsupported/cannot-tell cases.
  • Verify refusal message ordering, remediation guidance, path resolution, and interaction with existing refusal states.
tests/test_update_shape_kept_helper.py

Assessment against linked issues

Issue Objective Addressed Explanation
#536 Detect when a Python file that apply will write imports a name from a helper kept with --accept-local, but the kept helper does not define that name. ✅
#536 Prevent apply from leaving a broken set of files by refusing the update before it writes anything, and identify the missing names and paths so the user can resolve the conflict. ✅

Tips and commands

Interacting with Sourcery

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

Customizing Your Experience

Access your dashboard to:

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

Getting Help

…lidators pass only when no copy it names is one of them, both readers parse the bytes as Python's own import does, and a guarded import counts as answered only where its first `ImportError` handler binds the name, so a kept `repo_shape.py` saved with a BOM, or a friendly-error guard in the copy, no longer lets the bootstrap-time ImportError through (#536)

Fix round 1 on #562, from its review at ae3bd33: one MUST, two SHOULDs
and four NITs.

THE VALIDATOR CLAUSE (MUST). The refusal said "though the validators,
which never import it, pass" whichever copy imported the missing name.
When that copy is one of `kind.validators` (validate-pins.py or
validate-manifest.py in a project, validate-family.py in a holder) the
validators DO import it: on main that run goes red and is rolled back.
`_refuse_kept_copies_missing_names` now takes `kind` and says the clause
only when no copy in its list is a validator. With a mix, part of the
sentence would be false, so it is all or nothing. The scan moves into
`_kept_copies_lacking`, answering as it did, with its two loops
flattened to one so neither function passes Sonar's complexity limit.

BYTES, AS PYTHON READS THEM (SHOULD). Both readers decoded as UTF-8
before `ast.parse`. That keeps a BOM as U+FEFF, which no parser takes, and
fails a latin-1 source whose coding cookie declares it. So a kept
`repo_shape.py` saved with a BOM, as a Windows editor or PowerShell 5.1's
`Set-Content -Encoding UTF8` writes it, read as "cannot tell", and #536's
ImportError went through end to end. They now parse the bytes, as import
does. "Not UTF-8", a null byte (a ValueError before 3.11, a SyntaxError
from 3.11) and "does not parse" keep their "cannot tell" answers.

THE GUARD (SHOULD). An import inside a `try` that catches ImportError
was exempt whatever the handler did. So a handler that exits, re-raises
or passes, which is the friendly-error guard an upstream maintainer would
add for this very coupling, turned the refusal back into a failure at
bootstrap time. A guarded name now counts as answered only where the
first handler that catches ImportError binds that same local name (the
`as` name, else the name). The handler is read by the same walk as a kept
copy's definitions, `_bound_at`, and only the import's own scope is
guarded: a `def`, `class` or `lambda` inside the `try` runs outside the
handler. The innermost such `try` decides. `_bound_at` is split out of
`_names_defined`, which also brings that function under Sonar's
complexity limit.

NEW SYNTAX IN THE KEPT COPY (NIT). `_bound_at` now reads `:=` targets
(in a module-level comprehension too), `match` captures (`case X`,
`[*X]`, `{**X}`), `type X =` (3.12, guarded where `ast` has no
TypeAlias), and a `global` declared in a function or class the module
defines. A call may bind that one, so it reads as bound: the no-refusal
direction, the PR's own rule for what it cannot tell. `NAME = 1` then
`del NAME` stays a documented false negative, and `globals()`, `setattr`
and `exec` stay unread. The docstrings say which is which.

TESTS. The `val-name` case end to end: refused, naming
scripts/validate-manifest.py, without the clause. The clause in process
for both kinds and for a mix, and asserted present in the issue's case.
The BOM, coding-cookie and null-byte cases. The guard cases: refused when
the handler exits, re-raises or passes, when only a later handler binds,
when the handler binds the name but not the alias, for a `def` in the
`try`, and for an inner handler that passes inside one that binds; not
refused for a fallback `def`, a bound alias, a star import in the
handler, or a `ModuleNotFoundError` handler ahead of one that binds.
`NEW, OTHER` with a fallback for `NEW` alone names `OTHER` only. The
review's section-4 bindings, with other-scope controls, skipped where the
running Python cannot parse them. `typing.TYPE_CHECKING`. The seven
composite asserts are split, each with its own message.

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

Copy link
Copy Markdown

@sourcery-ai

sourcery-ai Bot commented Oct 15, 2026

Copy link
Copy Markdown

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

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

@brettheap
brettheap merged commit bbcfd34 into main Oct 15, 2026
5 checks passed
@brettheap
brettheap deleted the fix/update-shape-kept-helper branch October 15, 2026 11:04
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
…an merge for ruling 27

A clean merge of main bbcfd34, made for
ruling 27 with no hand resolution: git reported no conflict, and the merge's
tree equals the tree `git merge-tree --write-tree e0c7185 origin/main`
predicted (f8586d9). Main's #562 changed
update-shape.py, this PR's file, and git merged the two cmd_apply edits
without a conflict.

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
…ll-fixes, a clean merge for ruling 27

A clean merge of main bbcfd34, for ruling
27, with no hand resolution. git merge-tree --write-tree of the start head
6b73e70 with that main printed tree
da3fd77 and exited 0, and this commit's tree
is that tree.

Main brought #553 (shape-doctor.py), #561 (scripts/bump-leg.py) and #562
(update-shape.py and tests/test_update_shape_kept_helper.py). Only
update-shape.py is a file this PR also changes; git merged it without a
conflict.

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
… 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
…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 pushed a commit that referenced this pull request Oct 15, 2026
…y-hand test and #581 among them) into #569, so the PR's CI runs on current main

Merges origin/main fb786c1, read from the
shared store without a fetch. Main moved from 7d9bec9 (#569's last merge) by
#556, #557, #558, #559, #560, #562, #567, #568, #579, #580 and #581. #579
(#550) landed tests/test_setup_by_hand_commands.py, which this PR's
`--no-next-steps` makes red; the next commit answers it. The merge itself is
clean, and its tree is the pm569 prediction's #581 step (53b6d0f7)'s, so the
resolution prepared against the predicted main carries over.

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
…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 pushed a commit that referenced this pull request Oct 15, 2026
… fix/540-pin-source-unread

This merges origin/main 2bf8fd4 as the shared store held it at
2026-10-15T16:50Z, thirteen landings past the 7d9bec9 this branch last
merged: #555 (shape-doctor's manifest kinds), #565 (scaffold's
`scaffold-remote-exists`), #581 (adopt's `check` of a kept pin path and
an unwritable work directory), #568, #559, #557, #560, #580, #556, #558,
#567, #579 and #562. It is done so that the PR's CI merge ref and this
round's rewording run on current main.

The merge was clean, with no conflict hunk. `adopt-project.py`,
`scaffold-project.py`, `scripts/shape_materialize.py`, `docs/cli.md` and
`tests/test_adopt_root_symlinks.py` took main's changes beside this PR's
by the textual merge, and `scripts/render-cli-reference.py --check`
passes on the result.

Main: 2bf8fd4.

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
…ines each exit settles when it holds both directions, a kept non-.py copy is tested to be passed over, and _names_defined answers a path holding a NUL as before (#604)

Fix round 1 for R604's review of 90aba6a (APPROVE WITH NITS), as the
coordinator ruled.

F1. When one refusal held both kinds of line, its remediation said "merge
the upstream's version by hand into a kept copy that lacks a name, so it
defines every name above", which is false of a `(written) no longer
defines` line's names: those the kept copy must stop importing, not
define. And "Either A, and B, commit it" put two changes, possibly to two
files, under one "it". Each exit now names the lines it settles ("so it
defines every name its `(kept) lacks` line names"; "so it no longer takes
any name its `(written) no longer defines` line names"), and the sentence
says "commit each". A refusal that holds one kind of line gives that
kind's exit alone.

F2. The "a kept copy that is not .py" case sat at the root, where
`repo_shape` resolves to `repo_shape.py` and not `scripts/repo_shape.py`,
so the `.py` filter never decided it and R604's mutant r6 (no filter on a
kept copy) survived. It is now `scripts/notes.txt`, and so is #562's
forward twin (a written copy that is not `.py`).

F3. `_names_defined` caught only `OSError` around the read once #604
split the parse out, so a path holding a NUL raised the `ValueError` that
base had answered with None. It now catches what base caught around the
read: `OSError`, `ValueError` and `MemoryError`. `apply` cannot reach
either input, since a row path holding a NUL is refused first.

F4. The pair the two listers return is one `Disagreement` NamedTuple
(module, importer, names, side), so `_missing_name_refusal` and
`_missing_name_remediation` read it by field name. No behaviour change.

Tests (`tests/test_update_shape_kept_helper.py`, 105 to 108): the
assertions that pinned the old words now pin each exit's new words and
"commit each", end to end for both directions in one run and in process;
a one-direction refusal of each kind is held to give only its own exit;
the two non-`.py` cases sit under `scripts/`; and `_names_defined` is
held to answer None for a missing file, a path holding a NUL and a
`MemoryError` on the read.

Mutants over these bytes, each against the whole file, all killed: the
reverse check removed (9 failed); "defined on disk now" dropped (7); the
written module read from disk (9); r6, no `.py` filter on a kept copy
(1); its forward twin, no `.py` filter on a written copy (1); the old
remediation words (6); `except OSError` alone in `_names_defined` (1).

Lane: openreposhape-4 (openRepoShape-4)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 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
…t imports a name the module it writes beside it no longer defines, before anything is written, so a kept `scripts/bootstrap.py` no longer passes the validators and then dies on an ImportError (#604) (#616)

#562 (#536) asked one direction only: a copy this run WRITES that imports
from a module `--accept-local` KEEPS. A project whose committed edit had
`scripts/bootstrap.py` import a name from `scripts/repo_shape.py`
(`locally-modified`) ran `apply --at <a commit that dropped the name>
--accept-local scripts/bootstrap.py`, got exit 0, `pins ok` and `manifest
ok`, and its next `bootstrap.py --help` died on an ImportError. Upstream
does drop public names: cac6bb2 took `link_manifest_path` and
`declared_neutral_pins` out of `repo_shape.py`.

`_refuse_kept_copies_missing_names`, at its one call site before
`confirm`, now asks both directions: a written importer and a kept module
(#536, `_kept_copies_lacking`, unchanged), and a kept importer and a
written module (#604, `_kept_copies_importing_dropped`). Each kept `.py`
copy is read from its bytes on disk; each module it imports is resolved
with `_module_beside`; where that path is a copy this run writes, a name
is refused only when the module's bytes on disk define it now and the
bytes the run writes do not. Each pair is one `Disagreement` NamedTuple.
`_names_defined_in(data)` is split out of `_names_defined(path)`, which
answers exactly as before, so the written module is read from its target
bytes under the same rules.

The code stays `update-accept-local-missing-name`: one refusal lists
every pair, sorted, #536's lines unchanged and each reverse line saying
which copy is written and which kept. The opening sentence and the
validators clause (said only when no importer named, written or kept, is
a validator) are true of every line in either direction. The remediation
gives each exit in the words of the lines it settles (a `(kept) lacks`
line's names are merged in; a `(written) no longer defines` line's names
are no longer taken), says "commit each", gives a one-direction refusal
only its own exit, names `git show <target>:<source>` of each kept copy,
names `--accept-local` in prose and prints no NEXT line. Exit 2, nothing
written. README and the handbook list some of `apply`'s refusals, but
none names this code or promises that a re-run succeeds, so no doc
changes.

What it cannot tell, it does not refuse: a kept importer that does not
parse; a star import or a plain `import M`; a module, on disk or in the
bytes the run writes, that does not parse or holds a star import or a
module `__getattr__`. A module with no file yet (an `--add` file) defines
nothing to take away, and a name the bytes on disk lack too is the
project's doing.

Tests: `tests/test_update_shape_kept_helper.py`, 76 to 108, covering both
directions: #604's case end to end (refused, tree byte-identical, no
`copied` line, the premise measured), `--branch`, a kept validator
(without the validators clause), the lacked-before and still-defined
controls (both apply), a family holder, both directions in one refusal
with each exit's words, and in process each "cannot tell" case, an
`--add` module with no file, non-`.py` copies under `scripts/` in both
directions, each one-direction exit, and `_names_defined` over unreadable
paths. Seven mutants killed, R604's r6 among them.

Closes #604

Lane: openreposhape-4 (openRepoShape-4)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant