Repository navigation
adopt-project.py: a relative .gitmodules url the plan keeps in a leg is resolved from the source's remote and from the leg's, and where the two differ check finds it and execute refuses before any leg exists, so a recursive clone no longer fails after adoption verified (#190) - #245
Merged
Merged
Conversation
…eg and a submodule it registers to another, or keeps either in the root, is now a `check` finding and an `execute` refusal, and `plan` asks the one-leg question on `.gitmodules` itself (#166) A plan answering `.gitmodules -> code, upstream/ -> spec` (or `.gitmodules -> drop, upstream/ -> code`) used to pass `check`, run `execute` to "adoption verified", and leave an assembly whose `git clone --recurse-submodules` exits 128 on `No url found for submodule path 'spec/upstream/dependency'`. Reproduced on main before this change; refused before either leg exists after it. ONE helper, `submodule_plan_problems(entries, tree)`, read by both `cmd_check` (as findings) and `_refuse_an_unrunnable_plan` (as a refusal), so the plan `check` passes is the plan `execute` runs. The leg of a path is the `leg:` of the single entry `_covering` it: - `.gitmodules` and every gitlink (mode 160000): `root` is refused, one `plan-submodule-root` per path. The assembly root has exactly two mounts, `execute` writes it a fresh `.gitmodules` for them, and `validate.yml` reads every URL there as a leg. - When the source has a `.gitmodules`, every gitlink kept in spec or code must share its leg, one `plan-submodule-split` per gitlink; so `.gitmodules` cannot be dropped while one is kept. A gitlink dropped beside a `.gitmodules` that went to a leg is allowed: its registration stays behind inert (verified: recursive clone exit 0, `bootstrap ok`). All dropped together is allowed. - No `.gitmodules`: only the root rule, so an orphan gitlink adopted into a leg still adopts. - An unresolved, invalid, uncovered or doubly covered entry is skipped: it is already its own finding. A source with neither (the common case) returns nothing at once; its plan file and `check` output are byte-identical to main's. `contracts/path-classification.yaml` gains `ambiguous-gitmodules`, asked first with the other named ambiguities, whose question states the constraint: "A source's own `.gitmodules` goes to the SAME leg as every submodule it registers; `execute` writes the assembly root a fresh one for its two mounts", and that `root` is not an answer. Only the top-level file matches. With `root` refused, the root-kept submodule path #165 documents in `_verify` is no longer reachable from `execute`; `_verify` and `_mount_the_legs` are untouched and #165 stays open. Tests: the plan question; the `check` matrix of eleven answers, a gitlink a rule classified silently (`tools/dep`), an orphan gitlink; the refusal through importlib, all without git-filter-repo; and one end-to-end refusal under `needs_filter_repo` asserting no leg repository was made. The two policy tests take `.gitmodules` (its rule id) and a nested `docs/theme/.gitmodules` (spec). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tered in the source's `.gitmodules` with a url, because the assembly's recursive clone dies on one that is not even where the source's own clone skips it, and a `.gitmodules` that `git config` cannot read is refused instead of being read as registering nothing (#166) aaa9099 allowed a gitlink with no registration into a leg on the premise that it "was already unclonable in the source and is no worse in a leg". That premise is false. At the top level, `git clone --recurse-submodules` skips a gitlink that has no URL; once it is inside a leg, the clone recurses with `--init` and dies on it. The adversarial review's reproduction (`upstream/orphan`, no `.gitmodules`, `upstream/ -> code`) gave `check` 0, `execute` 0 "adoption verified", source clone rc 0, and assembly clone rc 128 `No url found for submodule path 'code/upstream/orphan'`. The same happens beside a `.gitmodules` that registers other paths. The sentence is withdrawn from the docstring; this message withdraws it from aaa9099's ("No `.gitmodules`: only the root rule, so an orphan gitlink adopted into a leg still adopts"). The rule, still in ONE helper, so `check` and `execute` cannot disagree. The helper is `submodule_plan_problems(entries, tree, registered)`, and it is now a pure function of the registrations its callers read: - `Source.registered_submodules(tree)` returns the paths that have a `submodule.<name>.path` and a non-empty `submodule.<name>.url`. It reads them with `git config -z --blob <commit>:.gitmodules --list`. It uses `--list` and not `--get-regexp`, because the second exits 1 both for "no key matched" and for "bad config line" (probed on git 2.43); `--list` exits 0 or 128. A key with no `<name>` is ignored, as git ignores it. With no `.gitmodules`, nothing is registered. - A gitlink kept in spec or code that is not registered is `plan-submodule-unregistered`, naming the gitlink and its leg. It is that and never also a split: no leg holds a URL for it. Dropped, it is fine. - A `.gitmodules` that `git config` cannot read is `plan-gitmodules-unreadable` when the plan keeps it, or any gitlink, in a leg. It is never read as registering nothing. - Unchanged: `root` refused for `.gitmodules` and every gitlink; a registered gitlink kept away from `.gitmodules` is `plan-submodule-split`; a dropped gitlink beside a leg's `.gitmodules` is inert. - Problems are listed root, unreadable, unregistered, split: the order to fix them in. `execute`'s refusal takes the first problem's code and names eight details, then "and N more". - The remediation says how to fix an unregistered gitlink: drop it, or register it in the source (an entry with path and url, committed) and re-run `plan`. Text that was untrue where it is printed: - The `ambiguous-gitmodules` question keeps the issue's sentence verbatim and "`root` is not an answer". Its "`check` finds it and `execute` refuses it" becomes "`adopt-project.py` refuses a plan that gives it". `shape-doctor.py --placement-plan` prints this question for already-split projects, whose plan no `check` or `execute` reads. The question also gains: "A submodule it does not register with a path and a url cannot be kept in a leg at all: drop it, or register it first." - Its `reason:` no longer claims that the repository registers submodules, since it is printed for an empty `.gitmodules` too. - The `plan-submodule-root` detail and the remediation said "`execute` writes it a fresh .gitmodules", which is false when `.gitmodules` is itself `root`. They now say that the root's `.gitmodules` "is reserved for its two leg mounts". - The doctor's verdict on the reviewer's adopted assembly is unchanged: `note`, "nothing is in the wrong leg", exit 0. Only the `reason:` and `question:` lines of its placement plan differ. Sonar hygiene: no literal in adopt-project.py or the test file newly reaches three uses. - `_git_path` is the one decoder for `Source.tree()` and the registrations, which must be compared in one spelling. - `GIT_CONFIG` replaces the subcommand literal in `_elector` and in `_mount_the_legs`' url write. That is a one-token change there; `_verify` is untouched. - `SUBMODULE_KEY_RE` parses the keys. - `_submodule_findings` keeps `cmd_check` at main's shape. Tests, tests/test_adopt_submodules.py (28 -> 52 cases): - The orphan test now expects `plan-submodule-unregistered` for code and spec, a pass for drop, and `plan-submodule-root` for root. - New: an unregistered gitlink beside a registered one (code, spec, drop). - New: a registration that is url-less, empty-url, name-less, at another path, or in an empty file is not one. Upper-case keys are one. - New: an unreadable `.gitmodules` (five answers). - The importlib refusal test now compares the refusal with `check`'s findings, detail for detail, across all four codes and four legal plans. - New: a mixed refusal of eleven or ten problems pins the first code and the 8-detail cap. - The `needs_filter_repo` end-to-end refusal is parametrised over the split plan and the reviewer's orphan plan. It asserts that the `--local-remote-dir` does not exist, that the `--work-dir` is empty, and that the source's HEAD, refs and status are unchanged. - Repeated literals go into constants and helpers (`adopt`, `check_plan`, `plan_of`, `add_submodule`, `commit_as_source_human`). The first test in the file uses them too. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ry that holds one of the source's submodules, whatever rule matched it, so it no longer proposes the `root` that `check` refuses, and `check` now prints the submodule remediation after its findings (#166) The second review of aaa9099, by lane openRepoShape-2, found that `plan` proposes what `check` refuses. `walk` sees paths, not modes. So a submodule vendored at `.specify/vendored`, `.cursor` or `.agents` came out `leg: root` with no question (rules `root-spec-kit` and `root-assistant-instructions`). One at `docs/theme` came out `spec` (`spec-governance`), also unasked. `check` then answered `plan-submodule-root`, or a split once `.gitmodules` was answered. Reproduced at 09d7889: `.specify/` written `root`, then `plan-submodule-root: .specify/vendored (entry .specify/)`. - `ask_about_submodules` runs in `cmd_plan` right after `walk`. Every entry that `_covering` says covers a gitlink takes the verdict `.gitmodules` itself gets: `leg: null`, `review_required: true`, rule `ambiguous-gitmodules` and its reason. It also takes that question, plus "This entry holds the submodule(s) <paths>: it is asked this question, not classified by its rule `<rule>`." When the entry holds other paths too, the question adds that it can be split. `walk`'s folding is unchanged; only the covering entry is replaced. - `check` on the untouched plan now reports `plan-unresolved` there, never `plan-submodule-root`. - The 2026-09-02 ruling is unchanged for files. The `ambiguous-gitmodules` header comment says that a vendored SUBMODULE under a root-ruled path is asked rather than rooted, because the root keeps no submodule of the source's. - A source with no gitlink gets its entries back untouched. On the suite's SYNTHETIC_TREE source, these are byte-identical to main's: the plan file, `plan`'s output, and `check`'s output both unresolved and resolved. Not fixed here, and now said so: `execute` refuses every SUBMODULE problem `check` finds, but not yet every finding. An entry that is uncovered, covered twice or has a misspelled leg still reaches the split; that is #168, which merges right after this. The docstrings that said "the plan `check` passes is the plan `execute` runs" now say exactly this, in `submodule_plan_problems`, `_answered_legs` and `_refuse_an_unrunnable_plan`. The same review's smaller findings: - The question and the remediation offered no way to be rid of one submodule except `git rm` after adopting. The rule has always allowed dropping one on its own, and both texts now say so: its registration stays behind in the leg, inert. - `check` prints SUBMODULE_REMEDIATION once after its findings when any of them is a submodule problem, as `execute`'s refusal does. `_submodule_findings` returns it, so `cmd_check` gains no branch. - The module docstring's list of what `check` validates now includes the submodule rule. docs/cli.md is generated from the first line only and is unchanged. - #162's kept-root mount test and the `_mount_the_legs` comment are re-labelled as mount defence in depth for a plan that `check` and `execute` refuse (#166). In `_mount_the_legs` only the comment changes. Tests in tests/test_adopt_submodules.py, 52 -> 77 cases: - `plan` asks on the entry holding a gitlink at `.specify/vendored`, `.cursor`, `.agents`, `docs/theme` and `upstream/dependency`, and on one entry holding two gitlinks. Each row is the `.gitmodules` row apart from its path and size. Its question names the gitlinks and the rule, and `check` on the untouched plan reports only `plan-unresolved`. - The `tools/dep` test now answers `tools/` as its rule would have, since `plan` asks there. - Branches no test reached before: a `.gitmodules` with no gitlink beside it; two submodules split across legs; a gitlink that is its own entry. Also uncovered, doubly covered and bad-leg entries: `check` reports each by its own finding, and the helper skips it. - The `check` matrix asserts the remediation is printed once, after the findings, exactly when there is a submodule finding. - One `needs_filter_repo` end-to-end test runs `execute` on accepted plans: both code, both spec, and the submodule dropped on its own. `git clone --recurse-submodules` of the assembly and `bootstrap.py` in the clone then both exit 0. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… submodule reader reads it in the leg, so a 0xFF byte or an `[include]` no longer changes the answer and a registration git ignores is no registration, says what git said when it cannot read one, and an entry holding a submodule among other paths names the entries that replace it (#166) Round 3, from lane openRepoShape-2's re-verification of 09d7889 (N1-N3, P3.3). N1. `Source.registered_submodules` read the blob with `git config --blob`, which git's submodule code never does: it reads bytes through a `char` (signed on x86), so a 0xFF byte ends the file there, and it follows `[include]`. A Latin-1 comment before a registration was a false refusal; one before a `path` override hid it, and the adoption verified and then failed to clone (#166's symptom); a relative include was a false `plan-gitmodules-unreadable`; an absolute one read a file on the operator's machine. The blob is now piped into `git config -z --no-includes --file - --list`. `--no-includes` is not optional: probed on git 2.43, `--file -` FOLLOWS includes by default (stdin sets no file, so git turns `respect_includes` on), unlike `--file <path>`. The `plan-gitmodules-unreadable` detail now says what git said, on one line ("git cannot read the source's .gitmodules for its submodules (fatal: bad config line 2 in standard input), ..."), including when the `.gitmodules` in the tree is no blob at all (`git cat-file` fails). N2. `_registrations` mirrors git's `parse_config` for `path` and `url`: a name that is empty or has a `..` component at `/` or `\` is ignored (`check_submodule_name`); a value starting with `-` is ignored and the one before it stands; a path belongs to the last name given it, and a name moved to another path removes its old path's entry whoever holds it (git's `cache_remove_path`); a `path` or `url` with no value, which `--list` prints but git's reader dies on, makes the file unreadable, except under a name git already ignores. Every one probed against a recursive `git submodule update --init` on git 2.43. "Matched exactly as git matches them" is gone from the docstring; it now says which of git's rules are mirrored. N3. Adding `src/vendored: drop` beside `src/` is `plan-covered-twice`. An asked entry holding more than one path now names the entries that replace it, each submodule an entry of its own, descending only into a directory that holds one ("replace it with an entry for each of `src/app/`, `src/vendored`, each with its own `leg:`"), and SUBMODULE_REMEDIATION says to REPLACE a directory entry, not add beside it. An entry holding only submodules, two or more, is split too, so one can be dropped on its own. P3.3 (partial). The `ambiguous-gitmodules` question keeps #166's sentence, framed "When a repository is adopted, ..." so it is true where `shape-doctor.py --placement-plan` prints it for a leg already split. Docstrings and comments that said "until #168" are corrected now that #168 is merged here: `_refuse_what_check_finds` refuses the findings `submodule_plan_problems` leaves to `check`'s own. Tests: the registration test gains the 0xFF, name, `-`, last-name, moved-name and no-value shapes; new tests for `[include]` relative and absolute, for git's words in the unreadable finding, and for the split the question names (beside: covered twice; the named split: plan ok); the accepted-plan e2e gains upper-case keys, a 0xFF comment and an include. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…or a source whose `src/` or `contracts/` holds a submodule, spells every path its question names so a non-UTF-8 or newline name no longer crashes it or empties `--out`, and caps the split it names; `check` reads a key with no value beside no submodule as git does, says where git fails on a `.gitmodules` it cannot read, and why a named entry registers nothing; `execute` refuses with the one list `check` prints (#166) Round 4, from this lane's re-verification of ca5fb3d (S1-S3, N1-N4) and lane openRepoShape-2's passes 4 and 5 (NEW-1, NEW-2, NEW-3, R5-1, R5-2). NEW-1 / S1. `cmd_plan` computed the collision prediction, the follow-ups and the seeding record after `ask_about_submodules` had set the entry holding a submodule to `leg: null`. A submodule under `src/` dropped main's follow-up for a harness there that reads `contracts/`, and called the code leg empty: a SEEDED warning, `seeded_from_template: true`, and an `--allow-empty-leg code` it never needed. One under `contracts/` lost the harness line and the `CONTRACTS_DIR` line. All three are now read from `walk`'s entries, before the question is asked; a source with no submodule plans byte for byte as before. NEW-2 / S2 / R5-1. Every path the question names, the submodules and the entries that replace a directory alike, is `_spelled`: between backticks, a byte that is not UTF-8 as `\xe9`, a newline as `\n`, any other character that is not printable by a backslash, and a backslash or a backtick escaped so the backticks still delimit it. A raw surrogate made `write_lf` raise mid-write and leave `--out` empty; a raw newline read back as a space. The spelled newline, between double quotes, is the YAML entry that covers the file. R5-2. Past SPLIT_ENTRIES_NAMED (8) entries the question no longer names them one by one: "replace it with an entry for each path directly in `src/`, 402 in all", or "in one of `docs/`, `docs/theme/`, other than those directories" when it is nested. That edit is the one `check` passes. S3. A `.gitmodules` whose one defect is a key with no value is fatal to git's submodule reader alone, which git runs only for a gitlink. Beside no submodule it is now read as registering nothing: main adopts and clones such a source, and this branch no longer refuses it. Measuring every shape end to end against main also found where git really fails on a file it cannot read, so `plan-gitmodules-unreadable` now says so: beside a submodule, `git filter-repo`'s `git fast-export` reads the checked-out `.gitmodules` for each gitlink and fails the extraction of either leg, wherever the plan sends the two. That includes both of them dropped, which this branch passed and main and this branch both died on after making both leg remotes; it is refused now. Only a plan that extracts no leg escapes it. Otherwise the finding names the assembly's recursive clone, which fails in a leg that keeps a file with a line git cannot parse (no submodule needed), or a `.gitmodules` that is itself a gitlink. SUBMODULE_REMEDIATION no longer offers dropping such a file beside a submodule as an exit. NEW-3 / N1. `check` and `_refuse_what_check_finds` read one list, `_entry_findings`: coverage, each entry's `leg:` (#169's leg paths among them) and the submodule problems. A plan `check` rejected as uncovered and split was refused by `execute` for the split alone; it is refused for both, in one refusal, with #168's remedy and the submodule one. `_refuse_an_unrunnable_plan` is main's again, so its literal `8` cap is gone with the code that had it. #168's tests pass unchanged. N2. `.gitmodules` is now read into `Registrations`, which also keeps why a path an entry names registers nothing: the entry holding it has no url git reads, its name is one git ignores as suspicious, its path starts with `-`, or another entry's move dropped it. The `plan-submodule-unregistered` detail says that, and "no .gitmodules entry ... registers its path with a url" only when no entry names it. N3. git calls the file read through `--file -` "standard input"; the finding says `.gitmodules`. N4. `_SplitTree` reads `GITLINK_MODE` (`_verify`'s literal is left to #173), and `cat-file` is `GIT_CAT_FILE`, as `config` is `GIT_CONFIG`. Tests: new ones for each of the above (follow-ups and seeding for `src/vendor/dep` and `contracts/vendor/x`; four names no plan line can hold, built through `update-index --index-info` and skipped where git refuses them; a newline entry typed from the question; the capped split, flat and nested; why an entry registers nothing, eight ways; where git fails, six shapes; one refusal for an uncovered and split plan through `cmd_execute`); the unreadable-file matrix now refuses both dropped, and says the extraction; the end-to-end refusal gains that case and the end-to-end acceptance a key with no value beside no submodule. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…it, so a `.gitmodules` that is itself a gitlink is pinned to the clone of its leg (#166) `_where_git_fails` names the extraction only for a FILE git cannot read beside a submodule: a `.gitmodules` that is itself a gitlink leaves `git fast-export` nothing to read, and git fails in the clone of the leg that keeps it (measured on main: execute 0, recursive clone 128). No test held that condition; `test_a_gitmodules_git_cannot_read_says_what_git_said` now asserts the ending of each of its four findings. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rce git will not record a name in, so the 13 tests of test_adopt_submodules.py that failed only on `tests-windows` no longer fail there (#166) PR #194 was green on Linux and macOS and red on `tests-windows`: 13 tests, two causes, both in the test helpers. adopt-project.py is unchanged: the plan it writes is LF on every platform (`write_lf`). 1. `edited()` found no `paths:` and no ` - path: <dir>` line in the plan (10 tests). `conftest.resolve` wrote the answered plan back with `Path.write_text`, which on Windows turns every LF into CRLF, and `edited` reads the plan as BYTES, for the LF `plan` writes. It now writes with `open(..., newline="\n")`: `write_text` has no `newline` before 3.10 and this standard runs on 3.9. 2. `raw_source` builds a source holding a name no filesystem need hold with `git update-index -z --index-info`, which exits 0 and says `Ignoring path` for a name git will not record, so the Windows runner committed a source with no `src/a\nb.py` and no `src/ven`d\or`, and the question never named them (3 tests: KeyError 'question', and a list without the file). The helper now reads the committed tree back with `git ls-tree -r -z` and skips the case when a name it was given is not there: asked of git, not of the operating system. Where git keeps the name the case runs, with its assertion unchanged. Two tests keep both from coming back on a platform that does not show them. `windows_text_mode` opens text files for writing as Windows does (CRLF for LF unless a `newline=` is named), around the one `answered()` it is for: with `resolve` reverted the plan holds CRLF and `edited` raises the ValueError of the CI log. A `.git/...` name, which every git leaves out of the tree, makes `raw_source` skip. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…t name the same repository from the leg as from the source, or `check` finds it and `execute` refuses it before any leg exists, so `url = ../dependency` no longer adopts as `adoption verified` into an assembly whose recursive clone cannot fetch it (#190) git resolves a `./` or `../` submodule url against the remote of the repository holding `.gitmodules`: the source's before the split, the leg's after it. On main and on #166's head the re-verifier's `relative-url` case checked 0, executed 0 with `adoption verified`, and the assembly's `git clone --recurse-submodules` exited 1 on `.../remotes/dependency' does not exist`. Each relative url the plan keeps in a leg is now resolved by git itself (`git submodule init` in a scratch repository whose `origin` is the remote in question), from the source's default remote (the adopted branch's, else `origin`, else its own directory) and from the leg's remote, and the two are compared with `remote_key`, and for two paths on this disk through its symlinks. Where they name one repository nothing changes; `check` says in one NOTE that it resolved from the GitHub remotes the plan names. Where they differ, or git cannot resolve one, `check` reports `plan-submodule-relative-url` naming the submodule, its url and both resolutions, with a remediation that works (commit the absolute url in the source and re-plan, or drop the submodule), and `execute` refuses it through #168's one list -- resolved again from the remotes it is about to create, so under `--local-remote-dir` its refusal says so. Refused, never rewritten: a rewritten leg `.gitmodules` is a blob `_verify` accounts for by sha and a url nobody committed, so `_verify` is unchanged. A credential in the source's remote is redacted wherever it is printed. A source with no submodule, or none kept with a relative url, starts no git process for this and prints exactly what the base printed. `tests/test_adopt_submodules.py`'s hand-written registrations take the dependency's absolute url: seven cases kept `../dependency` in code with no remote and expected no finding, which is #190's broken outcome. Stacked on #166 (fix/adopt-split-submodule-plan). Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…relative-submodule-url Brings in #166's Windows test fix (6fcf645: `resolve` writes the plan back LF, `raw_source` skips a source git will not record a name in) and its merge of main 22c9fdd (#175, #176, #192). No conflict; no code of this branch changed. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…bmodule-url #166 landed on main as e4decfb, the squash of PR #194, so this branch is no longer stacked on fix/adopt-split-submodule-plan. e4decfb's tree is 96c8353's, which 52cf9f9 already merged, so every conflict (8 blocks in adopt-project.py, 3 in tests/test_adopt_submodules.py, each a #190 line beside a #166 line) resolves to this branch's side: main's copy of #166 plus #190 on top. No line of #190 changed: its added and removed lines against e4decfb are those of 3eb55e5 against 6aaedc5. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, #201, #200 and #173 land beside #190, and `_entry_findings` hands `_leg_findings` the tree while still returning #190's NOTEs Main moved from e4decfb to 54aa69a: 20e13f0 (#172 via #198), 8df21e6 (#179 via #201), a5b09c3 (#174 via #200) and 54aa69a (#165 via #173). Five conflict hunks, all in adopt-project.py, each a #190 line beside a main line. Each takes BOTH sides, by lane 2's recipe (resolve_m5b_r190d.py, which asserts each hunk's two sides before it writes anything): 1. Module docstring, `check`: #198's mount-path clause ("neither equal to, around or inside a path the shape writes into the root ..."), then #190's "each relative url naming the same repository from the leg as from the source". 2. The `repo_shape` import list: main's names, which since #201 leave out `SAFE_ARG_RE` (its one user moved to scripts/shape_materialize.py, and #190 adds none), plus #190's `redacted, remote_key, remote_local_path`. 3. `_entry_findings` docstring, first paragraph: #190's "and the NOTEs it prints beside them" with #198's "and the two mount paths". 4. `_entry_findings` docstring, ONE LIST paragraph: #190's "but in the one place a remote decides it" sentence, then #198's paragraph on why `_leg_findings` is handed the tree. 5. `_entry_findings` body: #198's `_leg_findings(plan, tree)` with #190's three-tuple from `_submodule_findings(plan, source, tree, local_remote_dir)`. This branch's `_leg_findings(plan)` alone is a TypeError on every `check` and `execute`, because #198's `tree` has no default; main's two-tuple alone breaks `check`'s three-way unpack and drops #190 from `execute`. #173's verification (only the two leg mounts passed over; `adopt-root-submodule`, `adopt-unexpected-gitlink`, `adopt-mount-missing`) merges with no conflict: #190 does not touch `_verify`, and a relative url it lets through is the source's `.gitmodules` byte for byte. tests/test_adopt_submodules.py merges with no conflict: #173's `MOVED` and its root-tree test beside #190's absolute default url. #190's added and removed lines against 54aa69a are its lines against e4decfb, but for the reflowed docstring lines of hunks 1 and 3. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ked of a relative url kept in the SPEC leg and of `..\dependency`, so #190 coming back for the spec leg alone, or for Windows' separator alone, no longer passes every test (#190) Lane 2's review of b0c959e found two mutants of adopt-project.py that survived all 193 targeted tests: - M14, `if kept == leg == "code" and ...` in `_kept_relative_urls`, which resolves only the code leg's relative urls and reintroduces #190 for the spec leg (its rows P6 and P8 then end `adoption verified` over a failing recursive clone); - M13, `RELATIVE_URL_RE = re.compile(r"\.\.?/")`, which no longer takes `..\` for relative, where git does on every platform and strips it on Windows: there it is #190's own case. `test_execute_refuses_a_relative_url_its_leg_resolves_elsewhere` gains a `leg` parameter and two rows: - `spec-leg`: `.gitmodules` and the submodule kept in spec, url `./dependency`, so the answer names the spec leg's own remote (`remotes/Northwind-spec.git/dependency`), not the code leg's; - `backslash`: `..\dependency` in code, which git appends whole where `\` separates nothing (`remotes/Northwind-code.git/..\dependency`) and strips on Windows (`remotes/dependency`). `resolved_from_the_leg` reads the leg's name out of the finding and checks it. Each mutant, run over this file at c46dc57, fails exactly its row and nothing else: M14 1 failed (`[spec-leg]`), 26 passed; M13 1 failed (`[backslash]`), 26 passed. Unmutated, all five rows pass. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… RELATIVE source remote, is joined to the source's working tree before it is compared, so a source whose `origin` is `../src/Thing.git`, with the legs beside its dependency, no longer has its relative url refused when its recursive clone works (#190) Lane 2's review of b0c959e, P3-2: a source whose `remote.origin.url` is the relative `../src/Thing.git` and whose `.gitmodules` keeps `../dep` in a leg was refused by `execute --local-remote-dir`, though the legs sat beside the dependency and main adopted it into an assembly whose recursive clone worked. git resolves `../dep` against that remote to `../src/dep` and keeps it RELATIVE; `same_as` compared it as written with the leg's absolute answer, and `remote_local_path` names no path for a relative remote, deliberately, so the two never matched. What git does with it (git 2.43.0, probed here): `git submodule init` stores `../src/dep`, and `git submodule update` opens it from the TOP of the working tree, even when run from a subdirectory. So `_default_remote` now also returns that working tree for a remote that is a path and not an absolute one -- git's own test for the answers it leaves relative -- and `_resolved` joins git's answer to it. The join is not normalised: `same_as` asks the disk (`realpath`) whether the two name one directory, which is where a `..` after a symlink is decided. The finding says why its path is joined: `(its remote `origin`, relative to its working tree)`. The review suggested anchoring the BASE first, `normpath(source.path / url)`. That differs from git wherever git's arithmetic on the relative spelling is not normpath's: with `origin` `.` git resolves `../dep` to `dep`, the working tree's own `dep`, where the anchored base gives the directory beside it; with `../src/Thing.git/.` it gives `../src/Thing.git/dep`; and one `..` too many is git's `fatal: cannot strip one component off url '..'`, which an anchored base would resolve. Joining git's own answer keeps every one of those as git has it. Tests (tests/test_adopt_relative_submodule_url.py): - `test_a_relative_origin_resolves_from_the_source_working_tree`, with the origin spelled relative and absolute: under the legs' directory nothing is refused; with the legs elsewhere it is refused, the finding naming `<tmp>/Thing/../remotes/dependency` for the relative spelling and saying why, and the absolute spelling exactly as before; - `test_a_relative_url_its_leg_resolves_alike_adopts_and_clones` gains `[relative-origin]`: `execute` verifies, and the assembly's recursive clone and `bootstrap.py` exit 0 with the dependency in the leg. Each part reverted on its own, over the whole file (30 tests): no join, 2 failed (both `[relative-origin]` rows); the `which` clause dropped, 1 failed; joined to the process's cwd, 2 failed; the clause for every local path, 1 failed (`[absolute-origin]`); the join normalised, 1 failed. Unmutated, 30 passed. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ssh.github.com` as `github.com`, so `ssh://git@github.com:22/...` and `ssh://git@ssh.github.com:443/...` key like `git@github.com:...` and a source cloned that way no longer has its relative submodule url refused (#190) Lane 2's review of b0c959e, P3-4: `remote_key` read the `host:port` colon of a `<scheme>://` url as git's scp separator, so `ssh://git@github.com:22/testorg/dep` keyed as `github.com/22/testorg/dep`, and GitHub's documented SSH-over-the-HTTPS- port spelling `ssh://git@ssh.github.com:443/testorg/dep` as `ssh.github.com/443/testorg/dep`. `adopt-project.py`'s relative-url check compares keys and nothing else, so `check` found, and `execute` refused, `../dep` in a source whose `origin` was spelled that way, in the plan's own organisation, where main said `plan ok`. It failed safe; it was still a false refusal for a GitHub spelling. `remote_key` now takes `:<digits>` off the host of a `<scheme>://` url before it looks for the scp colon (`REMOTE_PORT_RE`), and maps `ssh.github.com` to `github.com` (`REMOTE_HOST_ALIASES`). Only a url with a scheme has a port: in the scp spelling `git@host:22/Repo.git` the `22` is the path's and stays, and a drive's colon has no digits after it, so `file:///D:/...` still keys as `D:\...` does. Over 24 spellings, the 8 that change are the port and `ssh.github.com` ones, each to its port-free `github.com` key; paths, drives, file urls, bare `Org/Repo` and scp keys are unchanged. `same_repository` asks the same keys, and its forge-tail match had hid this for an `Owner/Repo`; at a host's top level it had not, and `ssh://git@host:2222/Repo.git` and `git@host:Repo.git` are now one repository to `make siblings` and the doctor as well. THIS IS A SHAPE COPY. `scripts/repo_shape.py` is copied into every assembly root and family holder and digest-pinned there, so each one's next `update-shape.py check` shows `upstream-changed scripts/repo_shape.py`, as after #192's `load_yaml` (22c9fdd) and #157's `same_repository` (71cf5de). That is the right place for it: the project's own copy runs `same_repository` (`siblings.py`), and the fix is to the rule, not a helper only this checkout calls. No test pins its digest: digests are computed from the bytes copied, and like those two commits this one changes no pin. Tests: - tests/test_shape_doctor.py, `test_a_port_and_githubs_ssh_host_are_ not_part_of_a_remotes_key`, beside the one-repository table: six port and `ssh.github.com` spellings key as the scp one; a top-level repository with a port is `same_repository` with its scp spelling; the scp `:22/` path, another repository and another host do not key alike; a drive's file url still keys as the drive path. - tests/test_adopt_relative_submodule_url.py, `test_check_keeps_a_relative_url_its_github_leg_resolves_alike` gains `[ssh-port]` and `[ssh-github-443]`: no finding and the NOTE in the plan's own organisation, found in another. Each half reverted, over the new unit test, the one-repository table and the five `check` rows (7 tests): main's `remote_key` fails 3 (the unit test, `[ssh-port]`, `[ssh-github-443]`); the port kept, the same 3; no `ssh.github.com` alias, 2 (the unit test, `[ssh-github-443]`); a port taken off the scp spelling's path too, 1 (the unit test). The table passes under every one, as it must: no row of it changes. The three `check` rows that existed say exactly what they said. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 2 days and 22 hours by commenting |
Reviewer's GuideThe PR adds Git-backed validation for relative Sequence diagram for relative submodule URL validation and refusalsequenceDiagram
participant Check
participant Source
participant Git
participant LegRemote
participant Execute
Check->>Source: _entry_findings(plan, source)
Source->>Source: _default_remote(source)
Source->>Git: _resolved(source_remote, relative_urls)
Git-->>Source: source resolutions
Source->>LegRemote: _remote_urls(org, names, local_remote_dir, source)
Source->>Git: _resolved(leg_remote, relative_urls)
Git-->>Source: leg resolutions
Source->>Source: _Resolution.same_as(source, leg)
alt resolutions differ or cannot resolve
Source-->>Check: FINDING plan-submodule-relative-url
Check-->>Check: RELATIVE_URL_REMEDIATION
Execute->>Source: _refuse_what_check_finds(plan, source, local_remote_dir)
Source-->>Execute: refuse before leg repositories exist
else resolutions identify the same repository
Source-->>Check: NOTE or no finding
Execute->>Source: _refuse_what_check_finds(plan, source, local_remote_dir)
Source-->>Execute: continue with adoption
end
Flow diagram for source-to-leg relative URL comparisonflowchart TD
A[Kept submodule has a relative .gitmodules URL] --> B[Resolve from source remote with git submodule init]
B --> C[Resolve from leg remote with git submodule init]
C --> D{same_as resolutions?}
D -->|Yes| E[No finding; check may print a NOTE]
D -->|No| F[Report FINDING plan-submodule-relative-url]
F --> G[Commit an absolute URL or drop the submodule]
F --> H[execute refuses before creating any leg]
E --> I[Adoption proceeds without rewriting .gitmodules]
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
This was referenced Oct 11, 2026
…, #213 and #199 land beside #190, and the `repo_shape` import list carries #199's `double_quoted` with #190's `redacted`, `remote_is_a_path`, `remote_key` and `remote_local_path` Main moved from 54aa69a to 4835fb4: e7d1720 (#208 via #224), 404760f (#213 via #244) and 4835fb4 (#199 via #247). One conflict hunk, in adopt-project.py, and it takes BOTH sides: 1. The `repo_shape` import list. #199 adds `double_quoted`, which `y` now spells every quoted plan scalar with; #190 adds `redacted`, `remote_is_a_path`, `remote_key` and `remote_local_path`, which its relative-url check calls (`_Resolution.same_as` and `__str__`, `_default_remote`, `_relative_url_findings`). Main's line is kept as main wrote it, and #190's four names follow `parse_yaml` in the list's order. Either side's list alone leaves a name the merged code calls undefined. Merged with no conflict: - adopt-project.py, #199's other hunks: `_PLAIN_RE` ending at `\Z`, `y` through `double_quoted`, `_spelled`'s docstring, `_shown` and the four report lines that print through it, and `write_paths_listing` in the `shape_materialize` import and in `_extract_leg`. None touches a #190 line. #190's findings and NOTE already print every path and url through `_spelled`, which is what `_shown` falls back to for a name that is not printable. - scripts/repo_shape.py: #199's escape table (`DOUBLE_QUOTED_ESCAPES` with `\r`, `CODE_POINT_ESCAPES`, `double_quoted`, `_escaped`, `_escape_at`, the one-pass `_unescape`), with its module-docstring line and its `_BLOCK_SENTINEL` comment, and #213's `_split_key_flow` and `_split_flow`, are all in the YAML reader at the top of the file. #190's `REMOTE_PORT_RE`, `REMOTE_HOST_ALIASES` and `remote_key` are in the remote helpers some 1,400 lines below, and call nothing main changed. Each side's added and removed lines are the same before and after the merge, byte for byte. - update-shape.py (#208), scripts/shape_materialize.py (#199) and main's three test files are main's blobs; #190's three test files are this branch's. #190's added and removed lines against 4835fb4 are its lines against 54aa69a, but for the import hunk. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 11, 2026
This was referenced Oct 12, 2026
brettheap
added a commit
that referenced
this pull request
Oct 12, 2026
…project.py lists #314's `plan-manifest-invalid` after #270's `plan-unwritable-value` and before #245's relative-url findings, keeps `_repository_urls`' name-length refusal in #245's shape, and #314's tests unpack `_entry_findings`' third value #245 (161c6c9) conflicted with #314 in three hunks of adopt-project.py. #351 (d82a83e) merged clean. - `_entry_findings`, the docstring: #245's wording is kept, with the NOTEs it returns. #314's group is named after #270's value findings, before #245's relative urls, which stay "last". - `_entry_findings`, the list. #245 split the submodule findings into the `.gitmodules` split and the relative urls (`moved`), put `moved` last, and returns `notes` as a third value. The tail is now `findings + split + unwritable_findings + value_findings + moved`, and the remediations follow the same order. - #270's `plan-unwritable-value` still comes before #314's `plan-manifest-invalid`, as it did at 4b8da3d. So a value both rules judge is still refused under #270's code, and #270's hand-edited-value tests pin that. - #245's relative-url findings are still the last group, as its message says they are. - The docstring's paragraph on the order now says so. - `_repository_urls`: #245 moved the url table into `_remote_urls` and returns `(local, repositories, urls)` in one statement. #314's `refuse_unstorable_names` (`adopt-name-too-long`) is kept before that return, with `local` named once. - tests/test_adopt_manifest_values.py: `test_check_and_execute_read_one_list` and `test_a_text_value_the_reader_misreads_is_one_finding_in_the_list_check_prints` unpack `_entry_findings` into three names, because #245 made it return `notes` too. On the merged tree, without this, both raise `ValueError: too many values to unpack`. scaffold-project.py merged clean with #351. Its `_identity` refuses `scaffold-unwritable-value` before #314's `refuse_unstorable_names` and `_refuse_unholdable_manifest` run, so the scaffold has the same unwritable-first order as adopt. Five of #314's scaffold test cases expect the manifest refusal for values both judge: `--elected-by` TAB in two tests, then `\x0b`, `\x1c` and U+2028. The next commit applies #351's compose patch to them. `scripts/render-cli-reference.py --check` passes on the merged tree. 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
…refused `gh pr create`, and step 7 keys its way out to the exit it prints, so an `IS pushed` exit 2 is finished by hand, not re-run into a second pair of legs (#304) (#371) AGENTS.md step 7 told an assistant that any non-zero exit of `adopt-project.py execute` is "to fix the plan and re-run into fresh legs". That is the way out of a failed verification, exit 1. In GitHub mode `execute` has two more non-zero exits, both exit 2, and both come after both legs and the split branch are pushed: - `gh pr create` refused: `The branch IS pushed. Open the pull request by hand:` and the command. `execute` returned before `_verify`, so no table was printed and the pull request to open by hand was for a split nothing had checked. - The topic will not set: after `adoption verified`, `The split IS pushed and the pull request IS open. Set the topic by hand:` and three commands. On either, the way out is the printed command, and "re-run into fresh legs" abandons a finished adoption for a second pair of legs. No test reached either function, because `--local-remote-dir`, the path every test takes, skips both (`if not local:`). - **AGENTS.md step 7**, four lines longer (477 -> 481 on origin/main, the cap raised with a dated entry in `tests/test_repo_hygiene.py`; #309 landed first and its `464 -> 477` entry is kept as it is). It keys the advice to the exit the tool prints: exit 1 keeps its content (the source is untouched, fix the plan, re-run into fresh legs); exit 2 saying `IS pushed` (GitHub only, `gh pr create` or the topic refused) comes after both legs and the split are pushed and verified, so finish with the command it prints and never re-run into fresh legs. - **`adopt-project.py`, behaviour (fix round 1, below):** a refused `gh pr create` no longer returns before the verification. The by-hand command is printed after the table, and only when the table passed. - **`adopt-project.py`, the module docstring's EXIT CODES paragraph.** It gave exit 2 one meaning, "the question could not be asked". It now names the two exit 2s that come after the push and the verification, in the tool's own words, and says that a failed verification outranks both. `--help` carries none of it, so `docs/cli.md` is unchanged (`scripts/render-cli-reference.py --check`: `docs/cli.md is what the generator produces`). - **`tests/test_adopt_github_exits.py` (new, 11 tests).** The issue's reproduction as a test: a `#!/bin/sh` `gh` that refuses one command (the pattern of `tests/test_scaffold_leg_paths.py`, `WINDOWS_SKIP`) and a throwaway global git config rewriting `https://github.com/` to bare repositories under `tmp_path`, so no network. Each exit is asserted as exit 2 with its REFUSED block and its by-hand command word for word, a table, both legs and the split branch pushed; the command comes after the table in a merged stream; a verification that really fails gives exit 1 with a refused pull request and no command to open one. Step 7's anchors (`IS pushed`, `gh pr create`, the topic) and the EXIT CODES paragraph are asserted against what the tool prints, so rewording either side alone fails a test. The issue's second "what would fix it" bullet. Reproduced on d3f71d6 first (a source with its own `contracts/shape-pin.yaml` kept in the root fails the verification for real, nothing in the tool replaced): | scenario, GitHub mode, merged stdout+stderr | before (d3f71d6) | after | |---|---|---| | `gh pr create` refused, split verifies | exit 2; no table; `Open the pull request by hand` (line 19) | exit 2; table (27), `adoption verified` (33), then `Open the pull request by hand` (37), the last thing printed | | `gh pr create` refused, split FAILS verification | exit 2; `Open the pull request by hand` for a split that is wrong, never verified | exit 1; the finding and the footer; `No pull request was opened, and none should be: the split above did not pass verification.`; no command to open one | | topic refused, either | unchanged | unchanged (exit 2 / exit 1) | **The order, and why.** The split is pushed, then `gh pr create` is tried; a refusal prints its REFUSED block at once (what failed); the topic is not tried; `_verify` runs; then the by-hand lines are printed or withheld by what `_verify` said. Evidence for each step: - *The by-hand text has to come after the verdict.* The command is only right for a split that passed; a failed one must not get it (the brief), and a person who sees only the REFUSED block above, which names the exact `gh pr create ...` that was tried, may paste it. So `_open_it_by_hand` runs after `_verify`, and a failed split gets the sentence above in its place. - *It flushes stdout first.* Under a pipe stdout is block-buffered and the table is on it. In the merged logs above (stdout to the same file as stderr), every stderr line lands ahead of every stdout line on d3f71d6: the topic's by-hand commands print at line 7 of 26, before the table at 20. A test (`..._comes_after_the_table`) fails without the flush. - *The topic is not tried after a refused pull request*, as before (it returned early). Its by-hand message says the pull request IS open, so printing it would be false; the stub `gh` lets a topic through, and a test asserts it is never set on this path. - *Verifying first and opening the pull request only on a pass* was considered and not done: it reorders every successful run's output (the pull request line moves below `adoption verified`, and `_verify`'s `NEXT: review the pull request` is printed first), and it is not "the code path the topic case uses". It would fix something real, below. The code reuses the topic's path, with no second verification: `cmd_execute` already ran `_verify` after the topic step; the early `return 2` is gone, `_open_the_pull_request` prints the REFUSED block and returns False as before, and the by-hand lines moved to a small `_open_it_by_hand` called once the verdict is known. The exit is `2 if the pull request was refused or the topic failed`, and a failed verification outranks both with exit 1. **Tests for it** (`tests/test_adopt_github_exits.py`, 7 tests, all end to end through the real CLI): - `test_each_exit_after_the_push_is_exit_2_with_a_command_to_finish_by_hand[gh-pr-create-refused]` now expects the table, `adoption verified`, no pull request line, the topic never set; - `..._the_command_to_open_a_refused_pull_request_comes_after_the_table`: merged stream, table < `adoption verified` < by-hand, which is the last thing printed; - `..._a_failed_verification_outranks_a_refused_pull_request`: exit 1, the finding, no `Open the pull request by hand`, no `IS pushed`; - `..._the_same_source_verifies_when_gh_opens_the_pull_request`: its control, exit 0. Driving a real failing verification was practical (a source's own pin kept in the root, the way #188's tests do it), so nothing is injected. **Fails before, passes after:** the 5 changed or new tests fail on d3f71d6 (the step 7 reading, the EXIT CODES reading, the pr-create run, the order run, the failed-verification run); the topic run and the control pass there, as they should. **Mutation probes of the new hunk**, each failing at least one new test: restoring the early `return 2` (3 fail); ignoring the verdict in `_open_it_by_hand` (the failed-verification test); removing the flush (the order test); trying the topic after a refusal (the pr-create run); returning 0 for a refused pull request (2 fail); AGENTS.md or the docstring back to d3f71d6 (their readings). **S1, a closed stdout (should).** `_open_it_by_hand` flushed stdout with `sys.stdout.flush()`, which raises BrokenPipeError when nobody reads stdout any more (`execute ... | head`, `| true`), so the run ended in a traceback and the command to open the pull request, which main printed, was never printed. It now calls `_flush_stdout()`, which is lane 1's #188 helper under the same name and shape (`contextlib.suppress(OSError)`), at the same place in the file and with the same text, so the two merge to one definition. A test drives the real CLI with a stdout whose reading end is closed: the command is on stderr, there is no traceback, and the exit is 120, Python's own for a stdout it cannot flush as it exits, which main gave and 16aa9ff gave too (so the exit does not tell them apart; the traceback and the missing command do). | `pr` scenario, stdout closed (`\| true`) | c7c02fa (main) | 16aa9ff | this head | |---|---|---|---| | by-hand command | printed | **not printed, traceback in `_open_it_by_hand`** | printed | | exit | 120 | 120 (Python's, for the stdout it cannot flush as it exits) | 120 | **S1, the other case, NOT built here.** If the verification itself raises (a `git` that will not list a tree, a full disk), the by-hand line is still lost: a `CommandFailed` gives exit 2 with two REFUSED blocks and no word that the legs and the split are pushed, an `OSError` a traceback with exit 1, which step 7 reads as "a failed verification". #188's `_verify` catches those, reports them with its notice and the branch to delete, and exits 2; composed, both give `REFUSED verifying the split`, the notice, and "No pull request was opened, and none should be". That needs #188's handler, so it is its to compose, not this PR's to build. **S2.** origin/main is merged into the branch (merge commits c519f57 and a92fcad, no rebase or force). The one conflict was the AGENTS.md cap: #309's `464 -> 477` entry is kept as it is, this PR's entry is now `477 -> 481` and the cap is 481, the real `wc -l AGENTS.md` after the merge (477 on main, and step 7 grows from 5 lines to 9). The composition section above is refreshed against the current heads. **Nits.** - **N1.** The dated entry uses the file's em dash, and the five-versus-four sentence and "(fix round 1)" are gone: it describes the change, not the branch's history. - **N2.** `NEXT: review the pull request on <root>` moved out of `_verify` into `_next_steps(names)`, which `cmd_execute` calls only when no pull request was refused. After a refused one, the by-hand command is the next step and the last line. Under `--local-remote-dir` it is unchanged (see "Left out"). - **N3.** The printed command now works without a terminal. `gh pr create` without `--title` and `--body` asks for them, and where there is no terminal it exits 1 before any request (`must provide --title and --body ... when not running interactively`; the review ran it offline with a bogus token, an empty config directory and an `.invalid` host, and this fix round did not run `gh` at all), so an assistant told by step 7 to "finish with the command it prints" could not. The tool already knew both, so `_pull_request_command` builds the arguments once, `_open_the_pull_request` runs them, and `_open_it_by_hand` prints the same list, shell-quoted. A test reads back the arguments the stub `gh` was refused with and compares them with the parsed printed command; nothing runs the printed command. Two costs, stated: the body is the split commit's message, which lists every moved path, so for a large repository the printed command is as long as that list (the REFUSED block above it already prints the same text unquoted); and the quoting is POSIX, as #188's printed `git ... --delete` already is. `_open_it_by_hand`'s signature is now `(repositories, names, tracking, branch, message, passed)`, which the #188 note carries. - **N4.** The EXIT CODES paragraph said `execute` "has two more exit 2s". It now says it has other exit 2s once the leg repositories exist (a refused push, a refused mount) and that two of them are GitHub mode's alone. - **N5.** The follow-up claiming #188's `_flush_stdout()` covers the topic's by-hand commands printing above the table was false; it is corrected above ("Left out"). - **N6.** Nothing to do: the coordinator lands with an explicit squash message. - **N7.** Wording only, for the #188 composition: "was not verified" would be exact where #188's `_verify` could not verify at all. It is in the note on #188. - **N8.** The end-to-end environment keeps the real `gh` unreachable. `subprocess` goes on to the next PATH directory when the program it found cannot be executed (a `noexec` temp directory gives EACCES), and the next `gh` would be the operator's. So the run's environment now carries `GH_TOKEN=bogus-token-for-tests` and `GH_CONFIG_DIR` an empty directory, and PATH is the stub first and then the inherited PATH WITH every directory that holds a `gh` taken out (`without_a_real_gh`), which finds `git` and `python` because the directories are taken out only when each of `git`, `git-filter-repo`, `python3`, `python`, `sh`, `env`, `sed`, `tr`, `uname` and `cat` is found at the same place without them. Where it is not (`/usr/bin` on Linux holds `gh` beside `git`, and Homebrew's `bin` beside the `git` a run is meant to use), PATH is returned whole and the token and the config directory are what keep the real `gh` from acting. The stub records the token and config directory it was run with, and a test holds them to the bogus token and the empty directory. **Tests.** `tests/test_adopt_github_exits.py` is 11 tests now. On a throwaway worktree at 16aa9ff with this file: **5 failed, 6 passed**. Failing there: the EXIT CODES reading (N4), `[gh-pr-create-refused]` (N2), the order test (N2, N3), the printed-command test (N3) and the closed-stdout test (S1). Passing there, as they should: step 7, `[the-topic-will-not-set]`, the failed-verification test and its control, and the `without_a_real_gh` unit test and the environment test (N8: nothing in 16aa9ff's production code to fail, the environment is the test's). **Mutation probes** (15, each reverted by path in a throwaway worktree): the unguarded flush back, the flush removed, `_flush_stdout` not suppressing, the printed command cut to the old form, unquoted, run without the body, `NEXT` always or never said, the EXIT CODES paragraph reverted, the cap back to 477, the token or the config directory dropped from the environment, and `without_a_real_gh` either keeping every directory or dropping the shared ones: each is caught by at least one test, and a title with the leg names swapped, which no test caught at first, is caught by the title assertion added for it. - **Reproduction** (the issue's script, unchanged but for temp paths, on c7c02fa, git 2.43.0): both loops exit 2 with every message the issue quotes; the first printed no `VERIFICATION` table. - **Every non-zero exit of `execute`**, with line numbers, is tabulated in the lane's report. Also run: first `gh repo create` refused (exit 2, nothing created), second refused (spec leg exists, empty), split push refused by a hook (exit 2, legs pushed), and the naive re-run after the topic exit (refused at `gh repo create`, as a taken name is). - **Targeted tests on the head (c5a8a55, origin/main a1b3a4b merged in):** - `tests/test_repo_hygiene.py tests/test_cli_reference.py tests/test_adopt_github_exits.py`: 74 passed. - `tests/test_adopt_e2e.py tests/test_adopt_plan.py`: 75 passed, 1 skipped (`SHAPE_MEDXEHR_CLONE` is unset: the MedxEHR-backed plan test needs a read-only clone of MedxSoft/MedxEHR). - `tests/test_adopt_execute_refusals.py tests/test_adopt_verify.py`: 53 passed. - `tests/test_git_output_decoding.py` (origin/main's text-mode guard, which reads test files like this PR's): 67 passed. - `scripts/render-cli-reference.py --check`: `docs/cli.md is what the generator produces`. - origin/main, #184 (`fix/adopt-report-filter-repo-failure`, f416e13), #245 (`fix/adopt-relative-submodule-url`, d72544c, which now contains #309), and the lane-5 siblings F (69c54c9), K (21fbea4), L (8f43c96), M (14e2d68), Q (a30091f) and U (233dc71): all clean. #309 and #268 have landed; the one conflict with #309, the AGENTS.md cap in `tests/test_repo_hygiene.py`, is resolved in the merge commit (`477 -> 481`, cap 481 = `wc -l AGENTS.md`). - **#188** (`fix/adopt-late-failure-notice`, 918e8f2): conflicts in AGENTS.md (step 7) and in six regions of `adopt-project.py`, all in code this PR rewrote and #188 edits (nine regions in all, and three of those are #188's own against origin/main since #268 landed, so they are not this PR's). Not resolved in either branch. The note for lane 1, with the joined step 7 (482 lines, cap 482), the join of each region and the one patch #188's tests need, is posted on #188: the two `[...-pull-request]` parametrizations of `test_a_gh_step_after_the_split_gets_its_by_hand_exit` call `_open_the_pull_request` alone and look for the by-hand text, which is `_open_it_by_hand`'s now. Checked on a throwaway tree (87373a6, where #188 merges clean, plus this PR's diff plus #188, joined by hand, never committed): 106 passed over `tests/test_adopt_github_exits.py`, the whole of `tests/test_adopt_late_failure.py`, `tests/test_adopt_verify.py` and the cap test; without the one test patch exactly those two parametrizations fail. `_flush_stdout()` and the two imports are the same text on both branches, at the same place, so they merge to one definition. - `fix/adopt-record-submodule-placement` (#189, 3e86200), `fix/adopt-root-kept-symlinks` (6c44591), `fix/adopt-plan-quote-yaml-keywords` (e337f44): conflict in `adopt-project.py` (and, for #189, the cap line) in exactly the same regions with this PR absent (`git merge-tree origin/main <ref>`: 1 + 1, 3 and 3 conflict regions either way); none of them is this PR's. With #189 (cap 472, +8) as well the cap would be 489; arithmetic only. `README.md`, `docs/handbook.html` and `docs/cli.md` are not touched here, so the handbook's README-blob footer does not move. **At opening** (origin/main 8cdca5e, which merges clean): lane 1 has since moved `fix/adopt-plan-quote-yaml-keywords` to d38e143, which now merges clean, and #189 to 27d039b. - **README.md and docs/handbook.html: unchanged.** Neither states the rule: README's adoption section and the handbook's `#adopt` say that `execute` verifies and never say what a non-zero exit means; their "re-run" / "clean re-run" sentences are the scaffold's (`#partway`, lane 4's #309 territory). Changing the README would also move the handbook's machine-checked `README.md blob` footer for no gain. - **The verification footer and `--help`.** The footer (`_verify`, exit 1) is true for the exit it describes, and `tests/test_adopt_verify.py` holds its text; #188 rewrites it. `--help` carries no exit-code text. - **`NEXT: review the pull request on <root>` under `--local-remote-dir`** is still printed, as it always was, with no pull request ever opened. This PR stops saying it after a refused pull request (fix round 2); a local run is a test or rehearsal path, and its NEXT block is the only clone-and-bootstrap guidance it has. - **Candidate follow-ups, not done:** - GitHub mode opens the pull request BEFORE it verifies, so a failed verification leaves a pull request open for the wrong split (probed, a source's own pin kept in the root: `pull request .../pull/1` printed, then the finding, exit 1). Verify, then open, would avoid it, at the cost of reordering every successful run's output. - After a refused pull request the topic is not tried and the by-hand text does not list the three `gh repo edit` commands, so a person who opens the pull request by hand still has no topic set. - The topic's by-hand commands print above the table in a merged log (line 21 of 26 with #188 composed, line 7 on main): `_set_the_topic` prints them before `_verify` runs, so #188's `_flush_stdout()` only stops them jumping ahead of the leg lines. `_set_the_topic`'s docstring ("the caller reports it after the table") is false on main and stays false with #188. Printing them after the verification, the way this PR does for the pull request, would make it true. - **No remediation on three exit 2s** (a refused `gh repo create`, a refused split push, a failed `git filter-repo`): on c7c02fa each prints a REFUSED block or, for the last, nothing at all (probed: exit 2, empty stderr, two leg repositories left). #184 and #188 are the open PRs on these. An adversarial review of 16aa9ff found 2 should-fix items and 8 nits. Fix round 2 addresses all of them. A re-verification of c5a8a55 found 0 blockers, 0 should-fix items and 4 nits: - the end-to-end environment still passes `GH_HOST` and the enterprise tokens through; - this body's composition tips had moved (refreshed above); - two counts in lane 1's composition note were off (corrected before posting); - the `not verified` half of the `NEXT` condition has no test. No probe ran the real `gh`: every probe used a stub first on PATH, a bogus `GH_TOKEN` and an empty `GH_CONFIG_DIR`. Reviewed adversarially: the first review asked for changes (two should-fix, eight nits), and a fix round addressed all ten. The re-verify approved with four nits, left as follow-ups: the end-to-end environment passes `GH_HOST` and the enterprise tokens through, the composition section was stale at that day's heads, two sentences of the #188 note were imprecise, and the `not verified` half of the new `NEXT` condition has no test. CI is green on Linux, macOS and Windows, with SonarCloud passing, at 34b9500 (c5a8a55 merged with main 161c6c9). Closes #304 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
…ter #245 and #371 landed Main took #245 (161c6c9) and #371 (a983e6a) since this branch last merged it, and both changed `_entry_findings` where this PR does, so GitHub could not merge the branch. `adopt-project.py` was the only file that conflicted, in 2 hunks, both in `_entry_findings`; every other file came from main as it is. - The docstring: main's text, with its ending "and last each relative url of a kept submodule that names another repository from its leg (#190)" changed to "then each relative url ... (#190), and last each plan value `execute` reads late and `_plan_field_findings` asks about (#293, #294)", so this PR's findings still come last. - The return: main's three-tuple, with `notes` kept. The findings are `findings + split + unwritable_findings + moved + field_findings` and the remediations are the same order, ending in `field_remediation`. `cmd_check` and `_refuse_what_check_finds` already unpack three values on main, and both keep their `local_remote_dir=None` defaults, so this PR's tests that call `_refuse_what_check_finds(plan, source)` need no edit. 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 12, 2026
Main brings three commits since f795cf7: - 161c6c9 (#245): adopt-project.py resolves a relative `.gitmodules` url the plan keeps in a leg from the source's remote and from the leg's, and `check` finds the case where the two differ. It also touches scripts/repo_shape.py and three adopt tests, and adds tests/test_adopt_relative_submodule_url.py. - ab623dd (#296): tests/conftest.py puts a session `gh` guard (tests/gh_guard.py) first on PATH. It answers the doctor's two `machine`-row probes as a `gh` with no login, and fails any test whose other `gh` calls it refused. - a2cb7d3: bump-leg.py rewrites a tracked workflow the disk spells another way, under git's spelling. One conflict, in tests/conftest.py. Both sides appended at the end of the file, after the same helper: main its `gh` guard block, this branch `spelled()`. Both are kept, main's block first, as in the merge of b30595c. The merge's diff against this branch has exactly main's own lines, with two blank lines placed before `spelled()` instead of at the end. Its diff against main is this branch's own diff. adopt-project.py merged with no conflict. The merge's diff against each parent equals the other side's own diff (386 and 848 lines). Both sides changed only `cmd_execute`, in different lines: main hands `_refuse_what_check_finds` the `--local-remote-dir`. Every definition main changed is in `plan` and `check`, or in the refusal of what `check` finds, which `execute` raises before any leg exists. So the new `.gitmodules` finding needs no notice, and nothing the late failures go through changes. Checks: - No top-level name is bound twice in adopt-project.py, scaffold-project.py, scripts/repo_shape.py, scripts/shape_materialize.py, tests/conftest.py, tests/gh_guard.py, update-shape.py, scripts/family.py, shape-doctor.py, scripts/bump-leg.py and the two template validators, and no global is undefined. - `render-cli-reference.py --check` is ok. 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 12, 2026
…merged main a983e6a #386's branch is stacked on #356's fix/adopt-check-hand-edited-plan and was built at 5a5011d. #356 has since merged main a983e6a, which brings #245 (a relative `.gitmodules` url resolved from the source's remote and the leg's) and #371 (`execute` verifies before it exits on a refused `gh pr create`) into adopt-project.py, with #351, #350, #345 and #372 in other files. The merge is clean. `cmd_check` and `cmd_execute` still ask `_plan_type_findings` and `_refuse_a_mistyped_plan` first, right after `Plan.load` and before `_work_root`, so nothing #245 or #371 added runs on a mistyped `source:`, `legs:`, `pins:`, `allow_empty_legs:` or `paths:`. #245's new reader, `_kept_relative_urls`, takes `plan.entries`, which `_plan_list` already reads only from a list, and every reader of `plan.legs` asks its type. #245 gives `_refuse_what_check_finds` a `local_remote_dir` and rewrites its first line; `_refuse_a_mistyped_plan` shares only its shape and `REFUSED_FINDINGS_SHOWN`, not its text, so neither needs a change. Lane: openreposhape-2 (openRepoShape-2) 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
…elative-url findings join `_entry_findings` last, after this branch's `plan-root-symlink` and #255's values, and `execute` probes `core.symlinks` after #190's remote-aware refusal origin/main moved a2cb7d3 -> 92e29d9 (ten commits). The coordinator named 1f90a0f; 92e29d9 came after it and changes adopt-project.py too, so main is merged at its tip, by sha: - ab623dd (#296 via #349), tests/conftest.py: a session `gh` guard first on PATH; - 161c6c9 (#190 via #245), adopt-project.py and scripts/repo_shape.py: a relative `.gitmodules` url kept in a leg is resolved from the source's remote and from the leg's; `_entry_findings` takes `local_remote_dir` and returns its NOTEs as a third value; `_submodule_findings` returns the submodule findings and the relative-url findings apart, so that the relative urls come last; - d82a83e (#282 via #351), scaffold-project.py; - 0f67f7b (#331 via #350), the template's scripts/validate-family.py; - 3ae0109 (#326 via #345), scripts/family.py; - 28bdc84 (#337 via #372), single-repository.yaml; - a983e6a (#304 via #371), adopt-project.py and AGENTS.md: `execute` verifies before it exits on a refused `gh pr create`, and `_verify` no longer prints NEXT: `_next_steps` does, once the split is verified and no pull request was refused; - 8325d05 (#276 via #354), shape-doctor.py; - 1f90a0f (#323 via #380), scripts/shape_materialize.py (the pin-rewrite helpers), scripts/bump-leg.py and update-shape.py; - 92e29d9 (#319 via #377), adopt-project.py: `plan` asks about every path `execute` will not extract as it is named. scripts/shape_materialize.py, scripts/repo_shape.py and every other file merge with no conflict. adopt-project.py conflicts in four hunks, each resolved by keeping both sides: 1. The module docstring's `check` paragraph. Main's clause for the submodules gains "each relative url naming the same repository from the leg as from the source"; this branch's symlink clause follows it, as it followed the submodule clause before. 2. `_entry_findings`' docstring. Main's opening (the findings, what is printed once after them, and the NOTEs) and its order, with this branch's symlinks after the source's own submodules and before #255's values, and #190's relative urls last. 3. `_entry_findings`' return. Main's three values. The findings are, in order: each entry's path, coverage and the legs (with what the root keeps where the shape writes), the source's own submodules (`split`), this branch's `_root_symlink_findings`, #255's values, and #190's relative urls (`moved`) last. The remediation and the NOTEs are main's: the symlink findings carry their own fix and add none. 4. `cmd_execute`. Main's `_refuse_what_check_finds(plan, source, args.local_remote_dir)`, then this branch's `_refuse_symlinks_checked_out_as_files`, both before any leg, as before. Checked on the merged tree: - AST (fin183g/names.py, base a2cb7d3): no top-level name is bound twice in adopt-project.py, scaffold-project.py, scripts/repo_shape.py or scripts/shape_materialize.py, and none is loaded but bound nowhere. Main changed the signatures of `_entry_findings`, `_submodule_findings` and `_refuse_what_check_finds`, each by a trailing `local_remote_dir=None`; every call in the merged file is main's or passes two arguments, and this branch's tests call `_refuse_what_check_finds(loaded, source)`. - Both sides intact (fin183g/sides_intact.py): the only lines that differ from either side's own change are those of the four hunks. - What this branch changed in the materializer (fix B among it) and 1f90a0f's pin-rewrite helpers share no function: 1f90a0f adds `is_nested_key`, `with_scalar`, `pin_values_in` and `rewrite_fault`, and imports `YamlError`, `_strip_comment` and `parse_yaml`, none of them a name this branch binds. `place`, `copy_in_the_way` and `_refuse_unwritable` are as they were. - a983e6a changes what happens after the split is pushed (the pull request, the verification, NEXT). This branch's refusals all come earlier: `plan-root-symlink` and `adopt-symlinks-off` before any leg, and the materializer's before the split is committed. Its one line in `_verify` (`follows too many symlinks`) is kept. - 92e29d9 changes what `plan` asks, not what `check` or `execute` take. Left for the two commits after this one: - tests/test_adopt_root_symlinks.py's `refused_past_check` switches `_refuse_what_check_finds` off with a two-argument lambda, and `execute` now passes three, so the two tests that use it fail here with a TypeError. A rehearsal of this merge with the docstring commit gave `2 failed, 481 passed` over this branch's file and tests/test_adopt_{relative_submodule_url,github_exits,entry_paths, submodules,template_values,execute_refusals}.py, those two the only failures. The next commit fixes the fixture. - `_entry_findings`' docstring, as merged, says the two commands differ "in the one place a remote decides it". The commit after names the second place, this branch's symlink destinations asked of each command's own disk. 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 12, 2026
…refusal of `check`'s findings takes any arguments, so the two defence-in-depth tests reach the materializer again after #190 gave the refusal a third (#183, #178) The merge of main 92e29d9 brought 161c6c9 (#190 via #245), whose `cmd_execute` calls `_refuse_what_check_finds(plan, source, args.local_remote_dir)`. `refused_past_check` replaced that function with `lambda plan, source: None`, so on the merge both tests that use it, `test_a_caller_that_skips_check_meets_the_materializer_refusal` and `test_execute_hands_the_materializer_its_spelling`, failed with `TypeError: refused_past_check.<locals>.<lambda>() takes 2 positional arguments but 3 were given`, before `execute` reached the materializer they are about. The stub is now `lambda *_: None`, as main's own in tests/test_adopt_verify.py. Revert line (on a rehearsal of the merge, with git filter-repo): - as committed: `2 passed`; - with the two-argument lambda back: `2 failed`, each the TypeError above. 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 12, 2026
…r and validate-manifest.py about the manifest's values before anything exists, so `--id null`, `--tracking-branch true` and a blank `--elected-by` are refused (#258, #261) (#314) Both tools fill `templates/assembly-root/` from values a person typed or edited into a plan, and the new project then reads the result with two programs of its own: its YAML reader (`repo_shape`) and its `scripts/validate-manifest.py`. Neither tool asked either of them. Now each is asked, before a work directory or a remote exists and under `--dry-run` alike, and a value the project cannot hold is refused with the value named and the reason given. - **#258, the scaffold.** `id:` and `tracking_branch:` are written bare. `--id null` and `--id true` scaffolded three repositories whose first bootstrap failed `manifest-id`; `--tracking-branch true`, `007` or `null` pushed every repository on the branch as typed while bootstrap read the manifest back as `True`, `7` or `main` and put each leg on a local branch the remotes do not have, with no upstream, exit 0. Now `scaffold-manifest-misread` names the value and what the reader makes of it. - **#261, both tools.** A blank `--elected-by`, `--reference` or `--name`, `--elected-on 2026-W41-6` or `20261011` (read by `date.fromisoformat` from Python 3.11), `--org test+org`, and a plan edited to `visibility: secret` or `elected_by: null` were accepted, and `adoption verified` or a scaffold exit 0 followed. Now the scaffold says `scaffold-manifest-invalid`, `adopt-project.py plan` says `adopt-manifest-invalid`, `check` makes a `plan-manifest-invalid` FINDING of each, and `execute` refuses it through `check`'s own list (#168), before a leg repository exists. The sentence quotes the validator's own finding, so the words are its words. - **Names that cannot be directories.** A `--project` of 247 to 250 characters died in `Path.exists` with `OSError: File name too long` and a traceback under `--local-remote-dir` (scaffold from 247, adopt `execute`); `plan` admitted 251 to 255 that `check` refused as `unsafe-value: legs.spec`. `scaffold-name-too-long` and `adopt-name-too-long` refuse the first before anything exists, and `plan` now holds the three derived names to `checked_value` as `check` does. - **A validator that cannot be asked.** A copy of the standard whose `templates/assembly-root/scripts/validate-manifest.py` is missing, does not load, lacks what is asked of it (`DATE_RE`, `_findings`) or raises, is `manifest-validator-unusable` and not a traceback: it names the file, says the defect is the copy's and that nothing was created. A failed load is not cached. - `scripts/shape_materialize.py` (the one module both tools import, in no copy list): `manifest_misreads` renders each template line that carries a value and reads it with `parse_yaml`; `manifest_invalid` renders `project.yaml` from the whole substitution table and runs the validator the template ships, loaded from that file (no bytecode written into the template tree, `sys.path` put back); `refuse_manifest_values` and `manifest_value_problems` compose them; `misread_placeholders` names the placeholders `manifest_misreads` would refuse; `election_date` admits only what the validator's own `DATE_RE` admits, on every Python; `refuse_unstorable_names` holds a name to 255 bytes, `.git` included under `--local-remote-dir`, and shows a name too long at both ends. A placeholder is what `PLACEHOLDER_RE` finds in a line, not a second copy of its alphabet. - `scaffold-project.py`: `_refuse_unholdable_manifest` after the substitution table is built and before the plan is printed, the name rule before anything asks `gh api`; `--help` for `--id`, `--elected-by` and `--tracking-branch` says the rule. - `adopt-project.py`: `_flag_plan`, `_manifest_values` (the table `execute` renders from, asked of `_template_values` and not copied), `_manifest_findings` in `_entry_findings`, `_refuse_unholdable_values` in `cmd_plan`, the name rule in `_repository_urls`. - `docs/cli.md`: regenerated (four help paragraphs). - `tests/test_scaffold_manifest_values.py` (106 tests), `tests/test_adopt_manifest_values.py` (79). The templates write `id:`, `elected_on:`, `visibility:`, `tracking_branch:` and the leg paths bare, and the project's reader reads `null`, `true` and `007` as None, True and 7. Either the tool refuses what the project would misread, or the template quotes the placeholder (#259, which Brett has not decided). This refuses, because: 1. Every neighbouring fix refuses and none quotes: #225 (`--spec-path 2026`: "refused rather than quoted in the templates"), #181 (`adopt-manifest-misread`), #266, #251 (a chain link that is not a name). A scaffold that quoted while `adopt-project.py plan` refuses `--tracking-branch 007` would disagree with it about whether `007` is a branch. 2. Quoting changes the bytes of every new project's `project.yaml` and leg pins for every tool that renders them (the scaffold and adopt `execute`), and #259 asks for that to be decided once. (`family.py init` renders `templates/family-root/`, not these lines; it has the same defects in its own file and is #307.) 3. The question is asked of the TEMPLATE: each line is rendered and read as the project reads it, so the day a placeholder is quoted those values read back and nothing here refuses them, with no edit. `test_quoting_the_template_would_make_the_refusal_go_away` holds that. 4. A value that is a legal git branch name and that YAML reads as a boolean or null is not silently retyped: it is refused, with the reading named. **A behaviour change for input that used to work.** `--tracking-branch 2026` and `1.5` worked by accident (`str(2026)` gives `2026` back, and every reader of `tracking_branch` does `str()`: `bootstrap.py`, `update-shape.py`, `bump-leg.py`); they are refused as `adopt-project.py plan` refuses them on #181. #259's quoting would allow them again. The refusal's remediation says so in the true terms: it says only what holds of every value refused (the project's reader reads back something other than what was typed), names the failures that do hold (an id that is not text fails `validate-manifest.py`; `007` as a branch reads back as `7`), says that `2026` or `1.5` as a branch work today only because the tools that read it turn the number back into text and are still numbers to the project's reader, and gives the way forward, a value the reader takes for text (`atlas`, `release-2026`, `v2`). It no longer says the first bootstrap would fail, which is false of exactly those two values. The rule is the validator's own: `manifest_validator()` loads `templates/assembly-root/scripts/validate-manifest.py` from the template it ships, and `election_date` asks its `DATE_RE`. The alternative, moving the predicates into `scripts/repo_shape.py`, is a shape copy: every project in the estate would meet an `upstream-changed` row on its next `update-shape.py check` for a question no project asks of itself (the argument `shape_materialize` already makes for the leg-path rule and the commit trailers). Loading the file leaves the validator the one place the rule is written, and any rule it learns is asked here with no edit (`test_the_election_date_asks_the_validator_not_a_copy_of_its_rule`). Reproduction scripts: the issues' own loops (`repro258.sh`, `repro261.sh`), local remotes only. ``` base c3f4987 == --id null: scaffold exit 0 ... bootstrap exit 1 FINDING manifest-id: id is None == --tracking-branch 007: scaffold exit 0 ... bootstrap exit 0 spec leg on 7, upstream: fatal: no upstream configured for branch '7' == scaffold --elected-by ' ': exit 0, bootstrap exit 1: FINDING manifest-elected-by: elected_by is empty == adopt plan --org=test+org: check plan ok, execute exit 0, 1 x 'adoption verified'; bootstrap exit 1: manifest-leg-repository == adopt --project of 247 characters: execute exit 1 OSError: [Errno 36] File name too long this branch == --id null: scaffold exit 2, REFUSED scaffold-manifest-misread: --id is 'null', which is written into project.yaml as `id: null`, where the project's own YAML reader reads it back as no value at all and not as that text. == --tracking-branch 007: scaffold exit 2, REFUSED scaffold-manifest-misread: --tracking-branch is '007' ... reads it back as the number 7 == scaffold --elected-by ' ': exit 2 REFUSED scaffold-manifest-invalid: --elected-by is ' ', which the project's own validate-manifest.py refuses: manifest-elected-by: elected_by is empty. ... == adopt plan --org=test+org: plan exit 2 REFUSED adopt-manifest-invalid: --org is 'test+org' ... manifest-leg-repository ... == adopt --project of 247 characters: execute exit 2 REFUSED adopt-name-too-long ``` Each row is in the logs named in the report; every row of the two issues, and the extra rows lane 2 added to #258 (`--id 2026` and `007`, `--project Null` and `True`, `--tracking-branch TRUE`, `Null` and `1.50`), is a test that fails on c3f4987 (130 of the 170 new tests fail there: the rest assert what must still work, such as `release-1` and `v1.2.3`, or hold the API). Targeted runs on this head, all with the venv `git-filter-repo` 2.47.0 on PATH: | run | result | |---|---| | test_scaffold_e2e, _pin_and_reuse, test_naming_policy, test_manifest_validator, test_adopt_plan, test_adopt_execute_refusals, test_repo_hygiene, test_cli_reference, test_adopt_entry_paths, test_adopt_finding_spelling, test_adopt_gitmodules_values, test_yaml_flow_quotes, test_load_yaml_refusals, test_adopt_plan_escapes, test_pin_validator, test_neutral_pin_validator, and the two new files (170 of the tests) | 1087 passed, 1 skipped (`SHAPE_MEDXEHR_CLONE` unset) | | `tests/test_shape_doctor.py`, its own stream | 242 passed, 3 skipped (the PowerShell line, a Windows host only) | Mutation probes: each hunk reverted in a throwaway worktree, at least one new test fails for each of the eighteen of the first round and the ten of the second (the date rule, the scaffold gate, the misread scan, the validator question, the bytecode and `sys.path` guards, both name rules, adopt `plan`, the one list `check` and `execute` read, the grouping by value, the attribution, the leg-path and non-text filters, the list-`legs` guard, the no-revision guard, the unclassifiable-names guard; and the remediation wording, `misreads_elsewhere`, the placeholder pattern, `_line_shown`, the name tails and the three validator guards). Reach: every tracked YAML file (14) loaded with `repo_shape.load_yaml` on c3f4987 and on this head, parsed results identical (nothing here touches the reader; 7 are unrendered templates the reader refuses on both). Writers of the constructs: `scaffold-project.py` and `adopt-project.py execute` (assembly root); the family holder's writer is #307's. Nothing ran on Windows or macOS before this PR's CI: the new tests pass bytes and never `text=True` to git, write plans with `newline="\n"`, and the ones that copy the standard (`a_copy_of_the_standard`) copy a tree the way `tests/test_family.py` does. | against | result | the merged tree, run in a throwaway worktree and never committed | |---|---|---| | origin/main d82a83e | clean: this head contains it. Main was merged five times (49c21fa b421e6b, 567ee20 c0ed615, f45edf4 34ecbc2, 4b8da3d ab623dd, c578557 d82a83e); each conflict at the tail of `_entry_findings` was resolved by keeping every group, and on this head the tail is `findings + split + unwritable_findings + value_findings + moved`, with the remediations in the same order (see the merges below) | the runs below the merges are this tree | | origin/fix/render-single-pass 552763d (#260) | clean | 364 passed: the two new files, `test_template_values_single_pass.py`, `test_template_quoted_values.py` | | origin/fix/adopt-check-shape-paths 8a496d5 (P) | clean | 284 passed: the two new files, `test_adopt_root_shape_paths.py` | | origin/fix/leg-path-yaml-scalars b2b42f8 (#266) | clean | 722 passed: the two new files, `test_leg_path_yaml_scalars.py`, `test_adopt_leg_paths.py` | | origin/fix/adopt-plan-quote-yaml-keywords e337f44 (#181) | CONFLICT, two hunks in `adopt-project.py` `_entry_findings`: its docstring and its last statement | keep both and `misreads_elsewhere=True` (below): 1050 passed, 1 skipped: the two new files, `test_adopt_plan_yaml_quoting.py`, `test_adopt_leg_paths.py`, `test_adopt_plan.py`, `test_adopt_execute_refusals.py`, `test_cli_reference.py` | **With #181, and who does what.** Both branches add a findings group to `_entry_findings`. Keeping both, with #181's misread group just before this branch's (on this head, `findings + split + unwritable_findings + misread + value_findings + moved`), gives two findings for a plan value that is TEXT and that the template writes bare, which the reader reads back as a number or a boolean (`elected_on: "20261011"`, `visibility: "true"`, `id: "007"`): #181's `plan-manifest-misread` and this branch's `plan-manifest-invalid`, the second saying the retyped `id is 7`. `_manifest_findings(plan, misreads_elsewhere=True)` leaves such a value to the misread rule, by the placeholders `shape_materialize.manifest_misreads` refuses, so it is judged once. It is False here because with nothing else asking the misread question the validator's finding is the only one: always leaving such a value out makes this branch alone accept `id: "007"` at `check`, which it refuses, and 13 tests of this branch fail with it on. **Whoever lands second passes `misreads_elsewhere=True` at the one call in `_entry_findings`**; nothing is lost in either order, and `test_a_text_value_the_reader_misreads_is_one_finding_in_the_list_check_prints` fails for each of the three values on a merge that has not (two findings), and passes on one that has. Run on a throwaway merge with #181's e337f44: the three pairs are three single findings, and the other 16 rows of the 19-row plan and `check` driver are unchanged. Re-pointing #181's own gate at `shape_materialize.manifest_misreads` (one definition for both tools, which #258 asks) is the same person's job and is not done here: lane 1's branch is not this PR's. `#266` fires first at the scaffold (its leg-path rule, before the manifest gate), at adopt `plan` and at `check`, so the two messages about a leg path never disagree once it lands. `#260` makes a value holding a `{{KEY}}` acceptable as typed (see the first left-out item). - `adopt-project.py`'s misread side is #181's: `--tracking-branch 007` at `plan`, a plan edited to a non-text value (`id: 007` read as the number 7), and a value carrying a line break (`elected_on: "2026-09-02\nfoo: bar"`). This branch already refuses `--id true` and `--id 007` at `plan`, and `id: "007"`, `visibility: "true"` and `elected_on: "20261011"` at `check` and `execute`, through the validator's `manifest-id`, `manifest-visibility` and `manifest-elected-on`. - Leg paths (`--spec-path 2026`): #225's, now #266. `plan` refuses one through the validator's `manifest-leg-path` (it passes no `ignore`), and `check` ignores that code because it reports a leg path in the leg-path rule's own words (`ignore=LEG_PATH_CODES`), so a hand-edited `spec_path: "2026"` still checks `plan ok` on this branch as it did on main. #266 judges it first everywhere. - `scripts/family.py init`: the same defects in the holder; verified here (`--created-by ' '`, `--created-on 20261011` and `--id null` exit 0 and the holder's `validate-family.py` is red; `--tracking-branch true` is silent). **#307.** - `--pin null@<sha>` and a `--pin` product the reader misreads (the scaffold writes `neutral_product_pins` and a pin file product bare): not asked; #181 does adopt's. - `--org`: the scaffold's `gh repo create` still fails for a name longer than GitHub's 100 characters, at the first leg, before anything exists; that is GitHub's limit and not the validator's. - A refusal that depends on `--pin` without `--pin-source` is reached after `_declaration` has read the forge with `gh api` (a read); the leg-path and name rules come before it (the review's NIT-8). Remaining nits of the adversarial review that are not fixed here, one line each: - NIT-1: `plan` and `check` disagree about `--spec-path 2026` on this branch alone (see the second item above), and the scaffold's sentence names no flag for `--spec-path true`; #266's rule fires first everywhere once it lands. - NIT-2: `check` exits 2 with one REFUSED and no findings for a bad `org:` (`unsafe-value`) or an unparseable `elected_on:` in a plan with no `reference:` (`election-date-malformed`, naming `--elected-on` about a plan key); both are better verdicts than main's `plan ok`, but refusals and not FINDINGs. - NIT-6: before #260 lands, a value that holds a `{{KEY}}` is refused as `template-unsubstituted` (a defect of the tool), under `--dry-run` too; main accepted the dry run and wrote the wrong text or died after the push. With #260 both are accepted as typed. - NIT-8: with `--pin` and no `--pin-source`, a misread `--id null` is reached only after the forge read (above). - Two mutants of the review survive and are judged equivalent: `_reading_of`'s `len(parsed) == 1` and `key in values` in `_sentences_of`. An adversarial review of ca21b57 approved with nits. It had three should-fixes, none a code defect: - The misread remediation gave a false reason for `--tracking-branch 2026` and `1.5`. - A conflict with main in `_entry_findings`, and a stale body. - Double findings once lane 1's #181 lands. Fix round 1 (7defac7) handles all three and four nits. Its re-verification found no blocker or should-fix. It confirmed that taking the review's #181 suggestion unconditionally would drop this branch's only finding for `id: "007"`, `visibility: "true"` and `elected_on: "20261011"`. The `misreads_elsewhere` flag is therefore the handover: whichever of this PR and #181 lands second passes `True` at the one call in `_entry_findings`, and the guard test fails if it does not. A seeded run of 1324 edited plans on the flagged merge with #181 left no value with no finding and none with two. Two new nits are left: - A template validator whose `_findings` returns something other than a list still ends in a traceback. - The flag's call site in `_entry_findings` has no comment pointing at the handover. Tests on all three OSes passed at 7defac7, but SonarCloud's quality gate failed on its security rating for new code: `pythonsecurity:S8705` at `shape_materialize.run()`. That is the agentic argument-injection family #119 marked a false positive. The only new call into `run()` was `git rev-parse HEAD` in this tool's own checkout, with constant arguments. Fix round 2 (cf3e8d3) removes it, and a second git call with it. **SonarCloud still flags the same S8705 on cf3e8d3**, so that call was not its cause. The finding sits wholly inside `run()`, and PR #316 carries the same one with no new caller of `run()`. It needed no disposition in the end: main itself carried the identical finding until 13:01:12Z on 2026-10-12, opened by a `bump-leg.py`-only commit and closed by a `validate-pins.py`-only commit, and this PR's one analysis (11:55:50Z) was charged with it as new. SonarCloud passes from 4b8da3d on, with no change to `run()` or its callers. - `plan` and `check` ask the project's own validator about the manifest with a fixed, well-formed placeholder commit and tree digest. The question starts no process; the commands still read the source with `git`. The validator holds both to their form and compares them with nothing. - A second call went with it: `_template_values` also ran `tree_digest(SHAPE_ROOT, commit)` (`git ls-tree -r` over the whole standard) on every `plan` and `check`. The table body moved to `_template_table`, which runs nothing. `execute` still builds its table with the real commit and digest, which a new test asserts. - Both commands now refuse a blank `--elected-by` from a tree with no `.git` exactly as in a checkout, which fixes the review's NIT-9. There are 16 new tests: 14 fail on the previous `adopt-project.py`. - Main c0ed615 is merged (567ee20), with no conflict. - A fresh re-verification of the round found the code correct. `execute`'s 29-key table and its 50 pushed files are identical to 7defac7's. An audit hook saw 0 process starts in the manifest question. 14 of the 16 tests fail on 7defac7, and all nine mutants of the round are caught. Fix round 3 (ab66bc0) changes no behaviour: - It corrects a stale comment that pointed at `_template_values`. - It replaces two lane-private review ids that had been committed in a test. - It splits eight composite assertions SonarCloud flagged (`python:S9073`). - It defines the validator's path once (`MANIFEST_VALIDATOR`, `python:S1192`). Targeted runs: 289, 102 and 176 passed, and the suite collects (3499). **Four later merges of main, and the #351 compose commit.** - **4b8da3d merges main ab623dd.** It brought #270 (f795cf7), which asks whether a file can hold a value at all: `adopt-unwritable-value` at `plan`, and `plan-unwritable-value` at `check` and `execute`. - Some values are judged by both #270 and this branch. Examples: a lone surrogate, a line break or a bidi control in `visibility:` or `elected_on:`, and a control character or a line break in `elected_by` or `reference`, which the validator then reads as blank. - For such a value, #270's finding comes first, so `execute` refuses under #270's code. Both findings are listed, and both refuse before any leg exists. - #270's own hand-edited-value tests pin that order: three of them fail when it is reversed. - Why this order: #270 names the byte, the break or the control the person must retype. The validator describes the manifest that value would be written into, which is a consequence of it. - #309's `--spec-path` and `--code-path` help is kept beside this branch's `--tracking-branch` help. - **c578557 merges main d82a83e.** It brought #245 (161c6c9). #245 splits the submodule findings, puts its relative-url findings last, and returns `check`'s NOTEs as a third value. - This branch's group stays after #270's and before #245's, so the tail of `_entry_findings` is `findings + split + unwritable_findings + value_findings + moved`. - Two of this branch's tests now unpack that third value. - `_repository_urls` keeps `adopt-name-too-long` in #245's shape. - **The same merge brought #351 (#282).** Its `_identity` refuses an `--elected-by`, `--reference` or `--name` that no file can hold, as `scaffold-unwritable-value`, before this branch asks the validator. So the scaffold and adopt refuse in the same order. - a5438a5 applies the compose patch written with #351, unchanged. - Five of this branch's scaffold test cases now expect #351's refusal: `--elected-by` TAB in two tests, `\x0b`, `\x1c` and U+2028. - Without the patch, the merged tree fails exactly those five. - **ba58838 and c6f441b merge main 1f90a0f and 92e29d9** through `gh pr update-branch`, with no conflict. They bring #371 (a983e6a), #354 (8325d05), #380 (1f90a0f) and #377 (92e29d9). **A behaviour change since the head first approved (f45edf4).** `plan --reference $'\t'` and `plan --elected-by ' \n'` were refused as `adopt-manifest-invalid` and are now refused as `adopt-unwritable-value`. `plan --elected-by 'Bob\u202e'`, which f45edf4 planned, is now refused too. The same holds for `check` and `execute` on a value both rules judge: the code is now #270's. No test of this branch pins the old code, and the new order is the one #270's tests require. Targeted runs on a5438a5, with `git-filter-repo` 2.47.0 on PATH and the checkout's git identity blanked, are below. | run | files | result | |---|---|---| | this branch and #351 | `test_adopt_manifest_values.py`, `test_scaffold_manifest_values.py`, `test_scaffold_free_text_values.py` | 298 passed | | #270 | `test_adopt_template_values.py` | 77 passed, 1 skipped (`:223`: this checkout's own git config names a user, which outranks the test's `GIT_CONFIG_GLOBAL`; it passes in an export of the tree) | | #245 | `test_adopt_relative_submodule_url.py`, `test_adopt_gitmodules_values.py`, `test_adopt_submodules.py` | 352 passed | | the doctor | `test_shape_doctor.py` | 255 passed, 3 skipped (the PowerShell line, a Windows host only) | | neighbours | `test_scaffold_leg_path_collisions.py`, `test_cli_reference.py`, `test_repo_hygiene.py`, `test_scaffold_pin_collisions.py`, `test_adopt_clone_template.py`, `test_adopt_ignored_shape_files.py`, `test_adopt_literal_names.py`, `test_template_quoted_values.py`, `test_template_values_single_pass.py` | 506 passed | The same five runs were repeated on c6f441b, the landing head. #377's `test_adopt_entry_paths.py` and `test_adopt_plan_escapes.py` were added to the neighbours. Results: 298 passed; 77 passed and 1 skipped; 361 passed; 255 passed and 3 skipped; 644 passed. Reviewed adversarially (APPROVE WITH NITS, 0 blockers, 3 should-fixes); fix round 1 handles the three and four nits, and its re-verification found no blocker or should-fix. Fix round 2 validates with a placeholder commit, so the manifest question starts no process; its re-verification found the code correct. Fix round 3 is nits only. CI is green on Linux, macOS and Windows, with SonarCloud passing, at c6f441b (main 92e29d9 merged in). The combined tree with main 45e01f4 passed `test_agents_md_codes.py`, `test_shape_doctor.py` and `test_adopt_manifest_values.py` locally (391 passed, 3 skipped). A re-verification of the merge with ab623dd (4b8da3d) confirmed the merge, the refusal order and the composition. Its one nit, two stale sentences about the order, is fixed here. Closes #258 Closes #261 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
Merged by sha: origin/main at 16:30Z. Past a1b3a4b it brings these, among others: - f795cf7 (#255): `plan-unwritable-value`. - 161c6c9 (#245): relative `.gitmodules` urls. - a983e6a: `execute` verifies before it exits on a refused `gh pr create`. - 1f90a0f: the pin rewrite helpers in scripts/shape_materialize.py. - ab623dd: the session `gh` guard in tests/conftest.py. adopt-project.py conflicted in four hunks. Each resolution keeps both sides: 1. The module docstring's `check`. #181's clause about every value `execute` writes into the project's YAML now comes before the source's own submodules, which is where its finding comes. Main's relative url clause ends the sentence. 2. The `_entry_findings` docstring. This is main's text, with #181's misread values named after the root and before the submodules. 3. The `_entry_findings` return. - Findings: unsafe, coverage, legs and shape, misread (#181), submodules, unwritable (#255), relative url (#245). - Remediations: unsafe, misread, submodules, unwritable, relative url. - Main's NOTEs are third. 4. The mount line in `_print_what_will_happen`. #181's filter stays, so a leg path that is not text prints no `None/`, and main's `_shown` now wraps each mount and the branch. Everything else merged clean. At this merge, tests/test_adopt_template_values.py and tests/test_adopt_plan_yaml_quoting.py gave 514 passed. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 12, 2026
…est: `_entry_findings`' docstring keeps both paragraphs, F's empty-symlink one first and #314's value one after it, and nothing else conflicts (#303, #305) Main has moved since this branch was cut from c7c02fa: #245, #270, #371, #377 and #314 (3ef059c) landed in `adopt-project.py`, among others. Exactly one hunk conflicted, in `adopt-project.py`: `_entry_findings`' docstring, where this branch adds "THEN THE SOURCE'S EMPTY SYMLINKS (#303)" and #314 adds "A VALUE NO FILE CAN HOLD IS SAID BEFORE THE VALIDATOR'S WORD ON IT (#255 before #261)" after the same paragraph. Both are kept, this branch's first: it describes the code's order (`_source_path_findings` heads the list) and #314's describes the tail. The code itself merged without a hand, and `check` lists a source's paths, then its empty symlinks, then coverage and `_leg_findings` (with `plan-root-blocks-manifest`), then #166's split, #270's `plan-unwritable-value`, #314's `plan-manifest-invalid` and #245's relative urls last, so `execute`'s refusal still wears this branch's finding first. Rehearsed in a throwaway worktree before #314 landed: the resolved tree is the same tree object (47dfa8b) as 69c54c9 + origin/main 45e01f4 + #314's head c6f441b, and no compose fix was needed. 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 12, 2026
…351, #350, #345, #372, #371, #354, #380, #377 and the rest since ab623dd merge with no conflict, beside the pin functions this branch moved out of `scaffold-project.py` and its `plan --pin` in `adopt-project.py` Twelve commits of main since ab623dd, the base this branch last merged, reach five files this branch changes: `adopt-project.py` (161c6c9 #245, a983e6a #371, 92e29d9 #377), `scaffold-project.py` (d82a83e #351), `scripts/shape_materialize.py` (1f90a0f #380), `scripts/repo_shape.py` (161c6c9) and `docs/cli.md` (3ae0109 #345). Git merged every one with no conflict: - `scaffold-project.py`: #351's `_identity` value checks and its `unwritable_reason` import sit beside the imports that replaced the pin functions; pyflakes finds no undefined name in either tool (only the unused imports main itself has). - `adopt-project.py`: #245's `_entry_findings` now returns its NOTEs as a third value and takes `local_remote_dir`; `_pin_findings` is still read through `_leg_findings`, inside it. - `scripts/repo_shape.py` equals the tree `git merge-tree` gives for 6494714 with cd1fc79 (blob 091966b), and `validate-pins.py` and `tests/test_pin_source_env_collisions.py` are still Q's blobs. - `render-cli-reference.py --check`: ok. 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
… is refused and a name holding a lone surrogate is a finding, so `pins: [a, b]]` is not the pin `b]` and `name: "a\ud83db"` no longer kills `make validate` (#301, #280) (#390) Two reader-side defects the lane filed, in one commit on `fix/yaml-flow-brackets-surrogates`, cut from main c7c02fa. Below, "this head" is that commit, 21fbea4, the one the review approved; main was merged into the branch afterwards (last paragraph). **#301, `scripts/repo_shape.py`.** In YAML a plain scalar inside a flow collection ends at any of `[ ] { } ,`. `_split_flow` counts brackets to find its commas and never asked whether the count went below zero or ended above it, so a stray bracket was kept as text: - `pins: [a, b]]` read `['a', 'b]']`; - `k: [x [a, b]]` read `['x [a, b]']`; - since #284, `k: {a"]:}` read the key `a"]`. PyYAML refuses each. A new `_refuse_flow_indicator` refuses a PLAIN item or value (called from `_flow_value`) and a PLAIN key (called from `_split_key_flow`) of a flow collection that holds `[`, `]`, `{` or `}`. The refusal is a `YamlError`, so `load_yaml` turns it into `REFUSED yaml-unparsable` and names the scalar: ``a plain scalar in a flow collection holds `]`, which ends it in YAML: 'b]'``. These still read as before: - a quoted scalar's brackets (`k: ["a]", {"b}": "c]"}]`); - nested and empty flow collections; - a bracket in a BLOCK plain scalar (`k: a[b]`, `- see [1]`); - a bracket in a comment after a flow. No other function changed. **#280, `templates/assembly-root/scripts/validate-manifest.py`.** `_name_finding` now reports a display name holding any of U+D800 to U+DFFF as `manifest-name`: ``` FINDING manifest-name: name is 'a\ud83db', which holds U+D83D, a lone surrogate: no UTF-8 file, commit message or console can hold it. Write each character as itself, or as one `\U` escape (`\U0001F600`, not the pair `\uD83D\uDE00`). ``` The finding goes to stderr with the name escaped by `repr`, before the `manifest ok` line could raise. **Why the #280 refusal is in the validator and not in the reader.** Two places could refuse a lone surrogate, the reader and the validator. The evidence rules out the reader: - **The writer emits a lone-surrogate escape on purpose.** The escape is `\uDXXX`, a `\u` and four hex digits from D800 to DFFF. `double_quoted('caf\udce9')` is `"caf\udce9"`. That is how `adopt-project.py`'s plan keeps a path byte that is not UTF-8 (#199), and `check` must read it back. `tests/test_adopt_entry_paths.py` holds that round trip (120 passed with `test_adopt_plan.py` on this head). Lane 1's pending #189 branch (70f4d29, `submodule_record`) also writes `\udce9` escapes into `project.yaml`'s `submodules:` block, and its docstring relies on the reader resolving them. A reader refusal would break both. - **PyYAML 6.0.3 agrees with the reader.** The pure-Python `SafeLoader` reads `name: "a\ud83db"` as the lone surrogate. Only libyaml (`CSafeLoader`) refuses it, and it refuses even the escape pair `"\ud83d\ude00"`, which the Python loader reads as two surrogates, as this reader does. - **The defect is in a value, not in the YAML.** A display name is text a person chose, and no UTF-8 stream holds a surrogate. On c7c02fa and on this head, with local bare remotes only: | launcher | c7c02fa | 21fbea4 | |---|---|---| | `scripts/validate-manifest.py` | exit 1, `UnicodeEncodeError` traceback | exit 1, the `manifest-name` finding | | `make validate` | exit 2 on that traceback | exit 2 on the finding | | `shape-doctor.py --root .` | `INVALID (manifest)`, the row quotes `UnicodeEncodeError: ...` | `INVALID (manifest)`, the row quotes `FINDING manifest-name: ...` | | a real `bump-leg.py` | exit 0, prints `manifest ok: a\ud83db`, and COMMITS the subject `Bump spec leg to <sha> in a?b` (`errors="replace"`) | exit 2 `bump-leg-validators-red`, every byte rolled back, 0 commits | | `update-shape.py apply` | exit 0, prints `manifest ok: a\ud83db`, commits | exit 2 `update-validators-red`, restored, 0 commits | So all four launchers now agree, and each names the problem. **Readers and callers this changes.** - **#301:** every `parse_yaml`/`load_yaml` caller, and only for a document whose flow collection holds a plain scalar with a bracket. Such a document was read, or misread, before and is refused now. Every tracked YAML file loads the same on c7c02fa and on this head (14/14 raw, 18/18 rendered templates). So do the 6 YAML files of a scaffolded project (`project.yaml`, four `contracts/*.yaml` and `validate.yml`): 6/6 on a c7c02fa scaffold and 6/6 on a 21fbea4 scaffold. No writer emits such a scalar: - every flow sequence written in the tree is bare names or `[]` (adopt's `pins: [...]` and `allow_empty_legs: [...]`, `naming_block`'s `also_matches` and `referent_chain`); - `double_quoted` quotes any value holding a bracket. - **#280:** the project's own `scripts/validate-manifest.py` (a digest-pinned shape copy; it reaches existing projects through `update-shape.py apply`, which reports it `upstream-changed`). Every caller of that copy now stops at the finding: - `make validate`; - the CI step in `validate.yml`; - `shape-doctor.py`'s `manifest` row; - `bump-leg.py` and `update-shape.py apply` (their red-validator refusal and rollback); - a family holder's `make validate`, which runs each member's. `bootstrap.py` runs the same copy in its step (b), but on such a name it dies at its own first `print`, before step (b) (see Left out). With #314 merged as well, the scaffold's pre-check asks this validator about `--name`. In the tree of this head and #314 alone, `scaffold-project.py --name $'caf\xe9' --dry-run` is refused `scaffold-manifest-invalid` with this finding before anything is created (checked at #314's head 4b8da3d). With #351 (landed as d82a83e) and without #314, that `--name` is already refused, as `scaffold-unwritable-value`. **Evidence.** - **Issue rows.** All ten #301 documents reproduce on c7c02fa and are refused on this head. 8 are documents PyYAML refuses. 2 (`k: {a:}#}: b}`, `k: {}#":}`) PyYAML reads only by taking a `#` with no space before it for a comment, and they are refused by design, as `k: "a"#c` is. Every #280 row is in the table above. - **Seeded oracle fuzz.** Corpus: an earlier review's, seed 2490250, 785,646 documents in six contexts. PyYAML 6.0.3 is the oracle only. | | c7c02fa | 21fbea4 | |---|---|---| | read the way PyYAML reads it | 254,629 | 254,629 | | read, where PyYAML refuses | 179,999 | 112,562 | | refused, where PyYAML reads | 48,434 | 49,492 | | read, but not as PyYAML reads it | 2,330 | 1,272 | | the flow contexts alone: read where PyYAML refuses / refused where PyYAML reads | 77,073 / 15,413 | 10,306 / 16,465 | Only two kinds of document change: - 67,437 invalid documents c7c02fa read are refused; - 1,058 documents c7c02fa MISREAD are refused. 1,037 of these are PyYAML's `#` leniency, and 21 are a single-pair mapping in a flow sequence (`k: [a:[]]`), which the subset never read as a mapping. Nothing c7c02fa read correctly changes, nothing is newly read, and there are 0 crashes. Against the issue's two-line `depth < 0` sketch, run on the same corpus, this head refuses everything the sketch refuses, plus 21,631 more: 21,608 invalid documents (the opener class, such as `k: [a[]` and `k: [x [a, b]]`) and 23 misreads. The fuzz ran on 38738cf, whose `scripts/` and `templates/` are byte-identical to 21fbea4's. - **Tests fail on c7c02fa.** With the four test files copied onto c7c02fa: 45 failed, 191 passed. The failures are 31 bracket rows, 10 validator rows and the 4 launchers. - **Mutation probes.** Each hunk was reverted or weakened on its own, and each mutant fails new tests: - M1, no check on items and values: 23 failed; - M2, no check on keys: 8; - M3, no quoted-scalar exemption: 14; - M4, no quoted-key gate: 11; - M5, closers only: 3; - M6, no surrogate finding: 10; - M7, upper bound excludes U+DFFF: 2; - M8, lower bound excludes U+D800: 2. **Tests.** - New: `tests/test_yaml_flow_brackets.py`, with the ten issue rows, sixteen more positions, the refusal's wording, a file refusal, fifteen still-read rows and the writer round trip. - In `tests/test_manifest_validator.py`: five surrogate spellings, each under a pipe and under `PYTHONIOENCODING=utf-8`, plus four still-ok controls on either side of the range. - In `tests/test_validator_child_encoding.py`: a real bump, `update-shape.py apply`, the doctor and `make validate`, each on `name: "a\ud83db"`. - `tests/test_yaml_quote_boundaries.py`: its docstring said `k: "a"#c` was the only PyYAML disagreement. There are eight; seven are `BRACKET_IN_A_KEY` rows refused by design. Targeted runs on 21fbea4, Linux, Python 3.12.3, git-filter-repo 2.47.0, each in the background: - the reader and validator files (`test_yaml_flow_brackets`, `test_yaml_flow_quotes`, `test_yaml_flow_mappings`, `test_yaml_quote_boundaries`, `test_yaml_block_scalar_keys`, `test_load_yaml_refusals`, `test_adopt_plan_escapes`, `test_manifest_validator`, `test_pin_validator`, `test_neutral_pin_validator`, `test_template_quoted_values`): **462 passed**; - `test_validator_child_encoding`, `test_stdout_encoding`, `test_repo_hygiene`, `test_cli_reference`: **165 passed, 1 skipped** (`test_stdout_encoding.py:286`, a Windows-only default); - `test_adopt_plan`, `test_adopt_entry_paths`: **120 passed, 1 skipped** (`test_adopt_plan.py:393`, `SHAPE_MEDXEHR_CLONE` unset); - `test_shape_doctor`: **242 passed, 3 skipped** (`test_shape_doctor.py:5330`, PowerShell on a Windows host only); - this head merged with origin/main 86da808, which carries #316: `test_template_quoted_values`, `test_template_values_single_pass`, `test_yaml_flow_brackets`, `test_yaml_quote_boundaries`, `test_manifest_validator`: **412 passed**; - this head merged with #314 (ab66bc0): `test_scaffold_manifest_values`, `test_adopt_manifest_values`, `test_manifest_validator`, `test_yaml_flow_brackets`: **293 passed**. The full suite and the macOS and Windows legs did not run before this PR's CI. **Merge-tree, as of 2026-10-12T15:12Z** (`git merge-tree --write-tree --name-only`), against origin/main d82a83e and the heads named here; each sha is the one it had then, and main has moved since. - **Clean with this commit:** - origin/main d82a83e, which carries #316 (34ecbc2), #245 (head a0272b2, landed at 14:55Z as 161c6c9) and #351 (landed as d82a83e). #245's one change to `scripts/repo_shape.py` is in `remote_key`, away from the YAML reader, and #351 touches `scaffold-project.py` only; - #314 (4b8da3d). With it merged, the `--name` refusal above still holds (checked at 4b8da3d); - lane 1's `fix/adopt-late-failure-notice` (9f1778d) and `fix/adopt-plan-quote-yaml-keywords` (d38e143); - the sibling `fix/pin-source-env-collisions` (34dbd86, #275); - every other open PR head but one: 28 of the 29 open, all but #358. - **Conflicts that are not this commit's.** Each conflicts the same way with origin/main alone, in a file this commit does not touch: - #358 (ebffb08), in `shape-doctor.py`; - lane 1's `fix/adopt-root-kept-symlinks` (6c44591), in `adopt-project.py`. - **Five heads already conflict with origin/main itself, and merge clean with this commit:** #314, #356 (5a5011d), #382 (7e55d00) and lane 1's `fix/adopt-plan-quote-yaml-keywords` in `adopt-project.py`, and lane 1's `fix/adopt-late-failure-notice` in `tests/conftest.py`. - **One conflict is this commit's:** `fix/adopt-record-submodule-placement` (70f4d29, lane 1, #189), in one hunk, the import block of `tests/test_manifest_validator.py`. Both branches add to it: that branch adds `import os` and `import sys`, and this one adds `import subprocess` between them, so the resolution is all three. Its `repo_shape.py` and `validate-manifest.py` changes auto-merge. Not resolved in either branch. **The review.** An independent opus review at 21fbea4 approved it with nits: 0 blocker, 0 should-fix, 5 nit. All five are in text (this description, the author's report and the commit message), none in the code or the tests. Four were fixed in the description; the fifth, the commit message's spelling of the surrogate escape as a six-digit one, is fixed in this message: the escape is `\uDXXX`, a `\u` and four hex digits, D800 to DFFF. Its own seeded structured fuzz (seed 7301280: 120,000 nested flows in five block contexts, 600,000 documents, PyYAML 6.0.3 as the oracle only) found 0 changed reads: no document c7c02fa read the way PyYAML reads it changed, and nothing crashed. An instrumented reader returned no plain scalar holding `[ ] { }` inside a flow on this head, against 76,955 on c7c02fa. The author's seed 2490250 counts reproduced row for row, and all 13 mutants it tried were killed. **664 passed** on this head merged with origin/main ab623dd (16 test files; 361 on the head alone, 8 files). The one conflict, with lane 1's #189 branch, is the `tests/test_manifest_validator.py` import block, and resolves to all three imports. Three follow-ups it confirmed are not in this change: the bootstrap's raw name print and the flow entries still read (both under Left out), and a block value that opens with `]` or `}`, which c7c02fa and this head both read where PyYAML refuses. **Left out.** - **A plain flow entry that opens with an indicator, and an empty flow entry.** Most of what this head still reads where PyYAML refuses is one of two classes: - an entry opening with `:`, `#`, `?` or `-` (`k: {:":}` reads `{':"': None}`, `k: [:]`; 6,286 flow documents); - an empty entry from a leading or doubled comma (`k: [,a]` reads `['a']`, `k: [a,,b]` reads `['a', 'b']`; 3,858). Neither is a bracket, and #301 does not claim them. - **A single-pair mapping in a flow sequence.** `k: [a: b]` still reads as the string `'a: b'`, where PyYAML reads `[{'a': 'b'}]`. Only the ones holding a bracket are now refused. - **`templates/assembly-root/scripts/bootstrap.py`.** It prints the display name raw (`bootstrap: <name> (<root>)`) before its validators run. On a project named `"a\ud83db"`, `make bootstrap` still dies there with `UnicodeEncodeError`, on c7c02fa and on this head (filed as #391). - **`elected_by` and `reference`.** A surrogate in either is still accepted. Neither is printed by the validator, so nothing raises; the scaffold side of such values is #282. - **The escape pair `"\ud83d\ude00"`.** It is still two surrogates, not one character, as PyYAML's Python loader reads it. The finding asks for the character itself or its one `\U` escape instead of the reader inventing a joining rule. Reviewed adversarially at 21fbea4 (APPROVE WITH NITS: 0 blockers, 0 should-fixes, 5 nits, all in text and none in the code or the tests). The code and the tests needed no fix round; the nits are fixed in this message. Main was merged into the branch after the review with no conflict: at that merge (c1a119d, main 45e01f4 merged in) the branch's own diff against main is the one reviewed, the same 6 files with 422 lines added and 6 removed. CI is green on Linux, macOS and Windows, with SonarCloud passing, at c1a119d (main 45e01f4 merged in). The combined tree with main 3ef059c (#314 landed) was run locally before landing. Closes #301 Closes #280 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
…ut family.yaml before creating anything, `--dry-run` too, so `--id null`, `--tracking-branch true`, a blank `--created-by` and `--created-on 20261011` are refused (#307) (#397) `family.py init` rendered `templates/family-root/family.yaml` from `--id`, `--org`, `--name`, `--created-by`, `--created-on` and `--tracking-branch` and pushed it. The holder reads that file with its own YAML reader and its own `scripts/validate-family.py`, and `init` asked neither anything. Every row of #307 reproduces on c7c02fa (the issue's own script, with a stub `gh`): `--dry-run` and the real run both exit 0 for all 15 rows, and - `--created-by ' '`, `--created-on 20261011` / `2026-W41-6` / `tomorrow`, `--id null|true|false`, `--org null`, `--name 'Contoso Group'` leave a holder whose `validate-family.py` exits 1 from its first commit. `init` cannot be run again over it (`family-remote-exists`, no `--force`). - `--tracking-branch true`, `007`, `null`, `2026` exit 0 with `validate-family.py` exit 0 (it has no rule about `tracking_branch`), the holder pushed on that branch, `family.yaml` reading back as `True`, `7`, `None`, `2026`, and `update-shape.py apply --branch` printing `gh pr create --base True` / `--base 7` / `--base main` (for `null`) / `--base 2026`. **What changed.** `scripts/family.py` asks two questions about the manifest `init` is about to write, right after the substitution table is built: before the plan is printed, before the `--dry-run` return, before `_create_holder_remote` (which is where `gh repo create` runs against GitHub). Both refuse, name the flag, and say nothing was created. - `family_manifest_misreads` renders each line of the TEMPLATE that carries one of the values, reads it back with the holder's own reader (`repo_shape.parse_yaml`) and refuses a value that does not come back as the same text: `family-manifest-misread`. This is what catches `--id null`, `--org null`, `--created-on 20261011` and the silent `--tracking-branch true|007|null|2026`. A line the refusal quotes is shown the way Python spells it where it holds a line break or an escape, so a value cannot forge a second line of the refusal. - `family_manifest_invalid` renders the whole `family.yaml` and runs the validator the template ships over it, loaded from `templates/family-root/scripts/validate-family.py` (`Report` and `_check_envelope`), never re-implemented: `family-manifest-invalid`, in the validator's own words. This is what catches a blank `--created-by`, `--created-on 2026-W41-6` / `tomorrow` and `--name 'Contoso Group'`, and whatever the validator learns next. A misread is reported first and alone, because the validator's finding about it (`id is None`) is its consequence. A validator that is missing, broken, raising or exiting, whose `Report` has no `findings`, or that answers with a finding that is not text, is `family-validator-unusable`, not a traceback and not a zero. - A value no flag supplied is named by where it came from (`--id (not given, so --family lowercased)`). - `init --help` and `docs/cli.md` (regenerated, `render-cli-reference.py --check` ok) say what is held to what. No template is edited. A refused value is refused, not quoted or corrected. **Fix round 1 (the review of 14e2d68).** The first review found one broken test elsewhere in the suite and four claims no test guarded. This round is one commit on top of 14e2d68 (tests, plus one change to `scripts/family.py` for the validator that cannot be asked); the first commit is not amended. - **`tests/test_stdout_encoding.py` (the blocker).** The `family-init-dry-run` row ran `init --name <non-ASCII> --dry-run` and asserted the name is printed in the plan. The holder's own `validate-family.py` refuses that name (`naming-unclassified`: a family holder is one CamelCase token), so `init` now refuses it before the plan prints. That is this PR working as intended, so the TEST changed and the refusal did not: the row drops `--name`. The plan still prints the non-ASCII `--created-by` on its `created by` line, so the constrained stdout still has a value to escape (`created by 日本 ...` under `cp1252`, the characters themselves under UTF-8), and the row still fails if `family.py main` stops calling `escape_what_stdout_cannot_encode`. On the first head the old row failed (`1 failed`); the row as it is now passes on this head, and on this head merged with origin/main. - **A validator that cannot be asked.** The in-process call now sits in one `try` that also catches `SystemExit` and reads `report.findings` there, and a finding that is not text is refused by name. Three cases used to escape the refusal: a `Report` with no `findings` (an `AttributeError` traceback), a finding that is not a string (a `TypeError` from the regular expression) and a `_check_envelope` that calls `sys.exit(0)` (`init` exited 0, having printed and created nothing). Each is now `family-validator-unusable`, exit 2, nothing created; one test per case over a deliberately broken copy of the template, and one run at the command line for the exit. - **`_line_shown`.** A value with a line break, an ESC and a would-be forged line (`x\nFINDING forged: y\x1b[2J`, and a CR, U+2028 and U+0085 variant) is one escaped sentence: asserted on `family_manifest_misreads` itself, through `main` and what it prints (in process, so no argv crosses to Windows), and at the command line (skipped on Windows before any call, since argv is UTF-16 there; the in-process rows hold it on every OS). - **Two more guarded hunks.** The `sys.path` restore (also when the validator fails to load) and the comment-line skip each had a surviving mutant; each now has a test that kills it. The bytecode flag's restore is guarded by the same tests. - **The bytecode test** asked the REAL template's `__pycache__` before and after, so any stale `validate-family*.pyc` there failed it whatever the code did. It runs over a copy of the template made without a `__pycache__`, with the flag cleared first so that it is `family_validator` that must set it, and a control shows that importing the same file that way does write a `.pyc`. **How this composes with #259 (Brett ruled to quote the placeholders).** The misread check never says which lines are bare: it renders the real template line and reads it back. So once `fix/template-quoted-placeholders` (PR #395, first head 233dc71) quotes `id:`, `org:`, `created_on:`, `visibility:` and `tracking_branch:`, `--id null`, `--org null` and `--tracking-branch true|007|null|2026` read back exactly, the first question refuses nothing, and no edit is needed here. What the validator refuses stays refused: `--created-on 20261011` becomes `family-manifest-invalid` (`family-created-on`) instead of a misread, and a blank `--created-by` and `--name 'Contoso Group'` are unchanged. Proved three ways, all in `tests/test_family_init_values.py`: - the same values through a copy of the template with the placeholder forced quoted: accepted, and `parse_yaml(render(...))` reads the exact text back; - an end-to-end run of a forged one-commit checkout of `scripts/`, `templates/` and `contracts/` whose `family.yaml` is forced quoted: `init` exits 0, the holder's own `validate-family.py` exits 0, `family.yaml` reads back as typed, and the branch pushed is the typed one (a second forged checkout forced bare: refused, nothing created); - the tests that read the real template's own quoting to say what to expect: run here with the real template as it is, and on a tree with #259's branch merged in (below), so the file passes whichever of this and #259 lands first. **Once #259 is on main, 19 rows skip for good and one sentence goes unreached.** With the template quoted, the 16 end-to-end rows of `test_init_asks_the_holders_own_programs_before_anything_is_created` whose quoted answer is "the run is fine" and the 3 of `test_an_id_the_family_name_would_derive_is_named_by_where_it_came_from` skip with a message that says why (the 8 `test_a_quoted_value_is_held_as_typed` rows un-skip and pass), and nothing fails. `MANIFEST_MISREAD_REMEDIATION`'s "family.yaml writes a value like these unquoted" is then text no real template reaches; a template that writes one bare (the forged bare checkout the tests use) still does. Nothing is changed here for it: whichever of this and #259 lands second can drop the dead rows and reword that sentence in the same PR. **Same helpers as #314 (#261)?** #314's `manifest_misreads` / `manifest_invalid` are hard-wired to the assembly root (`project.yaml`, its templated list, `validate-manifest.py`'s `_findings`) and were not on main when this PR was written, so this PR writes the smallest version the family needs, in `scripts/family.py` (not `shape_materialize.py`, which #266, #270, #314 and #316 all edit). They differ only in the template, the validator's entry point and the sentence that names the file. #314 landed first (3ef059c), and this PR leaves both sets of helpers as they are: **unifying them is left to a follow-up on #459's checklist (#459 (comment) **Evidence, refreshed when this PR was opened (head abb27e6).** Main had then moved to 28bdc84: #345 landed as 3ae0109, #350 as 0f67f7b, and lane 3's `single-repository.yaml` as 28bdc84. The review's re-verify of abb27e6 ran on the head merged with 28bdc84: clean, with the six init, encoding, CLI-reference and family files at 305 passed / 9 skipped, and `test_family.py` with #345's `test_family_dry_run.py` at 126 passed. Merge-tree was also clean against #259's 3894aa8 (PR #395) and #314's 4b8da3d. The results below were taken before that; main has moved since. - Reproduction, before and after, with the issue's script: on c7c02fa every row exits 0; on this branch all 13 failing rows now exit 2 on `--dry-run` and on the real run, with no holder, no remote directory and nothing pushed; the two controls (`--tracking-branch main`, `--name ContosoGroup`) are unchanged. `--tracking-branch 2026` is refused too, as #314 does for the scaffold: it worked only because every reader calls `str()` on the number. - `tests/test_family_init_values.py`: 125 tests, 103 of them fail on c7c02fa; on this branch 117 pass and 8 skip (the 8 rows that need the real template to be quoted, which they run against the day it is). On 14e2d68, the first head, the 4 tests of this round that guard a code change are the ones that fail (the three validator cases and the exit at the command line); the rest guard what 14e2d68 already did. - Mutations: fifteen of the change in the first round (drop the call, move it after the `--dry-run` return, move it after the remote is created, empty either question, ignore the template's quoting, ignore the template's validator, drop the not-given naming, drop a finding no flag owns, write bytecode into the template, let a validator's `SystemExit` through at load, ask the validator first, drop the unusable-validator checks), and eight more in this round, each killed by a new test: `_line_shown` always in backticks (9 fail), the `sys.path` restore dropped (2), the bytecode flag's restore dropped (2), the comment skip dropped (1), `findings` read outside the `try` (1), the non-text check dropped (1), `SystemExit` let through the run (2, one a zero at the command line), and `main` without `escape_what_stdout_cannot_encode` (the `family-init-dry-run` row of `test_stdout_encoding.py`). The write-bytecode mutation is now killed by the new bytecode test. - Targeted runs on the head of this branch, abb27e6 (Linux, Python 3.12.3, git 2.43.0, git-filter-repo 2.47.0): `test_family_init_values.py test_family.py test_stdout_encoding.py test_cli_reference.py`: 276 passed, 9 skipped (the 8 rows that need a quoted template, and one Windows-only row). - Merged with origin/main at 161c6c9 (which has #316, #317, #318, #245 and #349): `test_family_init_values.py test_family.py test_family_add_refusals.py test_family_add_excludes.py test_family_member_path.py test_stdout_encoding.py test_cli_reference.py` 407 passed, 9 skipped; `test_shape_doctor.py` 255 passed, 3 skipped (the PowerShell row, which needs a Windows host); `test_update_shape_add.py test_update_shape_pr_base.py test_template_values_single_pass.py test_scaffold_ignored_shape_files.py` 223 passed (with the git identity blanked, because the standard's own checkout sets a `user.name` that scaffold and adopt tests read). Main then moved to 0f67f7b (#350 landed, which changes the validator this reads, and a scaffold PR): `test_family_init_values.py test_stdout_encoding.py test_cli_reference.py test_validate_family_duplicates.py` on this branch merged with it: 197 passed, 9 skipped. - Merge-tree (`git merge-tree --write-tree`), all clean: origin/main at 0f67f7b; #345 (abaaf3c, which landed as 3ae0109); #314 (4b8da3d); and `fix/template-quoted-placeholders` (3894aa8, #259). This branch + origin/main + #345 + #259's branch together are clean too, and `test_family_init_values.py test_family.py test_stdout_encoding.py test_cli_reference.py test_family_dry_run.py test_validate_family_duplicates.py test_template_text_placeholders.py` there: 442 passed, 20 skipped (the 19 rows above, which skip because #259 quotes the template, and one Windows-only row). #345 changes `cmd_bump`, `cmd_remove` and `build_parser`, not `cmd_init`; #350 (landed) added member-name checks to the validator and left `_check_envelope` and `Report`, the two names this change asks, as they are. #314 at 4b8da3d did not merge with origin/main by itself then (`adopt-project.py`, from what landed after its last merge), so it was not composed with the rest, but it was clean against this branch alone. #309, #312, #316, #317, #318 and #350, which the first round merged against or named, have landed. - `--created-by '{{ORG}}'`: on c7c02fa `init` created the remote and then died with `template-unsubstituted` once the worktree was rendered. Merged with main, #316's one-pass `render` writes the value as typed: `init` exits 0, `family.yaml` records `created_by: "{{ORG}}"` and the holder's own `validate-family.py` is green. This change does not touch it and does not refuse it. (On this branch alone, without #316, the pre-flight render refuses it before anything exists, with the same `template-unsubstituted` text.) **Left out.** - `AGENTS.md`, `README.md`, `docs/handbook.html`: no prose change (#309 has landed since; `init --help` carries the sentence). - A calendar check of `--created-on` (`2026-13-45` passes the validator's `DATE_RE`): the validator's rule, not copied here. - A branch git itself refuses (`a..b`, `x.lock`): #283. - `--org test+org` passes `validate-family.py`; not claimed. - `--created-by` with an ESC or OSC is printed raw by the plan's `created by` line on c7c02fa and here: not part of this change. - No run against GitHub (the `gh repo create` ordering is proved in-process, with every step that would reach GitHub replaced by one that fails the test), none on macOS or Windows. Reviewed adversarially at 14e2d68 (CHANGES NEEDED: 1 blocker, 1 should-fix, 6 nits). The blocker was a test elsewhere in the suite that the new refusal turned red, `tests/test_stdout_encoding.py`'s `family-init-dry-run` row; the should-fix was the line-escaping hunk (`_line_shown`), which no test guarded. Fix round 1 (abb27e6) changed that test and not the refusal, guarded `_line_shown` and the two other surviving mutants, closed the three ways a broken validator escaped `family-validator-unusable`, and rewrote the bytecode test over a copy of the template. Its re-verification at abb27e6 found every finding closed: APPROVE WITH NITS (0 blockers, 0 should-fixes, 2 nits, both about this description and the commit trailer and not about the code: the trailer names Opus 5.5 below, and the description's merge-tree and composition paragraphs are refreshed above). CI is green on Linux, macOS and Windows, with SonarCloud passing, at d216ba8 (main 8782333 merged in). Closes #307 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 13, 2026
… exists, a source symlink with an empty target and the source's own `project.yaml` kept in the root, and `plan` no longer offers `root` for that file (#303, #305) (#387) Two ways `check` said `plan ok` to a source `execute` could not adopt, both reproduced on c7c02fa with the issues' own scripts. **#303, a symlink whose target is empty.** A `120000` entry whose blob is the empty blob passed `check`, which never reads a link's target. - With symlinks on (git's default where the disk can hold one), `execute` created both leg repositories, then failed in its first clone of the source: exit 2, nothing on stderr, two empty legs left behind. Linux refuses a symlink to nothing. This happened whatever the plan answered: `docs/empty-link` sent to spec, and a top-level `empty-link` answered `root` or `drop`. - With `core.symlinks=false`, it said `adoption verified`. The spec leg held the empty link, a clone of that leg exited 128, and the assembly's recursive clone exited 1. The top-level link answered `root` or `code` verified the same way. Only `drop` gave an assembly that clones. **#305, the source's own `project.yaml` answered `root`.** `plan` asked about it under `ambiguous-top-level-yaml`, whose question offers `root`. Answered `root`, `check` said `plan ok`. `execute` wrote the shape's manifest beside the source's file as `shape/project.yaml`, said "nothing was overwritten" and `adoption verified`. `shape-doctor.py` then said NOT A SHAPE ROOT (exit 2) and pointed at `adopt-project.py plan`. `make bootstrap` failed `naming (exit 2), manifest (exit 1), pins (exit 2)`. Answered `code` or `spec`, the doctor said COMPLIANT and `bootstrap ok`. **What changed.** - **`plan-empty-symlink` (#303).** `_empty_symlink_findings` reports each `120000` row of size 0 in `Source.tree()`. The size `ls-tree -l` gives a symlink is the length of its target, so no target is read; a SHA-256 repository reports size 0 too. - It is listed right after #196's `plan-unsafe-path`, through `_source_path_findings`, on the line `_entry_findings` already had. So `check` prints it, and `execute` refuses it through `_refuse_what_check_finds` before any leg exists (#168). - `EMPTY_SYMLINK_REMEDIATION` is said once after the findings, in `check` and in `execute`'s refusal. The fix is in the source: give the link a target, or remove it, commit, and re-run `plan`. - `plan` notes each such link in `check`'s words, and still writes the plan. - **`ambiguous-project-yaml` (#305).** A new rule in `contracts/path-classification.yaml`, above `ambiguous-top-level-yaml`, for the file `project.yaml`. Its question asks spec, code or drop, and says `root` is not an answer for this file. `release.yaml` keeps the old question. - **`plan-root-blocks-manifest` (#305).** #313's machinery is extended, not copied: `_shape_blocks`, which `_root_shape_findings` runs over each disk, now refuses the root holding `project.yaml` outright, with no copy question after it. It fires for a kept row at that path (file or symlink), or a kept directory there, on a disk that keeps case or one that ignores it. The finding's remediation is #313's `_fix_the_kept_path`. - A submodule there stays `plan-submodule-root`'s, said once. - `_kept_there` is the at-or-under lookup the copy branch already did, factored out for both. - `execute` refuses it before any leg exists, through the same list. - **Files.** `adopt-project.py` (+205/-13), `contracts/path-classification.yaml` (+28), the two new test files (399 and 366 lines) and `tests/test_adopt_root_shape_paths.py` (+6/-2). **Decisions.** - **#303 is refused whatever the plan answers, and whatever `core.symlinks` says.** - Where symlinks are made, no answer adopts. The run fails at the first clone of the source, which checks the whole tree out before anything is taken out. Measured with the link in `docs/` (sent to spec), at the top level answered `root` and `drop`, and with every path rooted or dropped and both legs seeded: there the seeded legs were pushed and the assembly's clone exited 128. - `check` cannot know where `execute` will run. A check that passed `drop` with symlinks off would pass a plan the next Linux machine cannot adopt, and the root's own `validate` workflow runs on `ubuntu-latest`. - The remediation costs one commit in the source. - **#305 covers the manifest only.** The issue asked whether other rendered root paths need the same rule. Measured on c7c02fa, with the source's own file kept in the root at each of the 18 `assembly_root_paths()`: - `check` passes all 18. - These verify over a broken assembly: `contracts/spec-pin.yaml` and `contracts/code-pin.yaml` (DRIFTED, pins fail); `scripts/validate-pins.py`, `scripts/validate-manifest.py`, `scripts/repo_shape.py` and `contracts/repository-naming.yaml` (INVALID); `scripts/validate-repository-naming.py` (COMPLIANT, naming fails); `scripts/bootstrap.py` (COMPLIANT, and `make bootstrap` runs the source's own script). - `contracts/shape-pin.yaml` fails verification after both legs exist. - README.md, AGENTS.md, CLAUDE.md, .gitignore, .gitattributes, AGENTS-shape.md and the workflow come out COMPLIANT with the copy beside them. `Makefile` is rooted by default, and the follow-up merge is its designed exit. - `project.yaml` is the one path `plan`'s own question offered `root` for. The others are reached only by hand-editing a `contracts/` or `scripts/` answer to `root`. They are left for a follow-up issue (under Left out), not folded in here. - **A symlink at `project.yaml` is refused too.** On c7c02fa: - a link to the source's own file verified and was NOT A SHAPE ROOT; - a link to nothing had the manifest written through it, and came out COMPLIANT on Linux. A clone that makes no symlinks holds that link as a file naming its target, so it is refused anyway, and #183 refuses a link to nothing at a shape path regardless; - a directory `project.yaml/` verified and was NOT A SHAPE ROOT. - **A case twin `Project.yaml` kept in the root is refused, with the case aside.** It adopts COMPLIANT on Linux. On a disk that ignores case, `place` writes the manifest beside it. This is the trade #172 and #313 already make. **Evidence.** In this section and in the two below it, "this head" is 69c54c9, the branch's own commit and the one the review approved; main and #314 were merged into the branch afterwards (last paragraphs). - **Reproductions, on this head** (exits as printed): - #303 run 1: `check exit 1`, one `plan-empty-symlink` finding; `execute exit 2`, `REFUSED plan-empty-symlink`, `leg repositories left:` empty. - #303 run 2 (`core.symlinks=false`): the same finding and refusal; no leg, no verification. - Top-level `empty-link`, symlinks off and answered root, code or drop, and symlinks on and answered root or drop: each `check exit 1`, `execute exit 2`, no leg. - #305 root: `check exit 1`, one `plan-root-blocks-manifest` finding; `execute exit 2`, `leg repositories:` empty. - #305 code, spec and drop: `plan ok`, `adoption verified`, the root's `project.yaml` begins `schema_version: 1`, doctor COMPLIANT, `bootstrap ok`. - The plan's entry for `project.yaml` now reads `rule: ambiguous-project-yaml`, with no `(root)` in its question. - `project.yaml` as a link to a file, a link to nothing, a directory, and the twin `Project.yaml`: each `check exit 1` and `REFUSED plan-root-blocks-manifest`. - No row needs real GitHub or a cloud tool. - **New tests.** `tests/test_adopt_empty_symlinks.py` (28) and `tests/test_adopt_root_manifest.py` (27). - The symlinks are made with `git update-index --cacheinfo`, so no disk symlink is needed. The one test that checks a link out (a link given a target, end to end) is skipped on Windows by its marker, before it runs. - On c7c02fa: 37 failed, 18 passed. The 18 are the controls that must pass on both sides: a symlink with a target, the remediations adopting, `project.yaml` sent to a leg or dropped, a submodule there, other shape paths keeping their copy, and `release.yaml`'s question. - **One existing test changed.** `test_a_file_kept_at_a_path_the_shape_writes_is_not_a_finding` in `tests/test_adopt_root_shape_paths.py` held that a kept file at any shape path is no finding. It now holds that for every path but the manifest, which is `plan-root-blocks-manifest`. - **Mutation probes:** 14 of 14 hunks reverted one at a time, each killed by at least one new test. - **Targeted runs.** The adopt list and the doctor ran on a local state of this branch (7a22a21) whose files were this head's but for `tests/test_adopt_root_shape_paths.py`. That file was re-run on this head, so the adopt list is two runs, not one sum. - The 15 adopt files (`test_adopt_empty_symlinks`, `_root_manifest`, `_root_shape_paths`, `_leg_path_collisions`, `_execute_refusals`, `_plan`, `_e2e`, `_submodules`, `_verify`, `_mount_ignored`, `_ignored_shape_files`, `_entry_paths`, `_finding_spelling`, `_history_gitlinks`, `_spec_only`) on 7a22a21: 837 passed, 1 failed, 1 skipped (a MedxEHR clone the plan test needs). The one failure is the #313 test this head changes. - `tests/test_adopt_root_shape_paths.py` alone, on this head: 120 passed. - `tests/test_shape_doctor.py`, as its own stream: 242 passed, 3 skipped (PowerShell lines only a Windows host prints). - `tests/test_scaffold_leg_path_collisions.py`, `tests/test_repo_hygiene.py` and `tests/test_cli_reference.py`: 167 passed. - This head merged with origin/main at 87373a6, which brings `tests/conftest.py`'s new `git()` identity: the two new files, `tests/test_adopt_root_shape_paths.py` and the three above, 342 passed. **The review.** An independent opus review at 69c54c9 approved with nits: 0 blocker, 0 should, 8 nit. It reproduced every row of both issues on c7c02fa and on this head, and killed 6 of 6 mutants: two of the author's, re-run, and four of its own. Its targeted runs on this head merged with origin/main ab623dd (`git merge-tree` clean, with #349's session `gh` guard in place): - the new files, `tests/test_adopt_root_shape_paths.py`, `tests/test_scaffold_leg_path_collisions.py`, `tests/test_adopt_execute_refusals.py`, `tests/test_repo_hygiene.py` and `tests/test_cli_reference.py`: 379 passed; - the other adopt files, in two streams: 303 passed, 1 skipped (the MedxEHR clone), and 323 passed; - the clone-template, literal-names, template-values, plan-escapes, stdout-encoding and Windows-path tests, which cover #268 and #270: 266 passed, 2 skipped (an operator `user.name` set; Windows only); - `tests/test_shape_doctor.py`: 254 passed, 3 skipped (PowerShell lines only a Windows host prints). c7c02fa's own `tests/test_adopt_root_shape_paths.py` against this head's code gave 1 failed (the `project.yaml` row, `:191`) and 119 passed, so no other #313 behaviour changed. - **A cross-lane composition, with lane 1's #183.** On the tree merged with lane 1's `fix/adopt-root-kept-symlinks` (#183, 6c44591), `tests/test_adopt_root_symlinks.py::test_a_link_with_an_empty_target_is_fixed_in_the_source` fails: `AssertionError: assert 'plan-empty-symlink' == 'plan-root-symlink'`. An empty link at or above a shape path is reported by both findings, and `execute` refuses with the first, which is this branch's. The two remediations agree (fix it in the source). Whichever of the two lands second removes the duplicate report. The same file also fails 3 other tests on origin/main with #183 and without this branch (`[a-link-at-the-copy-itself]` and `test_the_materializer_refuses_before_it_writes`, two cases), because #313's `plan-root-blocks-shape-copy` and `materialize-copy-blocked` answer first; those are not this branch's. - **Two known nits, left for a later round.** - An empty link AT `project.yaml` (or at a #313 copy path) gets `plan-root-blocks-manifest`, whose fix offers `drop`, and `plan-empty-symlink`, which says `drop` is no fix. - `plan` notes every empty link (25 links print 25 NOTEs), where `check` names 20 and counts the rest. - The other nits are smaller. A symlink target that begins with a NUL byte passes `check` and fails like #303, and the issue's `_verify` bullet, are follow-up issues; the case twin `Project.yaml` is under Left out. The new tests' `SYMLINKS_OFF` replaces the Windows job's own `GIT_CONFIG_COUNT` settings (`core.autocrlf=false`, `core.longpaths=true`) for those child processes, which is harmless while none of them checks anything out. **Merge-tree, as of 2026-10-12T15:14Z.** `git merge-tree --write-tree --name-only` of this head (69c54c9), against heads fetched then; every sha below is the one it had then, and main has moved since (see the merge paragraph below). It is a textual merge only; the semantic meeting with #183 was tested by the review (the first item above). - origin/main: clean at 0f67f7b. That includes #316 (34ecbc2), #268 (e43fa95), #270 (f795cf7) and #245 (161c6c9), which have landed since this branch was cut. It was clean at 161c6c9 and at ab623dd, the review's merge, too. - Open PRs touching `adopt-project.py`: #184 (f416e13), #353 (dcbc37b), #356 (5a5011d), #371 (34b9500), #373 (5ac03de), #377 (c35c896), #378 (2da8691), #382 (7e55d00) and #384 (a96297e) are all clean. #356 and #382 conflict with origin/main alone (2 and 5 blocks, from #245's landing), not with this head. - #314 (4b8da3d) was NOT clean: one block, in `_entry_findings`' docstring, where each side adds a paragraph after the same one (this head's "THEN THE SOURCE'S EMPTY SYMLINKS (#303)" and #314's "A VALUE NO FILE CAN HOLD"). Keep both. It had been clean at f45edf4, and it had 3 blocks against origin/main alone since #245 landed, so it needed a merge of main in any case. - Lane 1's branches without a PR: `fix/adopt-late-failure-notice` (9f1778d), `fix/adopt-record-submodule-placement` (70f4d29) and `fix/adopt-plan-quote-yaml-keywords` (d38e143) were clean. `fix/adopt-plan-quote-yaml-keywords` was 3 blocks against this head at e337f44, and conflicts with origin/main alone in 4. - `fix/adopt-root-kept-symlinks` (6c44591, #183) conflicts in 3 blocks of `adopt-project.py`: the module docstring's `check` paragraph, `_entry_findings`' docstring and its return line. They are the same 3 that c7c02fa itself has with that branch, and this branch's one added docstring line sits inside the first. It has 5 blocks against origin/main alone. - Lane 5's `fix/adopt-pin-writes-pin-file` (d806e8f): clean. `docs/adopt-execute-exits` is #371, above. - When #183 lands, an empty symlink kept at or above a shape path is reported twice, by `plan-empty-symlink` and by #183's `plan-root-symlink` (the first review item above). A dangling link kept at `project.yaml` is reported twice too, by `plan-root-blocks-manifest` and `plan-root-symlink`, and an empty one three times. Whichever lands second should keep one. **Brought onto main past #314 (bbbb541).** Since this branch was cut from c7c02fa, main took #245 (relative `.gitmodules` urls), #270 (`plan-unwritable-value`), #371 (`execute` verifies before it exits on a refused `gh pr create`), #377 (`plan` asks about every path `execute` will not extract) and #314 (`plan-manifest-invalid`, 3ef059c), all in `adopt-project.py`. The merge of origin/main 3ef059c has ONE conflict, in `_entry_findings`' docstring, where this branch adds "THEN THE SOURCE'S EMPTY SYMLINKS (#303)" and #314 adds "A VALUE NO FILE CAN HOLD IS SAID BEFORE THE VALIDATOR'S WORD ON IT (#255 before #261)" after the same paragraph: both are kept, this branch's first, because it describes the head of the list and #314's its tail. Nothing in code conflicted. `check` and `execute` read one list, and it composes as each feature says: a source's unsafe paths (#196), its empty symlinks (#303), coverage, `_leg_findings` with `plan-root-blocks-manifest` (#305), #166's split, #270's `plan-unwritable-value`, #314's `plan-manifest-invalid`, then #245's relative urls last. So an empty symlink beside an invalid value is still refused wearing `plan-empty-symlink`, with no leg created; probed on the merged tree with an empty link plus `visibility: secret`, plus a line break in `visibility`, and with `project.yaml` answered `root` plus `visibility: secret`, each listing the findings in that order. No test expectation or order changed: the merged tree was rehearsed before #314 landed (69c54c9 + 45e01f4 + c6f441b) and is the same tree object as the pushed merge, and `git diff 3ef059c bbbb541` is this PR's five files and nothing else (+1004/-15). Targeted runs on bbbb541: the new files, `test_adopt_root_shape_paths`, `test_adopt_manifest_values`, `test_scaffold_leg_path_collisions`, `test_agents_md_codes` and `test_cli_reference` 418 passed; `test_shape_doctor` and #377's, #245's and #270's adopt tests 595 passed, 4 skipped (PowerShell lines only a Windows host prints; one operator `user.name`); the other adopt files 234 passed, 1 skipped (the MedxEHR clone) and the guards, literal-names, clone-template, plan-escapes, gitmodules, leg-paths, stdout and Windows-path tests 1101 passed, 1 skipped. **Left out.** - `_verify` and the materializer do not themselves refuse a root `project.yaml` that is not the shape's manifest. This is the issue's second bullet. Once `execute` refuses the plan before any leg, through the same list `check` prints, nothing reaches them with one. It is a follow-up issue. - A top-level file named `project.yaml` only once case is ignored (`Project.yaml`) is still asked `ambiguous-top-level-yaml`'s question, which offers `root`. `check` refuses that answer with the case aside, before any leg. - #171 (a failed clone exits 2 silently) is lane 1's PR #184. This PR only keeps the empty-symlink run from reaching that clone. - Follow-up issues proposed by this PR (two since filed): - the source's own file kept in the root at one of the other shape paths the root's tools read (the pins, the validators, `scripts/repo_shape.py`, the naming policy, `scripts/bootstrap.py`, `contracts/shape-pin.yaml`), each measured above; - a symlink whose target begins with a NUL byte (`\0spec.md`, size 8), which `plan-empty-symlink`'s size test does not see: it fails like #303 (filed as #388); - `_verify` asking whether the split's root `project.yaml` is the shape's manifest (filed as #389); - an entry git cannot check out where `execute` runs, other than an empty symlink or a NUL-led one (a target longer than the platform allows, say), which is still found only by the first clone, after both legs exist. #303 leaves this as a note. Reviewed adversarially at 69c54c9 (APPROVE WITH NITS: 0 blockers, 0 should-fixes, 8 nits). The review reproduced every row of both issues on c7c02fa and on that head, and killed 6 of 6 mutants. No fix round followed: the branch's own commit is as reviewed, and what was added after it is the merge described above. CI is green on Linux, macOS and Windows, with SonarCloud passing, at fce86fa (main 7a8b981 merged in). Its merge with main a73b4e9 passed the targeted set (the three adopt test files, `tests/test_shape_doctor.py` and `tests/test_scaffold_leg_path_collisions.py`): 597 passed, 3 skipped. Closes #303 Closes #305 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 13, 2026
…d `open_glass` share one `SHAPE_PIN_SOURCE_<PRODUCT>`, so `--pin` refuses the pair and `validate-pins.py` notes a declared one and reads the variable for neither (#275) (#409) `pin_source_env_name` turns every character outside `[A-Z0-9_]` into `_`, so `open-glass`, `open_glass` and `open.glass` are three legal product names, with three pin files (`contracts/<product lowercased>-pin.yaml`), and one variable, `SHAPE_PIN_SOURCE_OPEN_GLASS`. On ab623dd (this branch's main parent; the same rows reproduce on c7c02fa), with the issue's own probe against local bare remotes and a `gh` that exits 97: | step | ab623dd | this head (6494714) | |---|---|---| | `scaffold-project.py --pin open-glass@<A> --pin open_glass@<B>` | exit 0, three repositories | exit 2 `scaffold-pin-collides`, nothing created | | `validate-pins.py`, no source, on a project that already declares the pair | exit 0, both skips offer `SHAPE_PIN_SOURCE_OPEN_GLASS` | exit 0, one `note neutral-pin-source-collides`; the skips offer `--pin-source <product>=<path>` and a sibling checkout only | | `SHAPE_PIN_SOURCE_OPEN_GLASS=<open-glass>` | `neutral-pin-source-unreadable` for `open_glass`, exit 1 | exit 0, the note; the variable is read for neither pin, so each pin is read from its own source or skipped | | `SHAPE_PIN_SOURCE_OPEN_GLASS=<open_glass>` | `neutral-pin-source-unreadable` for `open-glass`, exit 1 | the same | | `--pin-source open-glass=... --pin-source open_glass=...` | exit 0 | both pins recompute, exit 0, the note | **What changed.** Every variable name is kept: an operator may have set one and the docs name them. What is done instead: - `scripts/repo_shape.py`: `pin_source_env_collisions(products)` is the one pure answer, `{variable: [products]}` for each variable two or more distinct products share. Distinct means distinct PIN FILES (`pin_product_key`, `lower()`, which is what `contracts/<product lowercased>-pin.yaml` is named by), so a product typed twice or in two cases is #241's file collision and never this one. `pin_source_env_shared` is the set of those keys. - `scaffold-project.py`: `_refuse_colliding_pins` (left where it is) names each variable's `--pin` arguments as typed after #241's file sentences, in the same `scaffold-pin-collides` refusal. Dry run and real run say the same no, before any process starts and ahead of the leg-path refusal. #241's wording and remediation are unchanged when only a file collides. A NEW pair is still refused, so the situation is not created anew. - `templates/assembly-root/scripts/validate-pins.py`: one NOTE per variable, `neutral-pin-source-collides`, naming every product and the variable, the keyed `--pin-source <product>=<path>` that reads each pin, and that which declaration stays is the human's decision. It is never a finding and never moves the exit code. A product whose variable is shared does not read it (rule 2 of `resolve_neutral_pin_source`), and its skip hint offers the keyed `--pin-source <product>=<path>` and not the variable or the bare form. A variable one product has to itself reads and is offered exactly as before. - `scripts/repo_shape.py`, the two chain readers: `link_pins_from_trees` does not read a variable two links of one chain share, and the `declared-unverified` hint stops naming it. Only a hand-written chain can have two such links, since scaffold takes every link but the first from the naming policy's alphanumeric `open<Product>` form. Each reader of `pin_source_env_name`, decided: | site | decision | |---|---| | `scaffold-project.py --pin` | refuse (above) | | `validate-pins.py` rule 2 and the skip hint | a note, and the variable is neither read nor offered for a shared product | | `repo_shape.py` `resolve_link_source` (via `link_pins_from_trees`) | skip the variable when another link of the chain shares it | | `repo_shape.py` `_chain_link_findings` hint | omit the variable for a shared link | | `adopt-project.py --pin`, `plan`, `check`, `execute` | not in this PR, see below | **Fix round 1 (from the review of 34dbd86).** **The design change: a declared pair is a NOTE and not a finding. This is the lane coordinator's decision, and Brett may reverse it.** The first round made `neutral-pin-source-collides` a FINDING. Main through ab623dd scaffolds such a project (`--pin open-glass@A --pin open_glass@B` exits 0), and `update-shape.py apply` runs the NEW `validate-pins.py` over the rewritten root, so for a project already declaring the pair the finding ended every apply as `REFUSED update-validators-red`, rolled back, for this update and every later one, until a person deleted a true declaration of descent. AGENTS.md tells an agent never to edit "a pinned file, a digest or a manifest to make a row go green", and the finding's remediation was exactly that, so the person was locked out. The coordinator chose the review's option (b): it is a NOTE now. The guard stays (the variable is read for neither pin), each pin still verifies from its own source, scaffold still refuses a new pair, and `validate-pins.py` says the rest in one line. To put the finding back, restore 34dbd86's `report.finding(...)` call in `_report_pin_source_collisions` (it is not a word swap for `report.advise`: `advise` takes one string, `finding` a code and a detail), give the finding the remediation 34dbd86 had, and turn the tests that expect the note (and `test_a_project_declaring_a_colliding_pair_takes_a_shape_update`) back into exit 1; the first round's versions of all of them are on 34dbd86. - **Proved on a real old project.** Main (ab623dd) scaffolded the pair into local bare remotes; `update-shape.py apply` to the first round's head 34dbd86 exited 2 `REFUSED update-validators-red` and rolled back (`git branch` shows only `main`); to this head it exits 0, the project's validator passes and prints the note, and the doctor's `pins` row is `ok` before and after. The doctor prints `passes` for a validator that exits 0 and does not echo its stdout, so the note itself is visible in `update-shape.py apply`'s output and in `validate-pins.py`'s, and the verdict moves for no reason but the update (`COMPLIANT, SHAPE BEHIND` to `COMPLIANT`). It is now a test: `test_a_project_declaring_a_colliding_pair_takes_a_shape_update`, over local stand-ins beside the project, so no `gh` is asked. - **Collisions by the variable.** Pairs were decided by `casefold()`, but the pin file is `lower()`. `openß`/`openss`, `openſ`/`opens`, `openfi`/`openfi` and `openς`/`openσ` (the letters are U+00DF, U+017F, U+FB01 and U+03C2 beside U+03C3) share a variable and have two pin files, and were never found. They are now, from the pin-file fold (`pin_product_key`). A pool of 31 names is checked pairwise against `neutral_pin_path` and `pin_source_env_name` themselves, and no single product is ever a collision. They are reachable only from a hand-written `project.yaml`. - **Windows.** The guard's only test needed the `#!` `gh` stub and was skipped on Windows. `test_a_shared_variable_answers_for_neither_pin_with_no_gh` puts a checkout of each pin beside the project, names one in the variable, and needs no `gh`: on ab623dd it is `neutral-pin-source-unreadable`, here each pin reads its own sibling. The hint and the non-ASCII rows are tested in-process, with no skip and no filesystem. - **The hint.** For a product two pins share, the skip hint offers the keyed `--pin-source <product>=<path>`: the bare form answers for every pin with no more specific source, so with two pins it gives the other the finding this issue is about. - **Trailers and title.** The trailers of the earlier commits (a30091f and 34dbd86 name Sonnet 5.5) are settled in this message, which carries Opus 5.5. The PR title now names `repo_shape.py`. - `scaffold-project.py` is not touched this round: `fix/adopt-pin-writes-pin-file` moves that code, and the change would only conflict with it. **Left out.** - `adopt-project.py` is untouched. Its half arrives with #273's PR, which moves scaffold's pin machinery into a shared module and routes adopt through `pin_source_env_collisions`. Once it lands, the one place that still keys by `casefold()` (`pin_variable_collisions` in `scripts/shape_materialize.py`) should read `pin_product_key`; it cannot differ for an ASCII `--pin`, which is all `PIN_ARG_RE` takes. - `scaffold-project.py`'s own `_colliding_pin_variables` also keys its argument lists by `casefold()` and one docstring there still says the project's validator reports the pair "as a finding"; both go with the code when #273 moves it. #312's `PinSources` keys `--pin-source` by `casefold()` too, so `openß=` and `openss=` are one key there; that is #312's. - A product in `neutral_product_pins:` and a chain link in `naming.referent_chain` that share a variable are read by two validators in one process, and neither sees both sets. Not detected here. - The doctor does not print a passing validator's stdout, so its `pins` row does not carry the note. Not changed: it would add a file to this PR. - No `--help` text changed, so `docs/cli.md` is untouched. `README.md` is at its line cap and its blob is pinned by `docs/handbook.html`, so the one README sentence this wants needs a regenerated handbook and waits for its own change. - `validate-pins.py` and `repo_shape.py` ship into projects, so every project's `shape-pin.yaml` reads `upstream-changed` for both until `update-shape.py apply`, which now succeeds for a project that declares such a pair. **Evidence.** All on the head (6494714), with git-filter-repo 2.47.0 on PATH, a `gh` that cannot be reached (a stub first on PATH, `GH_TOKEN` bogus, an empty `GH_CONFIG_DIR`), and the repository-local `user.name` blanked at command-line scope, as the standard's checkout sets one that two scaffold tests read. - `tests/test_pin_source_env_collisions.py` is 137 tests and passes on this head. Laid over 34dbd86, 50 fail and 87 pass: the note and update tests, the non-ASCII rows, the hint, the sibling guard, and `pin_product_key` itself (which 34dbd86 does not have); the scaffold refusals, the chain readers and the controls hold on both. - Targeted, run on ad6f569, whose tree differs from this head in `tests/test_pin_source_env_collisions.py` only (the last edits made one comparison read the committed blob and wrapped one line): the new file, `test_pin_validator`, `test_neutral_pin_validator`, `test_cli_reference`, `test_manifest_validator`, `test_naming_policy` and `test_repo_hygiene` **445 passed**; the scaffold pin, collision, referent-chain, leg-path and e2e files **245 passed**; `tests/test_update_shape*.py` **218 passed, 3 skipped** (junctions, NTFS only); `tests/test_shape_doctor.py` **254 passed, 3 skipped** (the PowerShell line, a Windows host only). - On this head itself: the new file, `test_repo_hygiene`, `test_cli_reference`, `test_pin_validator` and `test_neutral_pin_validator` **244 passed**. - This head merged with origin/main a983e6a, in a throwaway worktree: the new file, `test_pin_validator`, `test_neutral_pin_validator`, the scaffold pin, collision, referent-chain and e2e files, the whole doctor file and `test_cli_reference` **590 passed, 3 skipped** (the PowerShell line); the new file alone, in its final form, **137 passed**. origin/main had moved twice since; the merge-tree below is against it. - Twelve mutation probes, each hunk reverted alone: all caught. The note-to-finding one is caught by the update test (a committed mutant, since the test clones the repository's HEAD) and the guards by the sibling test, run with every test that needs a `#!` stub deselected as a Windows runner would; the hint one by an in-process test. - Nothing ran on macOS or Windows before this PR's CI. **Merge-tree, as computed during the fix round on 2026-10-12** (every sha and head is the one it had then; main and the other branches have moved since). `git merge-tree --write-tree` of this head (6494714) against origin/main 1f90a0f is clean, and so is the merge of origin/main a983e6a, which the merged-tree runs above used. #312, #309, #270, #268, #228, #245 and #351 have landed. The rest were computed on ad6f569 (the same tree but for one test file) against a983e6a. Open PRs touching this branch's files, all clean against the head: #314, #355 and #373 (`scaffold-project.py`), #343 (`validate-pins.py`), #390 (`repo_shape.py`) and #394 (`repo_shape.py`, which conflicts with origin/main by itself). The other 25 open PRs and the lane-5 branches `fix/adopt-check-link-and-manifest`, `fix/scaffold-free-text-values`, `fix/family-init-values`, `docs/adopt-execute-exits`, `fix/yaml-flow-brackets-surrogates` and `fix/adopt-pin-writes-pin-file` (#273's, 7a94b6b, also clean for this head) are clean too. #356, #382 and `fix/adopt-record-submodule-placement` conflict with origin/main and not with this head; #358, `fix/adopt-late-failure-notice` and `fix/adopt-plan-quote-yaml-keywords` conflict with both, with origin/main by themselves, so they are not this branch's. Neither are the five PRs below #200 that origin/main has not landed (#94, #96, #98, #100 and #161), which conflict with it too. The PR set is read from the refs under `refs/pull/` that origin/main's history has no `(#n)` subject for, not from GitHub, so a closed one may be in it. **One conflict, with `fix/link-source-values` (#338, 40762ca) in `scripts/repo_shape.py`, four hunks.** Not resolved in either branch; take both sides: (1) `resolve_link_source`'s signature, `default_source=None, env_shared: bool = False, *, tried: list | None = None`; (2) its docstring, both paragraphs; (3) `link_pins_from_trees`'s docstring, both paragraphs; (4) its call, `default_source, pin_product_key(name) in shared, tried=asked)` followed by #338's `if tried is not None: tried[name] = asked`. The auto-merged `resolve_link_source` body already composes: a shared variable is never an `env` candidate and so never appears in #338's `tried`. **Known limits after the re-verification (APPROVE WITH NITS, 0 blockers, 0 should-fixes, 3 nits, at 6494714).** For the four non-ASCII pairs this change now detects (`ß`/`ss`, `ſ`/`s`, `fi`/`fi`, `ς`/`σ`), the keyed `--pin-source <product>=<path>` form does NOT separate the two pins: #312's source lookup folds both keys into one, so one pin can get a false `unreadable` finding. A sibling checkout does separate them. Only a hand-written `project.yaml` can declare such a pair. The doctor's `pins` row still prints only "passes" for a validator that passes, so it does not carry the collision note; that needs `shape-doctor.py`, and is left for a follow-up. The wording for restoring the finding is under "Fix round 1" above: it is 34dbd86's `report.finding(...)` call, not a word swap. Reviewed adversarially at 34dbd86 (CHANGES NEEDED: 0 blockers, 2 should-fixes, 5 nits). The first should-fix was the finding itself, which locked a project that already declares the pair out of every `update-shape.py apply`; the second was a description that had gone stale. Fix round 1 (6494714) made the pair a NOTE, which is the coordinator's decision and one Brett may reverse (the way back is above), keyed collisions by the pin file's own fold, gave the guard a test that runs on Windows, and made a shared product's skip hint offer the keyed `--pin-source`. Its re-verification at 6494714 found every finding handled: APPROVE WITH NITS (0 blockers, 0 should-fixes, 3 nits: the keyed form and the non-ASCII pairs, the doctor's silence about the note, and the wording for restoring the finding, each stated above). CI is green on Linux, macOS and Windows, with SonarCloud passing, at b70a07d (main a73b4e9 merged in). Its merge with main 5a25387 passed the targeted set (the six pin and scaffold test files, `tests/test_validate_pins_unreadable_workflow.py`, `tests/test_ruleset_hint_tracking_branch.py` and `tests/test_shape_doctor.py`): 850 passed, 3 skipped. Closes #275 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 13, 2026
… writes its pin file as the scaffold does, so an adopted root passes `validate-pins.py` (#273); a `--pin-source` that cannot answer is `pin-source-unreadable` (#339) (#508) **Brought up to date after #409 squash-landed** by merging origin/main (9736424, #409's squash) as f9ed46f. Its `scaffold-project.py` conflict was resolved to this branch's side once `git diff b70a07d 9736424 -- scaffold-project.py` (#409's final head against its squash) came out empty. So the file is the one tested on this branch plus main's own later changes to it, and holds nothing of #409's; details below. This branch carries #275's adopt half. It was built on `fix/pin-source-env-collisions`, PR #409 (#275's scaffold half and its fix round), merged at 6494714 as 583f647, over the earlier 5690235 (34dbd86). After 6494714, #409 only merged main, five times, up to its final head b70a07d, and none of those merges has a hand edit (`git show --remerge-diff` is empty for each). Adopt's refusal of two products that share a source variable goes through #409's function, so this branch lands after #409. It was never rebased. `git diff b70a07d 9736424` is empty for each file #409 touched: `scaffold-project.py`, `scripts/repo_shape.py`, `templates/assembly-root/scripts/validate-pins.py` and `tests/test_pin_source_env_collisions.py`. What f9ed46f resolves, four files with one hunk each: - **`scaffold-project.py`.** - The conflict hunk is main's block from `PIN_COLLISION_REMEDIATION` through `_pin_collision_remediation`, which #409's squash edits: it adds `PIN_VARIABLE_REMEDIATION`, `_colliding_pin_variables` and `_pin_collision_remediation` and changes `_refuse_colliding_pins`. This branch moved all of that to `scripts/shape_materialize.py`. The hunk takes this branch's side. (The merge commit's message says "re-adds"; this is the exact account.) - #409's auto-merged import of `PIN_SOURCE_ENV_PREFIX, pin_source_env_collisions` is dropped, because nothing in the file uses either name any more. - It is not `git checkout --ours` of the whole file. Main changed the file beyond #409 since this branch last merged it: #355's `--tracking-branch` check (6c2a371) and a73b4e9's `RULESET_HINT`. Those lines stay. - The result is the file `git merge-tree` gives for this branch with main before the squash (518010b). Its diff from the file tested here has the same patch-id as main's own 3ef059c..518010b change to it. - **`templates/assembly-root/scripts/validate-pins.py`.** Main's side, which is #409's final blob, with main's #359 workflow reader in it. This branch had no change of its own to the file beyond #409. - **`adopt-project.py`, `_commit_the_split`.** - Main: #445 wraps the function in its late-failure `try`, and #394 builds an `append` with the plan's submodule record. - This branch: its only edits there were a comment and `neutral_pins=_recorded_pins(plan)` on the materializer call. - Both now sit on main's call inside the `try`: `append=append, neutral_pins=_recorded_pins(plan)`. - **`docs/cli.md`.** It is regenerated with `scripts/render-cli-reference.py`, and `--check` is ok. It differs from main's only in the `--pin`, `--pin-owner` and `--pin-source` help of `adopt-project.py plan`. `scripts/repo_shape.py`, `templates/assembly-root/scripts/validate-pins.py` and `tests/test_pin_source_env_collisions.py` are main's blobs, which are #409's: this branch adds nothing to them. Nothing of #409 is lost. Its scaffold behaviour is `shape_materialize.refuse_colliding_pins` and `pin_variable_collisions`, keyed by #409's `pin_product_key`. **73d65c6, after the merge.** - **The change.** `_read_pins` reads `plan.get("pins")` where it read `plan.data.get("pins")`. - **Why.** #445's `tests/test_adopt_late_failure.py` hands `_commit_the_split` a `SplitPlan` as its plan, a `dict` that adds `entries`. On f9ed46f, three of its tests stopped at "committing the split" with `AttributeError: 'SplitPlan' object has no attribute 'data'` (3 failed, 83 passed in that file). - **Nothing else changes.** `Plan.get` with no default returns `plan.data.get(key)` exactly, so a loaded plan reads as before. `adopt-project.py plan --pin` took a NAME only (`--pin openGlass`) and refused a trailing `@<commit>`. `plan`, `check` and `execute` each exited 0, and `execute` printed `adoption verified`. The manifest then listed the name in `neutral_product_pins:` with no `contracts/<product>-pin.yaml` beside it, and on its first run the adopted root's own `validate-pins.py` refused it as `neutral-pin-missing`. #273's table, reproduced on c7c02fa with the issue's own commands (local bare remotes, and a stub `gh` that exits 97): | `--pin` | c7c02fa | this branch | |---|---|---| | `openGlass` | `execute` exit 0, `- openGlass`; clone's `validate-pins.py` exit 2 `neutral-pin-missing` | `plan` exit 2 `pin-malformed`, with the scaffold's remediation (`--pin openGlass@<40 hex>`); no plan is written | | `opensoft/openGlass` | exit 0, `- opensoft/openGlass`; exit 2 `neutral-pin-missing` for `contracts/opensoft/openglass-pin.yaml` | `plan` exit 2 `pin-malformed` | | `spec` | exit 0; `validate-pins.py` exit 0, reading the leg's `contracts/spec-pin.yaml` as the neutral pin | `spec@<sha>` is `adopt-pin-collides` at `plan` | | `shape` | exit 0; `contracts/shape-pin.yaml` read as the neutral pin | `shape@<sha>` is `adopt-pin-collides` at `plan` | | `openGlass@<sha>`, `opensoft/openGlass@<sha>` | `plan` exit 2 `adopt-pin-malformed` | `plan`, `check` and `execute` exit 0, and `gh` is called 0 times. `contracts/openglass-pin.yaml` is written and the manifest lists `- openGlass`. In the clone, `validate-manifest.py` exits 0, and `validate-pins.py` exits 0 with `openGlass: pin commit … digest recomputes` | Brett ruled for decision 1's first option ("go with your recommendation on #273"): adopt takes the scaffold's pin and writes the scaffold's pin file. That reverses the NAME-only `--pin` from #6, and the comment that described it is replaced with the new contract. - **One implementation.** The scaffold's pin code moved into `scripts/shape_materialize.py`, next to `neutral_pin_path` and the materializer that writes the file. What moved: - `PIN_ARG_RE`, `parse_pin`, `pin_digest_from_source` and `pin_digest_from_gh`; - `declared_pin_values`, `declared_pin_products` and `PIN_COLLISION_REMEDIATION` (in #309's wording); - from #275's branch, `PIN_VARIABLE_REMEDIATION` and the variable check; - `_refuse_colliding_pins`, split into the pure `pin_collisions` and `pin_variable_collisions`, and `refuse_colliding_pins(refusal=...)`, which raises one refusal for both. The code did not move verbatim. `_refuse_colliding_pins` became the three functions above; `pin_digest_from_source` was rewritten for #339 (below); and fix round 1 changed three of them (below): `pin_variable_collisions` keys each pin by #275's `pin_product_key`, `pin_digest_from_source` spells a path that is not printable, and `pin_digest_from_gh` refuses a missing `gh`. The scaffold's behaviour is otherwise unchanged. `scaffold-project.py` imports what it calls under its old names. Lane 5's review ran 34 scaffold cases (every pin refusal, the `gh` stub's four modes, dry and real successes) on 34dbd86 and on this branch at 7a94b6b. rc, stdout, stderr, the `gh` log and the files made were identical, apart from the shape's own commit and digest and #339's row. Its pin tests and #275's pass as they are. `tests/test_git_output_decoding.py`'s exemption for `pin_digest_from_gh` follows the function to its new file. - **`plan`.** `--pin`, `--pin-owner` and `--pin-source` mean what they mean for the scaffold. `plan` reads the digest from `--pin-source` or `gh api`, and records product, repository, commit, digest and `digest_source` in the plan, so `check` and `execute` read no network for a pin. `plan` refuses: - a name with no `@<commit>`, as `pin-malformed`. That includes an empty `--pin ''`, which adopt used to drop and now refuses, as the scaffold always has; - `spec`, `code` and `shape`, in any case, as `adopt-pin-collides` (#273 decision 4), in the scaffold's sentence and remediation (#241, #309); - one product given twice, the same way, whether it is a repeat or two names that share one file (decision 5); - two products that one `SHAPE_PIN_SOURCE_<PRODUCT>` names, such as `open-glass` and `open_glass`, the same way, through #275's check; - a leg path over a pin's file, as `adopt-leg-path-collides`; - a `--pin-source` path that `check` would refuse in `digest_source:` (fix round 1), as `adopt-unwritable-value`. It is asked only when a `--pin` records the path; - with no `gh` on PATH and no `--pin-source`, `gh-not-found` (fix round 1). - **`check` and `execute`.** These are new `check` findings, and `execute` refuses each one before any leg exists. They come from `_leg_findings`, the list `check` and `_refuse_what_check_finds` share: - `plan-pin-unrecorded`: a name alone (an old plan's `pins: [openGlass]`, or a hand edit), or an entry missing its commit or digest; - `plan-pin-malformed`; - `plan-pin-collides`, which covers two pins at one file, a pin at a file the shape writes, and two products that share one source variable; - `plan-root-holds-pin`: a source path kept in the root at the pin's file. The materializer would write the shape's pin beside it under `shape/`, and `validate-pins.py` would read the source's file instead. How each finding ends: - `plan-pin-unrecorded` and `plan-pin-malformed` end with the `plan --pin <product>@<40 hex commit>` that records the pin whole. - `plan-pin-collides` ends with re-running `plan` with each product once, or, for two products that share a source variable, with one product of each such group. - `plan-root-holds-pin` ends with the kept path's own remedy: send that entry to a leg, `drop` it, or rename it in the source and re-run `plan`. A leg moved over a pin's file in the plan is `plan-leg-path-collides`. - **Also part of #386: its `pins:` row.** A hand-edited `pins:` that is not a list (a name such as `pins: openGlass`, a number, a mapping, a quoted string) is now `plan-pin-malformed` in `check` (`` `pins:` is 'openGlass', not a list of pins ``), and `execute` refuses it before any leg exists. - Where: `_read_pins` in `adopt-project.py`; the test row is `plan-pin-malformed-not-a-list` in `tests/test_adopt_pins.py`. - Before: on main, `check` said `plan ok` for the name, the mapping and the string, and gave a traceback for the number (reproduced by lane 5's review on cb7063f). #386 records that `execute` then wrote the name as eight one-letter pins. - Not fixed here: #386's other keys, `allow_empty_legs:`, `paths:`, `legs:` and `source:`, so #386 stays open. Lane 2's #435 takes all of #386, `pins:` included, and its `pins:` rule keeps only text items where this branch writes mappings; whichever lands second reconciles them (#435 (comment), and "Where the meaning meets" below). - **`execute`.** It writes each pin through `materialize_assembly_root(neutral_pins=...)`. The manifest lists the product with the owner stripped (decision 3), so the entry and the file agree. - **Docs.** `--help` is updated and `docs/cli.md` regenerated (`render-cli-reference.py --check` ok). `pin_digest_from_source` caught only `Refusal`. A directory that is no clone is git's own failure, so it was always `pin-source-unreadable`, exit 2. A path `subprocess.run` cannot enter never reaches git: it is a `FileNotFoundError`, `NotADirectoryError` or `PermissionError` out of `cwd=`, and nothing caught it. `scaffold-project.py --pin ... --pin-source <that>` ended `--dry-run` and the real run alike in a traceback, exit 1. Since the move above, `adopt-project.py plan --pin` reads its digest through the same function and ended the same way. Both reach it through `declared_pin_values`, and `check` and `execute` read no source, so that call is the only reader. The issue's table, run on ab623dd for the scaffold and on d806e8f for `adopt-project.py plan` (adopt has no `--pin-source` on ab623dd), then on this head. `gh` is a stub that logs and exits 97, with `GH_TOKEN` bogus and an empty `GH_CONFIG_DIR`; it was called 0 times in every run: | `--pin-source` | before | this branch | |---|---|---| | a directory that is not a clone | `pin-source-unreadable`, exit 2 | unchanged | | a path that does not exist | `FileNotFoundError` traceback, exit 1 | `pin-source-unreadable`, exit 2: `<path> could not answer for opensoft/openGlass @ <40 hex>: no such directory` | | a file | `NotADirectoryError`, exit 1 | exit 2, `... it is a file, not a directory` | | under a parent this user cannot enter | `PermissionError`, exit 1 | exit 2, `... Permission denied` | | a symlink loop (not in the issue) | `RuntimeError` out of `Path.resolve()` on 3.12, exit 1 | exit 2, `... Too many levels of symbolic links` | The real run and `--dry-run` give the same stdout and stderr, and nothing exists afterwards (no remote directory, no work directory; for adopt, no plan file). What changed, in `scripts/shape_materialize.py` only: - `pin_digest_from_source` catches `OSError` as well as `Refusal` and raises the existing `pin-source-unreadable` with its remediation unchanged. The message names the path and the reason. - `stat()` judges the source inside the guard, as #312 does in `validate-pins.py`, and the sentences are that file's: a missing path, or one beneath a file, is "no such directory"; a file is "it is a file, not a directory"; any other `OSError`, from `stat()` or from git's `cwd=`, is the system's own reason (`strerror`). It is not `exists()`/`is_dir()`, which before 3.14 swallow only a missing path. A source `stat()` can read but `cwd=` cannot enter, a directory with no search bit, fails in `tree_digest` and the same guard answers it. - `declared_pin_values` resolves the path through a guarded `_resolved_pin_source`. `Path.resolve()` raises a `RuntimeError` on a symlink loop before 3.13 (an `OSError` since), before the digest was asked for. As typed, `stat()` meets the loop and says so. - A source that answers is read exactly as before: a relative path resolves against the working directory, a symlink to a clone is followed, `digest_source` reads `local-clone (<path as typed>)`, and the digest is `tree_digest`'s. `tests/test_pin_source_unreadable.py` (46 tests in the #339 commit; fix round 1 adds 17 for S1 and moves one, below) runs each case through `scaffold-project.py --dry-run`, its real run and `adopt-project.py plan`: missing, a file, beneath a file, relative and missing, a parent and a source with no search bit (skipped on Windows and as root, by a mark, so before any fixture builds a directory), a 300-character name, a symlink loop (skipped on Windows) and a directory that is no clone. Each asserts exit 2, the refusal naming the path and the reason, its remediation, no traceback and nothing created. It also holds `--dry-run` and the real run to the same stdout and stderr, checks the path is printed escaped under an ASCII stderr and as a surrogate when it is not UTF-8, and checks that a relative source and a symlink to a clone read as before. The session `gh` guard fails any test whose run called `gh`. origin/main ab623dd reached this branch through implQ's branch: 34dbd86 is a30091f merged with ab623dd. One file conflicted there, `templates/assembly-root/scripts/validate-pins.py`, in seven hunks, because #312 (8cdca5e) replaced `keyed_sources` and `default_source` with `PinSources` and #275 added `env_shared` and `shared`. #312's blob is the same at its PR head, at the squash and at ab623dd, so it landed the file implQ resolved against. The resolution keeps #312's side in all seven and adds #275's on top: | # | hunk | resolution | |---|---|---| | 1 | `resolve_neutral_pin_source` signature | #312's `sources: PinSources` and 3-tuple return, plus #275's `env_shared: bool = False` | | 2 | its docstring tail | #312's `Returns (path, how, typed)` text, plus #275's "Rule 2 is skipped when `env_shared`" paragraph | | 3 | `_recompute_neutral_pin_digest` signature | #312's `sources`, plus #275's `shared: frozenset[str]` after `report` | | 4 | the `resolve_neutral_pin_source(...)` call inside it | #312's 3-tuple unpack, plus `env_shared = product.casefold() in shared` and the extra argument | | 5 | `_check_one_neutral_product_pin` signature | #312's `sources`, plus #275's `shared` | | 6 | its call of `_recompute_neutral_pin_digest` | `sources, report, shared` | | 7 | `_check_neutral_product_pins` docstring and body | #312's `PinSources(...)` and `_note_unused_pin_sources`, plus #275's paragraph, `_report_pin_source_collisions(products, report)` and `shared = frozenset(pin_source_env_shared(products))` | This branch's own merge of 34dbd86 (5690235) had no conflict, and neither had its merge of #409's fix round, 6494714 (583f647). That fix round made `neutral-pin-source-collides` a note and keyed "one product" by `repo_shape.pin_product_key`. At 421e4e5, `templates/assembly-root/scripts/validate-pins.py`, `scripts/repo_shape.py` and `tests/test_pin_source_env_collisions.py` were byte-identical to the tree `git merge-tree` gives for 6494714 with origin/main 3ef059c (blobs 5604007, 091966b, 5220787), so this branch added nothing to those three files beyond #409. The 34dbd86 merge brought in #335, #312, #317, #270 (f795cf7), #344 and #349 with no conflict. Earlier merges of origin/main (87373a6, for #309's reworded constant, and e43fa95) are unchanged. Fix round 1 merged origin/main three more times: - **cd1fc79 (2bcfa69).** It brought #245, #351, #350, #345, #372, #371, #354, #380, #377 and the rest since ab623dd, with no conflict. - **45e01f4 (4098b61).** It brought #396's `tests/test_agents_md_codes.py`. That test finds `scaffold-pin-collides` as a string constant of `scripts/shape_materialize.py`, which `SHIPPED` lists. - **3ef059c (421e4e5), #314.** One conflict, one hunk in `adopt-project.py`. This branch's `_refuse_an_unrecordable_pin_source` and #314's section "The project must be able to hold what `execute` writes into it (#261)" were added at the same place; both are kept. Where the two meet, lane 5's earlier recipe applies: - `_flag_plan` builds each pin as the `PIN_FIELDS` mapping `plan` writes, not `list(pins)` of products. So the plan `plan` asks the validator about is the plan it writes. - #314's `test_plan_asks_what_check_asks_of_the_same_keys` plans `--pin openGlass@<commit> --pin-source <stand-in>` where it planned the name alone, which `plan` now refuses. Then f9ed46f, after #409 squash-landed, and 73d65c6 after it (top of this description). On the merged tree exactly one test failed: `tests/test_adopt_template_values.py::test_check_finds_and_execute_refuses_a_hand_edited_value[pins-surrogate]`. It plans, then hand-edits the plan to `pins: ["openX\ud800"]`, a name-only pin with a lone surrogate. Before this branch the refusal's headline was `REFUSED plan-unwritable-value`. A name-only pin in a plan is now `plan-pin-unrecorded` (#273), so `check` has a second, independent finding for that very plan, and the headline is the first finding's: ``` REFUSED plan-pin-unrecorded: 2 finding(s) ... FINDING plan-pin-unrecorded: `pins:` entry 1, 'openX\ud800', is a name with no commit and no digest ... FINDING plan-unwritable-value: a `pins:` item is 'openX\ud800', which `execute` writes into the assembly root's `project.yaml`, and it holds a lone surrogate ... ``` Nothing about #270's contract changed. The lone surrogate is still found as `plan-unwritable-value`, listed under the refusal, and `execute` still refuses before any leg exists. `check` builds `_leg_findings`, where `_pin_findings` sits, before it calls `_value_findings`, so the pin finding comes first. d806e8f changes one assertion for that row, `pins` only: the headline is `REFUSED plan-pin-unrecorded: ` where it was `PLAN_REFUSED`. The other 15 rows and the finding assertions, including the `plan-unwritable-value` line, are untouched. Lane 5's review of 7a94b6b found 3 should-fix and 5 nits. Each is answered here: - **#409's `pin_product_key` (review N1).** The merge of #409's fix round could not carry it, because this branch had moved the function that used it out of `scaffold-project.py`. `pin_variable_collisions` now keys each pin by that function, where it casefolded. `openß` and `openss` are two pin files and one `SHAPE_PIN_SOURCE_OPENSS`, and a casefold named each of the two pins twice. No `--pin` or recorded `pins:` entry `PIN_ARG_RE` admits can spell either, so the test asks the function directly. `refuse_colliding_pins`' docstring now says, in #409's words, that the root's `validate-pins.py` only notes such a pair. - **S1, a `--pin-source` path that is not printable.** `pin_digest_from_source` names the path as typed unless it is not printable (`str.isprintable`), and then as Python's `repr` spells it (`shown_source`). That covers the refusal's head and git's own reason, which names the directory it ran in. Printed raw, an ESC reached the terminal, and a line break or a carriage return began a line of the path's choosing, a forged `REFUSED` line among them, from the scaffold and from adopt. The traceback #339 replaced had shown a `repr`. A printable path is named byte for byte as before. - **S2, no `gh` on PATH.** `pin_digest_from_gh` refuses as `gh-not-found`, exit 2, as `repo_shape.tree_digest_from_gh` names it, and offers `--pin-source`. It was a `FileNotFoundError` traceback, exit 1, from the scaffold and, new with this branch, from `adopt-project.py plan --pin`. - **S3, a hand-edited `commit:` or `tree_sha256:`.** The lane's decision is to accept it and document it (option (c)); no behaviour changed. A pair that disagrees passes `check` and `execute` (`adoption verified`), because neither reads a network for a pin, by design. The adopted root's own `validate-pins.py` then exits 1 with `neutral-pin-digest-mismatch` wherever it can read the product. `check` now says so once, after the pins it will write: "each pin as `plan` read it, not read again here: the root's own `validate-pins.py` recomputes its digest wherever it can read the product". Options (a), `execute` recomputes before it creates the legs, and (b), `check` recomputes from a clone still on the machine, are filed as #449. - **N2, a `--pin-source` that `plan` cannot record.** `plan` asks `check`'s question (`one_printable_line`) of the path it records in each pin's `digest_source:`, before any digest is read. It refuses one that fails as `adopt-unwritable-value`, in #270's words (`unwritable_reason`), or as "an invisible formatting character" where those name nothing. Before, it was planned, exit 0, and printed raw in the report. `check` then refused it as `plan-pin-malformed` and said to re-run the same `plan`. The report's `pin` line and `check`'s `write` line now spell the digest's source with `_shown`. The scaffold half is pre-existing (a line break in the path is written into the pin file; a byte that is not UTF-8 dies after all three repositories exist) and is filed as #450. - **N3, six hunks no test guarded.** Each now has a test that fails when it alone is reverted: - the two name guards (a `pins:` name holding an ESC and a forged line); - the lower-casing of the commit and the digest (an upper-cased hand edit); - the `digest_source` printability check; - the not-text check (`commit: 1234567` unquoted); - the gitlink filter (a submodule kept where the pin is written is `plan-submodule-root` alone). - **N4.** This body. - **N5, #339's third decision.** A `--pin-source` that nothing reads is still silent, now in adopt too. This PR does not decide it. #451, filed, lays out refuse, note and leave, so closing #339 loses nothing. Lane 5's second review, of 421e4e5, found no blocking item, 2 should-fix items and 3 nits. The two should-fix items are this body: the landing after #409 (above) and #386 (above). The code needed no change for them. f63d1ed adds two tests and no code: - **n1. `plan` asks nothing of a `--pin-source` no `--pin` records.** With the `args.pin and` guard dropped, such a path holding an ESC or a zero-width joiner was refused in a sentence naming no file ("`execute` writes into , and ..."), and every test still passed. `test_a_pin_source_no_pin_records_is_not_asked_about` holds the plan to exit 0 and `pins: []`, with the path named nowhere in the plan or the report. The ESC row is POSIX-only by the file's `POSIX_NAMES` mark; the zero-width joiner row runs everywhere. It holds #451's answer today, silence, so a decision there changes this test on purpose. - **n2. A printable `--pin-source` is printed as it is.** With `_spelled` in place of `_shown` in `plan`'s `pin` line, every test still passed. `test_a_printable_pin_source_is_printed_as_it_is` holds `plan`'s `pin` line, and `check`'s `write` line for the plan `plan` wrote, to their exact text, ending `(local-clone (<path>))` with no backtick. - **n3** needs a code change and is left out (below). **Fix round 2, on f63d1ed.** Each run had a `gh` stub first on PATH (it logs and exits 97), `GH_TOKEN` bogus, an empty `GH_CONFIG_DIR`, `git-filter-repo` 2.47.0 next on PATH, and the identity blanked with `GIT_CONFIG_COUNT`; the session `gh` guard came first inside each run. - **The pin set: 812 passed, 2 skipped.** It covers adopt pins, adopt plan, #339's file, #275's tests, the pin and neutral-pin validators, git output decoding, adopt template values, CLI reference, scaffold pin and reuse, scaffold pin collisions, #314's two files and #396's file. The skips are `test_adopt_plan.py:403` (no MedxEHR clone) and `test_adopt_template_values.py:223` (this host reads `git config user.name` from the repository, not from `GIT_CONFIG_GLOBAL`). That is fix round 1's 809 passed, plus the three new rows. - **Shape doctor and scaffold leg-path collisions: 369 passed, 3 skipped** (the PowerShell line exists only on a Windows host). - **Mutation probes**, each one line changed in a throwaway worktree, over `tests/test_adopt_pins.py` and `tests/test_pin_source_unreadable.py` less the five end-to-end adoptions: - the `args.pin and` guard dropped: 2 failed, both n1 rows; - `plan`'s `pin` line `_spelled`: 1 failed, the n2 test; - `check`'s `write` line `_spelled`: 2 failed, the n2 test and `test_an_upper_cased_hand_edit_is_written_lower_cased`; - `plan`'s `pin` line raw: 1 failed, the no-break-space test that already guarded that hunk. **On the merged tree, 73d65c6** (the merge f9ed46f and the `_read_pins` commit), with the same harness: - **The pin set** above, with #445's `test_adopt_late_failure.py`, `test_adopt_git_reads.py` and `test_adopt_filter_repo_failure.py` and #387's `test_adopt_empty_symlinks.py` and `test_adopt_root_manifest.py`: **988 passed, 2 skipped**. The skips are the two above. - **Shape doctor, scaffold leg-path collisions and the adopt suites:** **967 passed, 3 skipped** (the PowerShell line). The adopt suites are e2e, execute refusals, verify, root shape paths (#387's too), leg-path collisions, clone template, ignored shape files and submodules. - **On the merge alone, f9ed46f** (the same tree as before the `_read_pins` commit): `tests/test_adopt_late_failure.py` gave 3 failed, 83 passed, the three `AttributeError` rows above. - `render-cli-reference.py --check` is ok on the merged tree. pyflakes reports the same four warnings on `adopt-project.py`, `scaffold-project.py` and `scripts/shape_materialize.py` as before the merge. **Fix round 1, on 421e4e5.** Each run had `git-filter-repo` 2.47.0 on PATH and the identity blanked with `GIT_CONFIG_COUNT`. A `gh` stub that logs and exits 97 was first on PATH, with `GH_TOKEN` bogus and an empty `GH_CONFIG_DIR`, and the session `gh` guard came first inside each run. - **The pin set:** 809 passed, 2 skipped (the same files and skips as above). - **Doctor and scaffold:** 782 passed, 3 skipped (the PowerShell line exists only on a Windows host). It covers shape doctor, scaffold leg-path collisions, scaffold e2e, referent chain links, naming policy, repo hygiene and #351's free-text values. - **The adopt suites:** 882 passed. It covers e2e, execute refusals, verify, spec-only, leg paths, root shape paths, plan escapes, leg-path collisions, clone template and submodules. - **Before the #314 merge,** on 74ce4d9's tree: the first two sets gave 577 passed, 2 skipped and 782 passed, 3 skipped. On 2bcfa69's tree (the merges alone, before the fix) they gave 542 passed, 2 skipped and 782 passed, 3 skipped. After the 45e01f4 merge, `tests/test_agents_md_codes.py`, repo hygiene and CLI reference: 110 passed. - **Lane 5's second review** also ran 421e4e5 merged with main cb7063f and with 53bee5b: the pin set, the manifest-value set, and the doctor set with #357's `tests/test_doctor_validator_rows.py`. All passed, with only the skips above. Proof that fix round 1's tests test something: - **On 2bcfa69's tree,** with this head's two test files laid over it, 31 of the new rows fail: - S1: 17; - N2 and its report spelling: 7; - S2: 3; - the `pin_product_key` port: 2; - `check`'s S3 line: 1; - the not-UTF-8 `adopt plan` row: 1. The N3 rows pass there, as they must: they guard hunks this branch already had. - **Mutation probes.** Each hunk was reverted alone, in a throwaway worktree, and both test files were run: 18 of 18 were caught. - **The port:** the casefold keying (2 fail); the casefold lookup alone (2). - **S1:** the path named raw (13); git's reason named raw (2); every path `repr`'d, printable or not (27). - **S2:** no `except` (3). - **N2:** no `plan` check (11); only #270's question asked (1); the formatting-character reason said otherwise (1); the report's `pin` line raw (1); `check`'s `write` line raw (1). - **S3:** the line dropped (1). - **N3:** the `_pin_fix` name guard (1); the `_read_pins` path guard (1); the lower-casing (1); the `digest_source` check (1); the not-text check (1); the gitlink filter (1). - **The issue rows reproduced.** S1, S2 and N2 were reproduced on 2bcfa69's tree and shown gone on the fix (a probe driving both tools; `gh` was called 0 times). **Before fix round 1.** Runs were on 7a94b6b, with `git-filter-repo` 2.47.0 first on PATH and the repository-local `user.name` blanked with `GIT_CONFIG_COUNT` (a workstation whose standard checkout sets one fails `test_the_scaffold_refuses_with_no_elector` on main itself; CI sets none). 7a94b6b differs from ffd1297 in `tests/test_pin_source_unreadable.py` only, so the three suites that do not read that file ran on ffd1297: - the new file: 46 passed; - adopt pins, adopt plan, git output decoding: 172 passed, 1 skipped (the MedxEHR clone is absent); - scaffold pin and reuse, pin collisions, e2e, referent chain links, neutral pin and pin validators, #275's tests, naming policy, repo hygiene, CLI reference, scaffold leg-path collisions: 571 passed; - shape doctor: 254 passed, 3 skipped (the PowerShell line exists only on a Windows host); - the new file with repo hygiene, git output decoding and CLI reference, once more after the last edit: 176 passed. Earlier on 5690235 and d806e8f, before the #339 commit: the adopt suites (e2e, execute refusals, verify, spec-only, leg paths, root shape paths, plan escapes, leg-path collisions, clone template) 740 passed, and `tests/test_adopt_template_values.py` with #273's other adopt tests 489 passed, 2 skipped. They were not re-run on 7a94b6b: the #339 diff touches `pin_digest_from_source` and the call in `declared_pin_values`, which those suites reach through `test_adopt_pins.py` only. Nothing was run on macOS or Windows. Proof the new tests test something: - **On d806e8f.** 38 of the 46 fail, each on the traceback and its exit 1. The 8 that pass are the unchanged behaviour: the not-a-clone refusal (4 rows) and the valid-source rows (4). - **Mutation probes.** Ten hunks reverted one at a time, each failing at least one new test: the `OSError` catch (16 fail), the catch narrowed to `OSError` only (21), the missing-path clause (9), the clause narrowed to `FileNotFoundError` (3), the `S_ISDIR` judgement (3), the loop guard (4), no resolve at all (3), the old `.resolve()` call (4), the reason as `str(exc)` (12), and the file sentence as the OS's (3). - #273's own 52 tests still fail on c7c02fa (51 fail or error; the one that passes checks the test's own table), and its 18 mutation probes were each caught. **Re-verified after the merge.** An independent re-verification of f9ed46f and 73d65c6 found the code correct: - #409 is carried whole. A 30,000-case fuzz of `--pin` lists through #409's final refusal and this branch's port differed in 0 cases. - `scaffold-project.py` equals the tested file plus main's own changes that are not #409's (same patch-id). - `_commit_the_split` keeps #445's `try` and #394's `append`. - `_read_pins` reads a loaded plan as before. Its findings were all in this description (the #506 bullet and three stale facts), now fixed. Runs on the head and on its merge with main 48585e6: 988 passed, 2 skipped; and 967 passed, 3 skipped. `git merge-tree --write-tree` was run at 01:48Z on 2026-10-13, comparing hunks per file. Each PR head, fetched from `refs/pull/<n>/head` with no `gh`, was merged with 73d65c6 and with origin/main 9736424 alone. Two sets were asked about: - every PR this description listed before that main does not now contain; - every PR numbered above #468 whose head main does not contain: #473, #474, #490, #491, #493, #494, #495, #496, #497, #503, #504 and #505. **Open state, read with `gh` by the coordinator at 02:00Z and refreshed at 02:25Z after a re-verification on main 48585e6.** Every pull request named below is open. #184, #94, #96, #98, #100 and #161 are closed, and #465 landed as 48585e6. `fix/adopt-plan-quote-yaml-keywords` has no pull request. #506 (`fix/scaffold-reuse-seeded-legs`) opened after this list was made, and the re-verification reads it. - **Landed since the last draft of this list:** #343, #346, #355, #358, #373, #378, #382, #387, #394, #397, #401, #404, #409, #419, #452, #456, #465, #466 and #484. #394's `_commit_the_split` conflict and #401's `docs/cli.md` conflict are both resolved in f9ed46f, above. - **Clean, 20:** #266, #347, #356, #374, #376, #384, #395, #453, #467, #468, #473, #474, #490, #491, #493, #494, #495, #497, #503 and #505. #453, #468 and #491 were updated onto main since the first read and now merge clean. #507 (lane 5's #486-#488) shares only the generated `docs/cli.md` with this branch and merges clean with it. - **Conflicting with main alone, and this branch adds no hunk:** - #402: `shape-doctor.py` 5; - #496: `shape-doctor.py` 10 and `tests/test_shape_doctor.py` 1. **Conflicts this branch adds:** - **#435** (3efcc7a, lane 2's `fix/adopt-plan-field-types`, for #386). - It adds 2 hunks in `adopt-project.py`: 5 here, against 3 with main alone. They are `render_plan`'s `pins:` line and `Plan.pins`. - Keep this branch's `_pin_lines` and `_pin_products`. The rest is under "Where the meaning meets". - **#504** (37ef29a, `fix/adopt-root-kept-symlinks`). - It adds 1 hunk in `adopt-project.py`: 2 here, against 1 with main alone. It is the materializer call in `_commit_the_split`. - Keep both arguments and both comments: `append=append, neutral_pins=_recorded_pins(plan), spell=_spelled`. - **#506** (119d8c4, `fix/scaffold-reuse-seeded-legs`). - It adds 1 hunk in `scaffold-project.py`. With main alone, #506 conflicts only in `docs/handbook.html`. - #506 inserts `LEG_TEMPLATES` right after `declared_pin_products`, which this branch moved out, and rewrites `_reusable_remotes` further down. Keeping both sides breaks the file with an unterminated string literal. - Keep only #506's `LEG_TEMPLATES` block. Drop #506's `declared_pin_values`/`declared_pin_products`, which moved here, and this branch's three-line `_reusable_remotes` opener, which #506 rewrites below. Resolved that way, pyflakes is clean and #506's scaffold tests pass (88). - **`fix/adopt-plan-quote-yaml-keywords`** (ed9968f; a branch with no pull request). - It adds 2 hunks in `adopt-project.py`: 3 here, against 1 with main alone. - One is `render_plan`'s `pins:` line. Keep `_pin_lines`, which already sends product, repository and `digest_source` through `y`. - The other is `_refuse_unholdable_values`' signature. Keep `pins: dict`, with that branch's `_validator_judged` and `_named_as_read` above it. - `fix/adopt-late-failure-notice` is no longer on origin. #445, the late-failure notice, is on main and merged here. **Where the meaning meets:** - **#435 and #386.** The coordinator has said this on #435: #435 (comment) - #435's `_plan_key_findings` (`plan-bad-pins`) keeps only text items in `pins:`, and its `PINS_REMEDIATION` tells a person to write `pins: [openGlass]`. - This branch writes each pin as a mapping, and it makes a name alone `plan-pin-unrecorded`. - Read from #435's code, not run: on a tree with both, the plan `plan --pin` writes is `plan-bad-pins` in `check`. - Whichever lands second reconciles them, with three changes: - `_plan_key_findings` takes this branch's mappings, or leaves `pins:` items to `_read_pins`; - a `pins:` that is not a list gives one finding, not both `plan-bad-pins` and `plan-pin-malformed`; - `PINS_REMEDIATION` says the mapping `plan` writes. - Until that is done, `tests/test_adopt_pins.py::test_a_plan_with_a_whole_pin_checks_ok` fails. - **#314.** It landed, and it is merged here (above). - **`fix/adopt-plan-quote-yaml-keywords`.** Its pin tests are written against the name-only `--pin`. They need the `name@commit` form, and their expected `pins: [...]` line becomes `_pin_lines`'s mapping. - **Two unwritable values are refused in two runs (lane 5's second review, n3).** `plan --elected-by 'A<LF>B' --pin openGlass@<sha> --pin-source <a path holding an ESC>` refuses the elector alone, exit 2. Re-run with the elector retyped, it refuses the source; #270's refusal names every unwritable value at once. Reproduced on 421e4e5, with `gh` called 0 times. One refusal would need a code change, not made here: - `_refuse_an_unrecordable_pin_source` would split into a function that returns the sentence; - `_checked_plan_inputs` would ask it before #270's `unwritable_values` raise, and raise one `adopt-unwritable-value` that names both, with both remediations; - a test would add the two-value row. N2's question is `one_printable_line`, not `unwritable_reason`, so it cannot simply join #270's list. - **Docs.** The README, AGENTS.md and `docs/handbook.html` are unchanged, because none of them calls adopt's `--pin` a name. A sentence saying adopt now takes the scaffold's `--pin` is a follow-up. It costs an AGENTS.md line-cap raise, and a README edit forces the handbook's README blob to change. - **A product spelled `null`, `true` or `false`.** The scaffold already accepts `--pin null@<sha>`: it exits 0 and builds a root whose `validate-pins.py` exits 1 `neutral-pin-entry-malformed`. Adopt's `plan` writes `product: null` through `y`, and `check` then finds `plan-pin-malformed` and refuses it before any leg exists. The one fix for both tools is in `parse_pin`, and it would change the scaffold's behaviour. - **Wording shared with the scaffold.** `digest_source` is written bare into the pin file, so a `--pin-source` path holding ` #` is cut short there. `templates/assembly-root/contracts/neutral-product-pin.yaml` and `validate-pins.py`'s `neutral-pin-missing` remediation name only the scaffold as the writer. Two comments in `scripts/repo_shape.py` still name the scaffold's `pin_digest_from_gh`, in a digest-pinned shipped copy. - **Filed as follow-ups:** - #449: S3, a hand-edited `commit:` or `tree_sha256:` that disagrees, with options (a) and (b); - #450: the scaffold half of N2, a `--pin-source` path no file can hold; - #451: #339's third decision (N5), a `--pin-source` that nothing reads. - **`validate-pins.py` prints a `--pin-source` path raw** in `neutral-pin-source-unreadable`, the twin of S1. It is a shipped template that #409 and other open PRs edit, so it is left for its own change. No issue is filed for it yet. - **Windows and macOS** were not run here; CI ran both (below). Two assertions that depend on what Windows answers for a path (a name past the filesystem's limit, a path beneath a file) check only the refusal and its remediation there. Fix round 1's rows that need a control character or a byte that is not UTF-8 in a path, or a symlink to `git`, are skipped on Windows by a mark, and so is fix round 2's ESC row. The rows for a no-break space and a zero-width joiner run everywhere, and so does the in-process `gh-not-found` test. Reviewed independently by lane 5 at 7a94b6b (3 should-fix, 5 nits: fix round 1) and at 421e4e5 (0 blockers, 2 should-fix, 3 nits: fix round 2). After #409's squash, the re-verification of f9ed46f and 73d65c6 found the code correct; its 1 should-fix and 3 nits were in this description and are fixed. GitHub's update-branch then merged main twice (33bae3f, 2170526), with no hand edit. CI's tests, tests-macos and tests-windows and SonarCloud all passed at 2170526 (main cac6bb2 merged in). On that head merged with main 57c85e9 (#490, #474 and #468, none of which touches this branch's files), the pin and adopt set with `tests/test_update_shape_landing.py` gave 1112 passed, 2 skipped, and the doctor, adopt end-to-end, unfinished-merge and bootstrap set gave 1056 passed, 3 skipped (the doctor's PowerShell line, which exists only on a Windows host). Closes #273 Closes #339 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 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>
This was referenced Oct 16, 2026
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-1 (openRepoShape-1)
Claim: #190 (comment)
Builds on #166, which is on
mainas e4decfb (#194): it calls #166's.gitmodulesreader (Source.registered_submodules,Registrations) and adds to its one findings list (_entry_findings). The branch hasmainmerged in at ab623dd, so its diff againstmainis #190 alone:adopt-project.py,scripts/repo_shape.py(remote_key),tests/test_adopt_submodules.py,tests/test_shape_doctor.py, one constant oftests/test_adopt_gitmodules_values.py, and the newtests/test_adopt_relative_submodule_url.py.Head a0272b2. After the reviewed b0c959e:
main54aa69a.ssh.github.com, out ofremote_key.main4835fb4 (update-shape.pyapplyrewrites the manifest'sshape:block with fixed two- and four-space indents, so aproject.yamlindented four spaces, which the project's own reader and both validators accept, is rewritten into YAML that reader rejects, and the rolled-back refusal relays the validators' lockstep remedy #208, scripts/repo_shape.py_split_flowignores a backslash inside a double-quoted flow item, sopins: ["x\",y"]reads back as two itemsx\andy"where the same text as a block sequence item reads as the one itemx",y#213, adopt-project.py plan writes a plan its own loader cannot parse when a source name holds a line break (the macOSIcon\rfile), socheckrefuses the whole plan asyaml-unparsablewith no entry to edit #199). Its one conflict, therepo_shapeimport list, keeps both sides: adopt-project.py plan writes a plan its own loader cannot parse when a source name holds a line break (the macOSIcon\rfile), socheckrefuses the whole plan asyaml-unparsablewith no entry to edit #199'sdouble_quoted, and adopt-project.py keeps a source submodule's relativeurl = ../xunchanged in the leg, where git resolves it against the leg's remote, soexecutesaysadoption verifiedand the assembly's recursive clone fails unless the dependency happens to sit beside the leg #190'sredacted,remote_is_a_path,remote_keyandremote_local_path.mainat 9bc4147 (39ac59c), 0b996fd (60d08a1), 1abe078 (c3f4987) and 8ce5614 (d94d155).mainede15ce (adopt-project.pycheckaccepts a.gitmodulesvalue git's submodule reader rejects (update = bogus,update =,update = !cmd,shallow = maybe,fetchRecurseSubmodules = bogus), soplan okis followed byexecutedying ingit filter-repoafter both leg remotes exist, whether the tip or only the history holds the gitlink #215, scaffold-project.py--pin spec@<sha>(orcode@,shape@, or two--pins whose products differ only by case) passes--dry-run, then creates all three repositories, pushes both legs and dies withREFUSED materialize-collision, because the pin'scontracts/<product>-pin.yamlis a file the shape already writes #241, scripts/repo_shape.py treats a quote character in the MIDDLE of a plain scalar as opening a quote, sok: [it's, a]reads["it's, a"]where its block twin reads["it's", "a"],k: it's fine # notekeeps the comment in the value, and #244 turnsk: [a"b\", c]from right (by accident) into one item #249/scripts/repo_shape.py_scalartakes any text that starts and ends with a quote for a quoted scalar and anything else for plain text, so an unterminated"abcreads as"abc,- "a\"reads asa\, and"a" b "c"reads asa" b "c, where PyYAML refuses each #250, update-shape.pyapply --branch --push --pr: whengh pr createfails after the commit and the push succeeded, it exits 2 with "REFUSED committing the re-synced shape" and no NEXT line, and a re-run on that branch says "nothing to do" and exits 0, so the tool never opens the pull request #204, 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, shape-doctor.pyvalidator_rowfalls back to the standard's TEMPLATE validator when a root carries no copy of its own, and prints that path as the NEXT command without thePYTHONPATHit runs it under, so pasting themanifest,pinsorfamilyrow's NEXT dies withModuleNotFoundError: No module named 'repo_shape'and exit 1, the code a validator uses for a finding #238) with no conflict.mainb421e6b (adopt-project.pycheckaccepts plan pathsexecuterefuses asunsafe-value(a space or a non-ASCII character), soplan okis followed by a refusal and #166's split instructions can name entriesexecutewill refuse #196'sentry_path_problem, bump-leg.py (a real bump) and update-shape.pyapplyrun the project's own validators as children that cannot print its display name on a cp1252 stdout, so a project named日本is refused and rolled back asbump-leg-validators-redorupdate-validators-redunderPYTHONIOENCODING=cp1252:surrogateescape, and the refusal blames the validators #254). Its three conflict hunks are in_entry_findingsand in the docstring of_refuse_what_check_finds, and each keeps both sides: adopt-project.pycheckaccepts plan pathsexecuterefuses asunsafe-value(a space or a non-ASCII character), soplan okis followed by a refusal and #166's split instructions can name entriesexecutewill refuse #196'splan-unsafe-pathcomes first in the one list, and adopt-project.py keeps a source submodule's relativeurl = ../xunchanged in the leg, where git resolves it against the leg's remote, soexecutesaysadoption verifiedand the assembly's recursive clone fails unless the dependency happens to sit beside the leg #190's relative urls and NOTEs come last.main71b58a1 (adopt-project.pycheckpasses a plan that keeps the source's ownshape/in the root, andplace()then writes the shape's collision copy onto it after both legs are pushed: a realshape/README.mdis overwritten whileexecuteprints "nothing was overwritten" and endsadopt-lost, and ashape/Makefile/directory or a file namedshapeis a traceback #262/adopt-project.pycheckpasses a plan that keeps a plain file at a directory the shape writes into (.github,contracts,scripts), andexecutedies in aNotADirectoryErrororFileExistsErrortraceback after both legs are pushed #263's root-shape findings, and c0ed615'sfamily.py). Its two conflict hunks are docstrings, each the union of both sides: the module docstring'scheckparagraph, and the first paragraph of_entry_findings._leg_findings(plan, tree)now holds the root-shape findings, so the one list reads adopt-project.pycheckaccepts plan pathsexecuterefuses asunsafe-value(a space or a non-ASCII character), soplan okis followed by a refusal and #166's split instructions can name entriesexecutewill refuse #196's unsafe paths first, then the root's shape paths with the legs, and adopt-project.py keeps a source submodule's relativeurl = ../xunchanged in the leg, where git resolves it against the leg's remote, soexecutesaysadoption verifiedand the assembly's recursive clone fails unless the dependency happens to sit beside the leg #190's relative urls last.mainat cc5b6ae (87373a6) and d72544c (a1b3a4b).mainf795cf7 (adopt-project.py: since #247 a byte that is not UTF-8 in--elected-by,--referenceor the--sourcedirectory passesplanandcheck, andexecutepushes both legs before it dies withUnicodeEncodeErrorinwrite_lf#255's value findings, with 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 and templates/assembly-root/scripts/validate-pins.py: a bare--pin-source <path>whose path holds an=is read as<product>=<path>(raw.partition("=")), so a stand-in clone under a directory nameda=bis never consulted and the pin's digest silently goes togh apiand askip(pins ok, exit 0), or, when the text before the=is a declared pin's name, to aFileNotFoundErrortraceback #295). Its two conflict hunks are in_entry_findings. The one list is now adopt-project.pycheckaccepts plan pathsexecuterefuses asunsafe-value(a space or a non-ASCII character), soplan okis followed by a refusal and #166's split instructions can name entriesexecutewill refuse #196's unsafe paths, coverage, the legs and the root's shape paths, adopt-project.py: a plan that sends.gitmodulesto one leg and a submodule it registers to another passescheckandexecute, saysadoption verified, and the assembly's recursive clone then fails:planshould ask,checkshould find,executeshould refuse #166's submodules, adopt-project.py: since #247 a byte that is not UTF-8 in--elected-by,--referenceor the--sourcedirectory passesplanandcheck, andexecutepushes both legs before it dies withUnicodeEncodeErrorinwrite_lf#255's values, and last adopt-project.py keeps a source submodule's relativeurl = ../xunchanged in the leg, where git resolves it against the leg's remote, soexecutesaysadoption verifiedand the assembly's recursive clone fails unless the dependency happens to sit beside the leg #190's relative urls, with the remediations in that order. To give that order,_submodule_findingsreturns adopt-project.py: a plan that sends.gitmodulesto one leg and a submodule it registers to another passescheckandexecute, saysadoption verified, and the assembly's recursive clone then fails:planshould ask,checkshould find,executeshould refuse #166's findings and adopt-project.py keeps a source submodule's relativeurl = ../xunchanged in the leg, where git resolves it against the leg's remote, soexecutesaysadoption verifiedand the assembly's recursive clone fails unless the dependency happens to sit beside the leg #190's as two pairs.mainab623dd (bump-leg.pyplan_workflowscompares git's tracked workflow paths with the on-disk spelling exactly, so a tracked workflow renamed by case alone outside git on a case-insensitive filesystem (legs.ymltoLegs.yml) is refused asbump-leg-workflow-untrackedwhilegit statusis clean, and "commit it first" cannot clear it #332, tests: 149 tests in four files run the operator's realgh(407 calls togh --versionandgh auth status, andgh api userbesides with a login) through the doctor'smachinerow and the installed command's machine block, so a suite whose conftest says "NO NETWORK AND NO GITHUB" reaches the host on a logged-in machine, and the oneghstub the doctor tests have is called by that row without the test seeing it #296) with no conflict. adopt-project.py keeps a source submodule's relativeurl = ../xunchanged in the leg, where git resolves it against the leg's remote, soexecutesaysadoption verifiedand the assembly's recursive clone fails unless the dependency happens to sit beside the leg #190'sremote_keytest and tests: 149 tests in four files run the operator's realgh(407 calls togh --versionandgh auth status, andgh api userbesides with a login) through the doctor'smachinerow and the installed command's machine block, so a suite whose conftest says "NO NETWORK AND NO GITHUB" reaches the host on a logged-in machine, and the oneghstub the doctor tests have is called by that row without the test seeing it #296'sghguard rows sharetests/test_shape_doctor.py.What 5751460 (#212) and c2ff2b0 (#215) required. #212 spells the submodule that
_as_entrynames, so #190's finding now readsthe submodule `upstream/dependency` (entry `upstream/`)and its tests expect that. #215's tests registeredurl = ../bin a source with no remote, which #190 rightly finds, so they register an absolute url instead.adopt-project.pyis unchanged by either. Where a source's.gitmodulesholds both a value git refuses and a relative url, #215'splan-gitmodules-unreadableanswers alone, and the relative url is found once the value is fixed.What
git resolves a source submodule's RELATIVE
.gitmodulesurl (./x,../x) against the remote of the repository that holds the file. Before the split that is the source's remote; after it, it is the LEG's.adopt-project.pynow resolves each relative url the plan keeps in a leg from both places, and compares the two answers. It asks git to do the resolving:git submodule initruns in a scratch repository whoseoriginis the remote in question.remote_key, or, for two paths on this disk, as the same directory once symlinks and..are followed): nothing changes. Two cases produce this. One is legs on GitHub in the source's own organisation, where../depfromorg/Thingand fromorg/Northwind-codeboth reachorg/dep. The other is a local layout with the dependency beside the legs.checkprints one NOTE saying it resolved from the GitHub remotes the plan names, and thatexecute --local-remote-dirwill resolve again.checkreportsFINDING plan-submodule-relative-url. The finding names the submodule, its relative url, the source's base and git's answer from it, and the leg's remote and git's answer from it. A remediation follows once: commit the absolute url in the source's.gitmodulesand re-runplan, ordropthe submodule.executerefuses the same plan through adopt-project.pyexecuterefuses onlyplan-unresolvedandplan-stale, so a plancheckrejects as uncovered or with a bad leg creates and pushes both legs before it fails verification #168's one list, before any leg repository exists.checkandexecuteresolve from different remotes, on purpose.checkhas no--local-remote-dir, so it decides onhttps://github.com/<org>/<leg>.git.executedecides again on the remotes it is about to create, before it creates any. Under--local-remote-dir, the first line of its refusal says the finding was resolved from those remotes, and the remediation says thatcheck'splan oksettles nothing there..gitmoduleswould produce a blob that_verifyaccounts for by sha, and a url nobody committed. So_verifyis untouched: any relative url that reaches it names the same repository from the leg as from the source.origina relative path (git remote add origin ../src/Thing.git), git resolves../depto../src/dep, keeps that relative, andgit submodule updateopens it from the top of the source's working tree (probed on git 2.43, from the top and from a subdirectory). git's answer is joined to the source's working tree before it is compared, not normalised (the disk decides..after a symlink, throughrealpath), and the finding says(its remoteorigin, relative to its working tree). Joining git's answer, not anchoring the remote first, keeps git's own arithmetic where it is notnormpath's:origin.resolves../depto the working tree's owndep, and one..too many is git'sfatal: cannot strip one component off url '..'.scripts/repo_shape.py'sremote_keytakes:<digits>off the host of a<scheme>://url before it reads the scp colon, and readsssh.github.comasgithub.com, sossh://git@github.com:22/org/xand GitHub's documentedssh://git@ssh.github.com:443/org/xkey likegit@github.com:org/x. The scp spelling has no port (git@host:22/xkeeps22as a path component), and a drive's colon has no digits after it. Of 24 spellings probed, 8 keys change: the six GitHub port andssh.github.comspellings (five with a scheme, and the scpgit@ssh.github.com:org/x) becomegithub.com/org/x, andssh://git@host:22/Repo.gitandssh://[::1]:22/x/Repo.giteach become their port-free key.same_repository(the doctor,make siblings) asks the same keys: its forge-tail match already matched anOwner/Repospelled with a port, and now a repository at a host's top level matches too.scripts/repo_shape.pyis a SHAPE COPY, so every project's nextupdate-shape.py checkshowsupstream-changed scripts/repo_shape.py, as after scripts/repo_shape.pyload_yamlcatchesOSErrorandYamlErrorbut notUnicodeDecodeError, so a plan or manifest holding a non-UTF-8 byte makesadopt-project.py checkandexecuteexit 1 with a traceback instead ofREFUSED yaml-unreadable#192 (22c9fdd) and same_repository's trailing owner/repo fallback calls any two remotes that share their last two components one repository, so /srv/a/IRRS.git and /other/a/IRRS.git are the same clone #157 (71cf5de). It belongs there: a project's own copy runssame_repository(siblings.py). No test or contract pins its digest.repo_shape.redacted, which also hides the user of a url with a scheme (ssh://***@github.com:22/...).mainprints.In
tests/test_adopt_submodules.py, the default url ofregistration()and the url inUPPER_CASE_KEYSare now the dependency's absolute path. Seven of #166's cases wroteurl = ../dependencyby hand, in code, in a source with no remote, and expected no finding. That expectation is exactly #190's broken outcome. Those cases test the registration reader, not the url. For the same reason,tests/test_adopt_gitmodules_values.py(#215) registers its submodule athttps://example.invalid/b, not../b. Its tests are about values, and under #190 a relative url in a source with no remote is a finding of its own.Why (repro on
main)The re-verifier's
relative-urlcase: one gitlinkupstream/dependencywithurl = ../dependency,.gitmodulesand the gitlink both kept in code, and legs created under--local-remote-dir. Onmaine4decfb:On this branch,
checkexits 1 with the finding,executeexits 2 withREFUSED plan-submodule-relative-url, and no remotes directory is created.The review of the first version (b0c959e) found two layouts it refused that
mainadopts into a working assembly: a source whoseoriginis a relative path, with the legs beside its dependency, and a GitHuboriginspelled with a port or asssh.github.com. This version adopts both, asmaindoes (below, lane 2's rows O2 and X4).Tests
New:
tests/test_adopt_relative_submodule_url.py, 32 tests.checkagainst GitHub remotes. With an https, scp, other-case scp,ssh://git@github.com:22/...orssh://git@ssh.github.com:443/...originin the plan's own org,../dependencyresolves to the same repository from both places: no finding, one NOTE. In another org it is found, with the remediation printed once../dependency, and a url with more..than the remote has components, are found even in the plan's own org.origin..gitmodules) is adopt-project.py: a plan that sends.gitmodulesto one leg and a submodule it registers to another passescheckandexecute, saysadoption verified, and the assembly's recursive clone then fails:planshould ask,checkshould find,executeshould refuse #166's finding and nothing more.executeresolves again from the remotes it would create (https, scp, scp-other-case, and../../dependencyin another org). Its refusal comes before any remote exists and carries the--local-remote-dirfirst line. Without--local-remote-dir, its refusal ischeck's list line for line. A refusal for anything other than a relative url says nothing about local remotes.originis the RELATIVE../remotes/Thing.git: with the legs beside its dependency nothing is refused; with them elsewhere the refusal names the directory git opens,<tmp>/Thing/../remotes/dependency, and says why. The same remote spelled absolute is said as before.git filter-repo:../dependency,./dependencyand../../dependencykept in code,./dependencykept in SPEC, and..\dependency(which git appends whole where\separates nothing, and strips on Windows) are refused with exit 2, with no remotes and no work written, and the source is untouched.../dependencyand../../dependencyname the dependency from both places, the first also with the source'soriginwritten relative, adopt and verify. In each, the assembly'sgit clone --recurse-submodulesandbootstrap.pyexit 0 with the dependency checked out in the leg, and the leg's.gitmodulesstill holds the relative url the source committed.New:
tests/test_shape_doctor.py::test_a_port_and_githubs_ssh_host_are_not_part_of_a_remotes_key, beside the one-repository table: six port andssh.github.comspellings key as the scp one; a repository at a host's top level, with a port, issame_repositorywith its scp spelling; the scp:22/path, another repository and another host do not key alike; a drive's file url still keys as the drive path.Revert table. This round's new and changed tests, each with its change reverted: a mutation in a scratch clone at c2b5302, run over the test's whole file. Every other test in the file passes, unless the row says otherwise.
test_execute_refuses_a_relative_url_its_leg_resolves_elsewhere[spec-leg](new)kept == leg == "code"(lane 2's M14)...[backslash](new)RELATIVE_URL_REwithout\(lane 2's M13)...[dotdot],[dot-slash],[dotdot-dotdot](changed: alegparameter, and the leg's name checked)adopt-project.pyismain's 54aa69atest_a_relative_origin_resolves_from_the_source_working_tree[relative-origin](new)whichclause dropped; the join normalised...[absolute-origin](new)test_a_relative_url_its_leg_resolves_alike_adopts_and_clones[relative-origin](new)...[dotdot],[dotdot-dotdot](changed: the origin spelled by a condition)test_check_keeps_a_relative_url_its_github_leg_resolves_alike[ssh-port](new)main'sremote_key; the port kept...[ssh-github-443](new)main'sremote_key; the port kept; nossh.github.comalias...[https],[scp],[scp-other-case](changed: the other-org finding expected throughredacted, which leaves these three as they were)tests/test_shape_doctor.py::test_a_port_and_githubs_ssh_host_are_not_part_of_a_remotes_key(new)main'sremote_key; the port kept; no alias; a port taken off the scp spelling's path tooWith
adopt-project.pyreverted tomain's whole, 31 of the file's 32 tests fail. The one that passes istest_a_split_submodule_is_a_split_and_nothing_more: it guards against #190 adding a finding to #166's split, whichmainalready reports, and M8 below fails it. With their change reverted (main'stests/test_adopt_submodules.pyagainst this branch's code), the seven changed cases fail: 7 failed, 125 passed. Both held at c2b5302 againstmain54aa69a, and again, the same tests, at 60e9558 againstmain4835fb4.The implementer's mutations, at b0c959e: each mutation of
adopt-project.pyin its own copy ofmaine4decfb with that branch's files, over the then 25 tests of the new file. Row M0 is that branch unmutated. These are the implementer's numbers; lane 2's M13 and M14, in the table above, are other mutants.mainremote_keyexecuteresolves from GitHub's remotesorigin, not the tracked remoteexecuteignores--local-remote-dirLane 2 ran 19 mutants of its own at b0c959e and caught 17. The two that survived are its M13 (no
..\) and M14 (code leg only), and they fail the[backslash]and[spec-leg]rows above.Runs at b33562d, after the merge of
mainf795cf7, and at a0272b2.The red CI at 8ce5614 (GitHub's merge of d94d155), reproduced over 26 targeted files: 12 failed, 1376 passed, 2 skipped. The 12 are this PR's own tests, which expected the submodule unspelled.
The revert. With c0b1ebb's test changes reverted, at 18e161e, its two files give 22 failed and 197 passed: those 12, and 10 of adopt-project.py
checkaccepts a.gitmodulesvalue git's submodule reader rejects (update = bogus,update =,update = !cmd,shallow = maybe,fetchRecurseSubmodules = bogus), soplan okis followed byexecutedying ingit filter-repoafter both leg remotes exist, whether the tip or only the history holds the gitlink #215's.Eight files:
tests/test_adopt_relative_submodule_url.py tests/test_adopt_gitmodules_values.py tests/test_adopt_finding_spelling.py tests/test_adopt_submodules.py tests/test_shape_doctor.py, adopt-project.pycheckpasses a plan that keeps the source's ownshape/in the root, andplace()then writes the shape's collision copy onto it after both legs are pushed: a realshape/README.mdis overwritten whileexecuteprints "nothing was overwritten" and endsadopt-lost, and ashape/Makefile/directory or a file namedshapeis a traceback #262/adopt-project.pycheckpasses a plan that keeps a plain file at a directory the shape writes into (.github,contracts,scripts), andexecutedies in aNotADirectoryErrororFileExistsErrortraceback after both legs are pushed #263'stests/test_adopt_root_shape_paths.py, adopt-project.py: since #247 a byte that is not UTF-8 in--elected-by,--referenceor the--sourcedirectory passesplanandcheck, andexecutepushes both legs before it dies withUnicodeEncodeErrorinwrite_lf#255'stests/test_adopt_template_values.py, andtests/test_scaffold_leg_path_collisions.py(the guard against a publicCASE_ASIDEinadopt-project.py). At b33562d:git filter-repoon PATH: 1037 passed, 3 skipped (the PowerShell rows, Windows only);git filter-repois absent).The targeted set, 30 files:
checkandexecuteprint a source path raw in a FINDING, so a\r,\n,\x85, U+2028 or ESC in a name overwrites, splits or escapes the line on a terminal, whereplan's questions spell the same path with_spelled#212'stests/test_adopt_finding_spelling.pyandtests/test_adopt_ignored_shape_files.py;main'stests/test_adopt_history_gitlinks.py,tests/test_git_output_decoding.py,tests/test_stdout_encoding.py,tests/test_yaml_flow_mappings.py,tests/test_yaml_block_scalar_keys.pyandtests/test_update_shape_pr_base.py;checkaccepts a.gitmodulesvalue git's submodule reader rejects (update = bogus,update =,update = !cmd,shallow = maybe,fetchRecurseSubmodules = bogus), soplan okis followed byexecutedying ingit filter-repoafter both leg remotes exist, whether the tip or only the history holds the gitlink #215'stests/test_adopt_gitmodules_values.py, scripts/repo_shape.py treats a quote character in the MIDDLE of a plain scalar as opening a quote, sok: [it's, a]reads["it's, a"]where its block twin reads["it's", "a"],k: it's fine # notekeeps the comment in the value, and #244 turnsk: [a"b\", c]from right (by accident) into one item #249/scripts/repo_shape.py_scalartakes any text that starts and ends with a quote for a quoted scalar and anything else for plain text, so an unterminated"abcreads as"abc,- "a\"reads asa\, and"a" b "c"reads asa" b "c, where PyYAML refuses each #250'stests/test_yaml_quote_boundaries.pyandtests/test_template_quoted_values.py, and 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'stests/test_family_add_refusals.py.With
git filter-repoon PATH, at c0b1ebb: 1786 passed, 5 skipped (the MedxEHR clone, the three PowerShell rows, and one Windows-only stdout row). At 5d3caf3, with adopt-project.pycheckaccepts plan pathsexecuterefuses asunsafe-value(a space or a non-ASCII character), soplan okis followed by a refusal and #166's split instructions can name entriesexecutewill refuse #196'stests/test_adopt_entry_paths.pyadded as a 31st file: 1851 passed, 5 skipped, the same five. CI then passed on Linux, macOS and Windows at 5d3caf3. At 2e93649, the same 31 files: 1851 passed, 5 skipped, the same five. At b33562d: 1955 passed, 5 skipped, the same five. At a0272b2, the filesmainab623dd changed beside this PR's,tests/test_shape_doctor.py,tests/test_gh_guard.pyandtests/test_adopt_relative_submodule_url.py: 293 passed, 3 skipped.scripts/render-cli-reference.py --check: ok.Runs at 60e9558, after the merge of
main4835fb4. The 18 files, 962 tests:tests/test_adopt_submodules.py tests/test_adopt_e2e.py tests/test_adopt_execute_refusals.py tests/test_adopt_plan.py tests/test_shape_doctor.py tests/test_windows_paths.py tests/test_cli_reference.py tests/test_adopt_relative_submodule_url.py tests/test_adopt_verify.py tests/test_adopt_mount_ignored.py tests/test_adopt_leg_path_collisions.py;main'stests/test_adopt_plan_escapes.py(adopt-project.py plan writes a plan its own loader cannot parse when a source name holds a line break (the macOSIcon\rfile), socheckrefuses the whole plan asyaml-unparsablewith no entry to edit #199, 59) andtests/test_yaml_flow_quotes.py(scripts/repo_shape.py_split_flowignores a backslash inside a double-quoted flow item, sopins: ["x\",y"]reads back as two itemsx\andy"where the same text as a block sequence item reads as the one itemx",y#213, 17);tests/test_pin_validator.py tests/test_update_shape.py tests/test_update_shape_add.py tests/test_family.py tests/test_repo_hygiene.py.With
git filter-repoon PATH they gave 961 passed and 1 skipped; without it, 905 passed and 57 skipped. At c2b5302 the 647 gave 646 passed and 1 skipped, and 591 passed and 56 skipped. The shape copy gave 223 passed. 60e9558's one resolution was also run with each side alone, overtests/test_adopt_plan_escapes.pyandtests/test_adopt_relative_submodule_url.py:NameErroriny;Lane 2's probe rows, on c2b5302 and on
main54aa69a. Its own driver, everyexecutewith--local-remote-dir, then a recursive clone,make bootstrapand the doctor.mainoriginthe relative../src/Thing.git, legs beside it.gitmodulesthe source's blob, clone 0, bootstrap 0, COMPLIANT. b0c959e refused it.originssh://git@github.com:22/testorg/Thing.git../depelsewhere.gitignorenamingcodeandspec../../org/missing, alike, exists nowhereNo change for sources without a relative url. Four sources (no submodule, an absolute path url, an absolute https url, a relative url dropped) plan and check byte for byte alike: exit code, stdout, stderr and the plan file. That held at c2b5302 against
main54aa69a, and again at 60e9558 againstmain4835fb4, whose #199 changes whatplanwrites and prints. At b0c959e, withmaine4decfb and the same standard commit,execute --local-remote-dirand its verification table were byte-identical too, for no submodule, an absolute url kept in code, an absolute url dropped, and an absolute url kept in spec with an httpsorigin; all four endedadoption verified.Adversarial matrix, at b0c959e. The re-verifier's 58
.gitmodulescases, each run through plan, check,execute --local-remote-dirand a recursive clone, onmaine4decfb and on b0c959e. 57 cases are identical in every recorded outcome. Onlyrelative-urldiffers, as shown above. Three cases verify but fail the clone on both:dup-section-bad-url-last,two-names-same-path-last-bad-urlandname-single-dot. In each, the source's own recursive clone already fails, and none involves a relative url.The full suite was not run here.
tests-macosandtests-windowsrun on this pull request.Reviews
Lane 2 (openRepoShape-2) reviewed b0c959e: MERGEABLE WITH NITS, after the merge of
main; no P1 or P2.main: five hunks ofadopt-project.py_leg_findings(plan, tree)with #190's three-tuplekept == leg == "code", survives every test[spec-leg]rowRELATIVE_URL_REwithout\, survives every test[backslash]rowremote.origin.urlis compared unanchoredremote_keyreads a port as a path segment;ssh.github.comchecktakes no--local-remote-dirorg:makescheckexit 2; a deleted one namesNonenormpath(source.path / url)on the remote before resolving. That differs from git wherever git's arithmetic on the relative spelling is notnormpath's:origin.,../src/Thing.git/., and one..too many. So git resolves against the remote as written, and its answer is joined to the working tree, which is the step git takes at clone time.github.comkey" and that scp keys are unchanged. Precisely: two of the eight are not GitHub (ssh://git@host:22/Repo.git,ssh://[::1]:22/x/Repo.git) and each goes to its own port-free key, and the scpgit@ssh.github.com:spelling is the one scp key that changes, through the alias.check, passed to_entry_findingsasexecutepasses it, pluscheck's topics line. But every message that says wherecheckresolves a relative url from says GitHub's: the remediation printed after the finding ("check'splan oksettles nothing" under--local-remote-dir), the NOTE,execute's refusal header ("wherecheckresolves it from GitHub's"), and five docstrings. Each would have to be rewritten to stay true oncechecktakes the flag. A draft came to eleven places and 75 changed lines ofadopt-project.py, before its tests anddocs/cli.md. That is a second review of adopt-project.py keeps a source submodule's relativeurl = ../xunchanged in the leg, where git resolves it against the leg's remote, soexecutesaysadoption verifiedand the assembly's recursive clone fails unless the dependency happens to sit beside the leg #190's messages, not one flag.--local-remote-diris the test path; on the GitHub pathcheckandexecuteresolve from the same remotes, and the NOTE says which remotescheckasked.mainshares it, and it is outside adopt-project.py keeps a source submodule's relativeurl = ../xunchanged in the leg, where git resolves it against the leg's remote, soexecutesaysadoption verifiedand the assembly's recursive clone fails unless the dependency happens to sit beside the leg #190's cause. A relative url that resolves alike from the source and from the leg, but names a repository that exists nowhere, verifies, and the assembly's recursive clone fails on it. The source's own recursive clone fails on it the same way (N1 above). The adoption is faithful to a source that is already broken. A possible follow-up: under--local-remote-dir,git ls-remotethe leg-side answer and NOTE one that answers nothing.org:nit is left, because it is not a one-line message. A badorg:reacheschecked_value, whoseunsafe-valuerefusal is the onemain'sexecutegives that plan (_repository_urls);checkmeets it only when a relative url makes it readorg:, one step earlier. A deletedorg:reads asNonethere, as inmain'sexecute, so the finding names thehttps://github.com/None/...remoteexecutewould use. Reportingorg:as acheckfinding for every plan is a change tomain's handling oforg:, and belongs with it.This lane's own review of b0c959e stopped before it finished. It named four candidates: P3-2, P3-4 and P3-1 above, and one more, "the tracked-remote rule is never reached in a real clone". That one is not a finding:
_default_branchreturnsorigin/HEAD's branch without itsorigin/prefix (adopt-project.py:515), so a clone'sbranch.main.remoteis the one read.This lane's read-only re-check of c2b5302 found it MERGE-READY: no BLOCKER, no SHOULD-FIX, and two NITs kept as documented trade-offs. First, under
--local-remote-dironly, P3-2 refuses six relative-originlayouts thatmainclones. Four are degenerate origins:.,./,x/.and../src/Thing.git/.. Two have a url that climbs more..than its origin spells:../Thing.gitwith../../dep, and../src/Thing.gitwith../../../dep. In each, the source's own working tree opens another repository or cannot resolve the url, and the refusal comes before any leg exists. Second, P3-4'sremote_keykeys two services on one host behind different ports alike, and two unbracketed IPv6 hosts that differ only in their last group. Noadoptanswer changes, because its leg side is always a GitHub url or a local path.same_repositorymoves only for a top-level repository behind a port, 5 of 2,556 pairs.Closes #190
🤖 Generated with Claude Code