Repository navigation
family.py: add refuses member-duplicate-id before it mounts anything when the member's project.yaml records an id a row already pins, so adding a repository renamed at the forge no longer commits a second row the holder's own validate-family.py refuses (#330) - #381
Merged
Merged
Conversation
…ing when the member's `project.yaml` records an `id` a row already pins, so adding a repository renamed at the forge no longer commits a second row the holder's own `validate-family.py` refuses (#330) `add` asked whether the family already pins the member by NAME only: `family.holding` compares the row's project and its repository, case-blind since #206. A repository renamed at the forge keeps its `project.yaml`, so on a family pinning `testorg/Atlas` (id `atlas`), `add --member testorg/Atlas2` (a bare copy under the new name) passed every check, was mounted, and committed a second row recording id `atlas`. The holder's own `validate-family.py` (`_check_member_ids_unique`) then refused that commit as `member-duplicate-id` with exit 1, under `--no-members` too, so the holder's CI went red on a commit `add` itself had written. - `_probe_manifest` and `_probe_member` (#236's throwaway-clone probe) now return the `id` they read from the member's `project.yaml`. It is None only for a linked `project.yaml`, which the probe leaves to the mount, as before. - New `_checked_member_id`, asked in `cmd_add` after the probe and before `MountRollback` is taken or anything is mounted. It refuses as `member-duplicate-id`, the validator's own code (as #299's `member-path` reuses the validator's), names the row that already records the id, and gives the remedy: `family.py remove --member <row>` takes the old row out so this `add` can pin the project under its new name, or `family.py bump --member <row> --to <commit>` moves the row that is there. Two projects recording one id are a fault in one `project.yaml`, mended in that project. - The same check runs again in `_mount_and_pin` on the mount's own reading, the id the row records. A linked `project.yaml`, or a member whose id changed between the probe and the mount, is refused there and unwound by `MountRollback` like any other refusal after the mount. - It checks the rows the validator checks, the way the validator checks them: `str(id)`, compared exactly, skipping a row with no `project:` or no `id:` and a row whose `project:` an earlier row already spells exactly. So `add` refuses what the gate would refuse and nothing the gate would pass; a new test runs the holder's validator over each manifest `add` would write to show it. - Where `member-path-registered` (#300) applies too, a section under another name already giving the mount path, that refusal is said first: it is one of the holder's, asked before the member is read, so the add prints not one line on stdout and clones nothing. `member-duplicate-id` is said on the re-run, once that section is out. A test takes the section out with the commands the first refusal prints and re-runs the same add to show the order. Unchanged: `templates/family-root/scripts/validate-family.py`, which is not in the diff (it ships into every holder, so a change would be an `update-shape` for all of them); every other refusal `add` makes, in the same order; `add`'s output and commit for a member whose id no row records; `init`, `bump`, `remove` and `siblings`; every `--help` text, so `docs/cli.md` is not regenerated. Left out: `bump` does not ask whether the id at `--to` is one another row records, so a member whose `project.yaml` takes another member's id would be bumped into the same `member-duplicate-id`. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 8 hours and 38 minutes by commenting |
Reviewer's Guide
Sequence diagram for preflight duplicate member ID refusalsequenceDiagram
participant User
participant Family as family.py
participant Probe as ThrowawayCloneProbe
participant Rows as FamilyRows
participant Validator as validate-family.py
User->>Family: cmd_add
Family->>Family: _checked_add_target
Family->>Probe: _probe_member
Probe->>Probe: _probe_manifest
Probe-->>Family: commit, project_id
Family->>Rows: _checked_member_id
Rows-->>Family: existing row with matching id
Family-->>User: Refusal member-duplicate-id
User->>Validator: validate-family.py
Validator-->>User: family remains valid
Sequence diagram for mount-time member ID recheck and rollbacksequenceDiagram
participant User
participant Family as family.py
participant Mount as MountedMember
participant Rows as FamilyRows
participant Rollback as MountRollback
User->>Family: cmd_add
Family->>Family: _probe_member
Family->>Rollback: create MountRollback
Family->>Mount: _mount_and_pin
Mount->>Mount: _read_member_project
Mount-->>Family: mounted project_id
Family->>Rows: _checked_member_id
alt linked manifest or ID changed
Rows-->>Family: duplicate ID refusal
Family->>Rollback: unwind mount and holder changes
Rollback-->>User: Refusal member-duplicate-id
else ID is unique
Family-->>User: commit pinned member
end
Flow diagram for family add refusal orderingflowchart TD
A[family.py add] --> B[_checked_add_target]
B -->|member-path-registered| R[Refuse without cloning or mounting]
B --> C[_probe_member]
C --> D{project.yaml is linked?}
D -->|no| E[_checked_member_id]
D -->|yes| F[Defer ID check to mount]
E -->|duplicate ID| R2[Refuse member-duplicate-id before mount]
E -->|unique ID| G[_mount_and_pin]
F --> G
G --> H[_read_member_project]
H --> I[_checked_member_id]
I -->|duplicate or changed ID| J[MountRollback unwinds changes]
I -->|unique ID| K[Commit mounted member]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Author
|
(lane openRepoShape-3, 2026-10-12T14:51Z) @codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
brettheap
added a commit
that referenced
this pull request
Oct 12, 2026
…member's own `project.yaml` records, to deleting its own clone when the member's second reading fails, and to the object store git keeps for a linked-worktree holder, so an invented id, a leaked clone or an assumed `.git/modules` path no longer passes (#326) The fix round on the review of 8bcc57c (PR #400). Each of the three behaviours was right in the code, and no test held it. S1, never an invented id (ruling 3). Every member the family fixture scaffolds records `id:` as its name lowercased (`irqs`, `irln`). The reviewer's mutant M4 printed `project.lower()` for every non-linked manifest, and all 22 `add` tests passed. The new member `IRDI` is built by `one_commit_remote` with `id: routing-core`. The dry run prints that id, read from the member, and the real add then writes exactly the printed row. N3, the dry run's own clone removed on a refusal (ruling 2, "removed afterwards, also on a refusal"). Every refusal the tests provoked was made before that clone exists, so mutant M10 (`finally` turned into `else`, leaking the clone on its failure path) survived. The new test runs in process. It moves the member's remote away between the probe's clone and the dry run's own, so the second clone fails in git, for real. The dry run must then: - exit 2; - print `REFUSED reading`, the exact command and the hint, as every failed reading of the member is printed; - leave its clone's directory gone, TEMP empty and the holder as it was. The test does not stub `_probe_member`, whose return #381 changes, so it holds on that merge too. N3, the store named where git keeps it. Mutant M11 printed `.git/modules/<path>` and survived. The new test adds from a holder that is a linked worktree, where `.git` is a file and git keeps a mount's store under `.git/worktrees/<name>/modules/`. The store the dry run names must not exist before the real add, and must be the one the real add then makes. `add_state` reads `.git/objects` and `.git/modules` as absent where `.git` is a file, which is what a linked worktree has. Mutants, the reviewer's own (applied by its `mutants.py` in a scratch worktree at this tree), each over the review's selection `tests/test_family_dry_run.py -k add`, 25 tests here. Each survived at 8bcc57c, and each is killed now by its one new test, 24 others passing: - M4, the id made from the name: `' id: irdi' != ' id: routing-core'`; - M10, the clone leaked on its failure path: `the dry run's clone was left`; - M11, the store assumed: `the store printed, .../lane/.git/modules/ members/IRQS, is not the one git made`. Targeted set over this tree (Linux, git 2.43.0, CI's git-filter-repo pin): tests/test_family_dry_run.py, test_family.py, test_family_add_refusals.py, test_family_member_path.py and test_cli_reference.py: 272 passed in 192.45s, 0 skipped. Part of #326, which #400 closes. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 12, 2026
brettheap
added a commit
that referenced
this pull request
Oct 12, 2026
…icate-id` and to #399's `member-malformed` names, so a dry run that skipped either check no longer previews a row the real `add` refuses, a copy of a pinned project under a new name or a name beginning with `.`, `_` or `-` included (#326) Lane 3's #381 landed as f0a8bf6 before this PR: `add` refuses a member whose `project.yaml` records an `id` a row already pins, as `member-duplicate-id`, before it mounts anything. The review of 8bcc57c said whichever of the two landed second must give that refusal a row in `ADD_REFUSALS`, and that is this PR. #399 (`MEMBER_RE`, #366) also lands first and is merged here, so CI tests main, #399 and #400 together; both merges were clean. No behaviour changes. `cmd_add` asks `_checked_member_id` of the probe's reading just before the dry-run return, so the dry run reaches the check through the same function as the real run, with the same id, and `member-duplicate-id` comes out of `--dry-run` with the same stdout, the same stderr and exit 2. Only the `_add_dry_run` docstring, which listed the holder's refusals and the member's, now names it. - `ADD_REFUSALS` gains `duplicate-id`, last, in `add`'s own order: the member is `IRRS2`, a bare copy of the pinned `IRRS` made beside the holder from the remote its `.gitmodules` names and never in the module's shared remotes, as f0a8bf6's own tests build the renamed repository. A set-up may now return the directory of remotes the add reads from; the others return None. - It gains five `malformed-*` rows for #399's rule, after `malformed-dot`: a project beginning with `_`, `-` and `.`, and an owner beginning with `_` and `.`. `<org>/..` and a bare name, the table's old two, are refused by clauses that predate the rule, so none of them covered it. An owner beginning with `-` cannot be said as `--member <owner>/<project>` (argparse reads it as an option): it stays with the real run's test in `test_family_member_name.py`. Mutants, each run on the parametrized refusal test over the `malformed` and `duplicate-id` rows: - the dry run skips `_checked_member_id`: `[duplicate-id]` fails (the dry run exits 0 and prints the row for `IRRS2`; the real run exits 2), the other seven pass; - the dry run takes `--member` without `_checked_member`: the seven `malformed*` rows fail, `[duplicate-id]` passes; - `MEMBER_RE` loosened to the pre-#399 class: exactly the five new rows fail. Targeted set (Linux, git 2.43.0, CI's git-filter-repo pin): `test_family_dry_run.py`, `test_family_add_refusals.py`, `test_family_add_member_id.py`, `test_family_member_name.py`, `test_family_member_path.py`, `test_family.py`, `test_cli_reference.py` and `test_agents_md_codes.py`: 352 passed. Not changed: a linked `project.yaml` whose target records an id a row pins still gets exit 0 from the dry run, and the real run refuses it after the mount and unwinds, inside the module docstring's stated limit. 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 12, 2026
…}"` composes with #381's `member-duplicate-id` test, whose rewrite now matches the id as the template writes it and keeps its quoting, so the full suite no longer fails `test_a_member_whose_id_changes_between_the_probe_and_the_mount_is_unwound` on Linux (#259) #381 landed `member-duplicate-id` with a test that renumbers a member between the probe and the mount by rewriting `id: irqs` to `id: irrs` in the member's `project.yaml` with `re.sub(rb"(?m)^id: irqs$", ...)`, and asserts the line changed. #395 makes the assembly-root template write `id: "irqs"`, so that pattern matched nothing and the assertion failed. The two PRs were green separately and met only when #395 merged main, and the targeted runs that carried #395 never included this file. The rewrite is now `re.sub(rb'(?m)^id: ("?)irqs\1$', rb"id: \1irrs\1", text)`: it matches the id bare or quoted and writes the new id back in the quoting the old one had, so the rewritten member still loads. The `assert changed != text` stays as it was, so the test still fails loudly if the line ever stops matching. Swept `tests/` for every other regex or literal that matches an id, org, created_on, elected_on, visibility, tracking_branch, path or submodule_path line of a template-written file in its unquoted form. The only other one that matches such a line is `tests/test_shape_doctor.py`'s `_bad_family_id` (`^id: .*$` to `id: Bad Id`): it matches the quoted line too, and `Bad Id` is a plain scalar that loads, so it still makes `family-id` the one finding. Nothing else needed a change. 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 12, 2026
…mounts, reads the member from throwaway clones outside the holder for the row's id and digest, and prints the row, gitlink, `.gitmodules` section and trailers without mounting, writing or committing anything, and `add` takes `--trailer` as `bump` does, so every subcommand has the dry run the module's `EXIT CODES` line promises (#326) (#400) The second and last part of #326. Part 1 (#345, 3ae0109) gave `bump` and `remove` a `--dry-run`, and left `add` out until the changes to `cmd_add` then in flight had landed. Until now the only way to see what `add` would pin was to run it: clone the member, mount it, rewrite `family.yaml` and commit. `add --dry-run` leaves `cmd_add` after the probe and before `MountRollback` is taken or `git submodule add` runs. So every refusal `add` makes before its first write is the real run's own code, in its order, with the same code, the same words and exit 2. That covers the holder's refusals and the member's, which are asked of the probe's throwaway clone. Lane 3's #381 (f0a8bf6) added `member-duplicate-id` right after the probe, and `cmd_add` asks it before the dry-run return, through the same function, so `--dry-run` refuses it too: real and dry run give byte-identical output, exit 2, and a table row pins it. The same holds for #399's `member-malformed` names beginning with `.`, `_` or `-`. The dry run reads the member a second time, as the real run does. The real run's second reading is the mount, whose reading the row records. The dry run's is a second bare clone, made as the probe makes its own: in a temporary directory outside the holder, deleted whatever the answer. It is read at the commit the probe resolved. The `id:` comes from `_member_id_at`, through the function the checkout is read with. The digest comes from `tree_digest`, which reads objects and never a checkout, so it is the digest the real run computes, never a guess. A `project.yaml` that is a symbolic link is left to the real run's mount, which reads through it; the dry run says so and guesses no id. It prints, in `bump --dry-run`'s layout, and exits 0: - `commit none -> <sha>` and the digest; - the gitlink; - the `.gitmodules` section `git submodule add` writes: name, path, url, and whether the file is new; - the registration in `.git/config`, written or with its url rewritten; - the directory and object store the mount makes; - the row as `family.yaml` would carry it; - the trailers. Without `--at`, a note says the real run pins the tip it finds when it mounts, and gives the `--at` that pins the commit shown. The dry run never runs `git submodule add`. It never writes `.gitmodules`, `family.yaml`, the index, the work tree, `.git/config` or `.git/modules`. `add` takes `--trailer`, `bump`'s repeatable flag. Ruling 4 asks `--trailer` with `--dry-run` to print the trailers it would write, and `add` had none. The real run carries the lines into its one commit through `_commit_one`. Without the flag, the commit is the one `add` has always written. Git's own refusal at the mount is named, not made. With no `.gitmodules` in the work tree and one in the index or HEAD, `git submodule add` refuses before it writes anything, and the real run exits 2 there with the holder unchanged. The dry run says so in a note, with how to put the file back, and exits 0. That is how part 1 treats the checkout's own refusal. The module docstring's `EXIT CODES` line now says every subcommand takes `--dry-run`. `add`'s help names the dry run and says it reads the member. `docs/cli.md` is regenerated. AGENTS.md is not changed (line cap); its family section naming the dry runs and `add --trailer` is a follow-up. The tests are appended to tests/test_family_dry_run.py; part 1's tests are untouched. - A real add previewed, for a holder with members and for a family's first member (absent `.gitmodules` stays absent). These stay byte-identical: HEAD, the index, `git status`, `family.yaml`, `.gitmodules`, `.git/config`, the `.git/modules` listing, the work tree and the holder's objects. No `gh` call and no temporary clone left. The real add then writes exactly the printed row, digest, gitlink, section, registration and directory, and the holder validates. - 19 refusals before the first write, each the same stdout, stderr and exit 2 from the real run and the dry run, with the holder unchanged: the holder's, the member's, `member-duplicate-id` (a copy of a pinned project under a new name) and five `member-malformed` names beginning with `.`, `_` or `-`. - `--trailer` printed, and carried by the real commit. - `--at` previewed and pinned. - A registration the mount rewrites. - The `.gitmodules` note, for an unstaged and a staged deletion. - A linked manifest (POSIX only). - In process: both clones are outside the holder, under the run's TEMP. The holder's `git status`, asked while the dry run's clone exists, is unchanged. No git command in the holder writes, and no `git submodule` runs. - A member whose `project.yaml` records `id: routing-core` under the name `IRDI`: the dry run prints that id, read from the member and not made from its name, and the real add writes exactly the printed row. - The dry run's own clone failing (the member's remote moved away between the two readings): exit 2, its clone removed, the holder unchanged. And a holder that is a linked worktree: the object store printed is the one the real add makes. Mutants, each killed: - the dry-run return moved after `git submodule add`: `index changed`; - the temporary clone made inside the holder: the status asked while the clone exists; - the temporary directory not removed: the leftover check; - an id made from the name (`project.lower()`): the id test; - the dry run's own clone leaked on its failure path: the failing-clone test; - the store assumed to be `.git/modules/<path>`: the linked-worktree test. - the dry run skipping `_checked_member_id`: the `duplicate-id` row; - the dry run skipping `_checked_member`, or `MEMBER_RE` loosened to its pre-#399 class: the `malformed*` rows. Closes #326 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
14 of 17 tasks
brettheap
pushed a commit
that referenced
this pull request
Oct 12, 2026
…plan-manifest-invalid` joins `_entry_findings` after #255's values, and this branch's `plan-root-symlink` stays after the source's own submodules origin/main moved 92e29d9 -> dd8d0ea (twelve commits), merged by sha: - f0a8bf6 (#330 via #381), scripts/family.py; - 87d2eb7 (#333 via #379), update-shape.py; - cd1fc79 (#327 via #375), shape-doctor.py; - 45e01f4 (#348 via #396), tests/test_agents_md_codes.py: every code-shaped name AGENTS.md spells is a code a tool prints; - 3ef059c (#258 and #261 via #314), adopt-project.py, scaffold-project.py and scripts/shape_materialize.py: the project's YAML reader and its `validate-manifest.py` are asked about the manifest's values before anything exists; `check` reports `plan-manifest-invalid` through `_manifest_findings` in `_entry_findings`; `_repository_urls` refuses `adopt-name-too-long`; - f7f4b14 (#366 via #399), scripts/family.py; - cb7063f (#301 and #280 via #390), scripts/repo_shape.py and the template's validate-manifest.py: a bracket or brace in a plain flow scalar is refused; - 5678f68 (#278, #297 and #298 via #357), shape-doctor.py; - 53bee5b (#289 via #352), scripts/bump-leg.py; - f5cb616 (#369 via #398), scripts/shape_materialize.py: the docstrings of `force_stage` and `stage_written`; - a92e60b (#326 via #400), scripts/family.py; - dd8d0ea (#292 via #353), adopt-project.py: `_elector` keeps a `user.name` byte that is not UTF-8, `_remote_repository` reads an origin URL that is not UTF-8 as none, and `adopt-no-org` is asked before `checked_value`. scripts/shape_materialize.py, scripts/repo_shape.py and every other file merge with no conflict. adopt-project.py conflicts in two hunks of `_entry_findings`, each resolved by keeping both sides: 1. The docstring. Main's order, with #261's manifest findings after #255's values and before #190's relative urls; this branch's symlinks stay after the source's own submodules. 2. The return. Main's `_manifest_findings(plan)` call, and the findings in main's order: each entry's path (#196), coverage, the legs and what the root keeps where the shape writes, the source's own submodules (#166), this branch's `_root_symlink_findings`, #255's values, #261's manifest findings, and #190's relative urls last. The remediation and the NOTEs are main's. How each of main's commits that reach this branch's code meets it: - 3ef059c: `_manifest_findings` asks the validator the template ships about the values `execute` writes; it reads no symlink and no path the shape writes. Its materializer half adds the validator's helpers, `refuse_unstorable_names` and `MANIFEST_VALIDATOR`, and holds `election_date` to the validator's `DATE_RE`; it leaves `place`, `_materialize`, `copy_tree`, `template_files` and `_refuse_unwritable` as they were. In `execute`, its `adopt-name-too-long` (in `_repository_urls`) comes before #255's clone-URL refusal, `_refuse_what_check_finds` and this branch's `core.symlinks` probe, all before any leg. Its new paragraph of the module docstring follows the one 6b53a6d corrected; both merge with no conflict. - f5cb616: docstrings only. - dd8d0ea: `_remote_repository`, `_checked_plan_inputs`' organisation and `_elector`, none of them this branch's. - cb7063f: the plans `check` and `execute` read go through this reader. The plans `plan` writes for this branch's tests hold their entries as block mappings, each path `double_quoted` where it needs to be, and the rehearsal below read every one of them. Checked on the merged tree: - AST (fin183g/names.py, base 92e29d9): no top-level name is bound twice in adopt-project.py, scaffold-project.py or scripts/repo_shape.py, and none is loaded but bound nowhere. scripts/shape_materialize.py binds `YamlError` and `parse_yaml` twice, as main dd8d0ea does (1f90a0f's import and 3ef059c's, the same objects); the merge adds none. - Both sides intact (fin183g/sides_intact.py): the only lines that differ from either side's own change are those of the two hunks. - A rehearsal of this merge (tree d21f8b4, the same as this one) passed tests/test_adopt_root_symlinks.py with main's test_adopt_{manifest_values,git_reads,relative_submodule_url}.py, test_scaffold_manifest_values.py, test_agents_md_codes.py and test_yaml_flow_brackets.py: `440 passed`. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
… text placeholders, so `--id 007`, `--tracking-branch 2026`, `--spec-path true` or `--org null` read back as typed, and `family.py add` quotes a member id alike (#259) (#395) Brett Heap ruled on 2026-10-12: "quote #259". The templates now write each text field a tool fills from a value it was given between double quotes: | template | fields | |---|---| | `templates/assembly-root/project.yaml` | `id`, `elected_on`, `visibility`, `tracking_branch`, the spec leg's `path`, the code leg's `path` | | `templates/assembly-root/contracts/spec-pin.yaml`, `code-pin.yaml` | `submodule_path` | | `templates/family-root/family.yaml` | `id`, `org`, `created_on`, `visibility`, `tracking_branch` | `render` already spells a value between a YAML template's double quotes with `repo_shape.double_quoted` (#284). The single-pass `render` from #316, on main as 34ecbc2, does the same through `YAML_PLACEHOLDER_RE`. So each value reads back exactly. An ordinary value renders as it did before, plus the two quote characters. `family.yaml`'s `org:` is quoted along with the four fields #259 names. It is the same defect in the same file: `family.py init --org 2026` and `--org null` pass `checked_value`, and on c7c02fa the holder then fails `family-org` (#307 lists the `--org null` row). Taking `org:` back out is not a one-line revert. It fails 14 of the new tests (the 8 `family.yaml:org` in-memory cases, and all 6 `family.py init` rows, because every row of `FAMILIES` passes a misread `--org`), and it would also take the `ORG` row out of `TEXT_FIELDS` and put plain `--org` values into `FAMILIES`. The lane keeps `org:` quoted, as the same defect in the same file, unless Brett says otherwise. `scripts/family.py`: `_member_lines` writes each member's own `project.yaml` id into the holder's `members:` block. That id now reads back exactly, so a bare `007` in the row came back as 7, and `validate-family.py` would report `member-id-mismatch` for a member it had just pinned. `_member_id` quotes an id only when the reader would not read the bare text back as the same text. No row whose id reads back as text moves: `irrs` is still `id: irrs`. Two edges: - A legacy row whose bare id the reader misreads moves once. A holder written before this change, by `family.py add` of a member scaffolded with `--id 2026`, holds `id: 2026`. The next `bump`, `add` or `remove` of that holder rewrites it as `id: "2026"`. That is harmless: `validate-family.py` compares with `str()` and passes before and after. - Mixed versions. A `family.py` from before this change, run against a holder this branch wrote, turns `id: "007"` back into `id: 007`, and `validate-family.py` then fails `member-id-mismatch` (the member declares `007`, `family.yaml` records `7`). A current `family.py` that rewrites the holder next then writes that row as `id: "7"`. Nothing in this branch can prevent it: run `family.py` from a current checkout on a holder that has such a member. Left unquoted on purpose: `topic` (`xf-project-<id>`), every `repository` (`<org>/<Name>`; the slash keeps it text), `SHAPE_REPOSITORY` and `DIGEST_DEFINITION` (constants), and the block or list placeholders (`NEUTRAL_PRODUCT_PINS`, `*_NAMING`, `SHAPE_FILES`). **Why.** Every reader of these files uses `repo_shape`'s YAML subset. It reads a bare `007` as 7, `2026` and `20261011` as numbers, `1.5` as a float, `true` as True and `null` as None. The checks on the flags (`PROJECT_ID_RE`, `checked_value`, the leg-path rule) accept every one of these. Reproduced on c7c02fa, with local bare remotes only: - `scaffold-project.py --id 2026|007|true|null|20261011` exits 0, and the project's own `validate-manifest.py` then fails `manifest-id`. - `--spec-path 2026 --code-path 007` fails `manifest-leg-path`, and `validate-pins.py` exits 2. - `--tracking-branch 007` pushes each leg on `007`, and the root's `bootstrap.py` then puts the legs on a local `7` with `fatal: no upstream configured`. `true` and `null` do the same on `True` and `main`. - `adopt-project.py execute` with `--tracking-branch 007` does the same; `--id 007|20261011` gives `manifest-id`; leg paths `2026`/`007` give `manifest-leg-path`. - `family.py init --id 2026|007|true|null|20261011` gives `family-id`; `--org 2026|null` gives `family-org`; `--tracking-branch 007|true|null` reads back as 7/True/None. On this branch every one of those rows reads back as typed, bootstraps onto the typed branch with `origin/<branch>` as upstream, and passes the root's own validators. **Every reader checked.** Every reader reads these fields through `load_yaml`/`parse_yaml` into a dict. None uses a line regex, `split(":")`, `startswith("<field>: ")` or a whole-line rewrite of them: - `templates/assembly-root/scripts/bootstrap.py`: `tracking_branch`, leg `path`. - `validate-manifest.py`: `id`, `elected_on`, `visibility`, leg `path`. - `validate-pins.py`: `submodule_path` against the manifest's `path`. The pin side is read as it stands and the manifest side goes through `str()`, so the two differ in type exactly when a bare value is misread (that is the `pin-path-mismatch` row above). With both lines quoted they are the same text. - `validate-repository-naming.py`: `str(id)` for the topic. - `templates/family-root/scripts/validate-family.py`: `id`, `org`, `created_on`, `visibility`, `str()` member-id compare. - `siblings.py`, family `bootstrap.py`. - `scripts/bump-leg.py`: `tracking`, leg `path`. Its `rewrite_pin` rewrites only `commit:` and `tree_sha256:`. - `update-shape.py`: `tracking_branch_of`. `rewrite_manifest` touches only the `shape:` block. - `scripts/family.py`: `tracking()`, the member's id, and the `members:` rewrite. The member-id writer is the one fixed here. - `shape-doctor.py`: leg `path`, member row `path`, working-clone `str(id)` compare. - `adopt-project.py`: plan reads and `_template_values`. - `setup-project.py`: `family.yaml` `name`/`kind` only. - `scripts/shape_advisory.py`: the single-repository record only. The `str()` fallbacks (`bootstrap.py`, `adopt-project.py`'s `checked_value`) stay, because projects made before this keep their bare lines. **Pins.** The four files are TEMPLATED, not COPIED, so no row of any project's `contracts/shape-pin.yaml` names them. `Atlas` was scaffolded and `family.py init Contoso` was run on c7c02fa and on this branch. Each `contracts/shape-pin.yaml` differs only in `commit:` and `digests.tree_sha256`, which identify the standard itself and change with every commit; every per-file `sha256` row is identical. `update-shape.py check` of the c7c02fa project and holder against this branch reports every copied file `unchanged` (11 and 10 rows): "no copied file differs; `apply` would move the pin alone". **Tests.** - New: `tests/test_template_text_placeholders.py`, 123 tests: - Each of the 13 fields is rendered with each of 8 values the reader misreads (`2026 007 1.5 true null 20261011 ~ False`), and must read back exactly. - 6 `scaffold-project.py` runs, then a recursive clone, `bootstrap.py`, the three validators, and `bump-leg.py`, which refuses on the tracking branch and finds the leg at its path on a branch of its own. - 6 `adopt-project.py` `plan`/`check`/`execute` runs into local bare remotes, then a clone, `bootstrap.py` and the validators. - 6 `family.py init` runs. - One `family.py add` of members whose ids are `007` and `true`. - **All 123 fail on c7c02fa.** - `true`/`null` through adopt: `plan` writes them bare into the plan, and the plan's reader then misreads them. That belongs to the plan writer, #181 (lane 1). Those two rows quote the value in the plan by hand, as #181's writer will, and test `check`/`execute`. - Mutation probes: reverting any one of the 13 quoted lines fails 8 cases of its field. Reverting `_member_id` fails the member test (the rows read 7/True). - The `try/except YamlError` in `_member_id` is guarded by the follow-up commit. The review found that calling the parse directly left the member test passing. Ids such as `[`, `{`, `&a`, `*a` and `!t` reach the `except` (they raise `YamlError` bare), and `_member_id` returns each one exactly, as it does 34 adversarial inputs in all (the review's in-process probe). `test_a_member_id_the_reader_refuses_bare_is_rewritten_between_quotes` (7 cases: `[`, `{`, `&a`, `*a`, `!t`, an unclosed `'` and an unclosed `"`) opens a hand-edited holder whose row holds the id between double quotes, rewrites the `members:` block with `Family.rewritten`, and asks that the id read back exactly and the text come out byte for byte. With the `try/except` removed all 7 fail with the reader's `YamlError`, and with the `except` returning the bare text all 7 fail too; the old member test still passes under the first of those mutants. - 14 existing tests that asserted or edited a bare rendered line now spell it quoted: `test_manifest_validator` ×5, `test_pin_validator`, `test_reference_default`, `test_scaffold_e2e` ×2, `test_setup_project_py`, `test_setup_sh`, `test_family`, `test_shape_doctor` ×2. - Targeted runs on this branch: | files | result | |---|---| | new file + `test_template_quoted_values`, `test_scaffold_e2e`, `test_scaffold_pin_and_reuse`, `test_scaffold_leg_paths`, `test_scaffold_leg_path_collisions`, `test_reuse_visibility`, `test_reference_default` | 405 passed | | the six adopt files | 491 passed, 1 skipped (the MedxEHR clone is not set) | | `test_family`, `test_family_add_refusals`, `test_family_member_path`, `test_manifest_validator`, `test_pin_validator`, `test_bump_leg`, `test_update_shape`, `test_update_shape_pr_base`, `test_setup_project_py`, `test_park_resume_targets`, `test_repo_hygiene`, `test_cli_reference`, `test_setup_sh` | 603 passed | | `test_shape_doctor` | 241 passed, 3 skipped (Windows-only), 1 failed; that one test was fixed, and with its sibling it reran at 2 passed | **The review.** An independent opus review at 233dc71 (the branch's first commit) found no code defect. Its verdict was CHANGES NEEDED (0 blockers, 2 should-fixes, 6 nits) because it asked for this description and its composition counts to be corrected, and they are. Its evidence: - **Readers and rewriters.** Every reader in the shipped tree reads the 13 lines through `load_yaml`/`parse_yaml`, and no line regex or whole-line rewrite touches them. It read `bump-leg.py` `rewrite_pin`, `update-shape.py` `rewrite_shape_pin` and `rewrite_manifest`, `family.py` `rewritten`, the three validators, `siblings.py`, both `bootstrap.py`, the doctor, `setup-project.py`, the workflows, the Makefile and `setup.sh`. No test among main's 17 commits since c7c02fa (through #245's 161c6c9) edits one of these lines bare. - **`_member_id`, the one rewriter changed.** A mutant that writes every id bare fails the member test (the rows read back as `{'IRRS': 7, 'IRSS': True}`). A mutant that quotes every id fails `b" id: irqs" in lines`. So the minimal-churn choice is tested both ways. - **Old projects and holders.** Against a control commit (c7c02fa plus an unrelated README edit), a project and a holder made on c7c02fa give identical `update-shape.py check` and `shape-doctor.py` output, up to the shas. `check --at 233dc71` reports all 11 project files and all 10 holder files `unchanged` and says `no copied file differs`. The doctor's verdict is `COMPLIANT, SHAPE BEHIND (... no copied file differs)` for the control and for 233dc71. This branch's `family.py add` and `bump` on a c7c02fa holder leave the old `atlas` row and the header byte for byte, apart from the legacy-id edge described at the top. - **Tests.** All 123 new tests fail on c7c02fa (`123 failed`) and pass on 233dc71 (`123 passed`). The 14 changed existing tests pass (49 passed). - **Not covered.** Windows and macOS were reviewed by reading and not run: the plan edit and the member-row check are in bytes, and nothing writes to git's stdin in text mode. Of `test_shape_doctor.py` and `test_family.py` the review ran only the changed tests. **Follow-up commit.** A second commit, `3894aa8`, answers two nits from the review, and changes no behaviour. The `_member_id` docstring said "no existing holder's `members:` block moves by a byte", which was false: the next `bump`, `add` or `remove` of a holder written before this change rewrites a legacy `id: 2026` row as `id: "2026"`, harmlessly, because `validate-family.py` compares with `str()`. The docstring now says exactly that, and a grep of every comment and docstring the branch added for "by a byte" and "moves" found one more copy, in the member test's docstring, corrected the same way. The `try/except YamlError` in `_member_id` is guarded by a new test (see Tests), so the new file holds 130 tests. Every count above that says 123 (123 new tests, all failing on c7c02fa and passing on 233dc71) is for the first commit; the follow-up commit adds the 7. The `except` also carries a comment saying which ids reach it. **Composition at the landing head (4984651).** `49846518` is GitHub's update-branch merge of origin/main `cc8f2da4` (#376, a `--trailer` value carried into the commit as typed) into `3220c9ee`. Git reported no conflict, and the merge has no hand edit. #376 changed `scripts/family.py` and `tests/test_family.py`, which this branch also changes. None of the lines it added under `tests/` names a key the templates now quote. The coordinator ran targeted tests on this head, Linux only, before CI: - main's changed test files (`tests/test_commit_trailers.py`, `tests/test_family.py`, `tests/test_bump_leg.py`, `tests/test_update_shape.py`) with `tests/test_family_add_member_id.py`: 421 passed, 1 skipped (a case-insensitive disk only); - this branch's template set (`tests/test_template_text_placeholders.py`, `tests/test_template_quoted_values.py`, `tests/test_scaffold_manifest_values.py`, `tests/test_bootstrap_spelling.py`, `tests/test_bootstrap_after_bump.py`) with the hygiene, CLI-reference and AGENTS.md-codes tests: 652 passed. **Composition at 3220c9e.** `3220c9ee` is GitHub's update-branch merge of origin/main `70d0d1b3` into `e5e768bc`. It brings in three commits: #491 (da19735), in which `setup-project.py` asks at the person's own terminal; #374 (7ab11cf), update-shape's NEXT quoting; and #384 (70d0d1b), adopt's `_spelled` plan values. Git reported no conflict, and the merge has no hand edit. #491 changed `tests/test_setup_project_py.py` and `tests/test_setup_sh.py`, which this branch also changes. A grep of the lines main added under `tests/` for the keys the templates now quote found two hits, and neither reads a rendered template line: `tests/test_update_shape_next_quoting.py` writes its own `project.yaml`, and `tests/test_adopt_question_and_submodule_spelling.py` edits a plan's `- path:`/`question:` entry, which `adopt-project.py` writes bare. The coordinator ran targeted tests on the same tree (`e5a74d19`), Linux only, before CI: - six of main's eight changed test files (the setup, update-shape, `openRepoShape` command and doctor-row tests): 365 passed; - main's other two (adopt's spelling and repo hygiene), `tests/test_adopt_github_exits.py` and nine more files of this branch's sixteen-file set: 943 passed. **Composition, refreshed again 2026-10-13 (head e5e768b).** `6d108d18` merges origin/main `56713e21` into `6cbbac60`. That brings in two commits: 5e02abe, the doctor's and `validate-pins.py`'s gitlink repairs, and 56713e2 (#479), in which adopt's `plan` takes the tracking branch from the source's default branch. Git reported no conflict and `docs/cli.md` auto-merged; the merge has no hand edit. Two kinds of test failed on it, and two commits fix them: - **`7968f902`** changes one expected literal, at `tests/test_adopt_github_exits.py:675`, for `test_a_source_on_another_branch_opens_its_pull_request_against_it[master]` and `[trunk]`. - The test reads the adopted root's `project.yaml`, which this branch's template renders as `tracking_branch: "master"`, so it now expects the quoted line. - The look-alike lines at `:743` and `tests/test_adopt_tracking_branch.py:73-76` read a plan, which `adopt-project.py` writes bare, and they stay as they are. - **`e5e768bc`** adds `git("branch", branch, cwd=source)` to `tests/test_template_text_placeholders.py::test_an_adoption_reads_back_every_value_it_was_given`. - All six rows had failed at `check` with `plan-tracking-branch-missing`: #479 refuses a tracking branch (`2026`, `007`, `1.5`, `20261011`, `true`, `null`) the source does not have, and the test's source had only `main`. - The source now carries each row's branch, in the flag rows and the by-hand rows alike. The test's assertions are unchanged. Targeted runs, Linux only: - On e5e768b: main's seven changed test files, 211 passed; this branch's sixteen-file set, 1223 passed, 2 skipped. - On 7968f90: the same sixteen files gave 6 failed, which are the six rows e5e768b fixes. The scaffold, setup, doctor and family set gave 1094 passed, 22 skipped, and twelve further template-grep files gave 1259 passed. - `render-cli-reference.py --check` is ok. **Composition, refreshed 2026-10-13 (head 6cbbac6).** `d651b594` merges origin/main `aa4e1f59` (#508) into `d526f768`. - **The one conflict.** Git conflicted in one file, `tests/test_adopt_manifest_values.py`, in four hunks, all against #356 (965420b, #293/#294). #356 refuses an `elected_on:` that `election_date` does not read, as `election-date-malformed`, before the validator is asked (`_manifest_values` returns None, adopt-project.py:2258). - **Resolved by what the merged tree prints**, measured with `adopt-project.py plan`/`check`/`execute` into local bare remotes: - `id: "007"` checks clean and `execute` writes it as typed, so it stays in `TEXT_THE_TEMPLATE_NOW_HOLDS`. - `visibility: "true"` gets the validator's `manifest-visibility: visibility is 'true'`. - `elected_on: "20261011"` gets #294's one finding, "the plan's `elected_on:` is the text '20261011'", and the validator is not asked. That row keeps this branch's list, `TEXT_THE_VALIDATOR_REFUSES`, with main's sentence. - Two tests git merged cleanly now ask `said_of`, as main's own rows do: `test_check_refuses_a_text_value_the_validator_refuses` and `test_where_a_misread_rule_judges_the_rest_the_validator_still_answers`. - **Two more failures that no hunk showed**, fixed in the same merge: - `[branch-tilde]` in `tests/test_scaffold_manifest_values.py` was this branch's own `xfail(strict=True)` until #355 landed as 6c2a371, so it XPASSed. The mark is off, and the row asserts `REFUSED scaffold-bad-tracking-branch:`. - #391's `tests/test_bootstrap_spelling.py` and #490's `tests/test_bootstrap_after_bump.py` matched ` path: spec` and `tracking_branch: main` in a scaffolded `project.yaml`, and nine cases failed on the merge. They now match the quoted lines. - **The #507 interplay.** `6cbbac60` switches the holder to `family/members` before the real `add`s in `test_a_member_whose_id_yaml_would_misread_is_pinned_under_that_id`, so the test holds once PR #507's `family-on-tracking-branch` refusal lands. - On the clean merge of 6cbbac6 with #507's head 708f3bb, that file, all 13 `tests/test_family*.py` and `tests/test_shape_doctor.py::test_a_sibling_at_the_right_origin_with_another_id_is_not_the_member` give 686 passed, 19 skipped. - Without the switch line, the member test fails `REFUSED family-on-tracking-branch`. - **Targeted runs on 6cbbac6, Linux only:** - the template, adopt-plan and manifest files (14): 995 passed, 2 skipped; - scaffold e2e, setup, the doctor, leg-path collisions and every family file: 1094 passed, 22 skipped; - the 15 further files a grep for template lines found: 1503 passed; - `render-cli-reference.py --check`: ok. - **Re-verified independently** at d651b59 and 6cbbac6: APPROVE WITH NITS (0/0/3), all three in this paragraph and fixed here. The re-verification also ran the three rows, five mutations, and 26 more files (834 passed; 291 passed, 1 skipped). - **Not run locally:** the full suite, macOS and Windows; CI's three jobs are the check. **Composition, as measured on 2026-10-12.** The merge trees below were computed for 233dc71; the follow-up commit 3894aa8 changes only `scripts/family.py` comments and adds tests. At 15:28Z main was 3ae0109 (#245, #351, #350 and #345 had landed since the merge trees): 3894aa8 merges clean with it, and the review's re-verify ran the family tests on that merge (61 passed). Every sha below is the one it had then; main and the other branches have moved since. `git merge-tree --write-tree --name-only`, run after a fetch with automatic maintenance off on 2026-10-12: - **origin/main 161c6c9**, 17 commits past c7c02fa (the base): clean, tree 7ba8246. #316 (34ecbc2), #317, #318, #311 and #245 were on main by then. - **The 46 other remote branch heads** (the 30 open PR heads and 16 branches with no open PR): clean for 44. Among them #314 (4b8da3d), #266 (3410be7), #355 (a03abcf), lane 1's `fix/adopt-plan-quote-yaml-keywords` (d38e143), `fix/adopt-record-submodule-placement` (70f4d29) and `fix/adopt-late-failure-notice` (9f1778d), #380 (73068a3), and `fix/family-init-values` (14e2d68, #307). The two that conflict, each in the same file as it does with c7c02fa and with origin/main on its own: lane 1's `fix/adopt-root-kept-symlinks` (6c44591, `adopt-project.py`) and #358 `fix/doctor-manifest-kinds-unreadable-pin` (ebffb08, `shape-doctor.py`). This branch touches neither file. - #314 and d38e143 each conflicted with origin/main on their own, in `adopt-project.py`. Merged with this branch, neither gained a conflict. **Composition with #314, which lands before this branch.** Its misread gate (`manifest_misreads`) reads the template, so once the template quotes a field it refuses nothing there, and some of #314's tests expected that refusal. Composing with #314 (landed as 3ef059c), whose misread gate asks the template how a value is written. With `project.yaml` quoting `id`, `elected_on`, `visibility` and `tracking_branch`, the gate refuses nothing on the shipped templates, and 45 of #314's tests flipped on the merge. Forty expected a refusal the value no longer needs. Thirty-four of them now assert that the value is accepted, and the six whose premise was a bare placeholder now assert that no shipped template placeholder is misread. For example: a scaffold of `--id null|true|false|007|2026`, or of `--tracking-branch true|True|TRUE|false|False|null|Null|NULL|007|2026|1.5|1.50|0`, exits 0, and the manifest holds the text it was given. A dry run prints the plan. An id derived from `--project True|Null` is held as typed. `--id=null` reaches `gh repo create`. Adopt's `id: "007"` checks clean and `execute` writes it as typed. The rule's own sentence, remediation and line-break wording are still asserted, on a copy of the standard whose `project.yaml` writes the four lines bare. Four stay refused by #314's validator gate, now in its words about the text: `visibility is 'true'` and `elected_on is '20261011'`. One is a real break. `--tracking-branch=~` reads back as `~`, which git refuses as a branch name, and it dies at `git init -b ~`, after `gh repo create`. That is #283, so the row is `xfail(strict=True)` with that reason, and it turns XPASS (and fails) once PR #355's `git check-ref-format --branch` refusal lands. `--help` for `--id` and `--tracking-branch` no longer says such values are refused, and `docs/cli.md` is regenerated. **Composition with the other branches.** Where another branch's rules meet this one, the tests were run on the merged tree. Quoting changes what some of their tests see, and the changes are not all one kind: some tests expected a refusal the value no longer needs, some are still refused under another rule, and some guard a case quoting does not fix. Whoever lands second must tell them apart, so each branch's flips are classified below. - **#266** (lane 2, `fix/leg-path-yaml-scalars`). 27 flips, all from one cause: its rule asks a hard-coded bare line, `LEG_PATH_LINE = "path: {}\n"`, so it refuses leg paths the quoted templates now read back. 15 are `test_the_rule_agrees_with_every_template_line_that_holds_a_leg_path` (null, Null, NULL, ~, true, True, TRUE, false, False, FALSE, 2026, 007, 0, 1.5, 1.50) and 12 are this branch's own end-to-end tests (6 scaffold rows fail with `REFUSED scaffold-bad-leg-path`, 6 adopt rows with `FINDING plan-bad-leg-path`). Counted on the merge with b2b42f8. #266's head was then 3410be7, whose rule reads each template line that holds a leg path, renders the value into it as `render` does and asks the reader, so it has no hard-coded line left. The merge with this branch was clean (tree 1e348c6); that head was read and the 27 were not re-run on it. At b2b42f8, whichever of #266 and #259 landed second had to drop or re-point the bare rule; 3410be7 re-points it already. - **Lane 1's #181** (`fix/adopt-plan-quote-yaml-keywords`, d38e143). Its gate reads the template too, so `plan --tracking-branch 2026|007|true` exits 0 on the composed tree (exit 2 `adopt-manifest-misread` on d38e143). On the merge, `tests/test_adopt_plan_yaml_quoting.py` and this branch's 6 adopt end-to-end tests give 70 failed, 371 passed: - 57 refusals that are now unnecessary: `plan_refuses` ×14, `plan_names_an_id` ×2, `check_names_an_id` ×4, `check_reports` ×14 and `execute_refuses` ×14 (every value except `elected_on` and `visibility`), `text_the_loader_reads_as_other_text` ×7, and the two tests that need a bare placeholder (`test_the_gate_follows_the_loader_when_the_loader_learns_a_keyword`, `test_the_template_line_says_whether_a_value_is_quoted`). - 11 guards #181 loses. See "Do not flip". - 2 that fail only by cascade: `execute_refuses[pins='null']` and `[pins='True']` pass alone (`2 passed`). An earlier case no longer refuses, and its `execute` pushes the split branch. - The writer half of #181 (`y` asking the reader, so `true`/`null`/`True` are quoted in the plan) is still needed and composes cleanly. With it, `plan --tracking-branch true` reaches the quoted template intact, and this branch's two hand-quoted adopt rows become flag-driven. - **Do not flip these 12 to "accepted".** Each still needs a refusal or a finding, and quoting does not supply it: - **`~` (1, #314).** `test_a_value_the_reader_misreads_is_refused_before_anything_is_made[branch-tilde]`. `--tracking-branch=~` is not a branch name git accepts, and #314 refused it only as a misread. On the merge it passes the plan and dies at `git init -q --bare -b ~ .../Atlas-spec.git` with `fatal: invalid initial branch name: '~'`, and against GitHub that is after `gh repo create` has made all three repositories. That is lane 2's #283 (PR #355). `~` must stay refused, by #283's ref-name check, which is why #314's row for it is not flipped to "accepted". - **A hand-edited plan value that is not text (7, #181).** `test_check_names_a_plan_value_that_is_not_text_by_what_was_read[tracking_branch='007']`, `[tracking_branch='TRUE']`, `[tracking_branch='1.50']`, `[tracking_branch='[a]']`, `[visibility='{a: b}']`, `[id='007']` and `[elected_on='20260902']`. The plan's reader reads each as something other than text, and on the merge `check` passes. Measured on the merge: `tracking_branch: 007` and `id: 007` are adopted silently as `"7"` with every validator passing; `visibility: {a: b}` pushes both legs and then fails `manifest-visibility`; `tracking_branch: [a]` is still refused at `execute` (`unsafe-value`) before any leg exists. On d38e143 alone, `check` exits 1 `plan-manifest-misread` for each of those four. Re-point the gate at "a plan value that is not a string" and these keep their guard. - **A quoted value the validator refuses (4, #181).** `test_check_reports_a_plan_carrying_one[elected_on='20260902']` and `[visibility='true']`, and `test_execute_refuses_a_plan_carrying_one_before_any_leg_exists[elected_on='20260902']` and `[visibility='true']`. The value stays invalid for the validator (`DATE_RE`, the visibility choices), and #181's misread gate caught it only incidentally. On the composed tree `check` passes and `execute` pushes the legs, and the gate fails afterwards. That is what main does today, and #314's validator gate (`plan-manifest-invalid`) would catch it. - **main** (merge with ab623dd, tree cdeec7f, run by the review): this branch's new file, `test_adopt_template_values.py` (#270), `test_template_values_single_pass.py` (#316) and `test_template_quoted_values.py` give 431 passed, 1 skipped. The skip is `test_adopt_template_values.py:223`, which reads `git config user.name` and skips where the shared repository's `.git/config` sets one; it is environmental. The changed tests with `test_family_add_refusals.py` (#317, #318), `test_pin_validator.py` and `test_manifest_validator.py` give 132 passed. #270's `unwritable_reason` is value-based, not template-relative, so it refuses as it did and nothing flips. Main then added #245 (`adopt-project.py` and `repo_shape.remote_key`), which adds no test that edits a bare quoted line; it was not run on that merge. - **#380** (lane 3, `fix/pin-rewrite-indentation`; it landed since as 1f90a0f). It rewrites `commit:` and `tree_sha256:` only, and its tests render the pins from the templates. On the merge at dc0fa7b, `test_pin_rewrite_layout.py` and the 6 scaffold end-to-end tests gave 121 passed. Its head was then 73068a3; that merge was clean and was not re-run. - **#397** (`fix/family-init-values`, #307; measured at 14e2d68): its tests skip themselves where the template quotes. 137 passed, 19 skipped, 0 failed. With its fix-round head abb27e6, this branch's 3894aa8, main 0f67f7b and #345 together: 442 passed, 20 skipped (the 19 rows, and one Windows-only row). - **#345** (run at 875d802; it has since landed as 3ae0109): `test_family_dry_run.py` and the member test, 22 passed. Its `" id: irrs"` assertion holds, because ordinary ids stay bare. **Left out.** - The adoption plan writer `y` still writes `true`/`null` bare into the plan. That is #181 (lane 1). - `templates/assembly-root/contracts/neutral-product-pin.yaml` still writes `product`, `source_repository` and `digest_source` bare, and `family.py` writes a member row's `project:` bare. Neither is a #259 field. - A plan value someone edits to a non-text type (`tracking_branch: 007` or `id: 007`, unquoted, in the plan) is read by the plan's reader as the number 7. `str()` makes it `"7"`, and the quoted template now writes it faithfully. For `id` this is a change: on c7c02fa a bare `id: 007` failed loudly (`validate-manifest: manifest-id`, manifest line `id: 7`), and on this branch it is adopted silently as id `"7"` and every validator passes. `tracking_branch: 007` was already adopted silently as `7`. The plan side is not this branch's: the guard against it belongs with #181 (adopt plan writes YAML keywords and non-text values unquoted), whose gate should refuse a plan value that is not a string. The case is narrow, because `plan --id 007` itself writes `id: "007"`; only a hand edit reaches it. - No docs change: README, AGENTS.md, the handbook and the template comments show none of these lines; `docs/cli.md` is unchanged because no `--help` text changed. Reviewed adversarially at 233dc71 (CHANGES NEEDED: 0 blockers, 2 should-fixes, 6 nits; the code and the tests were sound, and the should-fixes were the composition counts and the claim about a hand-edited plan `id`). Fix round 3894aa8 changes no behaviour: it corrects the description and the two false "moves by a byte" claims, and guards `_member_id`'s `except YamlError` with a test of 7 cases. Its re-verification at 3894aa8 found every finding handled: APPROVE WITH NITS (0 blockers, 0 should-fixes, 1 nit: shas that had moved since the description was written, refreshed here). What was added after that re-verification, main and the composition with #314, is described under "Composition with #314". An independent re-verification of that composition at 77cd0b2 confirmed it with nits only (0 blockers, 0 should-fixes, 5 nits): the 45 flips reproduce test for test, breaking the real rule or the real template's quoting turns the compose tests red, and the `[branch-tilde]` row XPASSes on a merge with #355. **Composition with #381.** #395's quoted `id: "{{PROJECT_ID}}"` composes with #381's `member-duplicate-id` test (`tests/test_family_add_member_id.py`), whose id rewrite now matches the id bare or quoted and keeps its quoting, so the full suite, which failed that one test on Linux at 1ef3310, no longer fails `test_a_member_whose_id_changes_between_the_probe_and_the_mount_is_unwound` on Linux. CI is green on Linux, macOS and Windows, with SonarCloud passing, at 4984651 (main cc8f2da merged in by update-branch). Main af01068 (#505), which landed after, touches none of its files, and the combined tree passed the template, hygiene, CLI-reference, AGENTS.md-codes and setup tests (807 passed). Closes #259 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lane: openreposhape-3 (openRepoShape-3)
Claim: #330 (comment)
What
family.py addcompared a new member with the rows by NAME only (family.holding: the row's project and its repository, case-blind since #206). A repository renamed at the forge keeps itsproject.yaml, so adding it under the new name committed a second row recording the sameid, and the holder's ownvalidate-family.pythen refused that commit asmember-duplicate-id._probe_manifestand_probe_member, the throwaway-clone probe from family.pyaddmounts the member before it checks--ator the member'sproject.yaml, so a refused add (a leg, a short--at) leaves a staged gitlink, a.gitmodulesregistration and a cached object store, and the next add of a different member commits the refused leg's registration while validate-family.py and the doctor say ok #203, now return theidthey read from the member'sproject.yaml. It is None only for a linkedproject.yaml, which the probe leaves to the mount, as before._checked_member_id, asked incmd_addafter the probe and beforeMountRollbackis taken or anything is mounted. It refuses asmember-duplicate-id, the validator's own code, and names the row that already records the id. Its remedy isfamily.py remove --member <row>, which takes the old row out so the sameaddpins the project under its new name, orfamily.py bump --member <row> --to <commit>, which moves the row that is there._mount_and_pin, on the mount's own reading, which is the id the row records. A linkedproject.yaml, or a member whose id changed between the probe and the mount, is refused there and unwound byMountRollback._check_member_ids_uniqueasks them:str(id)compared exactly, skipping a row with noproject:or noid:, and a row whoseproject:an earlier row already spells exactly. Soaddrefuses what the gate would refuse and nothing the gate would pass. A test runs the holder's validator over each manifestaddwould write, to show it.addnever reads.gitmodules, so a section under ANOTHER name whosepathis alreadymembers/<P>(a "ghost" section) does not stop it: it exits 0 and commits two sections for one path,validate-family.pysaysfamily ok, and a recursive clone takes the LATER section, so with the ghost second it dies fetching the ghost's URL #300'smember-path-registered, a deliberate choice. Both refusals now sit inadd's preflight.member-path-registeredis one of the holder's refusals (_checked_mount_path, inside_checked_add_target), asked before the member is read. So when both apply, it is the one said:addprints not one line on stdout and clones nothing.member-duplicate-idis said on the re-run, once that section is out. Both repairs are needed before the project can be pinned under the new name anyway. Thecmd_adddocstring states the order. A new test takes the section out with the commands the first refusal prints, then re-runs the sameaddand getsmember-duplicate-id.Unchanged:
templates/family-root/scripts/validate-family.pyis not in the diff. Every other refusaladdmakes is unchanged and comes in the same order.add's output and commit for a member whose id no row records are unchanged, and so areinit,bump,removeandsiblings. No--helptext changed, sodocs/cli.mdis not regenerated.Why
Measured with local bare repositories only. A scaffolded
testorg/Atlas(idatlas) and a bare copy of it astestorg/Atlas2(same id) are added in turn to a holderFam.add --member testorg/Atlasadd --member testorg/Atlas2committed 64b0ffd0ac7c: .gitmodules, members/Atlas2, family.yamlREFUSED member-duplicate-id: Atlas2 is already a member of Fam under another name: testorg/Atlas2's project.yaml records id 'atlas', and the row Atlas pins testorg/Atlas with that id. ...; nomountingline;members/holdsAtlasonly; holder cleanvalidate-family.py --no-membersFINDING member-duplicate-id: Atlas2 and Atlas both record id 'atlas': one project is pinned twice.family ok: Fam (fam), 1 member(s)validate-family.pyfamily ok: Fam (fam), 1 member(s)Both refusals apply: the holder also has a committed section
ghostgivingmembers/Atlas2.add --member testorg/Atlas2REFUSED member-path-registered, naming the sectionghostand its url; nothing on stdoutgit config ... --unset-allcommands,git add .gitmodules, re-runvalidate-family.py --no-members:FINDING member-duplicate-idREFUSED member-duplicate-id: ...;validate-family.py --no-members:family okWhere the issue is out of date. The reproduction still holds exactly on main ab623dd. The line references (
:823,:525) have moved. The issue says "#236 (open, for #203)". #236 has since landed (6d8d88b) with the pre-mount probe andMountRollback, so a refusal after the mount no longer leaves a staged mount. The check is still placed before the mount, on the probe's reading, as the issue asks.Tests
tests/test_family_add_member_id.py, new, 14 tests:remove,bump), run as printed, ends with one row recording the id and a green gate;project.yamlrecording a pinned id, refused at the mount and unwound;member-path-registered(above);Targeted set on head cfa33e0 (Linux, git 2.43.0, git-filter-repo 2.47.0):
675 passed, 4 skipped in 1350.64s (0:22:30). Files:test_family_add_member_id.py,test_family.py,test_family_add_excludes.py,test_family_add_refusals.py(it holds #300'sgitmodules-path-twicevalidator tests),test_family_member_path.py,test_shape_doctor.py, and the four guardstest_repo_hygiene.py,test_cli_reference.py,test_git_output_decoding.py,test_stdout_encoding.py. The 4 skips are Windows-only rows: 3 PowerShell lines intest_shape_doctor.py, and 1 Windows default intest_stdout_encoding.py.Skipped on an OS:
test_a_linked_manifest_with_a_pinned_id_is_refused_by_the_mount_unwoundon Windows, where a tracked symlink is a plain file in a checkout. The existing linked-manifest test uses the same skip. The refusal after the mount is still covered on Windows by the test whose id changes between the probe and the mount.Mutants, each killed by the expected test:
project:filter;member-path-registereddeferred until after the probe and the id check. This one fails the new order test, and family.py:addnever reads.gitmodules, so a section under ANOTHER name whosepathis alreadymembers/<P>(a "ghost" section) does not stop it: it exits 0 and commits two sections for one path,validate-family.pysaysfamily ok, and a recursive clone takes the LATER section, so with the ghost second it dies fetching the ghost's URL #300's eight section tests too, because their stdout is no longer empty.Composes with
Each merge is
git merge-tree --write-tree <branch> cfa33e0, at the branch's head on 2026-10-12.fix/validate-family-member-duplicatevalidate-family.pytest_family_add_member_id.py+test_validate_family_duplicates.py: 34 passedfix/family-dry-runfamily.pytest_family_add_member_id.py: 14 passedfix/scaffold-tracking-branch-ref-formatfamily.pyfix/pin-files-not-a-listvalidate-family.pyfix/trailer-non-ascii-old-gitfamily.pyfix/ruleset-hint-tracking-branchfamily.pyfix/family-init-valuesfamily.py#350 leaves
_check_member_ids_uniqueas it is, so the parity test still holds on that merge. It changesmember-duplicate, which is a different finding.Left out
bumpdoes not check the id at--to.Closes #330
🤖 Generated with Claude Code
Summary by Sourcery
Prevent
family.py addfrom creating invalid duplicate member-ID rows by validating member IDs before and during mounting.Bug Fixes:
family.py addfrom committing a second member row when a repository renamed at the forge retains an ID already pinned by the family.validate-family.pyand existing mount-path checks.Enhancements:
removeor moving its pin withbump.Tests: