Skip to content

validate-repository-naming.py, repo_shape.py: a --link-source value is PRODUCT=PATH only when its key names a chain link, and a link source that cannot be entered is a WARNING with the system's reason, not a traceback (#338) - #453

Merged
brettheap merged 6 commits into
mainfrom
fix/link-source-values
Oct 13, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openreposhape-5 (openRepoShape-5)
Claim: #338 (comment)

What was wrong

Two faults, one in the parser and one in the lookup.

  • The parser. _link_sources_from_args split every --link-source value at its first =. A link tree under a directory named a=b, passed bare, became a source keyed to a link a that no chain names. Nothing asked for it, so the link stayed declared-unverified, exit 0. The WARNING then offered --link-source openXdox=<path> to the person who had passed exactly that. A mistyped key failed the same silent way.
  • The lookup. repo_shape.link_manifest_path asked Path.is_dir() and then is_file(). Before Python 3.14 those swallow only ENOENT, ENOTDIR, EBADF and ELOOP. So each of these ended the naming gate in a traceback, exit 1:
    • a keyed source under a parent this user cannot enter;
    • SHAPE_PIN_SOURCE_<LINK> pointing there;
    • a checkout ../openXdox at mode 000, which every --project run asks for;
    • a 300-character name.

What changed

How a value is read (LinkSources in scripts/validate-repository-naming.py). This mirrors #312's PinSources, with a chain link where that file has a declared pin:

  • The text before the first = is a LINK only when it casefolds to a link of a chain this run reads. Those are the entries of --referent-chain, and of each leg's naming.referent_chain: under --project. Both are read before any leg is classified.
  • The keyed reading wins even over a directory spelled like the whole value. ./ reads that directory.
  • Any other value is a bare PATH, taken whole.

What a lookup says (scripts/repo_shape.py):

What the validator prints (stderr, after each name's WARNINGs):

  • WARNING link-source-unreadable: <path> (<how>) could not answer for <link>: <reason>. This covers a --link-source asked for a link that holds a pin, a SHAPE_PIN_SOURCE_<LINK> likewise, and a checkout beside the project that exists and still cannot answer. A keyed value that also names a directory gets a second line saying how to ask for that directory with ./. A bare value holding = says it was read as a PATH.
  • NOTE --link-source <typed>: not used — <why>. for a value no such link asked for. The why is one of five: a later value replaced it; no name reads that link's own declaration (a keyed value); no name records a chain at all; no name reads any chain link's own declaration (a bare value, where chains exist and none is read); or every link that is read had a more specific source.
  • Three kinds of source are never named: a variable or a checkout asked only for a chain's LAST link (no one ever reads that link's own declaration); a variable or a checkout for a chain no classification reads (fix round 1: a spec leg's chain, a name with no referent, a chain whose shape fails; a --link-source value for such a chain is the NOTE); and a checkout that is absent or holds no manifest.
  • --quiet drops both, with the WARNINGs. So make bootstrap and the doctor's naming row print nothing new.

--help, the module docstring and docs/cli.md (regenerated) say all of this.

Fix round 1 (lane 5's review of 40762ca: 0 blockers, 2 should, 4 nits)

Second commit, 4b21263. Each review finding, in order:

  • A link manifest whose neutral_product_pins: is not a list of names is a tree that cannot answer (SHOULD-1). link_declarations iterated data.get(key) or [] as it stood, against the link_pins_from_trees docstring this change wrote ("nothing a source does raises out of here"). 5 ended the naming gate, the scaffold and the project's validate-manifest.py in TypeError: 'int' object is not iterable. A scalar openDox was read as the characters D, e, n, o, p, x, so the chain broke, and under --project that is a FINDING. A mapping was read as its keys, so openDox: yes DECLARED openDox. Each is now (None, why), as not a mapping is: declared-unverified, with the reason named ("neutral_product_pins is not a list", "holds an entry that is not text"). An absent or null value, an empty list, and a blank entry (null, '') in a list, declare nothing, as ever. The fix is in the shared function, so the scaffold and validate-manifest.py are reached too.
    Three behaviours change on purpose here, and the tests of fix round 2 pin each:
    • A falsy value is no longer an empty list. neutral_product_pins: 0, false or '' was read as no pin at all (or []), so a chain through the tree was BROKEN, and under --project that is a FINDING. It is now a tree that cannot answer: the link is declared-unverified, a WARNING, exit 0.
    • An entry that is not text is no longer read as text. [openInk, 0] and [openInk, false] dropped the entry as a blank, and [openInk, true] read it as the text True, so a chain through the list VERIFIED. Each is now declared-unverified, with a WARNING saying the list "holds an entry that is not text". That is stricter than the link tree's own validate-manifest.py, which takes str(pin) of an entry (_declared_pins, templates/assembly-root/scripts/validate-manifest.py:85): a chain is better left unverified than verified through a name nobody wrote.
    • The scaffold no longer refuses chain-not-declared over a pins value that is a scalar. With --pin-source a tree whose neutral_product_pins: is the scalar openInk, the scaffold used to refuse (REFUSED chain-not-declared: ... openXdox declares I, e, k, n, o, p, not openInk, exit 2, from the characters). It is now the declared-unverified WARNING and the scaffold goes on, exit 0, which is its documented rule for a tree that cannot answer: "a warning, never a refusal, because the check is offline". The warning says the tree was reached and could not be read (fix round 2, below).
  • The reach wording is corrected (SHOULD-2): "Who else this reaches" below, and the docstring of the test that made the claim. What stops crashing is the naming and manifest steps. make validate, validate.yml and make bootstrap still end in a traceback on the sibling row before Python 3.14, at a file this change does not touch (below). On 3.14 the sibling is passed over without a word there instead, which is Every Path.is_file() / is_dir() / exists() guard in the shipped tools answers False to any OSError on Python 3.14 (3.12 and 3.13 raise PermissionError), so a directory this user cannot search reads as ABSENT: validate-pins.py skips its workflow @<sha> check, update-shape.py says copy-missing and update-pin-missing ("adopt the project") for files that are there, the doctor says DRIFTED (5 copy-missing), and bump-leg.py's remaining guards, and every other tool's, skip or misreport instead of refusing by name #430.
  • The mutation gaps (nit). Six of the review's ten mutations survived. Five now die, each by a new test: a project.yaml that is a directory; a device or a FIFO as the source; the order A, B, A; a value holding = with no chain at all; and every failure of SHAPE_PIN_SOURCE_<LINK> for a link that holds a pin (a path that is not there, a directory with no manifest, a file that is no manifest), not only a permission. The sixth, the typed in self.used test in unread_lines, could not be killed because it was dead: a value is read as one link's keyed source or is the one bare PATH, so every lookup asks it of the same path and it is used or unread, never both. That was asserted over 293 tests and the test is removed.
  • A chain no classification reads (nit). classify consults a link's own declaration only for a name that claims a referent, with a recorded chain whose shape holds. For any other chain (one recorded on a spec leg; one on a name with no open<Product> to reach; one that does not begin at a held pin or does not end at a referent of the name) a source was still asked, so a path that was not there was reported as link-source-unreadable and a good tree was counted used, each for a question nobody put. NamingPolicy.reads_link_declarations now says when resolve_referent reads a chain, and LinkSources.link_pins asks it first: such a chain asks nothing, and a value for it is the NOTE that no name reads that link's declaration. A test runs resolve_referent with a mapping that records whether it was read, over every way a chain is and is not, and holds the new method to it.
  • Dead code and stale sentences (nit). declared_neutral_pins is removed: git grep over the tree, every pushed branch and every sibling worktree finds no caller. link_manifest_lookup's docstring no longer says "replaces" over a name that is gone. This body no longer says tests/test_naming_policy.py takes SEARCH_DENIED from validate-pins.py: a --pin-source value is PRODUCT=PATH only when the text before its first = names a declared neutral-product pin, so a bare stand-in under a=b/ recomputes the digest instead of falling to gh api and a skip, an explicit source that does not exist or is not a directory is the neutral-pin-source-unreadable finding instead of a FileNotFoundError traceback, and a value no pin used is noted (#295) #312: it defines a mark of its own, with the same condition and its own reason text.

Fix round 2 (lane 5's re-review of 4b21263: 0 blockers, 0 should, 3 nits)

Third commit, 2ac911c. Each finding, in order:

  • The three behaviours the body did not state are stated and tested (nit 1). Round 1's not-a-list fix changed three behaviours on purpose, each listed under SHOULD-1 above, and the re-review found two of them had no test. R1 (if declared is None read as if not declared, which makes 0, false and '' declare nothing again) and R2 (if pin is None or pin == "" read as if not pin, which drops a 0 or false entry as a blank) both passed all 210 cases of tests/test_naming_policy.py at 4b21263. Six rows join NOT_A_LIST_OF_NAMES, which runs at the CLI and in process: 0, false and '' as the value, and 0, false and true among the names. R1 now fails 9 cases and R2 fails 6.
  • The scaffold's warning is true for a tree it reached (nit 1, the third behaviour). Over a --pin-source whose neutral_product_pins: is not a list, round 1 turned the chain-not-declared refusal into a declared-unverified WARNING that says the tree "is not reachable here (a sibling checkout, SHAPE_PIN_SOURCE_, or --link-source = would let it be read)". --pin-source did reach the tree, and only its declaration could not be read, so that blamed a path that was right. Now link_pins_from_trees takes a keyword-only with_reasons; asked, it returns a LinkPins, a dict equal to the plain mapping that also keeps unreadable, {link: why} for each link whose source answered and whose manifest could not be read. _chain_link_findings words such a link "which was reached but could not be read as a declaration ()", in a branch of its own, and only the scaffold asks. The new text is asserted for each of the eleven not-a-list rows, with scaffold-project.py --dry-run run as a subprocess.
    • Every other path's text is the bytes it was. The sentence for a link no source reached is the same code in the same lines, and four tests hold it: the generic warning for a plain mapping, for an empty LinkPins and for an absent tree; the scaffold's own old warning for a clone that holds no project.yaml; the naming validator's two lines (its warning, and the link-source-unreadable line that names the reason); and validate-manifest.py's. The naming validator and validate-manifest.py do not ask for reasons, so they print what they printed.
    • One more scaffold path changes with it, on purpose. A tree the scaffold reached and could not read as a declaration for any other reason (a manifest that does not parse, or is not a mapping) used to say "not reachable here" too, and was no truer. It now says what it is, with the system's reason.
    • _declaration still returns its four values, so nothing else in scaffold-project.py moved.
  • A qualified chain link in the spy test (nit 2). _chain_read_cases had no case whose chain link is qualified, so reads_link_declarations could take the chain as typed and not through _chain_links (R3) and pass every test. With --referent-chain testorg/openXdox,openDox that turns a verified chain into a declared-unverified one. ("codexDox", {"openXdox"}, ("testorg/openXdox", "openDox"), True) is a case now, and R3 fails it.
  • The body (nit 3): the ten corrections of the re-review are applied (the reason a lookup returns, the fifth NOTE reason, the third kind of silent source, the SHOULD-1 bullet, "before Python 3.14", the validate-pins.py bullet, the 3.14.7 rows, the merge-tree section, the filed numbers, and the project's own validate-manifest.py).

The mutation proofs: all three survivors were reproduced first at 4b21263 (210 passed each) and each dies at 2ac911c. Six mutations of the new code each die too: the warning ignoring the reasons (12 cases fail), the lookup recording none (12), the scaffold not asking (11), with_reasons on by default (3), an absent tree called reached (3), and the reached sentence used for a tree nothing reached (17).

Where it departs from #312, and why

Who else this reaches

The guard is in repo_shape.py, so the same link_pins_from_trees lookup stops crashing for these callers too:

  • The project's own validate-manifest.py. It reaches the lookup through SHAPE_PIN_SOURCE_<LINK> or a sibling checkout, and now gives its usual declared-unverified WARNING instead of the traceback.
  • scaffold-project.py --referent-chain. A SHAPE_PIN_SOURCE_<LINK> under a locked parent no longer kills the plan, and --pin-source verifies the link.

Neither of these prints the reason a source could not be entered; the scaffold says it only for a tree it reached and could not read (fix round 2, above). A --pin-source the scaffold cannot enter is still a traceback: that is #339 (pin_digest_from_source), and this PR does not touch it.

What this does not reach, and what the doctor says.

The two copied scripts this change edits (scripts/repo_shape.py and scripts/validate-repository-naming.py) are copied into every project and pinned there. A project takes the change through update-shape.py (two upstream-changed rows), and a family holder takes the repo_shape.py row. Neither file has a second copy in this repository.

Evidence

  • Issue rows, main ab623dd (repro338.sh, a gh stub first on PATH). Under 3.10.21 and 3.12.3, rows 4-7 (keyed under a mode-000 parent, SHAPE_PIN_SOURCE_OPENXDOX there, a sibling at mode 000, a 300-character name) are a traceback, exit 1. Rows 2-3 (a=b, the typo) are a silent declared-unverified, exit 0. Under 3.14.7 row 1 verifies, and rows 2-7 are the silent declared-unverified WARNING, exit 0.
  • Issue rows, this PR. Under 3.10.21, 3.11.16, 3.12.3, 3.12.14, 3.13.15, 3.14.7 and 3.15.0rc2 (first commit), all 7 rows exit 0 with no traceback, and the output is byte-identical across versions. a=b verifies. The other five rows each name their source with its reason. Re-run on the fix-round head, 4b21263, under 3.10.21, 3.12.3 and 3.14.7: byte-identical to the first commit's run. Re-run on the fix-round-2 head, 2ac911c, under the same three: all 7 rows exit 0 with no traceback, the output is byte-identical across the three versions, and it is identical to the re-review's runs of the previous head over main.
  • Reach on main. Main gives a PermissionError traceback, exit 1, for scaffold --dry-run, the project's validate-manifest.py, validate-repository-naming.py --project and its --quiet form, each with SHAPE_PIN_SOURCE_OPENXDOX under a locked parent. This PR gives exit 0 for all four. validate-pins.py on the sibling row is the exception said above.
  • New tests. 17 cases (11 functions) in the first commit; 16 fail on ab623dd. The fix round adds 31 cases in tests/test_naming_policy.py, and 17 of them fail on 40762ca: the five shapes of pins that are not a list (each at the CLI, in-process and, for the scalar, under --project), and the chains nobody reads. The other 14 guard what a mutation could break (below) or the side the fix must not move (an absent, null or blank pin list still answers).
  • Targeted run on 4b21263, files test_naming_policy, test_manifest_validator, test_scaffold_referent_chain_links, test_cli_reference, test_repo_hygiene, test_neutral_pin_validator, test_scaffold_e2e, test_shape_doctor, test_git_output_decoding and test_pin_validator: 765 passed, 3 skipped. The 3 skips are the PowerShell-only rows. (40762ca: 734 passed, 3 skipped.) render-cli-reference.py --check is ok.
  • Mutation probes, first commit. Reverting the stat guard, the parse, keyed-over-directory, the NOTE lines, the chain's-end restriction, the sibling-absence filter, the manifest reason, the --quiet drop, the "read as a PATH" sentence, the ./ line, the project-chain read, the WARNING lines or the env reporting each fails at least one new test.
  • Targeted runs on 2ac911c (fix round 2). Under 3.12.3, test_naming_policy (238 cases: 210 and 28 new), test_cli_reference, test_manifest_validator, test_scaffold_referent_chain_links, test_repo_hygiene and test_setup_project_py: 468 passed; and test_scaffold_e2e, test_scaffold_pin_and_reuse, test_scaffold_leg_path_collisions, test_scaffold_pin_collisions, test_neutral_pin_validator, test_git_output_decoding and test_pin_validator: 312 passed. Under 3.10.21 and under 3.14.7, test_naming_policy, test_manifest_validator and test_scaffold_referent_chain_links: 321 passed each. render-cli-reference.py --check is ok, and ast.parse(feature_version=(3, 9)) passes on the three changed files.
  • Mutation probes, fix round. The review's ten, re-run on 4b21263: MW1, MW2, MW3, MW5 and MW11 now die (they survived), and MW4, MW7, MW12 and MW13 still die; MW6 is the dead test, removed. (Fix round 2 kills the three the re-review found, R1 to R3, and six more: see above.) Nine more for the new code (the list check, the entry check, the null and blank tolerances, reads_link_declarations always true, without the shape check, without the held pins, and the validator ignoring it): eight die. The ninth, reads_link_declarations ignoring whether the name has a referent, survives as an equivalent mutant: with no referent _chain_shape_problem's last question always reports a problem, so the explicit early return is redundant, and is kept because resolve_referent has the same first line. The spy test holds the two together.
  • Windows. The mode-000 rows skip on Windows and as root, using a SEARCH_DENIED mark this file defines, with the same condition as validate-pins.py: a --pin-source value is PRODUCT=PATH only when the text before its first = names a declared neutral-product pin, so a bare stand-in under a=b/ recomputes the digest instead of falling to gh api and a skip, an explicit source that does not exist or is not a directory is the neutral-pin-source-unreadable finding instead of a FileNotFoundError traceback, and a value no pin used is noted (#295) #312's. The new FIFO and /dev/null rows skip where those do not exist. a=b and the 300-character name run everywhere. On Windows the long-name row asserts the line, not POSIX's reason. None of it was run on Windows or macOS.

Merge-tree

Recomputed at 19:04Z with git merge-tree --write-tree of 2ac911c against current origin/main 53bee5b and against every one of the other 37 pushed branches that touches a file this PR changes (a scan of each head against its merge-base with main):

Left out

Each of these was found by lane 5's reviews of this change, and is filed or folded:

Also not done here:

  • Python 3.9 (the floor) was not run because it is not installed here. The grammar of the three changed files was checked with ast.parse(feature_version=(3, 9)). Windows and macOS were not run.

Closes #338

🤖 Generated with Claude Code

Summary by Sourcery

Make link-source handling unambiguous and ensure inaccessible or invalid link trees produce actionable warnings instead of tracebacks.

New Features:

  • Interpret --link-source values as keyed sources only when their key names a chain link, while preserving paths containing = and reporting unused values.
  • Report unreadable link sources and their system-provided reasons as warnings or notes without changing successful validation into findings.

Bug Fixes:

  • Prevent link-source lookups from raising tracebacks for inaccessible, invalid, overlong, non-file, or malformed manifest paths.
  • Treat malformed neutral_product_pins declarations as unverifiable rather than crashing or inferring pins from invalid data.
  • Avoid querying or reporting sources for chains that no classification actually reads.

Enhancements:

  • Replace link manifest path probing with guarded, reason-returning lookups and optional source-trial diagnostics.
  • Improve scaffold warnings to distinguish trees that were reached but whose declarations could not be read from trees that were unreachable.

Documentation:

  • Update CLI documentation and help text with source parsing, diagnostics, and warning behavior.

Tests:

  • Add comprehensive coverage for source parsing, unreadable paths, malformed manifests, unused sources, unconsulted chains, and scaffold diagnostics across supported environments.

Chores:

  • Remove obsolete manifest lookup helpers and align shared source handling across naming validation, manifest validation, and scaffolding.

brettheap and others added 3 commits October 12, 2026 15:11
…lue is `PRODUCT=PATH` only when the text before its first `=` names a chain link, and a link source is asked with `stat()` inside a guard, so a link tree under `a=b/` verifies instead of staying declared-unverified, a source that cannot be entered (keyed, `SHAPE_PIN_SOURCE_<LINK>` or beside the project) is a `link-source-unreadable` WARNING with the system's reason instead of a `PermissionError` traceback before Python 3.14, and a value no link used is noted (#338)

`_link_sources_from_args` split every `--link-source` value at its first
`=`. A bare path holding one (`a=b/openXdox`) became a source keyed to a
link `a` that no chain names, which no lookup asked for: the link stayed
declared-unverified, exit 0, under a WARNING offering the very
`--link-source` that had been passed. A mistyped key failed the same
silent way. And `link_manifest_path` asked `is_dir()`, then `is_file()`,
which before 3.14 raise every `OSError` but ENOENT, ENOTDIR, EBADF and
ELOOP, so a keyed source under a parent this user cannot enter, a
`SHAPE_PIN_SOURCE_<LINK>` there, a checkout beside the project at mode 000
or a 300-character name ended the naming gate in a traceback, exit 1.

How a value is read now (`LinkSources`, #312's `PinSources` with a chain
link where that file has a declared pin):
- The text before the first `=` is a LINK only when it casefolds to a link
  of a chain this run reads: `--referent-chain`, or each leg's
  `naming.referent_chain:` under `--project`, read before any leg is
  classified.
- The keyed reading wins even over a directory spelled like the whole
  value; `./` reads that directory.
- Any other value is a bare PATH, taken whole.

What a lookup says now:
- `repo_shape.link_manifest_lookup` asks `stat()` inside the guard and
  returns the reason a source did not answer. It replaces
  `link_manifest_path`, whose two callers now need the reason; no other
  file named it. `declared_neutral_pins` keeps its signature on top of
  `link_declarations`, which also says why a manifest could not be read.
  `resolve_link_source` and `link_pins_from_trees` take a keyword-only
  `tried` that records each source asked, in order, with its reason;
  nothing in `repo_shape` prints.
- A `--link-source` asked for a link that holds a pin and unable to
  answer, a `SHAPE_PIN_SOURCE_<LINK>` likewise, and a checkout beside the
  project that is there and still cannot answer are each a
  `WARNING link-source-unreadable: <path> (<how>) could not answer for
  <link>: <reason>` on stderr, beside the declared-unverified WARNING.
- A `--link-source` no such link asked for is a `NOTE --link-source
  <typed>: not used — <why>.` on stderr: replaced by a later value, no name
  reads its link's declaration, no chain at all, or a more specific source
  answered. A value holding `=` read as a PATH says so.
- A variable or a checkout asked only for the chain's last link is never
  said, and a `--link-source` keyed to that link is a NOTE: its own
  declaration is never read, and a CI job may point
  `SHAPE_PIN_SOURCE_<REFERENT>` at a checkout with no manifest for
  `validate-pins.py`'s digest.
- `--quiet` drops both with the WARNINGs, so `make bootstrap` and the
  doctor's `naming` row print nothing new.

Where this departs from #312, and why:
- An explicit source that cannot answer stays a WARNING, exit 0, where
  `--pin-source` made it a finding. This validator's exit 1 says a name is
  wrong; `SHAPE_PIN_SOURCE_<PRODUCT>` also answers the digest question, from
  a checkout that need not carry a manifest; and the lookup falls through,
  so a later source may still verify the link. `--help` and the module
  docstring say so.
- The lookup still falls through to the next source, as it always has,
  where `--pin-source` keyed is final.
- The lines go to stderr as `WARNING`/`NOTE`, this file's spelling, where
  #312's notes are `  note` lines on stdout.

The guard is in `repo_shape.py`, so it also reaches the project's own
`validate-manifest.py` (`link_pins_from_trees(chain, root)`, the same
lookup, which died on the same traceback) and `scaffold-project.py
--referent-chain` (a `SHAPE_PIN_SOURCE_<LINK>` under a locked parent died at
the plan). Neither prints the reason; both stop crashing. A `--pin-source`
the scaffold cannot enter is #339 and is not touched. Both files are
COPIED into every project and pinned, so a project takes this through
`update-shape.py`. `docs/cli.md` is regenerated.

Tests in `tests/test_naming_policy.py`:
- A tree under `a=b/`, bare: verified, nothing said, by `--referent-chain`
  and under `--project`.
- `openXdox=x/openXdox` beside a directory of that name: the keyed
  reading's WARNING names the other reading; `./` verifies.
- The issue's mistyped key, alone and beside good sources: named, and
  read as a path.
- Replaced, repeated, chain-end and chainless values are NOTEs, exit 0;
  `--quiet` is silent.
- Keyed, env and sibling sources at mode 000 (POSIX, not root) and a
  300-character name: the WARNING with the system's reason, exit 0, no
  traceback.
- A link manifest at mode 000, not UTF-8, or not a mapping: named.
- A chain-end source with no manifest or at mode 000: never said.
- The project's own gate, `--quiet` and `validate-manifest.py` with a
  sibling at mode 000, and the scaffold's `--dry-run` with a locked
  `SHAPE_PIN_SOURCE_OPENXDOX`: no traceback.
- `link_pins_from_trees(..., tried=)` records each source and reason.

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…neutral_product_pins:` is not a list of names is a tree that cannot answer, named with its reason, so `5` is no longer a `TypeError` and a scalar `openDox` no longer reads as the characters `D`, `e`, `n`, `o`, `p`, `x`, and a chain no classification reads asks no source and says nothing of it, so a spec leg's chain no longer reports an `../openYdox` nothing needed (#338)

Fix round 1 on `fix/link-source-values`, from lane 5's review at 40762ca.

`link_declarations` iterated `data.get(key) or []` as it stood. `5` ended
the naming gate, the scaffold and `validate-manifest.py` in `TypeError: 'int'
object is not iterable`; a scalar `openDox` was the six characters, so the
chain broke ("openXdox declares D, e, n, o, p, x, not openDox") and under
`--project` that is a FINDING; a mapping was its keys, so `openDox: yes`
DECLARED `openDox`; a number among the names was its digits. All of that
contradicted the docstring this change wrote on `link_pins_from_trees`
("nothing a source does raises out of here"). A value that is not a list, or
a list holding an entry that is not text, is now `(None, why)`, like `not a
mapping`: the link is declared-unverified and the reason is named. An absent
or empty list, and a blank entry in one, declare nothing, as ever.

`NamingPolicy.reads_link_declarations` says when `resolve_referent` asks what
a link declares: only for a name in `<Domainx><Product>` form whose recorded
chain's shape holds. `LinkSources.link_pins` asks it first, so a chain the
classification never reads (one recorded on a spec leg, one on a name with no
referent to reach, one that does not begin at a held pin) asks no source:
a path that is not there was a `link-source-unreadable` WARNING and a good
tree was counted used, each for a question nobody put. Such a value is now
the NOTE it always should have been. A test holds `reads_link_declarations`
and `resolve_referent` together over every way a chain is, and is not, read.

`declared_neutral_pins` is removed: nothing in this tree, on any pushed
branch or in any sibling worktree calls it. `link_manifest_lookup`'s
docstring no longer says "replaces" over a name that is gone, and the dead
`typed in self.used` test in `unread_lines` is dropped (every lookup asks a
value of the same path, so it is used or unread, never both; asserted over
293 tests before it was removed).

Tests: 31 new cases in `tests/test_naming_policy.py`. 17 fail on 40762ca (the
pins that are not a list, and the chains nobody reads); the rest kill the
mutations the review found surviving (a `project.yaml` that is a directory,
a device or a FIFO as the source, A-B-A replacement, a value holding `=`
with no chain, and every failure of `SHAPE_PIN_SOURCE_<LINK>`, not only a
permission). The test docstring on the project's own gates no longer says
`make validate`, `validate.yml` or `make bootstrap` stop crashing on a
sibling at mode 000: their naming and manifest steps do, and they still end in
`validate-pins.py`'s unguarded `(sibling / ".git").exists()`, which is not
this change's.

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…and could not read as a declaration is named as reached and unreadable, with why, so `--pin-source` over a scalar `neutral_product_pins:` no longer warns that its own tree "is not reachable here", and the falsy-pins, falsy-entry and qualified-chain-link branches of fix round 1 are held by tests that fail when any is reverted (#338)

Fix round 2, from lane 5's re-review of 4b21263 (approve with nits, 0 blockers, 0 should, 3 nits).

THE WARNING. Fix round 1 made a link manifest whose `neutral_product_pins:` is not a list of names a tree that cannot answer, so the scaffold, which used to REFUSE `chain-not-declared` over a scalar `openInk` (read as the characters `I`, `e`, `k`, `n`, `o`, `p`), now warns declared-unverified and goes on. The warning it reused says the tree "is not reachable here (a sibling checkout, SHAPE_PIN_SOURCE_<LINK>, or --link-source <LINK>=<path> would let it be read)". That is true of a tree nothing found and untrue of one `--pin-source` did reach, where only the declaration could not be read, so it sent the person to fix a path that was right.

`link_pins_from_trees` takes a keyword-only `with_reasons`. Asked, it returns a `LinkPins`, a dict subclass equal to the plain mapping (an unread link is still absent from it) that also keeps `unreadable`, `{link: why}` for each link whose source answered and whose manifest could not be read: not a list, an entry that is not text, not a mapping, not parseable, not readable. `_chain_link_findings` words a link in it as "which was reached but could not be read as a declaration (<why>)", in a branch of its own, and leaves the sentence for every other link exactly as it was, in the lines it was in. Only the scaffold asks. The naming validator names the reason on a `link-source-unreadable` line of its own and `validate-manifest.py` prints none, so both print exactly what they printed, and so does the scaffold for a tree that is absent or holds no `project.yaml`. Nothing else in `scaffold-project.py` moved: `_declaration` still returns its four values.

THE TESTS. Three of reviewW2's mutations survived all 210 cases of `tests/test_naming_policy.py` at 4b21263. R1 (`if declared is None` read as `if not declared`) and R2 (`if pin is None or pin == ""` read as `if not pin`) restore two behaviours the first round changed on purpose and never tested: a `0`, `false` or `''` pin list was an empty list, so a chain through it was BROKEN, and is now unverified; a `0`, `false` or `true` among the names was dropped as a blank or read as the text `True`, so a chain through it VERIFIED, and is now unverified. Six rows join `NOT_A_LIST_OF_NAMES`, which runs at the CLI and in process. R3 (`reads_link_declarations` taking the chain as typed, not through `_chain_links`) survived because no spy case had a qualified link; `testorg/openXdox` is one now. The scaffold row is run for every row of `NOT_A_LIST_OF_NAMES`, with the exact warning asserted, and the other readers' text is held by four tests: the generic warning for a plain mapping, an empty `LinkPins` and an absent tree; the scaffold's own old warning for a clone with no `project.yaml`; the validator's two lines; and `validate-manifest.py`'s.

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

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-5, 2026-10-12T19:11Z) @codex review

@sourcery-ai

sourcery-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

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

You can request another review in 4 days and 2 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@sourcery-ai

sourcery-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR makes link-source interpretation chain-aware, hardens repository and manifest lookup against filesystem and data-shape errors, and reports unread or unused sources as non-failing diagnostics while preserving verification fall-through; it also updates scaffold behavior, documentation, and extensive regression tests.

Sequence diagram for chain-aware link-source resolution

sequenceDiagram
    participant CLI as Naming validator
    participant Sources as LinkSources
    participant Policy as NamingPolicy
    participant Shape as repo_shape
    participant FS as Filesystem

    CLI->>Sources: LinkSources(values, chain_links)
    Sources->>Sources: classify each value as keyed LINK=PATH or bare PATH
    CLI->>Sources: link_pins(policy, target)
    Sources->>Policy: reads_link_declarations(name, pins, chain)
    Policy-->>Sources: whether classification reads link declarations
    Sources->>Shape: link_pins_from_trees(chain, sources, tried)
    loop each chain link
        Shape->>Shape: resolve_link_source(link, tried)
        Shape->>FS: link_manifest_lookup(source)
        FS-->>Shape: manifest or system reason
        alt manifest available
            Shape->>Shape: link_declarations(manifest)
            Shape-->>Sources: declared pins
        else source or declaration unreadable
            Shape-->>Sources: fall through or record reason
        end
    end
    Sources-->>CLI: link pins plus source diagnostics
Loading

Sequence diagram for source diagnostics and scaffold warnings

sequenceDiagram
    participant Validator as Naming validator
    participant Sources as LinkSources
    participant Shape as repo_shape
    participant Scaffold as scaffold-project
    participant Report as stderr report

    Validator->>Sources: link_pins(..., tried=tried)
    Sources->>Shape: link_pins_from_trees(..., tried=tried)
    Shape-->>Sources: pins and attempted source reasons
    Sources->>Sources: record used, unread, and unanswered values
    Validator->>Report: unread_lines()
    Report-->>Validator: WARNING link-source-unreadable
    Validator->>Report: unused_lines()
    Report-->>Validator: NOTE source not used
    Scaffold->>Shape: link_pins_from_trees(..., with_reasons=True)
    Shape-->>Scaffold: LinkPins with unreadable reasons
    Scaffold->>Report: report reached-but-unreadable declaration
    Report-->>Scaffold: declared-unverified WARNING
Loading

Flow diagram for non-raising link manifest lookup

flowchart TD
    A[Source path] --> B[stat inside guard]
    B -->|missing or invalid path| C[Return system reason]
    B -->|directory| D[stat project.yaml]
    B -->|regular file| E[Use source as manifest]
    B -->|other file type| C
    D -->|missing| F[Return no project.yaml]
    D -->|not regular file| G[Return project.yaml is not a file]
    D -->|stat or filesystem error| C
    D -->|regular file| E
    E --> H[load_yaml and validate mapping]
    H -->|neutral_product_pins absent or empty| I[Return empty declarations]
    H -->|not a list or non-text entry| J[Return declaration reason]
    H -->|valid list of names| K[Return declared pins]
Loading

File-Level Changes

Change Details Files
Make --link-source parsing depend on known chain links rather than blindly splitting on '='.
  • Added LinkSources to resolve CLI and project referent-chain links before classifying targets.
  • Treat only matching chain-link prefixes as PRODUCT=PATH; preserve other values, including paths containing '=', as bare paths.
  • Track replacement, usage, unreadable, and unused source values for diagnostics, including ./ disambiguation and --quiet behavior.
scripts/validate-repository-naming.py
docs/cli.md
Make link-source and manifest lookup failure-safe while preserving fall-through semantics and exposing reasons.
  • Replaced is_dir/is_file probing with guarded stat-based lookup covering permission, path-length, device/FIFO, and malformed manifest cases.
  • Return explicit reasons for inaccessible sources, missing manifests, invalid manifest types, and invalid neutral_product_pins values without raising.
  • Add ordered tried-source tracking and optional LinkPins unreadable metadata for callers such as the scaffold.
scripts/repo_shape.py
Prevent unrelated or unreadable chains from producing incorrect classification failures and improve scaffold reporting.
  • Added reads_link_declarations to avoid querying chains that no classification uses.
  • Treat non-list or non-text neutral_product_pins declarations as declared-unverified warnings rather than iterating invalid data or creating findings.
  • Update scaffold warnings to distinguish a reached tree whose declaration could not be read from a tree that was never reachable.
scripts/repo_shape.py
scaffold-project.py
Add broad regression coverage for parsing, lookup safety, diagnostics, malformed declarations, and downstream callers.
  • Cover equals-containing paths, keyed-versus-directory precedence, mistyped and replaced values, source usage notes, quiet mode, environment/sibling sources, and inaccessible or invalid paths.
  • Cover malformed neutral_product_pins values, qualified chains, chains not read by classification, and reached-versus-unreachable warning text.
  • Exercise naming validation, manifest validation, scaffold dry runs, direct lookup APIs, and CLI documentation generation.
tests/test_naming_policy.py
docs/cli.md

Assessment against linked issues

Issue Objective Addressed Explanation
#338 Interpret each --link-source value as PRODUCT=PATH only when PRODUCT names a chain link; otherwise treat the whole value as a bare path, with ./ available to force the path reading. ✅
#338 Report every --link-source value that no link used, including why it was unused, without turning it into a finding. ✅
#338 Handle unreadable or invalid keyed, bare, environment, and sibling sources without tracebacks, reporting the system’s reason as a warning; document the behavior in help and the CLI reference. ✅

Possibly linked issues


Tips and commands

Interacting with Sourcery

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

Customizing Your Experience

Access your dashboard to:

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

Getting Help

… read" warning is held for a link manifest that is not a mapping and for one that does not parse, so narrowing it to the not-a-list reasons, which passed every earlier case, fails four (#338)

TEST ONLY. No script changes.

The scaffold's declared-unverified warning says a tree "was reached but could
not be read as a declaration" for every tree its source answered and whose
manifest could not be read, not only for a `neutral_product_pins:` that is not
a list: a manifest that is not a mapping, or does not parse, said "is not
reachable here" of a file the scaffold had just opened, on main and at 4b21263,
and says what it is now. That was a decision of the last fix round, and
reviewW2's re-verification of 2ac911c found nothing held it: a mutation that
records `unreadable` only when the reason ends in "is not a list" or "is not
text" (the narrower reading, one line in `link_pins_from_trees`) survived all
238 cases of `tests/test_naming_policy.py`, because every row of
`NOT_A_LIST_OF_NAMES` has one of those two reasons.

`UNREADABLE_AS_A_DECLARATION` has the two other kinds, each a manifest the
repository's own parser reads the same on every platform: `- openInk` (not a
mapping) and a `neutral_product_pins: [openInk` that never closes (not a flow
collection). Each runs through `scaffold-project.py --dry-run` with
`--pin-source` a git clone, asserting the exact warning with the reason and no
"not reachable here", and through `link_pins_from_trees(..., with_reasons=True)`
in process, asserting `unreadable` and the warning built from it.
`_scaffold_with_link_tree` takes `raw=True` for a manifest that must not begin
with the two-line header every other row carries.

With the narrowing applied, those four cases fail and the other 238 pass, which
is the survivor reproduced; with it reverted, all 242 pass. Targeted runs on
this head: `test_naming_policy`, `test_cli_reference`, `test_manifest_validator`,
`test_scaffold_referent_chain_links`, `test_repo_hygiene` and
`test_setup_project_py` 472 passed; `test_scaffold_e2e`,
`test_scaffold_pin_and_reuse`, `test_scaffold_leg_path_collisions`,
`test_scaffold_pin_collisions`, `test_neutral_pin_validator`,
`test_git_output_decoding` and `test_pin_validator` 312 passed.

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
…(), which both roots' bootstrap.py now import instead of defining twice (#391)

Why. SonarCloud failed PR #456 on duplication (14.7% on new code against
a 3% gate): spelled() and escape_unencodable_stdout(), with the two sets
spelled() reads, were written out once in each root's
scripts/bootstrap.py, and a test held the two copies identical. The
premise behind the copies, that a root cannot import anything of this
repository's, was false: every project and every family holder carries
scripts/repo_shape.py, the pinned shared library, and both bootstraps
already import Refusal, find_repo_root and load_yaml from it. Brett
Heap's decision (2026-10-12): move the two helpers into repo_shape.py
and have both bootstraps import them; no Sonar exclusion.

Where. A new section at the END of scripts/repo_shape.py, after die():
ESCAPED_CATEGORIES, ESCAPED_BIDI_CONTROLS, escape_unencodable_stdout()
and spelled(), copied from the bootstrap source byte for byte. Their
code is unchanged (the AST of each, docstrings aside, equals both old
copies); only the docstring sentences the move made untrue are reworded
("this file", "this script", "the same in the assembly root's
bootstrap.py and the family root's"). `import unicodedata` joins the
imports after `import sys`, and the module docstring's "WHAT THIS
MODULE OWNS" list gains one bullet after `Refusal`. Those three places
are clear of the open PRs that also edit this file (#394, #409, #416,
#453): each of them merges with this commit without a conflict in
repo_shape.py.

Both templates/*/scripts/bootstrap.py lose the two definitions, the two
sets and `import unicodedata`, and add the two names to their existing
`from repo_shape import` line (the family root's becomes the
parenthesised form). escape_unencodable_stdout() is still main's first
statement, and the module level is still only `sys.path.insert` and the
import. Against origin/main, each bootstrap now differs only in that
import line and the spelled() call sites.

Tests (tests/test_bootstrap_spelling.py). The test
test_the_two_roots_carry_the_same_function is replaced by an ast
check that each root's
`from repo_shape import` names both and that nothing in the file
defines or assigns either function or either set; one more test per
root asserts that the name its module exposes IS repo_shape's object.
The unit rows of spelled() (keep-raw, escape, the full code-point sweep,
the non-string values, the one-line check) and of
escape_unencodable_stdout() now run once against repo_shape itself
instead of once per root. The end-to-end rows run a real scaffolded
project and a real family holder, which carry scripts/repo_shape.py, so
they exercise the moved code where it ships; they and the bytecode
regression test are unchanged.

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
…ncode and spells each value it prints, so `name: "a\ud83db"` or `日本` under `PYTHONIOENCODING=ascii` no longer ends `make bootstrap` in a traceback (#391) (#456)

`templates/assembly-root/scripts/bootstrap.py` `main` opened with `print(f"bootstrap: {name} ({root})")`, before any step ran, and `templates/family-root/scripts/bootstrap.py` with the same line (`:180`, with a literal dash in it).

- `name: "a\ud83db"` reads as a string holding a lone surrogate, which no UTF-8 stdout can write. `scripts/bootstrap.py` exited 1 with `UnicodeEncodeError`, `make bootstrap` exited 2, and `validate-manifest.py`, the validator that names the problem, was never reached.
- `日本` under `PYTHONIOENCODING=ascii` ended the same way.
- The family root's line holds a literal `—`, so under that stdout it raised for EVERY name (`InkRouter` included). The assembly root's own wording has the same dash in two lines and a middle dot in the register readout.
- A raw ESC or CR in a name is encodable on every stdout and reached the terminal as the control it is (`bootstrap: a^[b^Mc (...)`).

Both files are shape copies, digest-pinned in the project that holds them, and a project carries `scripts/repo_shape.py` and no `shape_materialize.py`, so neither can call #214's helper. Both already import `Refusal`, `find_repo_root` and `load_yaml` from that `repo_shape.py`.

`scripts/repo_shape.py`, the pinned shared library every project and every family holder carries beside its `bootstrap.py`, gains two functions and the two sets one of them reads (a new section at its end, `scripts/repo_shape.py:2524`). Both roots' `bootstrap.py` import the two functions on the `from repo_shape import` line they already had, and define neither:

- `escape_unencodable_stdout()` is the first statement of each root's `main`. A `strict` or `surrogateescape` stdout becomes `backslashreplace`, the handler stderr already has: what the stdout holds is written as it was, what it cannot hold as `\u65e5`. Any other handler is left alone and the encoding is never changed. It is `validate-pins.py`'s guard from #295 (`validate-pins.py:851`), which is inline there and cannot be imported.
- `spelled(value)` writes the escape Python spells (`\x1b`, `\r`, `\n`, `\u2028`, `\ud83d`, `\u202e`) for EXACTLY four kinds of character, chosen by Unicode category and not by `str.isprintable`:
  - the controls (Cc: C0, DEL and C1);
  - the surrogates (Cs);
  - the line and paragraph separators (Zl, Zp: U+2028 and U+2029);
  - the bidirectional controls: U+061C, U+200E, U+200F, U+202A to U+202E and U+2066 to U+2069.

  Every other character prints as it did before this change: the zero-width non-joiner (U+200C) a Persian name needs, the zero-width joiner (U+200D) of an emoji sequence, the ideographic space (U+3000) of `山田` U+3000 `太郎`, the no-break space (U+00A0), the soft hyphen (U+00AD), any other format, space or unassigned character, and a backslash. The four categories are fixed ranges, so the answer does not move with the interpreter's Unicode database; `unicodedata` is the standard library's.

Helper decision: `repo_shape.double_quoted` / `_escaped` (`scripts/repo_shape.py:347`, `:371`) was read first and still cannot serve: it quotes and escapes `"` and `\`, so a plain printable name would not stay byte-identical. The two new helpers now live beside it, in the same file, the one library both roots import from. `shape_materialize.escape_what_stdout_cannot_encode` is in no project; `shape_advisory.printable` flattens to `?` and is in no copy list.

Every print of a value the script read from the manifest, a register row or the disk is spelled:

- assembly root: the name and the root (first line); a leg's role, path and tracking branch in step (a); a register row's holder, act, objects, state and expiry, the register's path, and the reader's reason when it cannot be read.
- family root: the name and the root (first line), a member's project and directory name, `--members-make` (heading and failure line). The member's own command is still handed the RAW target.

A plain name prints exactly what it printed before: the new tests spell out the whole output of a plain project and a plain holder, and that golden also passes on the base. So does every valid name that holds one of the characters kept raw above (see the tests).

**Files.**

- `scripts/repo_shape.py`: `escape_unencodable_stdout()`, `spelled()`, `ESCAPED_CATEGORIES` and `ESCAPED_BIDI_CONTROLS`, in a new section after `die()`; `import unicodedata`; one bullet in the module docstring's "WHAT THIS MODULE OWNS".
- `templates/assembly-root/scripts/bootstrap.py` and `templates/family-root/scripts/bootstrap.py`: both names on the existing `from repo_shape import` line (the family root's becomes the parenthesised form); `escape_unencodable_stdout()` first in `main`; every print of a value read from the manifest, a register row or the disk goes through `spelled`. Against origin/main each differs in nothing else.
- `tests/test_bootstrap_spelling.py` (new).

`docs/cli.md` is unchanged: no `--help` text moved.

**Fix round 1 (review of 118c85a).** The first head's `spelled` wrote as an escape every character `str.isprintable` refuses. Independent review (its S1) showed that is wider than controls, surrogates and line separators: it also refuses every space but ASCII space, every format character and every unassigned code point, so a valid name that `validate-manifest.py` accepts and prints raw on the very next lines was escaped by this script's first line (a Persian name with its ZWNJ, an emoji sequence with its ZWJ, `山田` U+3000 `太郎`, a no-break space, a soft hyphen). The printed `git submodule update --init <path>` command was then wrong to paste for such a leg path, and what was escaped moved with the interpreter's Unicode database (Python 3.9 ships Unicode 13, 3.12 ships 15). The claim "prints byte for byte" was false for those names.

**The coordinator decided, and Brett may reverse it:** escape by category, not by `str.isprintable`, the four kinds listed above. Everything else prints raw, as on the base. The bidirectional controls stay escaped on purpose, so a name cannot reorder the text around it on a terminal. The alternative the review offered, and this does not take, is to keep `isprintable`, correct the claims, and pin ZWNJ and U+3000 as escaped by a test row.

The other round-1 changes: the lone-surrogate rows now assert `FINDING manifest-name` and no `Traceback` unconditionally, since #280 landed in #390 (cb7063f), and the `make bootstrap` row asserts there is no `Traceback` either; the docstrings, the test module's docstring and the unit rows say what is escaped and what is not. The branch has merged origin/main (53bee5b, which carries #390); the merge touched neither bootstrap file.

**Fix round 2 (CI and SonarCloud).**

- **CI** (2733c6a). The test module loaded each root's `bootstrap.py` through `importlib`, which wrote `templates/*/scripts/__pycache__/bootstrap.cpython-*.pyc`, and `tests/test_scaffold_leg_path_collisions.py`, which reads every file of the assembly template as UTF-8, then failed on it. `load()` now sets `sys.dont_write_bytecode` around the load, and a regression test asserts that loading either root, from the tree and from a copy in an empty directory, leaves no file behind.
- **SonarCloud** (d0aea3e). The quality gate failed on duplication, 14.7% on new code against 3%: `spelled()` and `escape_unencodable_stdout()`, with the two sets, were written out once in each root, and a test held the two copies identical. The premise for the copies, that a root cannot import anything of this repository's, was false: both bootstraps already import from `scripts/repo_shape.py`, which every project and every family holder carries. **Brett Heap decided** to move the two helpers into `repo_shape.py` and have both roots import them, with no Sonar exclusion. The code moved byte for byte (the AST of each function and set, docstrings aside, equals both old copies); only the docstring sentences the move made untrue are reworded. The new section, `import unicodedata` after `import sys`, and the docstring bullet after `Refusal` sit where none of the open pull requests that also edit `repo_shape.py` edits: #409, #453 and #394 each merge with this head with no conflict, in that file or any other (`git merge-tree --write-tree` against each branch's head, 2026-10-12). #416 landed as 580d7df without touching `repo_shape.py`.
- The branch merged origin/main ec8e09a (e4b0386) first; that merge touched none of this pull request's files. `git merge-tree --write-tree` of this head with current main is clean (2026-10-12).

**How it was tested.** `tests/test_bootstrap_spelling.py` (new, 205 tests at this head, local bare remotes, no `gh`). Answered in bytes, stdout decoded strictly in the encoding the run was given (stderr with `replace`), under `utf-8:strict`, `utf-8:surrogateescape`, `cp1252:surrogateescape`, `cp1252:strict`, `ascii:strict` and `utf-8:replace`:

- name rows in a real scaffolded project and a real family holder:
  - spelled: plain, lone surrogate, ESC and CR, `日本`, newline/tab/DEL, U+2028, the right-to-left override (U+202E) with the right-to-left mark (U+200F), an emoji, a quote and a backslash, two lone surrogates;
  - kept raw, as on the base: a no-break space (on all six stdouts, the cp1252 ones hold it), a Persian name with its ZWNJ, an emoji sequence with its ZWJ, `山田` U+3000 `太郎`, a soft hyphen, a zero-width space with a byte-order mark. For each, the first line is compared in BYTES with what the plain `print` it replaced wrote (assembly and family), and a full run with the validators shows the validator's own `manifest ok: <name>` line raw beside it;
- every other print above: a leg's role, path and tracking branch (and the four lines that name a branch, one with a remote-tracking ref elsewhere), a register row, a register's path and the reader's reason, the root's own directory name in both roots, a member row and `--members-make` (with `MAKE` the interpreter, so a member runs only if the RAW target reached it);
- the run reaches the validators: a lone-surrogate name prints `FINDING manifest-name`, `make bootstrap` exits 2 and `bootstrap FAILED: manifest (exit 1)`, with no `Traceback` and no frame in `bootstrap.py`;
- `spelled` and `escape_unencodable_stdout` directly, in `repo_shape` itself, once rather than once per root: every code point swept once against a set written from the specification (explicit ranges), which is itself checked against `unicodedata.category`; that each root's `from repo_shape import` names both and that neither root defines or assigns either function or either set (an `ast` check of the file); that the name each root's module exposes IS `repo_shape`'s object; that the reconfigure is `main`'s first statement; and that loading a root writes no bytecode under `templates/`.

The end-to-end rows run a real scaffolded project and a real family holder, and each carries `scripts/repo_shape.py`, so they run the moved code where it ships.

Results (`Python 3.12.3`, Linux; head d0aea3e = the move, on e4b0386 = the merge of origin/main ec8e09a):

| run | result |
|---|---|
| `test_bootstrap_spelling.py`, `test_scaffold_leg_path_collisions.py`, `test_stdout_encoding.py`, `test_template_quoted_values.py`, `test_template_values_single_pass.py`, `test_repo_hygiene.py`, `test_agents_md_codes.py` | 684 passed, 1 skipped (`tests/test_stdout_encoding.py:286`, Windows-only) |
| the five `test_yaml_*.py`, the six `test_update_shape*.py`, `test_scaffold_e2e.py`, `test_family.py`, `test_validate_family_unnamed_mounts.py`, `test_cli_reference.py`, `test_windows_paths.py` | 706 passed, 3 skipped (`tests/test_update_shape_add_symlink.py:831`, NTFS junctions) |
| mutation: `repo_shape.spelled` escapes nothing | 61 failed, 144 passed: 29 unit rows (every escape row, the code-point sweep, the non-string and one-line rows) and 32 end-to-end rows in a scaffolded project and a family holder (the name lines, a leg's role and path, a branch, a register row and a register's path, the root's directory, the member rows) |
| `test_bootstrap_spelling.py` again, the mutation reverted | 205 passed |

Earlier heads, not re-run on this one: on af26b19 the then 260-test file gave 197 failed, 63 passed against the base (cd1fc79) templates, 59 failed with both copies' `spelled` reverted to `str.isprintable`, and caught all 12 mutations of the category sets; reverting each hunk of the first round's fix, one at a time, still failed a new test (34 mutations, listed in the report); on 118c85a the existing bootstrap, scaffold, adopt, update-shape and doctor tests gave 496 passed, 1 skipped, and 305 passed. macOS and Windows have not run here; CI runs them on this head. Every new test reads and writes bytes and compares against `project.resolve()`.

What the rest of the run says (the lone-surrogate and `make bootstrap` rows assert it at this head): a lone-surrogate name prints `bootstrap: a\ud83db (<root>)`, reaches step (b), and ends

```
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. ...
bootstrap FAILED: manifest (exit 1)
```

exit 1 (`make bootstrap` 2), with no `Traceback` on a pipe, a TTY or `make`.

**What is left out.**

- The validators' own stdout. `validate-manifest.py`'s `manifest ok: <name>` still prints through a stdout with no handler: a valid `日本` under `ascii` ends in ITS traceback (the issue's "Not in this issue"), and an ESC or CR name is accepted and the line writes the raw control.
- A manifest value that is passed to git rather than printed: `tracking_branch: "a\ud83db"` (or a leg's `branch:`) raises from `subprocess` inside step (a), before the validators; a NUL does too (`ValueError: embedded null byte`). Reproduced on origin/main 53bee5b. `validate-manifest.py` never reads either key, so it accepts the manifest (`manifest ok`, exit 0) and NO tool names the value. Not a `print`; follow-up below.
- stderr: `print(str(exc), file=sys.stderr)` (Refusal text built by `repo_shape`, multi-line, manifest text `repr`-escaped; stderr cannot raise) and the family's `FETCH INCOMPLETE` text (git's own words).
- `templates/family-root/scripts/siblings.py`, which carries the SAME first line twice (`:676` in `dispatch_all`, `:906` in `_run_placement`) with the same plain `print`, and prints member rows raw. `make siblings`, `make park` and `make resume` on `name: "a\ud83db"` still end in a `UnicodeEncodeError` traceback (reproduced on origin/main), and under `PYTHONIOENCODING=ascii` they end so for every name (the literal dash). It already imports from `repo_shape`, where both helpers now are, so they are two more names on its existing import line. Follow-up below.
- Behaviour that differs from the base, all deliberate:
  - a control, a surrogate, a line or paragraph separator or a bidirectional control in a value now prints as its escape where the base wrote it raw, so a Hebrew or Arabic name that holds an explicit U+200E, U+200F or U+061C prints `\u200f` for it;
  - a root path holding a byte that is not UTF-8 prints `\udcXX` where the root `bootstrap` shim's #214 handler wrote the raw byte back;
  - a name holding the four characters `\x1b` prints as one holding ESC does;
  - the wording's dash and middle dot print `\u2014` and `\xb7` on a stdout that cannot hold them, instead of raising.
- The digest: no digest of either bootstrap or of `repo_shape.py` is recorded in this repository. The scaffold, `family.py init` and adopt compute each `files:` row from the bytes they write, and `update-shape.py` compares upstream blobs at two commits. A project scaffolded from origin/main 580d7df, checked against this head (d0aea3e), reads `upstream-changed` for `scripts/bootstrap.py` and `scripts/repo_shape.py` and nothing else (assembly root: 2 changed, 9 unchanged; family holder: 2 changed, 8 unchanged). `apply` copies both and re-pins, the project's own validators say `11 copied shape file(s) match their digests` (the holder's, 10), each copy equals its upstream blob, and `scripts/bootstrap.py` runs. Run for real, local remotes only.
- Compatibility: a project that keeps a locally edited `scripts/repo_shape.py` (`check` reads it `both`, and `update-shape.py apply --accept-local scripts/repo_shape.py` keeps the project's bytes) while taking the new `bootstrap.py` meets an `ImportError` the next time it bootstraps: `scripts/bootstrap.py` ends in `cannot import name 'escape_unencodable_stdout' from 'repo_shape'`, exit 1. Following `check`'s own instruction avoids it: `check` says to merge upstream's bytes into the edited file by hand, commit it, and only then `apply --accept-local`, and a merge that takes the new section brings both names. A project that skips the merge gets the error and nothing reports the break after `apply`: its validators still pass, because they import neither name; `check` then reads everything unchanged; and the project's `validate.yml` never runs `bootstrap.py`, so its CI stays green. The first command to fail is the one run after `git clone`. Guarding it is an item on #458's checklist (#458 (comment)): `apply` would refuse a kept `--accept-local` copy that lacks a name another copied shape file imports from it. It is the same coupling as the names the bootstraps already import from that file. Run for real on a project scaffolded from 580d7df.

Follow-ups found by lane 5's review of this branch, each reproduced on origin/main and filed separately:
- `siblings.py`'s two first-line prints (and its member rows): `make siblings`, `make park` and `make resume` on a lone-surrogate `family.yaml` name end in a `UnicodeEncodeError` traceback (#454).
- A lone surrogate or a NUL in `tracking_branch` (or a leg's `branch:`) ends `make bootstrap` in a subprocess traceback in step (a), and `validate-manifest.py` accepts the manifest (#455).
- #454 and #455 (open, off the milestone) and the two #459 checklist items moved from #463 and #464 can import `spelled` and `escape_unencodable_stdout` from `repo_shape.py`, wherever they print a value.

Reviewed adversarially at 118c85a (CHANGES NEEDED: 0 blockers, 2 should-fixes, 2 nits); fix round 1 (af26b19) was re-verified APPROVE WITH NITS (0/0/2). Fix round 2 stopped the test module writing bytecode into `templates/` (2733c6a) and moved the two helpers into `scripts/repo_shape.py` on Brett Heap's decision (d0aea3e); its independent re-verification read APPROVE WITH NITS (0/0/3), all three about this text and settled here. CI is green on Linux, macOS and Windows, with SonarCloud passing, at d0aea3e (main ec8e09a merged in).

Closes #391

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

scripts/repo_shape.py was the one file that conflicted, in four hunks where
this branch's `tried` and `with_reasons` meet #409's `env_shared`; each takes
both sides, as the review of 2ac911c resolved it:
- the `resolve_link_source` signature is `default_source=None, env_shared:
  bool = False, *, tried: list | None = None` (`env_shared` stays positional,
  because #409's callers pass it positionally, and `tried` stays keyword-only);
- its docstring keeps this branch's two #338 paragraphs, then #409's
  `env_shared` paragraph (#275);
- the `link_pins_from_trees` docstring keeps this branch's three paragraphs,
  then #409's, and the body opens with `found`, this branch's `unreadable`, and
  #409's `links` and `shared`;
- the call is `resolve_link_source(name, root, keyed_sources, default_source,
  pin_product_key(name) in shared, tried=asked)`, then `tried[name] = asked`.
A variable two links share is never a candidate, so `tried` holds no env entry
for it and nothing is reported, which is #409's intent kept whole. `docs/cli.md`
and scaffold-project.py merged on their own; #409's reached-warning edit in
`_chain_link_findings` and this branch's reached branch sit in different lines.

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

Copy link
Copy Markdown

@brettheap
brettheap merged commit cac6bb2 into main Oct 13, 2026
5 checks passed
@brettheap
brettheap deleted the fix/link-source-values branch October 13, 2026 03:23
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment