Repository navigation
The doctor reads where a file LIVES against where the policy puts it: a placement row, code-in-spec and spec-in-code, and a plan the human resolves (#99) - #100
Merged
Conversation
… a `placement` row, code-in-spec and spec-in-code, and a plan the human resolves (#99) Brett Heap, 2026-09-10: "what about openRepoShape --doctor that will fix the repo back into the correct 3leg version. we have to look for code in spec and spec in code". The doctor landed read-only in #96; this is the READING half of that sentence, and it moves nothing. A new row, `placement`, on an assembly root. For each mounted leg it takes the tracked paths with ONE `git ls-files`, classifies them against `contracts/path-classification.yaml` -- the same policy `adopt-project.py plan` uses, walked with that tool's own `walk()` by import rather than a second copy that would drift -- and names every path the policy would have put somewhere else, with the rule id that judged it. The fold is the adoption's: a directory whose files agree is ONE path however many files are under it, so a leg of several thousand reads in a line and answers in seconds. `ok` when nothing is misplaced, `note` when the only paths are ones the policy will not call (a question for a human is never a finding), FINDING otherwise -- and then the verdict is `MISPLACED (n paths)`, exit 1, under DRIFTED and over INVALID. A leg off its pin outranks it because the paths this row read are then not the paths the pin describes; a red validator does not, because it names itself in the table either way while nothing else here can see where a file lives. What belongs to EVERY repository is never judged: the four files `templates/spec-root/` and `templates/code-root/` themselves ship, READ FROM THOSE TEMPLATES at run time so the scaffold's own output can never be a finding, plus the front door and the forge furniture -- `LICENSE`, `.gitattributes`, `CODEOWNERS`, `.github/**`, which a spec leg carries because it runs its own CI. `--json` prints the list under `everywhere`. `--placement-plan <file>` writes the paths as an adoption-plan-style YAML: the entries are `adopt-project.py plan`'s entries exactly, plus `in_leg:` and an empty `resolution:`, so resolving one teaches the other. It carries `kind: placement-plan` and NOT `adoption-plan`, deliberately -- an adoption plan is the input to `adopt-project.py execute`, which creates repositories and rewrites history, and a file that called itself one would be that command aimed at a project already split. `adopt-project.py` refuses the kind by name and the refusal says which tool consumes one. It is the only thing this command writes, it writes only the file named, and it creates no directory. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…l test for a root that has no placement question at all The footer's provenance line records which commit each regeneration of the page was cut at, so it names this one. And the `row is None` branch of `write_placement_plan` -- a directory that is not a shape root, which has no `placement` row rather than an `n/a` one -- says a different sentence from the family holder's and now has a test that reads it. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…acement plan says it is empty `check_placement` had the per-leg loop inline with four ways out of it, which put a function's worth of branching inside a function whose job is to decide a status. `audit_leg` is that loop: one leg in, its summary row and its two lists out, and each of the three reasons a leg cannot be audited returns from its own line. Nothing about the report changes. Two smaller ones beside it. `placement_row` took a `check_id` that was the same string at every call. And a plan with no entries wrote a bare `paths:` with nothing under it — a value the YAML reader returns as null and a reader would have to interpret; it now writes `paths: []` and one line saying there is nothing here to resolve. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 3 days and 14 hours by commenting @sourcery-ai review. Upgrade to get a review now.
Contributor
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideIntroduces a shared-policy placement row for shape-doctor that finds code/spec paths in the wrong mounted leg, reports MISPLACED findings without moving repository content, and emits a distinct human-resolvable placement plan with extensive documentation and safety-focused tests. Sequence diagram for placement auditing and plan generationsequenceDiagram
participant Human
participant Doctor as shape-doctor.py
participant Root as Assembly root
participant Leg as Mounted leg
participant Git as git
participant Policy as adopt-project.py policy
participant Plan as Placement plan
Human->>Doctor: --placement-plan FILE
Doctor->>Root: check_placement()
loop each mounted spec/code leg
Doctor->>Git: git ls-files -z
Git-->>Doctor: tracked paths
Doctor->>Policy: PathPolicy.load(path-classification.yaml)
Doctor->>Policy: walk(policy, tracked paths)
Policy-->>Doctor: folded paths and rule decisions
Doctor->>Doctor: classify misplaced paths and notes
end
alt misplaced paths found
Doctor->>Plan: write_lf(FILE)
Plan-->>Human: unresolved entries with in_leg and resolution
Doctor-->>Human: MISPLACED, nothing moved
else
Doctor-->>Human: no misplaced paths: OK or note, nothing moved
end
Flow diagram for placement findings and verdict precedenceflowchart TD
A[Run shape-doctor] --> B[Audit mounted legs]
B --> C[git ls-files once per leg]
C --> D[Shared adoption walk and path policy]
D --> E{Path belongs elsewhere?}
E -->|No, policy undecided| F[note; human review required]
E -->|No, path is in correct leg| G[placement ok]
E -->|Yes| H[placement finding with path and rule]
H --> I[--placement-plan writes unresolved YAML]
I --> J[Human resolves entries]
J --> K[Future fix uses paired leg PRs and root pin bump]
H --> L{Higher-priority finding?}
L -->|Leg off pin or drift| M[DRIFTED]
L -->|No| N[MISPLACED exit 1]
N --> O[INVALID findings remain below MISPLACED]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Author
|
Merged Both hygiene caps conflicted and both were resolved by KEEPING BOTH dated entries in the order they landed, rather than letting this branch's entry stand alone: AGENTS.md 400 → 408 (#97) → 423 (#99), README.md 1324 → 1328 (#97) → 1374 (#99). @codex review |
…and both caps chained rather than replaced #97/#98 landed the generated CLI reference and its drift test while this branch was open, and this row added a flag to `shape-doctor.py --help` — so `docs/cli.md` is regenerated by its own generator rather than edited, which is the rule that file exists under. Two conflicts, both in the hygiene caps, and both resolved by KEEPING BOTH dated entries in the order they landed rather than letting the later branch's entry stand alone: AGENTS.md 400 -> 408 (#97) -> 423 (#99), README.md 1324 -> 1328 (#97) -> 1374 (#99). A cap whose history skipped a movement would be a cap nobody could argue with. `docs/handbook.html`'s footer names both regenerations and carries the MERGED README's blob. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings remain in shape-doctor.py.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a read-only placement audit for mounted legs, with human-resolvable plans and safeguards against destructive plan misuse.
Changes:
- Audits file placement using the adoption policy.
- Adds
--placement-planoutput and distinct plan-kind handling. - Updates tests and documentation.
File summaries
| File | Summary |
|---|---|
tests/test_shape_doctor.py |
Adds placement, verdict, folding, and plan coverage. |
tests/test_repo_hygiene.py |
Updates documentation cap coverage. |
shape-doctor.py |
Implements placement auditing and plan generation; has 2 critical findings and 4 moderate findings. |
README.md |
Documents placement behavior; has 2 nit findings. |
docs/handbook.html |
Regenerates handbook documentation; has 1 nit finding. |
AGENTS.md |
Updates operational guidance and caps. |
adopt-project.py |
Distinguishes and rejects placement plans during adoption. |
Review details
Suppressed comments (1)
README.md:1165
- This new sentence is the only write-mode qualification in the README, but the earlier paragraph in the same section still says the doctor “writes nothing” without mentioning the exception. Make this qualification explicit (for example, “Unlike the read-only run above...”) so the README does not give contradictory instructions about whether
--placement-planwrites a file.
printing what it will do and asking first; until then the moves are yours.
`--placement-plan` is the only thing this command writes, and it writes only
- Files reviewed: 8/8 changed files
- Comments generated: 8
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1208
to
+1212
| plan = (f"{PYTHON} {ctx.shape / 'shape-doctor.py'} --root {ctx.root} " | ||
| "--placement-plan placement-plan.yaml # writes the paths above " | ||
| "as a plan to resolve by hand. Moving one is a pull request on " | ||
| "each leg and a pin bump in the root, so this command makes " | ||
| "neither") |
Comment on lines
1089
to
1091
| FAMILY holder gets **family**, **manifest kinds**, **shape currency**, | ||
| **members** and **machine**; a directory that is neither gets what it is | ||
| CALLED under the naming policy, what IS in it, and the way in. |
Comment on lines
1392
to
1394
| <a href="#families">family holder</a> gets <strong>family</strong>, <strong>manifest | ||
| kinds</strong>, <strong>shape currency</strong>, <strong>members</strong> and | ||
| <strong>machine</strong>; a directory that is neither gets what it is CALLED under the naming |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved placement-audit safety findings and documentation inconsistencies must be addressed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (7)
README.md:1092
placementis registered for family holders as ann/arow (CHECKSapplies it toFAMILY), but this family-holder row list still omits it (and the existingagent-filesrow). Update the family list and regenerate the handbook so the documented row contract matches the doctor output.
FAMILY holder gets **family**, **manifest kinds**, **shape currency**,
**members** and **machine**; a directory that is neither gets what it is
CALLED under the naming policy, what IS in it, and the way in.
docs/cli.md:504
- The new flag makes this tool write a placement plan, but the generated CLI index still describes
shape-doctor.pyas “Writes nothing” (scripts/render-cli-reference.pysupplies that purpose text). Update the generator's purpose and regeneratedocs/cli.md; otherwise the CLI reference contradicts the option documented here.
--placement-plan FILE
write the `placement` row's paths to FILE as an
adoption-plan-style YAML to resolve by hand. The only
thing this command writes, and it still moves nothing:
a path changing legs is a pull request on each leg and
a pin bump in the root
docs/handbook.html:1395
- The generated handbook repeats the same omission: the implementation and family test emit a
placementn/arow, but this family-holder list does not mention it. Regenerate this section from the corrected README rather than leaving the handbook's row contract stale.
<a href="#families">family holder</a> gets <strong>family</strong>, <strong>manifest
kinds</strong>, <strong>shape currency</strong>, <strong>members</strong> and
<strong>machine</strong>; a directory that is neither gets what it is CALLED under the naming
shape-doctor.py:1288
- Both
ctx.shapeandctx.roothave already been resolved to absolute paths, so a plan written inside a repository embeds host-specific filesystem paths. That makes the artifact non-portable and violates this repository's host-absolute-path hygiene if the plan is committed; emit portable/relative metadata or omit these fields.
adopt.emit(lines, "standard", ctx.shape.as_posix())
lines.append("")
lines.append("root:")
adopt.emit(lines, "path", ctx.root.as_posix(), 2)
shape-doctor.py:1244
- This preamble is also emitted for
review_requiredentries, whoseleg: nullexplicitly means the policy does not know which repository they belong in. Saying every entry is already in the wrong repository can cause a human to move an ambiguous path without resolving the question first; describe these as paths needing a human decision.
"# EVERY ENTRY SAYS A TRACKED PATH IS IN THE WRONG REPOSITORY.",
shape-doctor.py:1192
- The manifest path is untrusted here, but
ctx.root / reltreats an absoluterelas an absolute path and follows..or a symlinked directory. The project's validator rejects absolute and parent paths (templates/assembly-root/scripts/validate-manifest.py:381-393), yet this row still runs when that validator is red, so a malformedproject.yamlcan make the doctor rungit ls-filesin an unrelated checkout and expose its file list/sizes in JSON or a plan. Validate the leg path and containment, or skip placement when the manifest path is invalid, before auditing.
role = str(leg.get("role") or "?")
rel = str(leg.get("path") or role)
entry, wrong, unsure = audit_leg(adopt, policy, patterns, role, rel,
ctx.root / rel)
shape-doctor.py:1078
- Catching
Refusalhere makes a failedgit ls-fileslook like a successful audit.check_placementreceives no wrong/review entries and returnsok, so a leg whose index cannot be read can still yieldCOMPLIANT(especially whenlegscan readHEAD). Propagate this as an inability-to-answer/finding and prevent--placement-planfrom emitting an empty plan for the failed audit.
try:
files = leg_tracked_files(mount)
except Refusal as exc:
entry["state"] = f"git could not list it: {exc.detail}"
return entry, [], []
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
…sitory, one the manifest points outside the root, a name no plan can carry, and a verdict that named a count of zero (Copilot on #100) Seven findings from the review, each kept as a test rather than as a fixed line, because each of them LOOKED right and would have reported something untrue about somebody's repository. `git -C` DISCOVERS: run in a plain directory it walks up and answers from the enclosing repository, so a leg mount that is not a submodule would have had the ASSEMBLY ROOT's index read as if it were the leg's contents. And `path:` comes out of `project.yaml`, which this command does not own -- an absolute path, a `..` or a symlink makes `root / rel` a directory somewhere else entirely, and `ls-files` there would have put another checkout's paths and sizes into this report and into the plan. Both are refused before any git call the second one could reach, and a leg mounted AT the root is refused for the neighbouring reason: its contents are the root's. A LEG NOBODY READ IS NEVER `ok`. "Every tracked path classifies as the leg it is in" is not a claim to make about a leg that declined, so the row is `note` while one is there, `n/a` when none could be read, and `counts.unread_legs` carries it. The same rule covers a name this command cannot write: git allows any byte but `/` and NUL, so a filename can hold a newline -- which would split one plan entry across two lines of YAML -- or a sequence that is not UTF-8 at all, which raises UnicodeEncodeError in the writer. Those are excluded from the classification and reported by `repr`, the only spelling safe in every output this command has. And the verdict needed a path to name. `run_checks` turns an exception out of any check into a FINDING with an empty detail, and the branch read a length -- so a row that never got as far as classifying anything would have answered `MISPLACED (0 paths)`. It now needs a non-empty list and otherwise reads `INVALID (placement)`, which is what it is. The README and the handbook named five rows for a FAMILY holder where the command returns six; both now name `placement` at `n/a`. README 1374 -> 1375. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
The fix round on Copilot's reviewSeven of the nine taken, each as a test rather than as a fixed line — the Taken.
Declined, and why.
|
Contributor
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate issues remain in placement auditing, plan generation, remediation guidance, and generated documentation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (12)
Previously missed (1) — in code that hasn't changed since the last review.
shape-doctor.py:1467
run_checksconverts anOSErrorfromcheck_placementinto aFINDINGwithdetail={}, but this guard only refusesn/a. In that caseplacement_plan_textwrites a syntactically valid empty plan andmainannounces it as written even though the audit failed. Refuse rows without the placement detail (for example, without theirlegsblock) instead of generating a plan from them.
adopt-project.py:733
- This remediation tells the user to resolve the plan and hand it back to
shape-doctor.py, but the proposed CLI only adds--placement-plan; there is no--fix/--planconsumer in this change. Following the printed remediation therefore leads to a command that does not exist. Explain that the current follow-up is the human-reviewed leg PRs and root pin bump, or explicitly state that no repair mode consumes the plan yet.
"Resolve every `resolution:` and hand it back to the "
"doctor.")
docs/cli.md:491
- PR #98 has now merged into
main, while this branch is based on3d46ab5as the description notes. Because this file is generated and this hunk overlaps the file that #98 introduced, merge currentmainand rerunpython3 scripts/render-cli-reference.py; otherwise the generated-reference check will be stale or conflict when this PR lands.
usage: shape-doctor.py [-h] [--root ROOT] [--json] [--placement-plan FILE]
shape-doctor.py:1151
any(mount.iterdir())is outside the guarded error handling. If a declared leg is a real but unreadable directory,iterdir()raisesPermissionError;run_checksthen turns the whole check into a genericFINDINGwith empty detail, producingINVALID (placement)instead of the documented unaudited-legnote. Probe population inside anOSErrorhandler and return an unaudited entry.
if not (mount.is_dir() and any(mount.iterdir())):
shape-doctor.py:1163
- After
not_its_own_repositorysucceeds, an independently cloned repository at a manifest leg path with no superproject gitlink is treated as an auditable leg.check_legsconsiders that path unmounted, so this row can classify unrelated checkout contents and emit a misleading plan. Checkrecorded_gitlink(root, rel)before listing files and mark the leg unaudited when it is absent.
return entry, [], []
shape-doctor.py:1359
review_requiredentries are included inentries, but they haveleg: nullprecisely because the policy cannot tell whether they belong in spec or code and the row remains anote. This header therefore tells the human that an ambiguous path is already in the wrong repository, contradicting the resolution guidance below; phrase it as a placement decision instead.
"# EVERY ENTRY SAYS A TRACKED PATH IS IN THE WRONG REPOSITORY.",
shape-doctor.py:1031
- Filtering these names before classification lets a real misplaced path disappear from the doctor's verdict. For example,
spec/tool\ns.pystill has a.pyextension and--jsoncan safely escape it, but this code makes the rownote/COMPLIANT; the same happens when no plan is requested. Keep the path in the audit/JSON and make plan generation refuse or safely encode it instead of dropping it.
path = record.decode("utf-8", "surrogateescape")
if UNWRITABLE_IN_A_PLAN.search(path):
unwritable.append(repr(path))
continue
shape-doctor.py:1318
- The remediation command interpolates
ctx.shapeandctx.rootraw. If either valid path contains spaces (the root is explicitly user-supplied), the shell splits it and the row'sNEXTcommand fails instead of writing the plan. Shell-quote each path argument and cover a space-containing path in the command test.
plan = (f"{PYTHON} {ctx.shape / 'shape-doctor.py'} --root {ctx.root} "
"--placement-plan placement-plan.yaml # writes the paths above "
"as a plan to resolve by hand. Moving one is a pull request on "
"each leg and a pin bump in the root, so this command makes "
"neither")
shape-doctor.py:1403
- The plan emits the caller-supplied
ctx.rootthroughadopt.y, but that writer only escapes backslashes and quotes; it does not encode newline/control/surrogate characters. A valid checkout path containing a newline therefore produces malformed YAML (and a non-UTF-8 path can raiseUnicodeEncodeError, which this handler does not catch). Refuse unsafe metadata or extend the shared scalar writer before claiming every generated plan parses.
adopt.emit(lines, "path", ctx.root.as_posix(), 2)
shape-doctor.py:335
- The shared policy classifies
.gitlab-ci.yml,.circleci/**,azure-pipelines.yml, andJenkinsfileascodefor the same leg-local CI reason as.github/workflows/**. Since this allowlist omits them, a spec leg with any of those ordinary CI files is falsely reported asMISPLACED; add the other policy CI patterns here (and cover one in the test).
EVERYWHERE = (
"LICENSE", "LICENSE.*", "LICENCE", "NOTICE", "COPYING",
"CONTRIBUTING.md", "CODE_OF_CONDUCT.md", "SECURITY.md", "SUPPORT.md",
"CHANGELOG.md", ".gitattributes", ".editorconfig", ".mailmap",
"CODEOWNERS", ".github/**",
shape-doctor.py:305
repo_shape._tokenise()usestext.splitlines(), which treats U+0085, U+2028, and U+2029 as line separators. Those valid Git filename characters are not excluded by this regex, soadopt.y()can write one into a quotedpath:value and the generated plan will no longer parse; include the Unicode line-separator characters in the unwritable check.
UNWRITABLE_IN_A_PLAN = re.compile(r"[\x00-\x1f\x7f\ud800-\udfff]")
shape-doctor.py:1058
Path.resolve()can raiseRuntimeErrorfor a symlink loop, but this guard catches onlyOSError. Apath:inproject.yamlthat names such a loop therefore escapesaudit_legand aborts the doctor with a traceback instead of recording that the leg could not be audited.
try:
here, base = mount.resolve(), root.resolve()
except OSError as exc:
return f"the mount cannot be resolved on this machine: {exc}"
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
…e file it points at (Copilot on #100) `stat` follows, and here that reads metadata OUTSIDE the very root `outside_the_root` had just finished keeping this row inside of: a tracked `secret -> /outside/big` would have put that file's size in the report and in the plan. The two reasons agree. A symlink's BLOB in git is the target path it holds, so the link's own length is the honest size for a row that reports what the index carries; the target's size is a different file's fact and nothing here is entitled to it. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
Eighth finding taken, at Codex could not review this PR. @codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate and critical findings remain in placement auditing and plan/remediation handling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (6)
Previously missed (1) — in code that hasn't changed since the last review.
shape-doctor.py:1174
- On an existing but unreadable mount,
any(mount.iterdir())can raisePermissionError; if that check succeeds,git_outcan still raise anOSErrorbefore it produces aRefusal. Both escapeaudit_leg, sorun_checksturns the whole placement row into a genericFINDING(and potentiallyINVALID (placement)) instead of recording that leg as unreadable and preserving the report's note/n/a behavior. Catch these inspection/listing errors in this path and record them on the leg.
adopt-project.py:733
- This remediation says to resolve the file and “hand it back to the doctor,” but this PR does not add a command that consumes a resolved placement plan (
shape-doctor.pystill has no--fix --plan). Until that later mode exists, the refusal leaves the user with an impossible next step; explain that the plan is currently human-only and must not be passed to this tool, or give the supported manual PR/pin-bump workflow.
"Remediation: this is a PLACEMENT plan, written by "
"`shape-doctor.py --placement-plan` for a project that is "
"ALREADY split; its entries name paths inside mounted "
"legs. This tool splits a repository that is not, so "
"`execute` here would be aimed at an assembly root. "
"Resolve every `resolution:` and hand it back to the "
"doctor.")
shape-doctor.py:1365
- The generated plan includes
review_required: trueentries for paths the policy explicitly cannot classify, but this header says every entry is already a path in the wrong repository. That wording can prompt a human to move an ambiguous path before answering its question; describe entries as needing a placement decision instead.
"# EVERY ENTRY SAYS A TRACKED PATH IS IN THE WRONG REPOSITORY.",
shape-doctor.py:333
- The shared
root-front-doorpolicy also classifiesREADMEandREADME.rstas root (contracts/path-classification.yaml:146-170), but neither pattern is in this exemption list. A leg using either normal front-door spelling will therefore be reported as misplaced even though this row treats the other front-door files as repository-wide; include both patterns so the allowlist matches the policy.
EVERYWHERE = (
"LICENSE", "LICENSE.*", "LICENCE", "NOTICE", "COPYING",
"CONTRIBUTING.md", "CODE_OF_CONDUCT.md", "SECURITY.md", "SUPPORT.md",
shape-doctor.py:1062
- A symlinked leg whose target is another directory inside
rootpasses this check because the resolved target is still underbase;not_its_own_repository()then compares against that same target and audits the other leg's index under the wrong role. The outside-root guard handles only external targets, so reject a symlinked mount (or otherwise verify the resolved path is the declared gitlink) before listing its files.
here, base = mount.resolve(), root.resolve()
shape-doctor.py:1474
- If
check_placementraises anOSErrororRefusal,run_checkssupplies aFINDINGrow with onlydetail={"error": ...}. This guard rejects onlyNone/n/a, soplacement_plan_textthen writes a valid-looking empty plan (paths: []) even though the audit failed. Reject errored or incomplete audit details before writing, rather than turning an unreadable audit into an empty plan.
if row is None or row.status == NA:
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+1157
to
+1161
| if not (mount.is_dir() and any(mount.iterdir())): | ||
| # The `legs` row already reports this, with `make bootstrap` beside | ||
| # it. Here it is only why this leg has no answer. | ||
| entry["state"] = "not populated" | ||
| return entry, [], [] |
This was referenced Sep 11, 2026
brettheap
added a commit
that referenced
this pull request
Sep 11, 2026
… a root or a name with a space in it is a line that runs (#101) (#102) Lane: openreposhape-2 (openRepoShape-2) Claim: #101 (comment) Closes #101. ## What was wrong Copilot asked for shell quoting on PR #100, against the `placement` row. The finding was real and was never that row's: **every** next command since #96 builds its arguments with an f-string, so a root at `/srv/my projects/Atlas` produced a line argparse reads as three arguments and refuses — in front of whoever pasted it. That is exactly the failure `validator_row` already carries a comment about (`python3 validate-pins.py in <root>`, Copilot on PR #96), one class of value along. ## Why one form will not do, and why not `shlex.quote` The two shells are not one dialect, and **each reads the other's form as a different value with no error to say so**. `sh` reads PowerShell's doubled apostrophe as two adjacent quoted strings and concatenates them, so `'O''Brien'` arrives as `OBrien` — a different path, silently. And `\` is a literal character to PowerShell, whose escape is the backtick. Both directions are asserted (`test_the_two_shells_are_not_one_dialect`). `shlex.quote` could not be the one function either. Its module is documented as designed for Unix shells only, so it carries no promise at all about the shell a Windows reader is in — and since #49 this standard has a native Windows entry point and `shape-doctor.py` runs there by contract. Nor would its spelling do: `'C:\Users\Jane O'"'"'Neill\proj'` is a line a person is meant to **retype**, on a machine whose stock shell is Windows PowerShell 5.1 (`setup-project.py`'s own docstring), where that value is written `'C:\Users\Jane O''Neill\proj'`. ## What landed One `quote_arg(value, platform=None)` in `shape-doctor.py`: * a value matching the bare alphabet (`^[A-Za-z0-9_./:@%+=,-]+$`, plus backslashes on Windows) is returned **unchanged** — a table whose whole job is to be read would be worse, not better, for `--root '/srv/work/Atlas'` on every line of every clean run; * otherwise `'…'` with `'\''` for `sh`, and `'…'` with `''` for PowerShell; * `platform=` so both forms are testable on either host. **It lives in `shape-doctor.py`, not in `scripts/repo_shape.py`** — where the standard's other platform-aware constant (`PYTHON`) sits — because that file is a SHAPE COPY: `scripts/shape_materialize.py` writes it into every assembly root and every family holder and the shape pin digests it, so one function added there would put an `upstream-changed scripts/repo_shape.py` row in front of **every project in the estate** on its next `update-shape.py check`, for a helper no project's own copy would ever call. `shape-doctor.py` is in no copy list, by its own contract: it compares a project against the checkout it is RUN FROM. (`repo_shape.SAFE_PATH_RE` answers a different question anyway — what may become an argument to `git`, where every command is a list run with `shell=False` and the threat is argument injection, not a shell.) **Nineteen next-command strings** now pass through it — every row of all three root kinds, the two `NOT A SHAPE ROOT` way-in lines included, and the `machine` row's. `--json` carries the same quoted strings, because they are commands. `<your-org>` in the scaffold line is **not** quoted and must not be: it is a placeholder the reader replaces, not a value this command knows. ## The proof ``` FINDING naming Some Thing: FINDING naming-unclassified: Some Thing NEXT python3 …/validate-repository-naming.py --explain 'Some Thing' n/a the way in this is a git repository with no shape manifest… NEXT python3 …/adopt-project.py plan --source '…/my proj'\''ects/Some Thing' --project 'Some thing' ``` **33 tests added (8 functions), 0 existing tests changed** — `tests/test_shape_doctor.py` is pure addition, 0 lines removed, because the fixtures' paths have no spaces and every command they assert is one the quoter returns unchanged: * both forms parametrized over both platforms, over a space, an apostrophe, both at once, and a Windows drive path; * the POSIX form proved by `shlex.split` **round-trip** — the shell's own reader — rather than by a string the suite spelled; * the PowerShell form asserted exactly, apostrophes doubled; * the two dialects proved incompatible — `sh` reads `'O''Brien'` as `OBrien`; * the same Windows path quoted on POSIX and bare on Windows, said out loud; * a **whole report** of a project at `<tmp>/do ct'or/Atlas` with five rows red at once, each next command read by `shlex.split` and the root asserted to arrive as one token; * end to end: the shape-currency row's line run **verbatim** through `bash -c`, trailing `#` comment and all — it reports the drift (exit 1, naming `scripts/bootstrap.py`), where the unquoted spelling of the same line exits 2 before `python3` is reached, with bash's own `unexpected EOF while looking for matching` complaint. ## Housekeeping * `git diff --stat origin/main -- templates/` — **empty**. * `python3 scripts/render-cli-reference.py --check` — `[ok] docs/cli.md is what the generator produces`; no `--help` text moved. * Rule 1 (`test_no_committed_file_names_a_host_absolute_path`): the example paths are `/srv/...` and `C:\Users\Jane O'Neill\...` — this repository's own convention of putting a space in the Windows user name, which is what the examples illustrate anyway (`scripts/repo_shape.py`: "Jane Doe"). * No README/AGENTS.md change and therefore **no cap moved**: neither promises a copy-pasteable command, and `Run the one the row names` is unchanged prose that is now true in more cases. The `Invoke-WebRequest` single-match rule is untouched. ## Neighbouring, not taken `update-shape.py check` prints its own `NEXT …apply --root {root} --upstream {upstream.label}…` line with the same interpolation. It is out of #101's scope (which names `shape-doctor.py`) and wants its own issue and its own decision about where the quoter then lives. A path with a **non-ASCII** character in it is still flattened by `ascii_text`, which every row's text goes through — `/srv/josé/proj` prints as `/srv/jos?/proj`, quoted or not. That is this file's documented PURE ASCII rule ("a verdict a cp1252 console cannot render is a verdict nobody reads"), and unpicking it for the `next` field alone is a separate decision about the whole report, not a quoting bug. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by Sourcery Make every command emitted by shape-doctor.py safe to read and run on the target platform, including paths and names that require shell quoting. New Features: - Add platform-aware shell argument quoting for commands emitted by shape-doctor.py, supporting POSIX shells and PowerShell. - Use platform-specific project setup commands so Windows reports setup-project.py while POSIX systems use setup.sh. Bug Fixes: - Ensure paths, project names, and other interpolated values containing spaces, apostrophes, shell metacharacters, or newlines remain valid single command arguments. - Normalize values before quoting so subsequent report formatting cannot invalidate quoted apostrophes. Enhancements: - Apply consistent quoting across all generated next-command strings, including JSON output and remediation commands. - Add platform-selectable interpreter formatting for cross-platform command generation. Tests: - Add comprehensive coverage for POSIX and PowerShell quoting, shell dialect differences, platform-specific command generation, awkward paths, and normalization edge cases. - Verify complete reports and pasted commands handle roots containing spaces and apostrophes correctly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Sep 11, 2026
brettheap
added a commit
that referenced
this pull request
Sep 12, 2026
…and reaches a verdict in three functions instead of one body, dropping its cognitive complexity from 22 to 13 (#134) `python:S3776` was OPEN against `check_placement` at 22, over the 15 Sonar allows. The body opened with four different `n/a` refusals before it had classified a single path, closed with a five-branch precedence ladder, and wrote one leg's half of the reason as a conditional expression nested three deep inside a `join`. A reader who came here because the row said `n/a` had to read past the audit to find out which of the four refusals said it. Three helpers, one per decision, each carrying the part of the WHY that moved with it: `placement_policy` the three refusals that are THIS CHECKOUT being short rather than the repository being wrong, including the unreadable policy file, whose `n/a`-not-a-finding argument moved into the docstring intact. `leg_audit_summary` one leg's half of the row's reason: its counts, or the `state` that says why it has none -- with the Copilot PR #100 argument for printing the state rather than a zero. `placement_verdict` the precedence ladder, and why each branch outranks the one below it. It reads the counts out of the `detail` the row prints, so a number this judges on and a number the reader is shown cannot drift apart. Behaviour is byte-identical: the same strings, in the same order, on every branch. The row's `next` command is built before the verdict rather than after the first of its branches, which is a pure f-string and writes nothing. `placement_row` still puts the `--json` keys on every branch, the `placement` id and label are untouched, and no message text, exit code or written file changes. Measured with complexipy (SonarSource's spec): `check_placement` 25 -> 13, `placement_policy` 6, `placement_verdict` 7, `leg_audit_summary` 2. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Sep 12, 2026
…arguing its own place, dropping the ladder's cognitive complexity from 33 to 11 (#134) `python:S3776` was OPEN against `verdict_for` at 33, the highest in the file and over twice the 15 Sonar allows. The precedence the module docstring argues at length was one body with the argument scattered through it as comments between branches, so the order -- the thing a reader comes to this function for -- had to be reconstructed from forty lines of counting, comprehensions and early returns. Five helpers. The order of the calls in `verdict_for` is now the precedence, readable as the list it is, and each rung argues its own place in its own docstring: `drifted_file_reasons` the drifted copies quoted in the line, and why the two `if` chains were two groups: the states that MADE it drifted, then the states in which the standard and the copy no longer map onto each other. Same states, same order, still named only when non-zero. `drifted_reasons` that a drifted copy, a leg off its pin and a member off its pin are one verdict because they are one fact. `misplaced_verdict` UNDER DRIFTED AND OVER INVALID, and the PR #100 argument for why the list has to be non-empty rather than the row merely red. `invalid_verdict` the named gates, each of which somebody else owns. `behind_verdict` both kinds of behind, and why the pin-only one quotes commits where the other quotes counts. Behaviour is byte-identical: the same verdict strings and the same exit codes (0, 1, 2, 3) in the same order of precedence. `counts` is read inside the two helpers that use it instead of once at the top, which is the same dictionary read the same way. The catch-all stays in `verdict_for` with its comment, because it is the rung that exists so that a rung nobody wrote still fails loudly. Measured with complexipy (SonarSource's spec): `verdict_for` 38 -> 11, and no helper above 10. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Sep 12, 2026
… one question per function, every one of them under the 15 cognitive complexity allowed and behaviour byte-identical (#134) (#142) * shape-doctor.py: the leg-shape-files check answers one question per function instead of three in one body, dropping its cognitive complexity from 26 to 8 (#134) `python:S3776` was OPEN against `check_leg_shape_files` at 26, over the 15 Sonar allows. The body did three separate jobs: it classified every shape file of every leg through a five-way `if`/`elif` chain, it worded the row's reason, and it decided which of the missing files a copy would even fix. Each of those is a question with its own answer, and a reader who came here because the row said something unexpected had to trace all three to find out which one said it. Four helpers, one per decision, each carrying the part of the WHY that moved with it: `shape_file_state` the five-way chain: what ONE file is against the template's copy, and why `rendered` is an answer rather than `differs`. `leg_shape_comparison` ONE leg: its `--json` entry and the files that are not there, with both early returns saying which kind of "nothing to compare" they are. `leg_shape_summary` one leg's half of the row's reason. `missing_shape_file_fix` the next command, or why a copy is not one -- `copy_command` is still the only place a shell verb is spelled (#105), and the rendered-template argument from #96 moved into the docstring intact. Behaviour is byte-identical: the same strings, in the same order, on every branch. `LABEL_LEG_SHAPE_FILES` still names all three returns, the `note`-not-`FINDING` comment stays on the branch it argues, and the `CHECKS` entry is untouched. Measured with complexipy (SonarSource's spec): `check_leg_shape_files` 31 -> 8, and no helper above 6. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * shape-doctor.py: the placement check loads a policy, audits the legs and reaches a verdict in three functions instead of one body, dropping its cognitive complexity from 22 to 13 (#134) `python:S3776` was OPEN against `check_placement` at 22, over the 15 Sonar allows. The body opened with four different `n/a` refusals before it had classified a single path, closed with a five-branch precedence ladder, and wrote one leg's half of the reason as a conditional expression nested three deep inside a `join`. A reader who came here because the row said `n/a` had to read past the audit to find out which of the four refusals said it. Three helpers, one per decision, each carrying the part of the WHY that moved with it: `placement_policy` the three refusals that are THIS CHECKOUT being short rather than the repository being wrong, including the unreadable policy file, whose `n/a`-not-a-finding argument moved into the docstring intact. `leg_audit_summary` one leg's half of the row's reason: its counts, or the `state` that says why it has none -- with the Copilot PR #100 argument for printing the state rather than a zero. `placement_verdict` the precedence ladder, and why each branch outranks the one below it. It reads the counts out of the `detail` the row prints, so a number this judges on and a number the reader is shown cannot drift apart. Behaviour is byte-identical: the same strings, in the same order, on every branch. The row's `next` command is built before the verdict rather than after the first of its branches, which is a pure f-string and writes nothing. `placement_row` still puts the `--json` keys on every branch, the `placement` id and label are untouched, and no message text, exit code or written file changes. Measured with complexipy (SonarSource's spec): `check_placement` 25 -> 13, `placement_policy` 6, `placement_verdict` 7, `leg_audit_summary` 2. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * shape-doctor.py: the placement plan is composed a block at a time instead of in one run of appends, dropping its cognitive complexity from 22 to 7 (#134) `python:S3776` was OPEN against `placement_plan_text` at 22, over the 15 Sonar allows. The function wrote all four blocks of one YAML file into a single list: the header, every leg, every path to resolve, and what was never judged. Nothing separated them but blank-line appends, so a reader looking for the line that wrote a key they did not expect had to count indents to find out which block they were even in. Four helpers, one per block of the file, each carrying the part of the WHY that moved with it: `placement_plan_header` the first block, and that `kind:` is the SAFETY property -- `PLACEMENT_PLAN_KIND`, never `PLAN_KIND`, is what stops `adopt-project.py execute` being handed one of these. `placement_plan_leg` ONE leg's counts, and why a tracked name no plan can carry is written as a comment with its newlines flattened rather than as a value. `placement_plan_entry` ONE path, in an adoption plan entry's keys exactly -- the claim the adoption's own reader is held to in `test_the_placement_plans_entries_are_adoption_plan_entries`. `placement_plan_paths` the `paths:` block, including the empty one that says what its emptiness means when a leg could not be read at all. Behaviour is byte-identical: the same lines, in the same order, emitted through the same `adopt.y` and `adopt.emit`. Each helper builds and returns its own list rather than appending into the caller's, which is the only change to how the lines are accumulated; the file they join into is unchanged, `--placement-plan` still writes exactly one file where the caller named it, and `write_placement_plan` is untouched. Measured with complexipy (SonarSource's spec): `placement_plan_text` 22 -> 7, and no helper above 4. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * shape-doctor.py: the members check finds a member's state, judges it and words the row in three functions instead of one loop, dropping its cognitive complexity from 29 to 8 (#134) `python:S3776` was OPEN against `check_members` at 29, over the 15 Sonar allows. One loop body read both copies of every member, decided which of three faults it had, decided whether its state was worth saying, and fed three different lists that the wording below the loop then read back. A reader who came here because a member was called `not populated` had to hold the whole loop in their head to see that the same pass had also decided whether a working clone was beside the holder. Three helpers, one per decision, each carrying the part of the WHY that moved with it: `member_state` both copies of ONE member as they are found, and why nothing is asked of a mount that is not populated: `git rev-parse HEAD` inside an empty submodule answers about the HOLDER, so an unfetched member would otherwise be reported as checked out at the holder's commit. The 40-hex rule for `pin.commit` moved here too. `member_problem` the three faults, in the only order they make sense in, and that a member at its pin but on a branch is deliberately not one of them. `members_verdict` the finding and the two `ok` wordings, and why only the pinned copies can make this row red -- the PR #96 argument about `family.py add` leaving a branch behind stays on the branch that records it. Behaviour is byte-identical: the same strings, in the same order, on every branch, and the `--json` entry for a member has the same keys in the same order it always had. The `if`/`elif` ladder over the three faults became three early returns in the helper, which answers in the same order and therefore with the same message. Measured with complexipy (SonarSource's spec): `check_members` 26 -> 8, and no helper above 8. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * shape-doctor.py: each rung of the verdict ladder is its own function arguing its own place, dropping the ladder's cognitive complexity from 33 to 11 (#134) `python:S3776` was OPEN against `verdict_for` at 33, the highest in the file and over twice the 15 Sonar allows. The precedence the module docstring argues at length was one body with the argument scattered through it as comments between branches, so the order -- the thing a reader comes to this function for -- had to be reconstructed from forty lines of counting, comprehensions and early returns. Five helpers. The order of the calls in `verdict_for` is now the precedence, readable as the list it is, and each rung argues its own place in its own docstring: `drifted_file_reasons` the drifted copies quoted in the line, and why the two `if` chains were two groups: the states that MADE it drifted, then the states in which the standard and the copy no longer map onto each other. Same states, same order, still named only when non-zero. `drifted_reasons` that a drifted copy, a leg off its pin and a member off its pin are one verdict because they are one fact. `misplaced_verdict` UNDER DRIFTED AND OVER INVALID, and the PR #100 argument for why the list has to be non-empty rather than the row merely red. `invalid_verdict` the named gates, each of which somebody else owns. `behind_verdict` both kinds of behind, and why the pin-only one quotes commits where the other quotes counts. Behaviour is byte-identical: the same verdict strings and the same exit codes (0, 1, 2, 3) in the same order of precedence. `counts` is read inside the two helpers that use it instead of once at the top, which is the same dictionary read the same way. The catch-all stays in `verdict_for` with its comment, because it is the rung that exists so that a rung nobody wrote still fails loudly. Measured with complexipy (SonarSource's spec): `verdict_for` 38 -> 11, and no helper above 10. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Oct 11, 2026
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
…d `open_glass` share one `SHAPE_PIN_SOURCE_<PRODUCT>`, so `--pin` refuses the pair and `validate-pins.py` notes a declared one and reads the variable for neither (#275) (#409) `pin_source_env_name` turns every character outside `[A-Z0-9_]` into `_`, so `open-glass`, `open_glass` and `open.glass` are three legal product names, with three pin files (`contracts/<product lowercased>-pin.yaml`), and one variable, `SHAPE_PIN_SOURCE_OPEN_GLASS`. On ab623dd (this branch's main parent; the same rows reproduce on c7c02fa), with the issue's own probe against local bare remotes and a `gh` that exits 97: | step | ab623dd | this head (6494714) | |---|---|---| | `scaffold-project.py --pin open-glass@<A> --pin open_glass@<B>` | exit 0, three repositories | exit 2 `scaffold-pin-collides`, nothing created | | `validate-pins.py`, no source, on a project that already declares the pair | exit 0, both skips offer `SHAPE_PIN_SOURCE_OPEN_GLASS` | exit 0, one `note neutral-pin-source-collides`; the skips offer `--pin-source <product>=<path>` and a sibling checkout only | | `SHAPE_PIN_SOURCE_OPEN_GLASS=<open-glass>` | `neutral-pin-source-unreadable` for `open_glass`, exit 1 | exit 0, the note; the variable is read for neither pin, so each pin is read from its own source or skipped | | `SHAPE_PIN_SOURCE_OPEN_GLASS=<open_glass>` | `neutral-pin-source-unreadable` for `open-glass`, exit 1 | the same | | `--pin-source open-glass=... --pin-source open_glass=...` | exit 0 | both pins recompute, exit 0, the note | **What changed.** Every variable name is kept: an operator may have set one and the docs name them. What is done instead: - `scripts/repo_shape.py`: `pin_source_env_collisions(products)` is the one pure answer, `{variable: [products]}` for each variable two or more distinct products share. Distinct means distinct PIN FILES (`pin_product_key`, `lower()`, which is what `contracts/<product lowercased>-pin.yaml` is named by), so a product typed twice or in two cases is #241's file collision and never this one. `pin_source_env_shared` is the set of those keys. - `scaffold-project.py`: `_refuse_colliding_pins` (left where it is) names each variable's `--pin` arguments as typed after #241's file sentences, in the same `scaffold-pin-collides` refusal. Dry run and real run say the same no, before any process starts and ahead of the leg-path refusal. #241's wording and remediation are unchanged when only a file collides. A NEW pair is still refused, so the situation is not created anew. - `templates/assembly-root/scripts/validate-pins.py`: one NOTE per variable, `neutral-pin-source-collides`, naming every product and the variable, the keyed `--pin-source <product>=<path>` that reads each pin, and that which declaration stays is the human's decision. It is never a finding and never moves the exit code. A product whose variable is shared does not read it (rule 2 of `resolve_neutral_pin_source`), and its skip hint offers the keyed `--pin-source <product>=<path>` and not the variable or the bare form. A variable one product has to itself reads and is offered exactly as before. - `scripts/repo_shape.py`, the two chain readers: `link_pins_from_trees` does not read a variable two links of one chain share, and the `declared-unverified` hint stops naming it. Only a hand-written chain can have two such links, since scaffold takes every link but the first from the naming policy's alphanumeric `open<Product>` form. Each reader of `pin_source_env_name`, decided: | site | decision | |---|---| | `scaffold-project.py --pin` | refuse (above) | | `validate-pins.py` rule 2 and the skip hint | a note, and the variable is neither read nor offered for a shared product | | `repo_shape.py` `resolve_link_source` (via `link_pins_from_trees`) | skip the variable when another link of the chain shares it | | `repo_shape.py` `_chain_link_findings` hint | omit the variable for a shared link | | `adopt-project.py --pin`, `plan`, `check`, `execute` | not in this PR, see below | **Fix round 1 (from the review of 34dbd86).** **The design change: a declared pair is a NOTE and not a finding. This is the lane coordinator's decision, and Brett may reverse it.** The first round made `neutral-pin-source-collides` a FINDING. Main through ab623dd scaffolds such a project (`--pin open-glass@A --pin open_glass@B` exits 0), and `update-shape.py apply` runs the NEW `validate-pins.py` over the rewritten root, so for a project already declaring the pair the finding ended every apply as `REFUSED update-validators-red`, rolled back, for this update and every later one, until a person deleted a true declaration of descent. AGENTS.md tells an agent never to edit "a pinned file, a digest or a manifest to make a row go green", and the finding's remediation was exactly that, so the person was locked out. The coordinator chose the review's option (b): it is a NOTE now. The guard stays (the variable is read for neither pin), each pin still verifies from its own source, scaffold still refuses a new pair, and `validate-pins.py` says the rest in one line. To put the finding back, restore 34dbd86's `report.finding(...)` call in `_report_pin_source_collisions` (it is not a word swap for `report.advise`: `advise` takes one string, `finding` a code and a detail), give the finding the remediation 34dbd86 had, and turn the tests that expect the note (and `test_a_project_declaring_a_colliding_pair_takes_a_shape_update`) back into exit 1; the first round's versions of all of them are on 34dbd86. - **Proved on a real old project.** Main (ab623dd) scaffolded the pair into local bare remotes; `update-shape.py apply` to the first round's head 34dbd86 exited 2 `REFUSED update-validators-red` and rolled back (`git branch` shows only `main`); to this head it exits 0, the project's validator passes and prints the note, and the doctor's `pins` row is `ok` before and after. The doctor prints `passes` for a validator that exits 0 and does not echo its stdout, so the note itself is visible in `update-shape.py apply`'s output and in `validate-pins.py`'s, and the verdict moves for no reason but the update (`COMPLIANT, SHAPE BEHIND` to `COMPLIANT`). It is now a test: `test_a_project_declaring_a_colliding_pair_takes_a_shape_update`, over local stand-ins beside the project, so no `gh` is asked. - **Collisions by the variable.** Pairs were decided by `casefold()`, but the pin file is `lower()`. `openß`/`openss`, `openſ`/`opens`, `openfi`/`openfi` and `openς`/`openσ` (the letters are U+00DF, U+017F, U+FB01 and U+03C2 beside U+03C3) share a variable and have two pin files, and were never found. They are now, from the pin-file fold (`pin_product_key`). A pool of 31 names is checked pairwise against `neutral_pin_path` and `pin_source_env_name` themselves, and no single product is ever a collision. They are reachable only from a hand-written `project.yaml`. - **Windows.** The guard's only test needed the `#!` `gh` stub and was skipped on Windows. `test_a_shared_variable_answers_for_neither_pin_with_no_gh` puts a checkout of each pin beside the project, names one in the variable, and needs no `gh`: on ab623dd it is `neutral-pin-source-unreadable`, here each pin reads its own sibling. The hint and the non-ASCII rows are tested in-process, with no skip and no filesystem. - **The hint.** For a product two pins share, the skip hint offers the keyed `--pin-source <product>=<path>`: the bare form answers for every pin with no more specific source, so with two pins it gives the other the finding this issue is about. - **Trailers and title.** The trailers of the earlier commits (a30091f and 34dbd86 name Sonnet 5.5) are settled in this message, which carries Opus 5.5. The PR title now names `repo_shape.py`. - `scaffold-project.py` is not touched this round: `fix/adopt-pin-writes-pin-file` moves that code, and the change would only conflict with it. **Left out.** - `adopt-project.py` is untouched. Its half arrives with #273's PR, which moves scaffold's pin machinery into a shared module and routes adopt through `pin_source_env_collisions`. Once it lands, the one place that still keys by `casefold()` (`pin_variable_collisions` in `scripts/shape_materialize.py`) should read `pin_product_key`; it cannot differ for an ASCII `--pin`, which is all `PIN_ARG_RE` takes. - `scaffold-project.py`'s own `_colliding_pin_variables` also keys its argument lists by `casefold()` and one docstring there still says the project's validator reports the pair "as a finding"; both go with the code when #273 moves it. #312's `PinSources` keys `--pin-source` by `casefold()` too, so `openß=` and `openss=` are one key there; that is #312's. - A product in `neutral_product_pins:` and a chain link in `naming.referent_chain` that share a variable are read by two validators in one process, and neither sees both sets. Not detected here. - The doctor does not print a passing validator's stdout, so its `pins` row does not carry the note. Not changed: it would add a file to this PR. - No `--help` text changed, so `docs/cli.md` is untouched. `README.md` is at its line cap and its blob is pinned by `docs/handbook.html`, so the one README sentence this wants needs a regenerated handbook and waits for its own change. - `validate-pins.py` and `repo_shape.py` ship into projects, so every project's `shape-pin.yaml` reads `upstream-changed` for both until `update-shape.py apply`, which now succeeds for a project that declares such a pair. **Evidence.** All on the head (6494714), with git-filter-repo 2.47.0 on PATH, a `gh` that cannot be reached (a stub first on PATH, `GH_TOKEN` bogus, an empty `GH_CONFIG_DIR`), and the repository-local `user.name` blanked at command-line scope, as the standard's checkout sets one that two scaffold tests read. - `tests/test_pin_source_env_collisions.py` is 137 tests and passes on this head. Laid over 34dbd86, 50 fail and 87 pass: the note and update tests, the non-ASCII rows, the hint, the sibling guard, and `pin_product_key` itself (which 34dbd86 does not have); the scaffold refusals, the chain readers and the controls hold on both. - Targeted, run on ad6f569, whose tree differs from this head in `tests/test_pin_source_env_collisions.py` only (the last edits made one comparison read the committed blob and wrapped one line): the new file, `test_pin_validator`, `test_neutral_pin_validator`, `test_cli_reference`, `test_manifest_validator`, `test_naming_policy` and `test_repo_hygiene` **445 passed**; the scaffold pin, collision, referent-chain, leg-path and e2e files **245 passed**; `tests/test_update_shape*.py` **218 passed, 3 skipped** (junctions, NTFS only); `tests/test_shape_doctor.py` **254 passed, 3 skipped** (the PowerShell line, a Windows host only). - On this head itself: the new file, `test_repo_hygiene`, `test_cli_reference`, `test_pin_validator` and `test_neutral_pin_validator` **244 passed**. - This head merged with origin/main a983e6a, in a throwaway worktree: the new file, `test_pin_validator`, `test_neutral_pin_validator`, the scaffold pin, collision, referent-chain and e2e files, the whole doctor file and `test_cli_reference` **590 passed, 3 skipped** (the PowerShell line); the new file alone, in its final form, **137 passed**. origin/main had moved twice since; the merge-tree below is against it. - Twelve mutation probes, each hunk reverted alone: all caught. The note-to-finding one is caught by the update test (a committed mutant, since the test clones the repository's HEAD) and the guards by the sibling test, run with every test that needs a `#!` stub deselected as a Windows runner would; the hint one by an in-process test. - Nothing ran on macOS or Windows before this PR's CI. **Merge-tree, as computed during the fix round on 2026-10-12** (every sha and head is the one it had then; main and the other branches have moved since). `git merge-tree --write-tree` of this head (6494714) against origin/main 1f90a0f is clean, and so is the merge of origin/main a983e6a, which the merged-tree runs above used. #312, #309, #270, #268, #228, #245 and #351 have landed. The rest were computed on ad6f569 (the same tree but for one test file) against a983e6a. Open PRs touching this branch's files, all clean against the head: #314, #355 and #373 (`scaffold-project.py`), #343 (`validate-pins.py`), #390 (`repo_shape.py`) and #394 (`repo_shape.py`, which conflicts with origin/main by itself). The other 25 open PRs and the lane-5 branches `fix/adopt-check-link-and-manifest`, `fix/scaffold-free-text-values`, `fix/family-init-values`, `docs/adopt-execute-exits`, `fix/yaml-flow-brackets-surrogates` and `fix/adopt-pin-writes-pin-file` (#273's, 7a94b6b, also clean for this head) are clean too. #356, #382 and `fix/adopt-record-submodule-placement` conflict with origin/main and not with this head; #358, `fix/adopt-late-failure-notice` and `fix/adopt-plan-quote-yaml-keywords` conflict with both, with origin/main by themselves, so they are not this branch's. Neither are the five PRs below #200 that origin/main has not landed (#94, #96, #98, #100 and #161), which conflict with it too. The PR set is read from the refs under `refs/pull/` that origin/main's history has no `(#n)` subject for, not from GitHub, so a closed one may be in it. **One conflict, with `fix/link-source-values` (#338, 40762ca) in `scripts/repo_shape.py`, four hunks.** Not resolved in either branch; take both sides: (1) `resolve_link_source`'s signature, `default_source=None, env_shared: bool = False, *, tried: list | None = None`; (2) its docstring, both paragraphs; (3) `link_pins_from_trees`'s docstring, both paragraphs; (4) its call, `default_source, pin_product_key(name) in shared, tried=asked)` followed by #338's `if tried is not None: tried[name] = asked`. The auto-merged `resolve_link_source` body already composes: a shared variable is never an `env` candidate and so never appears in #338's `tried`. **Known limits after the re-verification (APPROVE WITH NITS, 0 blockers, 0 should-fixes, 3 nits, at 6494714).** For the four non-ASCII pairs this change now detects (`ß`/`ss`, `ſ`/`s`, `fi`/`fi`, `ς`/`σ`), the keyed `--pin-source <product>=<path>` form does NOT separate the two pins: #312's source lookup folds both keys into one, so one pin can get a false `unreadable` finding. A sibling checkout does separate them. Only a hand-written `project.yaml` can declare such a pair. The doctor's `pins` row still prints only "passes" for a validator that passes, so it does not carry the collision note; that needs `shape-doctor.py`, and is left for a follow-up. The wording for restoring the finding is under "Fix round 1" above: it is 34dbd86's `report.finding(...)` call, not a word swap. Reviewed adversarially at 34dbd86 (CHANGES NEEDED: 0 blockers, 2 should-fixes, 5 nits). The first should-fix was the finding itself, which locked a project that already declares the pair out of every `update-shape.py apply`; the second was a description that had gone stale. Fix round 1 (6494714) made the pair a NOTE, which is the coordinator's decision and one Brett may reverse (the way back is above), keyed collisions by the pin file's own fold, gave the guard a test that runs on Windows, and made a shared product's skip hint offer the keyed `--pin-source`. Its re-verification at 6494714 found every finding handled: APPROVE WITH NITS (0 blockers, 0 should-fixes, 3 nits: the keyed form and the non-ASCII pairs, the doctor's silence about the note, and the wording for restoring the finding, each stated above). CI is green on Linux, macOS and Windows, with SonarCloud passing, at b70a07d (main a73b4e9 merged in). Its merge with main 5a25387 passed the targeted set (the six pin and scaffold test files, `tests/test_validate_pins_unreadable_workflow.py`, `tests/test_ruleset_hint_tracking_branch.py` and `tests/test_shape_doctor.py`): 850 passed, 3 skipped. Closes #275 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
… writes its pin file as the scaffold does, so an adopted root passes `validate-pins.py` (#273); a `--pin-source` that cannot answer is `pin-source-unreadable` (#339) (#508) **Brought up to date after #409 squash-landed** by merging origin/main (9736424, #409's squash) as f9ed46f. Its `scaffold-project.py` conflict was resolved to this branch's side once `git diff b70a07d 9736424 -- scaffold-project.py` (#409's final head against its squash) came out empty. So the file is the one tested on this branch plus main's own later changes to it, and holds nothing of #409's; details below. This branch carries #275's adopt half. It was built on `fix/pin-source-env-collisions`, PR #409 (#275's scaffold half and its fix round), merged at 6494714 as 583f647, over the earlier 5690235 (34dbd86). After 6494714, #409 only merged main, five times, up to its final head b70a07d, and none of those merges has a hand edit (`git show --remerge-diff` is empty for each). Adopt's refusal of two products that share a source variable goes through #409's function, so this branch lands after #409. It was never rebased. `git diff b70a07d 9736424` is empty for each file #409 touched: `scaffold-project.py`, `scripts/repo_shape.py`, `templates/assembly-root/scripts/validate-pins.py` and `tests/test_pin_source_env_collisions.py`. What f9ed46f resolves, four files with one hunk each: - **`scaffold-project.py`.** - The conflict hunk is main's block from `PIN_COLLISION_REMEDIATION` through `_pin_collision_remediation`, which #409's squash edits: it adds `PIN_VARIABLE_REMEDIATION`, `_colliding_pin_variables` and `_pin_collision_remediation` and changes `_refuse_colliding_pins`. This branch moved all of that to `scripts/shape_materialize.py`. The hunk takes this branch's side. (The merge commit's message says "re-adds"; this is the exact account.) - #409's auto-merged import of `PIN_SOURCE_ENV_PREFIX, pin_source_env_collisions` is dropped, because nothing in the file uses either name any more. - It is not `git checkout --ours` of the whole file. Main changed the file beyond #409 since this branch last merged it: #355's `--tracking-branch` check (6c2a371) and a73b4e9's `RULESET_HINT`. Those lines stay. - The result is the file `git merge-tree` gives for this branch with main before the squash (518010b). Its diff from the file tested here has the same patch-id as main's own 3ef059c..518010b change to it. - **`templates/assembly-root/scripts/validate-pins.py`.** Main's side, which is #409's final blob, with main's #359 workflow reader in it. This branch had no change of its own to the file beyond #409. - **`adopt-project.py`, `_commit_the_split`.** - Main: #445 wraps the function in its late-failure `try`, and #394 builds an `append` with the plan's submodule record. - This branch: its only edits there were a comment and `neutral_pins=_recorded_pins(plan)` on the materializer call. - Both now sit on main's call inside the `try`: `append=append, neutral_pins=_recorded_pins(plan)`. - **`docs/cli.md`.** It is regenerated with `scripts/render-cli-reference.py`, and `--check` is ok. It differs from main's only in the `--pin`, `--pin-owner` and `--pin-source` help of `adopt-project.py plan`. `scripts/repo_shape.py`, `templates/assembly-root/scripts/validate-pins.py` and `tests/test_pin_source_env_collisions.py` are main's blobs, which are #409's: this branch adds nothing to them. Nothing of #409 is lost. Its scaffold behaviour is `shape_materialize.refuse_colliding_pins` and `pin_variable_collisions`, keyed by #409's `pin_product_key`. **73d65c6, after the merge.** - **The change.** `_read_pins` reads `plan.get("pins")` where it read `plan.data.get("pins")`. - **Why.** #445's `tests/test_adopt_late_failure.py` hands `_commit_the_split` a `SplitPlan` as its plan, a `dict` that adds `entries`. On f9ed46f, three of its tests stopped at "committing the split" with `AttributeError: 'SplitPlan' object has no attribute 'data'` (3 failed, 83 passed in that file). - **Nothing else changes.** `Plan.get` with no default returns `plan.data.get(key)` exactly, so a loaded plan reads as before. `adopt-project.py plan --pin` took a NAME only (`--pin openGlass`) and refused a trailing `@<commit>`. `plan`, `check` and `execute` each exited 0, and `execute` printed `adoption verified`. The manifest then listed the name in `neutral_product_pins:` with no `contracts/<product>-pin.yaml` beside it, and on its first run the adopted root's own `validate-pins.py` refused it as `neutral-pin-missing`. #273's table, reproduced on c7c02fa with the issue's own commands (local bare remotes, and a stub `gh` that exits 97): | `--pin` | c7c02fa | this branch | |---|---|---| | `openGlass` | `execute` exit 0, `- openGlass`; clone's `validate-pins.py` exit 2 `neutral-pin-missing` | `plan` exit 2 `pin-malformed`, with the scaffold's remediation (`--pin openGlass@<40 hex>`); no plan is written | | `opensoft/openGlass` | exit 0, `- opensoft/openGlass`; exit 2 `neutral-pin-missing` for `contracts/opensoft/openglass-pin.yaml` | `plan` exit 2 `pin-malformed` | | `spec` | exit 0; `validate-pins.py` exit 0, reading the leg's `contracts/spec-pin.yaml` as the neutral pin | `spec@<sha>` is `adopt-pin-collides` at `plan` | | `shape` | exit 0; `contracts/shape-pin.yaml` read as the neutral pin | `shape@<sha>` is `adopt-pin-collides` at `plan` | | `openGlass@<sha>`, `opensoft/openGlass@<sha>` | `plan` exit 2 `adopt-pin-malformed` | `plan`, `check` and `execute` exit 0, and `gh` is called 0 times. `contracts/openglass-pin.yaml` is written and the manifest lists `- openGlass`. In the clone, `validate-manifest.py` exits 0, and `validate-pins.py` exits 0 with `openGlass: pin commit … digest recomputes` | Brett ruled for decision 1's first option ("go with your recommendation on #273"): adopt takes the scaffold's pin and writes the scaffold's pin file. That reverses the NAME-only `--pin` from #6, and the comment that described it is replaced with the new contract. - **One implementation.** The scaffold's pin code moved into `scripts/shape_materialize.py`, next to `neutral_pin_path` and the materializer that writes the file. What moved: - `PIN_ARG_RE`, `parse_pin`, `pin_digest_from_source` and `pin_digest_from_gh`; - `declared_pin_values`, `declared_pin_products` and `PIN_COLLISION_REMEDIATION` (in #309's wording); - from #275's branch, `PIN_VARIABLE_REMEDIATION` and the variable check; - `_refuse_colliding_pins`, split into the pure `pin_collisions` and `pin_variable_collisions`, and `refuse_colliding_pins(refusal=...)`, which raises one refusal for both. The code did not move verbatim. `_refuse_colliding_pins` became the three functions above; `pin_digest_from_source` was rewritten for #339 (below); and fix round 1 changed three of them (below): `pin_variable_collisions` keys each pin by #275's `pin_product_key`, `pin_digest_from_source` spells a path that is not printable, and `pin_digest_from_gh` refuses a missing `gh`. The scaffold's behaviour is otherwise unchanged. `scaffold-project.py` imports what it calls under its old names. Lane 5's review ran 34 scaffold cases (every pin refusal, the `gh` stub's four modes, dry and real successes) on 34dbd86 and on this branch at 7a94b6b. rc, stdout, stderr, the `gh` log and the files made were identical, apart from the shape's own commit and digest and #339's row. Its pin tests and #275's pass as they are. `tests/test_git_output_decoding.py`'s exemption for `pin_digest_from_gh` follows the function to its new file. - **`plan`.** `--pin`, `--pin-owner` and `--pin-source` mean what they mean for the scaffold. `plan` reads the digest from `--pin-source` or `gh api`, and records product, repository, commit, digest and `digest_source` in the plan, so `check` and `execute` read no network for a pin. `plan` refuses: - a name with no `@<commit>`, as `pin-malformed`. That includes an empty `--pin ''`, which adopt used to drop and now refuses, as the scaffold always has; - `spec`, `code` and `shape`, in any case, as `adopt-pin-collides` (#273 decision 4), in the scaffold's sentence and remediation (#241, #309); - one product given twice, the same way, whether it is a repeat or two names that share one file (decision 5); - two products that one `SHAPE_PIN_SOURCE_<PRODUCT>` names, such as `open-glass` and `open_glass`, the same way, through #275's check; - a leg path over a pin's file, as `adopt-leg-path-collides`; - a `--pin-source` path that `check` would refuse in `digest_source:` (fix round 1), as `adopt-unwritable-value`. It is asked only when a `--pin` records the path; - with no `gh` on PATH and no `--pin-source`, `gh-not-found` (fix round 1). - **`check` and `execute`.** These are new `check` findings, and `execute` refuses each one before any leg exists. They come from `_leg_findings`, the list `check` and `_refuse_what_check_finds` share: - `plan-pin-unrecorded`: a name alone (an old plan's `pins: [openGlass]`, or a hand edit), or an entry missing its commit or digest; - `plan-pin-malformed`; - `plan-pin-collides`, which covers two pins at one file, a pin at a file the shape writes, and two products that share one source variable; - `plan-root-holds-pin`: a source path kept in the root at the pin's file. The materializer would write the shape's pin beside it under `shape/`, and `validate-pins.py` would read the source's file instead. How each finding ends: - `plan-pin-unrecorded` and `plan-pin-malformed` end with the `plan --pin <product>@<40 hex commit>` that records the pin whole. - `plan-pin-collides` ends with re-running `plan` with each product once, or, for two products that share a source variable, with one product of each such group. - `plan-root-holds-pin` ends with the kept path's own remedy: send that entry to a leg, `drop` it, or rename it in the source and re-run `plan`. A leg moved over a pin's file in the plan is `plan-leg-path-collides`. - **Also part of #386: its `pins:` row.** A hand-edited `pins:` that is not a list (a name such as `pins: openGlass`, a number, a mapping, a quoted string) is now `plan-pin-malformed` in `check` (`` `pins:` is 'openGlass', not a list of pins ``), and `execute` refuses it before any leg exists. - Where: `_read_pins` in `adopt-project.py`; the test row is `plan-pin-malformed-not-a-list` in `tests/test_adopt_pins.py`. - Before: on main, `check` said `plan ok` for the name, the mapping and the string, and gave a traceback for the number (reproduced by lane 5's review on cb7063f). #386 records that `execute` then wrote the name as eight one-letter pins. - Not fixed here: #386's other keys, `allow_empty_legs:`, `paths:`, `legs:` and `source:`, so #386 stays open. Lane 2's #435 takes all of #386, `pins:` included, and its `pins:` rule keeps only text items where this branch writes mappings; whichever lands second reconciles them (#435 (comment), and "Where the meaning meets" below). - **`execute`.** It writes each pin through `materialize_assembly_root(neutral_pins=...)`. The manifest lists the product with the owner stripped (decision 3), so the entry and the file agree. - **Docs.** `--help` is updated and `docs/cli.md` regenerated (`render-cli-reference.py --check` ok). `pin_digest_from_source` caught only `Refusal`. A directory that is no clone is git's own failure, so it was always `pin-source-unreadable`, exit 2. A path `subprocess.run` cannot enter never reaches git: it is a `FileNotFoundError`, `NotADirectoryError` or `PermissionError` out of `cwd=`, and nothing caught it. `scaffold-project.py --pin ... --pin-source <that>` ended `--dry-run` and the real run alike in a traceback, exit 1. Since the move above, `adopt-project.py plan --pin` reads its digest through the same function and ended the same way. Both reach it through `declared_pin_values`, and `check` and `execute` read no source, so that call is the only reader. The issue's table, run on ab623dd for the scaffold and on d806e8f for `adopt-project.py plan` (adopt has no `--pin-source` on ab623dd), then on this head. `gh` is a stub that logs and exits 97, with `GH_TOKEN` bogus and an empty `GH_CONFIG_DIR`; it was called 0 times in every run: | `--pin-source` | before | this branch | |---|---|---| | a directory that is not a clone | `pin-source-unreadable`, exit 2 | unchanged | | a path that does not exist | `FileNotFoundError` traceback, exit 1 | `pin-source-unreadable`, exit 2: `<path> could not answer for opensoft/openGlass @ <40 hex>: no such directory` | | a file | `NotADirectoryError`, exit 1 | exit 2, `... it is a file, not a directory` | | under a parent this user cannot enter | `PermissionError`, exit 1 | exit 2, `... Permission denied` | | a symlink loop (not in the issue) | `RuntimeError` out of `Path.resolve()` on 3.12, exit 1 | exit 2, `... Too many levels of symbolic links` | The real run and `--dry-run` give the same stdout and stderr, and nothing exists afterwards (no remote directory, no work directory; for adopt, no plan file). What changed, in `scripts/shape_materialize.py` only: - `pin_digest_from_source` catches `OSError` as well as `Refusal` and raises the existing `pin-source-unreadable` with its remediation unchanged. The message names the path and the reason. - `stat()` judges the source inside the guard, as #312 does in `validate-pins.py`, and the sentences are that file's: a missing path, or one beneath a file, is "no such directory"; a file is "it is a file, not a directory"; any other `OSError`, from `stat()` or from git's `cwd=`, is the system's own reason (`strerror`). It is not `exists()`/`is_dir()`, which before 3.14 swallow only a missing path. A source `stat()` can read but `cwd=` cannot enter, a directory with no search bit, fails in `tree_digest` and the same guard answers it. - `declared_pin_values` resolves the path through a guarded `_resolved_pin_source`. `Path.resolve()` raises a `RuntimeError` on a symlink loop before 3.13 (an `OSError` since), before the digest was asked for. As typed, `stat()` meets the loop and says so. - A source that answers is read exactly as before: a relative path resolves against the working directory, a symlink to a clone is followed, `digest_source` reads `local-clone (<path as typed>)`, and the digest is `tree_digest`'s. `tests/test_pin_source_unreadable.py` (46 tests in the #339 commit; fix round 1 adds 17 for S1 and moves one, below) runs each case through `scaffold-project.py --dry-run`, its real run and `adopt-project.py plan`: missing, a file, beneath a file, relative and missing, a parent and a source with no search bit (skipped on Windows and as root, by a mark, so before any fixture builds a directory), a 300-character name, a symlink loop (skipped on Windows) and a directory that is no clone. Each asserts exit 2, the refusal naming the path and the reason, its remediation, no traceback and nothing created. It also holds `--dry-run` and the real run to the same stdout and stderr, checks the path is printed escaped under an ASCII stderr and as a surrogate when it is not UTF-8, and checks that a relative source and a symlink to a clone read as before. The session `gh` guard fails any test whose run called `gh`. origin/main ab623dd reached this branch through implQ's branch: 34dbd86 is a30091f merged with ab623dd. One file conflicted there, `templates/assembly-root/scripts/validate-pins.py`, in seven hunks, because #312 (8cdca5e) replaced `keyed_sources` and `default_source` with `PinSources` and #275 added `env_shared` and `shared`. #312's blob is the same at its PR head, at the squash and at ab623dd, so it landed the file implQ resolved against. The resolution keeps #312's side in all seven and adds #275's on top: | # | hunk | resolution | |---|---|---| | 1 | `resolve_neutral_pin_source` signature | #312's `sources: PinSources` and 3-tuple return, plus #275's `env_shared: bool = False` | | 2 | its docstring tail | #312's `Returns (path, how, typed)` text, plus #275's "Rule 2 is skipped when `env_shared`" paragraph | | 3 | `_recompute_neutral_pin_digest` signature | #312's `sources`, plus #275's `shared: frozenset[str]` after `report` | | 4 | the `resolve_neutral_pin_source(...)` call inside it | #312's 3-tuple unpack, plus `env_shared = product.casefold() in shared` and the extra argument | | 5 | `_check_one_neutral_product_pin` signature | #312's `sources`, plus #275's `shared` | | 6 | its call of `_recompute_neutral_pin_digest` | `sources, report, shared` | | 7 | `_check_neutral_product_pins` docstring and body | #312's `PinSources(...)` and `_note_unused_pin_sources`, plus #275's paragraph, `_report_pin_source_collisions(products, report)` and `shared = frozenset(pin_source_env_shared(products))` | This branch's own merge of 34dbd86 (5690235) had no conflict, and neither had its merge of #409's fix round, 6494714 (583f647). That fix round made `neutral-pin-source-collides` a note and keyed "one product" by `repo_shape.pin_product_key`. At 421e4e5, `templates/assembly-root/scripts/validate-pins.py`, `scripts/repo_shape.py` and `tests/test_pin_source_env_collisions.py` were byte-identical to the tree `git merge-tree` gives for 6494714 with origin/main 3ef059c (blobs 5604007, 091966b, 5220787), so this branch added nothing to those three files beyond #409. The 34dbd86 merge brought in #335, #312, #317, #270 (f795cf7), #344 and #349 with no conflict. Earlier merges of origin/main (87373a6, for #309's reworded constant, and e43fa95) are unchanged. Fix round 1 merged origin/main three more times: - **cd1fc79 (2bcfa69).** It brought #245, #351, #350, #345, #372, #371, #354, #380, #377 and the rest since ab623dd, with no conflict. - **45e01f4 (4098b61).** It brought #396's `tests/test_agents_md_codes.py`. That test finds `scaffold-pin-collides` as a string constant of `scripts/shape_materialize.py`, which `SHIPPED` lists. - **3ef059c (421e4e5), #314.** One conflict, one hunk in `adopt-project.py`. This branch's `_refuse_an_unrecordable_pin_source` and #314's section "The project must be able to hold what `execute` writes into it (#261)" were added at the same place; both are kept. Where the two meet, lane 5's earlier recipe applies: - `_flag_plan` builds each pin as the `PIN_FIELDS` mapping `plan` writes, not `list(pins)` of products. So the plan `plan` asks the validator about is the plan it writes. - #314's `test_plan_asks_what_check_asks_of_the_same_keys` plans `--pin openGlass@<commit> --pin-source <stand-in>` where it planned the name alone, which `plan` now refuses. Then f9ed46f, after #409 squash-landed, and 73d65c6 after it (top of this description). On the merged tree exactly one test failed: `tests/test_adopt_template_values.py::test_check_finds_and_execute_refuses_a_hand_edited_value[pins-surrogate]`. It plans, then hand-edits the plan to `pins: ["openX\ud800"]`, a name-only pin with a lone surrogate. Before this branch the refusal's headline was `REFUSED plan-unwritable-value`. A name-only pin in a plan is now `plan-pin-unrecorded` (#273), so `check` has a second, independent finding for that very plan, and the headline is the first finding's: ``` REFUSED plan-pin-unrecorded: 2 finding(s) ... FINDING plan-pin-unrecorded: `pins:` entry 1, 'openX\ud800', is a name with no commit and no digest ... FINDING plan-unwritable-value: a `pins:` item is 'openX\ud800', which `execute` writes into the assembly root's `project.yaml`, and it holds a lone surrogate ... ``` Nothing about #270's contract changed. The lone surrogate is still found as `plan-unwritable-value`, listed under the refusal, and `execute` still refuses before any leg exists. `check` builds `_leg_findings`, where `_pin_findings` sits, before it calls `_value_findings`, so the pin finding comes first. d806e8f changes one assertion for that row, `pins` only: the headline is `REFUSED plan-pin-unrecorded: ` where it was `PLAN_REFUSED`. The other 15 rows and the finding assertions, including the `plan-unwritable-value` line, are untouched. Lane 5's review of 7a94b6b found 3 should-fix and 5 nits. Each is answered here: - **#409's `pin_product_key` (review N1).** The merge of #409's fix round could not carry it, because this branch had moved the function that used it out of `scaffold-project.py`. `pin_variable_collisions` now keys each pin by that function, where it casefolded. `openß` and `openss` are two pin files and one `SHAPE_PIN_SOURCE_OPENSS`, and a casefold named each of the two pins twice. No `--pin` or recorded `pins:` entry `PIN_ARG_RE` admits can spell either, so the test asks the function directly. `refuse_colliding_pins`' docstring now says, in #409's words, that the root's `validate-pins.py` only notes such a pair. - **S1, a `--pin-source` path that is not printable.** `pin_digest_from_source` names the path as typed unless it is not printable (`str.isprintable`), and then as Python's `repr` spells it (`shown_source`). That covers the refusal's head and git's own reason, which names the directory it ran in. Printed raw, an ESC reached the terminal, and a line break or a carriage return began a line of the path's choosing, a forged `REFUSED` line among them, from the scaffold and from adopt. The traceback #339 replaced had shown a `repr`. A printable path is named byte for byte as before. - **S2, no `gh` on PATH.** `pin_digest_from_gh` refuses as `gh-not-found`, exit 2, as `repo_shape.tree_digest_from_gh` names it, and offers `--pin-source`. It was a `FileNotFoundError` traceback, exit 1, from the scaffold and, new with this branch, from `adopt-project.py plan --pin`. - **S3, a hand-edited `commit:` or `tree_sha256:`.** The lane's decision is to accept it and document it (option (c)); no behaviour changed. A pair that disagrees passes `check` and `execute` (`adoption verified`), because neither reads a network for a pin, by design. The adopted root's own `validate-pins.py` then exits 1 with `neutral-pin-digest-mismatch` wherever it can read the product. `check` now says so once, after the pins it will write: "each pin as `plan` read it, not read again here: the root's own `validate-pins.py` recomputes its digest wherever it can read the product". Options (a), `execute` recomputes before it creates the legs, and (b), `check` recomputes from a clone still on the machine, are filed as #449. - **N2, a `--pin-source` that `plan` cannot record.** `plan` asks `check`'s question (`one_printable_line`) of the path it records in each pin's `digest_source:`, before any digest is read. It refuses one that fails as `adopt-unwritable-value`, in #270's words (`unwritable_reason`), or as "an invisible formatting character" where those name nothing. Before, it was planned, exit 0, and printed raw in the report. `check` then refused it as `plan-pin-malformed` and said to re-run the same `plan`. The report's `pin` line and `check`'s `write` line now spell the digest's source with `_shown`. The scaffold half is pre-existing (a line break in the path is written into the pin file; a byte that is not UTF-8 dies after all three repositories exist) and is filed as #450. - **N3, six hunks no test guarded.** Each now has a test that fails when it alone is reverted: - the two name guards (a `pins:` name holding an ESC and a forged line); - the lower-casing of the commit and the digest (an upper-cased hand edit); - the `digest_source` printability check; - the not-text check (`commit: 1234567` unquoted); - the gitlink filter (a submodule kept where the pin is written is `plan-submodule-root` alone). - **N4.** This body. - **N5, #339's third decision.** A `--pin-source` that nothing reads is still silent, now in adopt too. This PR does not decide it. #451, filed, lays out refuse, note and leave, so closing #339 loses nothing. Lane 5's second review, of 421e4e5, found no blocking item, 2 should-fix items and 3 nits. The two should-fix items are this body: the landing after #409 (above) and #386 (above). The code needed no change for them. f63d1ed adds two tests and no code: - **n1. `plan` asks nothing of a `--pin-source` no `--pin` records.** With the `args.pin and` guard dropped, such a path holding an ESC or a zero-width joiner was refused in a sentence naming no file ("`execute` writes into , and ..."), and every test still passed. `test_a_pin_source_no_pin_records_is_not_asked_about` holds the plan to exit 0 and `pins: []`, with the path named nowhere in the plan or the report. The ESC row is POSIX-only by the file's `POSIX_NAMES` mark; the zero-width joiner row runs everywhere. It holds #451's answer today, silence, so a decision there changes this test on purpose. - **n2. A printable `--pin-source` is printed as it is.** With `_spelled` in place of `_shown` in `plan`'s `pin` line, every test still passed. `test_a_printable_pin_source_is_printed_as_it_is` holds `plan`'s `pin` line, and `check`'s `write` line for the plan `plan` wrote, to their exact text, ending `(local-clone (<path>))` with no backtick. - **n3** needs a code change and is left out (below). **Fix round 2, on f63d1ed.** Each run had a `gh` stub first on PATH (it logs and exits 97), `GH_TOKEN` bogus, an empty `GH_CONFIG_DIR`, `git-filter-repo` 2.47.0 next on PATH, and the identity blanked with `GIT_CONFIG_COUNT`; the session `gh` guard came first inside each run. - **The pin set: 812 passed, 2 skipped.** It covers adopt pins, adopt plan, #339's file, #275's tests, the pin and neutral-pin validators, git output decoding, adopt template values, CLI reference, scaffold pin and reuse, scaffold pin collisions, #314's two files and #396's file. The skips are `test_adopt_plan.py:403` (no MedxEHR clone) and `test_adopt_template_values.py:223` (this host reads `git config user.name` from the repository, not from `GIT_CONFIG_GLOBAL`). That is fix round 1's 809 passed, plus the three new rows. - **Shape doctor and scaffold leg-path collisions: 369 passed, 3 skipped** (the PowerShell line exists only on a Windows host). - **Mutation probes**, each one line changed in a throwaway worktree, over `tests/test_adopt_pins.py` and `tests/test_pin_source_unreadable.py` less the five end-to-end adoptions: - the `args.pin and` guard dropped: 2 failed, both n1 rows; - `plan`'s `pin` line `_spelled`: 1 failed, the n2 test; - `check`'s `write` line `_spelled`: 2 failed, the n2 test and `test_an_upper_cased_hand_edit_is_written_lower_cased`; - `plan`'s `pin` line raw: 1 failed, the no-break-space test that already guarded that hunk. **On the merged tree, 73d65c6** (the merge f9ed46f and the `_read_pins` commit), with the same harness: - **The pin set** above, with #445's `test_adopt_late_failure.py`, `test_adopt_git_reads.py` and `test_adopt_filter_repo_failure.py` and #387's `test_adopt_empty_symlinks.py` and `test_adopt_root_manifest.py`: **988 passed, 2 skipped**. The skips are the two above. - **Shape doctor, scaffold leg-path collisions and the adopt suites:** **967 passed, 3 skipped** (the PowerShell line). The adopt suites are e2e, execute refusals, verify, root shape paths (#387's too), leg-path collisions, clone template, ignored shape files and submodules. - **On the merge alone, f9ed46f** (the same tree as before the `_read_pins` commit): `tests/test_adopt_late_failure.py` gave 3 failed, 83 passed, the three `AttributeError` rows above. - `render-cli-reference.py --check` is ok on the merged tree. pyflakes reports the same four warnings on `adopt-project.py`, `scaffold-project.py` and `scripts/shape_materialize.py` as before the merge. **Fix round 1, on 421e4e5.** Each run had `git-filter-repo` 2.47.0 on PATH and the identity blanked with `GIT_CONFIG_COUNT`. A `gh` stub that logs and exits 97 was first on PATH, with `GH_TOKEN` bogus and an empty `GH_CONFIG_DIR`, and the session `gh` guard came first inside each run. - **The pin set:** 809 passed, 2 skipped (the same files and skips as above). - **Doctor and scaffold:** 782 passed, 3 skipped (the PowerShell line exists only on a Windows host). It covers shape doctor, scaffold leg-path collisions, scaffold e2e, referent chain links, naming policy, repo hygiene and #351's free-text values. - **The adopt suites:** 882 passed. It covers e2e, execute refusals, verify, spec-only, leg paths, root shape paths, plan escapes, leg-path collisions, clone template and submodules. - **Before the #314 merge,** on 74ce4d9's tree: the first two sets gave 577 passed, 2 skipped and 782 passed, 3 skipped. On 2bcfa69's tree (the merges alone, before the fix) they gave 542 passed, 2 skipped and 782 passed, 3 skipped. After the 45e01f4 merge, `tests/test_agents_md_codes.py`, repo hygiene and CLI reference: 110 passed. - **Lane 5's second review** also ran 421e4e5 merged with main cb7063f and with 53bee5b: the pin set, the manifest-value set, and the doctor set with #357's `tests/test_doctor_validator_rows.py`. All passed, with only the skips above. Proof that fix round 1's tests test something: - **On 2bcfa69's tree,** with this head's two test files laid over it, 31 of the new rows fail: - S1: 17; - N2 and its report spelling: 7; - S2: 3; - the `pin_product_key` port: 2; - `check`'s S3 line: 1; - the not-UTF-8 `adopt plan` row: 1. The N3 rows pass there, as they must: they guard hunks this branch already had. - **Mutation probes.** Each hunk was reverted alone, in a throwaway worktree, and both test files were run: 18 of 18 were caught. - **The port:** the casefold keying (2 fail); the casefold lookup alone (2). - **S1:** the path named raw (13); git's reason named raw (2); every path `repr`'d, printable or not (27). - **S2:** no `except` (3). - **N2:** no `plan` check (11); only #270's question asked (1); the formatting-character reason said otherwise (1); the report's `pin` line raw (1); `check`'s `write` line raw (1). - **S3:** the line dropped (1). - **N3:** the `_pin_fix` name guard (1); the `_read_pins` path guard (1); the lower-casing (1); the `digest_source` check (1); the not-text check (1); the gitlink filter (1). - **The issue rows reproduced.** S1, S2 and N2 were reproduced on 2bcfa69's tree and shown gone on the fix (a probe driving both tools; `gh` was called 0 times). **Before fix round 1.** Runs were on 7a94b6b, with `git-filter-repo` 2.47.0 first on PATH and the repository-local `user.name` blanked with `GIT_CONFIG_COUNT` (a workstation whose standard checkout sets one fails `test_the_scaffold_refuses_with_no_elector` on main itself; CI sets none). 7a94b6b differs from ffd1297 in `tests/test_pin_source_unreadable.py` only, so the three suites that do not read that file ran on ffd1297: - the new file: 46 passed; - adopt pins, adopt plan, git output decoding: 172 passed, 1 skipped (the MedxEHR clone is absent); - scaffold pin and reuse, pin collisions, e2e, referent chain links, neutral pin and pin validators, #275's tests, naming policy, repo hygiene, CLI reference, scaffold leg-path collisions: 571 passed; - shape doctor: 254 passed, 3 skipped (the PowerShell line exists only on a Windows host); - the new file with repo hygiene, git output decoding and CLI reference, once more after the last edit: 176 passed. Earlier on 5690235 and d806e8f, before the #339 commit: the adopt suites (e2e, execute refusals, verify, spec-only, leg paths, root shape paths, plan escapes, leg-path collisions, clone template) 740 passed, and `tests/test_adopt_template_values.py` with #273's other adopt tests 489 passed, 2 skipped. They were not re-run on 7a94b6b: the #339 diff touches `pin_digest_from_source` and the call in `declared_pin_values`, which those suites reach through `test_adopt_pins.py` only. Nothing was run on macOS or Windows. Proof the new tests test something: - **On d806e8f.** 38 of the 46 fail, each on the traceback and its exit 1. The 8 that pass are the unchanged behaviour: the not-a-clone refusal (4 rows) and the valid-source rows (4). - **Mutation probes.** Ten hunks reverted one at a time, each failing at least one new test: the `OSError` catch (16 fail), the catch narrowed to `OSError` only (21), the missing-path clause (9), the clause narrowed to `FileNotFoundError` (3), the `S_ISDIR` judgement (3), the loop guard (4), no resolve at all (3), the old `.resolve()` call (4), the reason as `str(exc)` (12), and the file sentence as the OS's (3). - #273's own 52 tests still fail on c7c02fa (51 fail or error; the one that passes checks the test's own table), and its 18 mutation probes were each caught. **Re-verified after the merge.** An independent re-verification of f9ed46f and 73d65c6 found the code correct: - #409 is carried whole. A 30,000-case fuzz of `--pin` lists through #409's final refusal and this branch's port differed in 0 cases. - `scaffold-project.py` equals the tested file plus main's own changes that are not #409's (same patch-id). - `_commit_the_split` keeps #445's `try` and #394's `append`. - `_read_pins` reads a loaded plan as before. Its findings were all in this description (the #506 bullet and three stale facts), now fixed. Runs on the head and on its merge with main 48585e6: 988 passed, 2 skipped; and 967 passed, 3 skipped. `git merge-tree --write-tree` was run at 01:48Z on 2026-10-13, comparing hunks per file. Each PR head, fetched from `refs/pull/<n>/head` with no `gh`, was merged with 73d65c6 and with origin/main 9736424 alone. Two sets were asked about: - every PR this description listed before that main does not now contain; - every PR numbered above #468 whose head main does not contain: #473, #474, #490, #491, #493, #494, #495, #496, #497, #503, #504 and #505. **Open state, read with `gh` by the coordinator at 02:00Z and refreshed at 02:25Z after a re-verification on main 48585e6.** Every pull request named below is open. #184, #94, #96, #98, #100 and #161 are closed, and #465 landed as 48585e6. `fix/adopt-plan-quote-yaml-keywords` has no pull request. #506 (`fix/scaffold-reuse-seeded-legs`) opened after this list was made, and the re-verification reads it. - **Landed since the last draft of this list:** #343, #346, #355, #358, #373, #378, #382, #387, #394, #397, #401, #404, #409, #419, #452, #456, #465, #466 and #484. #394's `_commit_the_split` conflict and #401's `docs/cli.md` conflict are both resolved in f9ed46f, above. - **Clean, 20:** #266, #347, #356, #374, #376, #384, #395, #453, #467, #468, #473, #474, #490, #491, #493, #494, #495, #497, #503 and #505. #453, #468 and #491 were updated onto main since the first read and now merge clean. #507 (lane 5's #486-#488) shares only the generated `docs/cli.md` with this branch and merges clean with it. - **Conflicting with main alone, and this branch adds no hunk:** - #402: `shape-doctor.py` 5; - #496: `shape-doctor.py` 10 and `tests/test_shape_doctor.py` 1. **Conflicts this branch adds:** - **#435** (3efcc7a, lane 2's `fix/adopt-plan-field-types`, for #386). - It adds 2 hunks in `adopt-project.py`: 5 here, against 3 with main alone. They are `render_plan`'s `pins:` line and `Plan.pins`. - Keep this branch's `_pin_lines` and `_pin_products`. The rest is under "Where the meaning meets". - **#504** (37ef29a, `fix/adopt-root-kept-symlinks`). - It adds 1 hunk in `adopt-project.py`: 2 here, against 1 with main alone. It is the materializer call in `_commit_the_split`. - Keep both arguments and both comments: `append=append, neutral_pins=_recorded_pins(plan), spell=_spelled`. - **#506** (119d8c4, `fix/scaffold-reuse-seeded-legs`). - It adds 1 hunk in `scaffold-project.py`. With main alone, #506 conflicts only in `docs/handbook.html`. - #506 inserts `LEG_TEMPLATES` right after `declared_pin_products`, which this branch moved out, and rewrites `_reusable_remotes` further down. Keeping both sides breaks the file with an unterminated string literal. - Keep only #506's `LEG_TEMPLATES` block. Drop #506's `declared_pin_values`/`declared_pin_products`, which moved here, and this branch's three-line `_reusable_remotes` opener, which #506 rewrites below. Resolved that way, pyflakes is clean and #506's scaffold tests pass (88). - **`fix/adopt-plan-quote-yaml-keywords`** (ed9968f; a branch with no pull request). - It adds 2 hunks in `adopt-project.py`: 3 here, against 1 with main alone. - One is `render_plan`'s `pins:` line. Keep `_pin_lines`, which already sends product, repository and `digest_source` through `y`. - The other is `_refuse_unholdable_values`' signature. Keep `pins: dict`, with that branch's `_validator_judged` and `_named_as_read` above it. - `fix/adopt-late-failure-notice` is no longer on origin. #445, the late-failure notice, is on main and merged here. **Where the meaning meets:** - **#435 and #386.** The coordinator has said this on #435: #435 (comment) - #435's `_plan_key_findings` (`plan-bad-pins`) keeps only text items in `pins:`, and its `PINS_REMEDIATION` tells a person to write `pins: [openGlass]`. - This branch writes each pin as a mapping, and it makes a name alone `plan-pin-unrecorded`. - Read from #435's code, not run: on a tree with both, the plan `plan --pin` writes is `plan-bad-pins` in `check`. - Whichever lands second reconciles them, with three changes: - `_plan_key_findings` takes this branch's mappings, or leaves `pins:` items to `_read_pins`; - a `pins:` that is not a list gives one finding, not both `plan-bad-pins` and `plan-pin-malformed`; - `PINS_REMEDIATION` says the mapping `plan` writes. - Until that is done, `tests/test_adopt_pins.py::test_a_plan_with_a_whole_pin_checks_ok` fails. - **#314.** It landed, and it is merged here (above). - **`fix/adopt-plan-quote-yaml-keywords`.** Its pin tests are written against the name-only `--pin`. They need the `name@commit` form, and their expected `pins: [...]` line becomes `_pin_lines`'s mapping. - **Two unwritable values are refused in two runs (lane 5's second review, n3).** `plan --elected-by 'A<LF>B' --pin openGlass@<sha> --pin-source <a path holding an ESC>` refuses the elector alone, exit 2. Re-run with the elector retyped, it refuses the source; #270's refusal names every unwritable value at once. Reproduced on 421e4e5, with `gh` called 0 times. One refusal would need a code change, not made here: - `_refuse_an_unrecordable_pin_source` would split into a function that returns the sentence; - `_checked_plan_inputs` would ask it before #270's `unwritable_values` raise, and raise one `adopt-unwritable-value` that names both, with both remediations; - a test would add the two-value row. N2's question is `one_printable_line`, not `unwritable_reason`, so it cannot simply join #270's list. - **Docs.** The README, AGENTS.md and `docs/handbook.html` are unchanged, because none of them calls adopt's `--pin` a name. A sentence saying adopt now takes the scaffold's `--pin` is a follow-up. It costs an AGENTS.md line-cap raise, and a README edit forces the handbook's README blob to change. - **A product spelled `null`, `true` or `false`.** The scaffold already accepts `--pin null@<sha>`: it exits 0 and builds a root whose `validate-pins.py` exits 1 `neutral-pin-entry-malformed`. Adopt's `plan` writes `product: null` through `y`, and `check` then finds `plan-pin-malformed` and refuses it before any leg exists. The one fix for both tools is in `parse_pin`, and it would change the scaffold's behaviour. - **Wording shared with the scaffold.** `digest_source` is written bare into the pin file, so a `--pin-source` path holding ` #` is cut short there. `templates/assembly-root/contracts/neutral-product-pin.yaml` and `validate-pins.py`'s `neutral-pin-missing` remediation name only the scaffold as the writer. Two comments in `scripts/repo_shape.py` still name the scaffold's `pin_digest_from_gh`, in a digest-pinned shipped copy. - **Filed as follow-ups:** - #449: S3, a hand-edited `commit:` or `tree_sha256:` that disagrees, with options (a) and (b); - #450: the scaffold half of N2, a `--pin-source` path no file can hold; - #451: #339's third decision (N5), a `--pin-source` that nothing reads. - **`validate-pins.py` prints a `--pin-source` path raw** in `neutral-pin-source-unreadable`, the twin of S1. It is a shipped template that #409 and other open PRs edit, so it is left for its own change. No issue is filed for it yet. - **Windows and macOS** were not run here; CI ran both (below). Two assertions that depend on what Windows answers for a path (a name past the filesystem's limit, a path beneath a file) check only the refusal and its remediation there. Fix round 1's rows that need a control character or a byte that is not UTF-8 in a path, or a symlink to `git`, are skipped on Windows by a mark, and so is fix round 2's ESC row. The rows for a no-break space and a zero-width joiner run everywhere, and so does the in-process `gh-not-found` test. Reviewed independently by lane 5 at 7a94b6b (3 should-fix, 5 nits: fix round 1) and at 421e4e5 (0 blockers, 2 should-fix, 3 nits: fix round 2). After #409's squash, the re-verification of f9ed46f and 73d65c6 found the code correct; its 1 should-fix and 3 nits were in this description and are fixed. GitHub's update-branch then merged main twice (33bae3f, 2170526), with no hand edit. CI's tests, tests-macos and tests-windows and SonarCloud all passed at 2170526 (main cac6bb2 merged in). On that head merged with main 57c85e9 (#490, #474 and #468, none of which touches this branch's files), the pin and adopt set with `tests/test_update_shape_landing.py` gave 1112 passed, 2 skipped, and the doctor, adopt end-to-end, unfinished-merge and bootstrap set gave 1056 passed, 3 skipped (the doctor's PowerShell line, which exists only on a Windows host). Closes #273 Closes #339 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lane: openreposhape-2 (openRepoShape-2)
Claim: #99 (comment)
Closes #99. Follows #95 / PR #96 (
3d46ab5), which landed the doctor read-onlywith a rows registry and an empty
fixslot.Brett Heap, 2026-09-10:
This is the reading half of that sentence. It moves nothing.
The row
placement, on an assembly root (n/aon a family holder, absent where thereis no shape root at all). For each mounted leg it takes the tracked paths with
ONE
git ls-files, classifies them againstcontracts/path-classification.yaml— the same policyadopt-project.py planuses, walked with that tool's ownwalk()by import rather than asecond copy that would drift — and names every path the policy would have put
somewhere else, each with the rule id that judged it.
The fold is the adoption's too: a directory whose files agree is one path
however many files are under it, so
src/with a thousand.pyfiles in thespec leg is one line of the report and one entry of the plan. One git call per
leg, one process for the classifying, no subprocess per path.
okwhen nothing is misplaced;notewhen the only paths are ones thepolicy will not call — a question for a human is never a finding, which is the
line #96 drew for the leg files and which this row could otherwise cross by a
different route;
FINDINGotherwise, and then the verdict isMISPLACED (n paths), exit 1, underDRIFTEDand overINVALID. A legoff its pin outranks it because the paths this row read are then not the paths
the pin describes; a red validator does not, because it names itself in the
table either way while nothing else in this report can see where a file lives.
What it never judges, and it says so
The four files
templates/spec-root/andtemplates/code-root/themselvesship —
README.md,AGENTS.md,CLAUDE.md,.gitignore— read from thosetemplates at run time, so the standard's own scaffold can never produce a
finding; plus the front door and the forge furniture (
LICENSE,.gitattributes,CODEOWNERS,.github/**— whole, because a spec leg runsits own CI and
.github/workflows/**classifies ascode). The path policyhas no such list of its own; this row defines it,
--jsonprints it undereverywhere, and the plan writes it underignored:.The plan, and why its
kind:differs--placement-plan <file>writes the paths as an adoption-plan-style YAML: theentries are
adopt-project.py plan's entries exactly, plusin_leg:(the legthe file is in today) and an empty
resolution:, so a person who has resolvedan adoption plan has already resolved this one.
It carries
kind: placement-planand notadoption-plan, deliberately. Anadoption plan is the input to
adopt-project.py execute, which creates tworepositories and rewrites history with
git filter-repo; a file that calleditself one would be that command aimed at a project already split, with
--yesthe only thing in the way.
adopt-project.pyrefuses the kind by name andthe refusal says which tool consumes one. The constant is declared once, beside
PLAN_KINDin the file that owns plan kinds.--placement-planis the only thing this command writes; it writes only thefile named, creates no directory, and refuses by name on a root that has no
placement audit (exit 3, the documented "usage or environment").
A later
shape-doctor.py --fix --plan <file>carries a resolved plan out bypaired leg pull requests and a root pin bump, printing what it will do and
asking first. Until it lands the moves are the human's, and the row says so.
Every
Check.fixin the registry is stillNone.Evidence
Both live estates on this machine, read-only, unchanged by the run:
templates/is untouched:git diff --stat origin/main -- templates/is empty.Tests
Fourteen new tests in
tests/test_shape_doctor.py, plus the three row-setassertions this row joins. The scaffold's own legs are
ok(which is not atautology — all four of their files classify as
root); both directions atonce give
MISPLACED (2 paths)with each path and rule named and both in--json; aLICENSEand a.github/workflows/*.ymlin the spec leg stayok;examples/alone is anoteand stillCOMPLIANT; a thousand files under onedirectory are ONE path; committing the file instead of staging it makes the
verdict
DRIFTEDwhile the placement row is still a FINDING in the table; theplan is written, the repository under the root is byte-for-byte unmoved, and its
entries are handed to
adopt-project.py's ownPlanand_leg_findings, whichdo not refuse on their shape and refuse on exactly the unresolved ones; and
adopt-project.py check --planrefuses a placement plan by name.Caps moved with dated entries naming #99: AGENTS.md 400 → 415, README.md
1324 → 1370.
docs/handbook.htmlre-cut for the README's new blob, with theregeneration named in its footer.
One standing dependency: #98
shape-doctor.py --helpgained--placement-plan, sodocs/cli.md— whichPR #98 generates from every tool's
--helpand guards with a drift test —will need regenerating. At the time of writing #98 is still OPEN and
origin/mainis3d46ab5, this branch's base, sogit merge origin/mainis ano-op and there is no
docs/cli.mdto regenerate. If #98 lands first, thisbranch must merge
mainand re-runpython3 scripts/render-cli-reference.pybefore it merges, or that test goes red on
main.🤖 Generated with Claude Code
Summary by Sourcery
Audit mounted leg contents for policy-defined placement drift and produce a safe, human-resolved plan without moving files.
New Features:
placementdoctor row that audits tracked files in mounted spec and code legs against the shared path-classification policy and reports misplaced paths with their governing rules.--placement-planto write a human-resolvable placement plan without modifying the repository.Bug Fixes:
MISPLACEDwhen actual misplaced paths exist, while preserving drift and validator precedence.Enhancements:
placement-plankind so placement plans cannot be passed to adoption execution accidentally.Documentation:
Tests:
Chores: