Repository navigation
shape_materialize.py: RULESET_HINT names the run's tracking branch where it named main, as the pull request's base and the ref the scaffold and the family holder pushed (#324) - #373
Merged
Conversation
…where it named `main`, and the scaffold, the family holder and both adopt legs pass it, so a `--tracking-branch trunk` run's refused push no longer advises `push -u origin main:seed/scaffold` (a branch its work clone does not have) and `gh pr create --base main` (a base its repository does not have) (#324) `RULESET_HINT` is what a person reads when an organisation's ruleset has just refused a direct push: seed a branch, open a pull request. It hard-coded `main` as the local branch of `git -C <work> push -u origin main:seed/scaffold` and as `gh pr create --base main`. A run made with `--tracking-branch trunk` makes its work clones with `git init -b trunk` and pushes `trunk`, so the first command died on `error: src refspec main does not match any` and the second named a base the repository does not have. The hint now carries a `{tracking}` field in exactly those two places, and each place that prints it passes the run's tracking branch: - `scaffold-project.py`: `_seed_legs` (a leg's push) and `_build_assembly_root` (the root's) pass `args.tracking_branch`. - `scripts/family.py`: `_create_holder_worktree` passes `args.tracking_branch`. - `adopt-project.py`: `_extract_leg` and `_seed_leg` pass the plan's `tracking`, the branch `_open_the_pull_request` already uses as its own `--base`. - No caller lacked a tracking branch to pass. Each value has been through `checked_value` by then (it admits only `[A-Za-z0-9._/@+~-]` and no leading `-`), so it is printed as it stands, like `{repo}`. - `tests/test_ruleset_hint_tracking_branch.py` runs each of the five callers against local bare remotes whose `pre-receive` hook refuses everything but `seed/*` (a ruleset in miniature, installed through `GIT_TEMPLATE_DIR`), once with `--tracking-branch trunk` and once with the default. It pins the printed block against a literal copy of the hint as it was, runs the printed push where it can run (the scaffold's legs and root, the family holder) and sees `seed/scaffold` arrive, and scans every shipped `RULESET_HINT.format(` for a call that leaves `tracking=` out, which would raise `KeyError` inside the failure path that exists to explain a refusal. Unchanged, and diffed on the five callers' printed output before and after: for the default `main` the hint is byte-identical. What the hint advises is the same, and so are its two exits (seed by pull request, or ask an operator with the bypass right; never `--admin`, per AGENTS.md); only the branch it names moves. Left out: an adoption's printed `push -u origin` still cannot run, because `git filter-repo` removed `origin` from the extracted clone and the seeded clone never had one (#177, open). The branch it now names is the one the clone holds whenever the plan's tracking branch is the source's default branch, which is where the adoption's own pull request already lands; #177's own hint is the place to push `HEAD`. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 8 hours and 56 minutes by commenting |
Reviewer's GuideUpdates ruleset-refusal instructions to use the invocation’s tracking branch for both the suggested seed push and pull-request base, propagates that value through all five rendering paths, and adds focused integration, text, and AST coverage. Sequence diagram for tracking-branch ruleset refusal guidancesequenceDiagram
participant Caller as Scaffold Family or Adopt caller
participant Materializer as shape_materialize.py
participant Work as Work clone
participant GitHub as GitHub
Caller->>Materializer: RULESET_HINT.format(tracking=tracking_branch)
Materializer-->>Caller: Hint with tracking push source and PR base
Caller->>Work: git push -u origin tracking:seed/scaffold
Work-->>Caller: Ruleset refusal or seed branch created
Caller->>GitHub: gh pr create --base tracking --head seed/scaffold
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Author
|
(lane openRepoShape-3, 14:34Z) @codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This was referenced Oct 12, 2026
…arried, `HEAD` for both adopt legs, so an extracted leg's hint no longer advises pushing the source's own branch of the tracking branch's name in place of the commit the tool pushed (#324) Review of the first commit of this branch found that `{tracking}` was one value doing two jobs. As the base of `gh pr create` it is right everywhere. As the local side of `git -C <work> push -u origin <ref>:seed/scaffold` it is the ref the refused push CARRIED only where the tool pushed the branch by name, which the scaffold and the family holder do from a clone made on it. An adoption pushes `HEAD:refs/heads/<tracking>`, and the extracted leg's clone holds the source's own branch of that name, filtered by `git filter-repo`: another commit than the one pushed whenever the adopted branch is not the tracking branch. Once `origin` exists (#177) the printed push would have seeded the leg with the wrong line's history, where on `main` it failed loudly; and for a source with no branch of that name the hint still named a ref the clone does not have. The hint now has two fields where it had one: - `{local}` is the ref in `{work}` the refused push carried, and stands in the push command only. The scaffold's two pushes and the family holder's pass `args.tracking_branch`, which is what they push; `_extract_leg` and `_seed_leg` pass `HEAD`, which is what they push. - `{tracking}` is the remote's branch and stands in `--base` only, passed by every caller as before. Tests (`tests/test_ruleset_hint_tracking_branch.py`): - the pre-change literal gains a marker for the local ref, so the text for `main` is still pinned byte for byte, and a case pins `HEAD` with `trunk`; - the scan of `RULESET_HINT.format(` calls requires every field the template has, not only `tracking`; - the test hook records the commit it refuses in `refused`, and each of the five callers asserts that the ref its hint names is AT that commit, not only that a branch of the name exists; - an adoption's source is on `master` with a second line named like the tracking branch, so the extracted clone holds a `<tracking>` that is another commit than the push, and the test asserts that too, so it cannot pass by a scenario in which `HEAD` and the tracking branch happen to agree; - a source with no branch of the tracking branch's name is adopted too: the clone has none, and the hint names `HEAD`. Unchanged: the text for the scaffold's two pushes and the family holder is byte-identical for any branch to what the first commit printed, and for the default branch to what `main` printed before this branch. An adoption's push line reads `HEAD:seed/scaffold` where it read `main:seed/scaffold` for the default branch; the hint's other lines and its two exits are untouched. Left out: `gh pr create --base <tracking>` cannot run in any of the five places, for `main` as for any other branch, because the repository the push was refused at is empty (`gh repo create` is called without a README), so the pull request has no base to land on; this is older than this branch and is named in the pull request. The printed adopt push still cannot run for want of `origin` (#177). Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
Author
|
(lane openRepoShape-3, 15:43Z) fix round 1 (independent review, codex is out of quota): Head is now
Tests on |
…s are two asserts each, so a failure names the half that failed (SonarCloud python:S9073) (#324) The text test for another branch asserted `"bypass right" in printed and "pull request" in printed` and the test for the two fields asserted `"main" not in printed and printed.count("trunk") == 1`. Each is split into one assert per condition, in the same order; neither had a message, so no failure message changes, and nothing else in the file does. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This was referenced Oct 12, 2026
…king-branch Conflict in adopt-project.py, resolved to main's file byte for byte. #445 gave adoption its own ADOPT_RULESET_HINT, which already names the tracking branch (`{repo} has no {tracking} ...`) and prints no command, so this branch's two adopt-leg edits to RULESET_HINT calls are dropped. The scaffold and family call edits merged cleanly and are unchanged. RULESET_HINT's comment no longer names the adopt legs as callers. tests/test_ruleset_hint_tracking_branch.py drops the adopt push-line cases and pins instead that ADOPT_RULESET_HINT holds no `main` and that both adopt legs print it for the run's tracking branch. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…king-branch No conflict. #355 asks git about `args.tracking_branch` (refuse_bad_branch) before anything exists, and refuses a value git would not echo as written, so the value every RULESET_HINT call passes as `{local}` and `{tracking}` is the one git accepted as a branch. adopt-project.py is still main's. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
changed the title
RULESET_HINT names the run's tracking branch where it named main, and the scaffold, the family holder and both adopt legs pass it, so a --tracking-branch trunk run's refused push no longer advises push -u origin main:seed/scaffold (a branch its work clone does not have) and gh pr create --base main (a base its repository does not have) (#324)RULESET_HINT names the run's tracking branch where it named main, as the base of its pull request and as the local ref the scaffold and the family holder pushed, so a --tracking-branch trunk run's refused push no longer ad
brettheap
changed the title
RULESET_HINT names the run's tracking branch where it named main, as the base of its pull request and as the local ref the scaffold and the family holder pushed, so a --tracking-branch trunk run's refused push no longer adRULESET_HINT names the run's tracking branch where it named main, as the pull request's base and the ref the scaffold and the family holder pushed (#324)
This was referenced Oct 12, 2026
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Contributor
Author
|
(lane openRepoShape-3, 2026-10-12T23:32Z) @codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
… exists, a source symlink with an empty target and the source's own `project.yaml` kept in the root, and `plan` no longer offers `root` for that file (#303, #305) (#387) Two ways `check` said `plan ok` to a source `execute` could not adopt, both reproduced on c7c02fa with the issues' own scripts. **#303, a symlink whose target is empty.** A `120000` entry whose blob is the empty blob passed `check`, which never reads a link's target. - With symlinks on (git's default where the disk can hold one), `execute` created both leg repositories, then failed in its first clone of the source: exit 2, nothing on stderr, two empty legs left behind. Linux refuses a symlink to nothing. This happened whatever the plan answered: `docs/empty-link` sent to spec, and a top-level `empty-link` answered `root` or `drop`. - With `core.symlinks=false`, it said `adoption verified`. The spec leg held the empty link, a clone of that leg exited 128, and the assembly's recursive clone exited 1. The top-level link answered `root` or `code` verified the same way. Only `drop` gave an assembly that clones. **#305, the source's own `project.yaml` answered `root`.** `plan` asked about it under `ambiguous-top-level-yaml`, whose question offers `root`. Answered `root`, `check` said `plan ok`. `execute` wrote the shape's manifest beside the source's file as `shape/project.yaml`, said "nothing was overwritten" and `adoption verified`. `shape-doctor.py` then said NOT A SHAPE ROOT (exit 2) and pointed at `adopt-project.py plan`. `make bootstrap` failed `naming (exit 2), manifest (exit 1), pins (exit 2)`. Answered `code` or `spec`, the doctor said COMPLIANT and `bootstrap ok`. **What changed.** - **`plan-empty-symlink` (#303).** `_empty_symlink_findings` reports each `120000` row of size 0 in `Source.tree()`. The size `ls-tree -l` gives a symlink is the length of its target, so no target is read; a SHA-256 repository reports size 0 too. - It is listed right after #196's `plan-unsafe-path`, through `_source_path_findings`, on the line `_entry_findings` already had. So `check` prints it, and `execute` refuses it through `_refuse_what_check_finds` before any leg exists (#168). - `EMPTY_SYMLINK_REMEDIATION` is said once after the findings, in `check` and in `execute`'s refusal. The fix is in the source: give the link a target, or remove it, commit, and re-run `plan`. - `plan` notes each such link in `check`'s words, and still writes the plan. - **`ambiguous-project-yaml` (#305).** A new rule in `contracts/path-classification.yaml`, above `ambiguous-top-level-yaml`, for the file `project.yaml`. Its question asks spec, code or drop, and says `root` is not an answer for this file. `release.yaml` keeps the old question. - **`plan-root-blocks-manifest` (#305).** #313's machinery is extended, not copied: `_shape_blocks`, which `_root_shape_findings` runs over each disk, now refuses the root holding `project.yaml` outright, with no copy question after it. It fires for a kept row at that path (file or symlink), or a kept directory there, on a disk that keeps case or one that ignores it. The finding's remediation is #313's `_fix_the_kept_path`. - A submodule there stays `plan-submodule-root`'s, said once. - `_kept_there` is the at-or-under lookup the copy branch already did, factored out for both. - `execute` refuses it before any leg exists, through the same list. - **Files.** `adopt-project.py` (+205/-13), `contracts/path-classification.yaml` (+28), the two new test files (399 and 366 lines) and `tests/test_adopt_root_shape_paths.py` (+6/-2). **Decisions.** - **#303 is refused whatever the plan answers, and whatever `core.symlinks` says.** - Where symlinks are made, no answer adopts. The run fails at the first clone of the source, which checks the whole tree out before anything is taken out. Measured with the link in `docs/` (sent to spec), at the top level answered `root` and `drop`, and with every path rooted or dropped and both legs seeded: there the seeded legs were pushed and the assembly's clone exited 128. - `check` cannot know where `execute` will run. A check that passed `drop` with symlinks off would pass a plan the next Linux machine cannot adopt, and the root's own `validate` workflow runs on `ubuntu-latest`. - The remediation costs one commit in the source. - **#305 covers the manifest only.** The issue asked whether other rendered root paths need the same rule. Measured on c7c02fa, with the source's own file kept in the root at each of the 18 `assembly_root_paths()`: - `check` passes all 18. - These verify over a broken assembly: `contracts/spec-pin.yaml` and `contracts/code-pin.yaml` (DRIFTED, pins fail); `scripts/validate-pins.py`, `scripts/validate-manifest.py`, `scripts/repo_shape.py` and `contracts/repository-naming.yaml` (INVALID); `scripts/validate-repository-naming.py` (COMPLIANT, naming fails); `scripts/bootstrap.py` (COMPLIANT, and `make bootstrap` runs the source's own script). - `contracts/shape-pin.yaml` fails verification after both legs exist. - README.md, AGENTS.md, CLAUDE.md, .gitignore, .gitattributes, AGENTS-shape.md and the workflow come out COMPLIANT with the copy beside them. `Makefile` is rooted by default, and the follow-up merge is its designed exit. - `project.yaml` is the one path `plan`'s own question offered `root` for. The others are reached only by hand-editing a `contracts/` or `scripts/` answer to `root`. They are left for a follow-up issue (under Left out), not folded in here. - **A symlink at `project.yaml` is refused too.** On c7c02fa: - a link to the source's own file verified and was NOT A SHAPE ROOT; - a link to nothing had the manifest written through it, and came out COMPLIANT on Linux. A clone that makes no symlinks holds that link as a file naming its target, so it is refused anyway, and #183 refuses a link to nothing at a shape path regardless; - a directory `project.yaml/` verified and was NOT A SHAPE ROOT. - **A case twin `Project.yaml` kept in the root is refused, with the case aside.** It adopts COMPLIANT on Linux. On a disk that ignores case, `place` writes the manifest beside it. This is the trade #172 and #313 already make. **Evidence.** In this section and in the two below it, "this head" is 69c54c9, the branch's own commit and the one the review approved; main and #314 were merged into the branch afterwards (last paragraphs). - **Reproductions, on this head** (exits as printed): - #303 run 1: `check exit 1`, one `plan-empty-symlink` finding; `execute exit 2`, `REFUSED plan-empty-symlink`, `leg repositories left:` empty. - #303 run 2 (`core.symlinks=false`): the same finding and refusal; no leg, no verification. - Top-level `empty-link`, symlinks off and answered root, code or drop, and symlinks on and answered root or drop: each `check exit 1`, `execute exit 2`, no leg. - #305 root: `check exit 1`, one `plan-root-blocks-manifest` finding; `execute exit 2`, `leg repositories:` empty. - #305 code, spec and drop: `plan ok`, `adoption verified`, the root's `project.yaml` begins `schema_version: 1`, doctor COMPLIANT, `bootstrap ok`. - The plan's entry for `project.yaml` now reads `rule: ambiguous-project-yaml`, with no `(root)` in its question. - `project.yaml` as a link to a file, a link to nothing, a directory, and the twin `Project.yaml`: each `check exit 1` and `REFUSED plan-root-blocks-manifest`. - No row needs real GitHub or a cloud tool. - **New tests.** `tests/test_adopt_empty_symlinks.py` (28) and `tests/test_adopt_root_manifest.py` (27). - The symlinks are made with `git update-index --cacheinfo`, so no disk symlink is needed. The one test that checks a link out (a link given a target, end to end) is skipped on Windows by its marker, before it runs. - On c7c02fa: 37 failed, 18 passed. The 18 are the controls that must pass on both sides: a symlink with a target, the remediations adopting, `project.yaml` sent to a leg or dropped, a submodule there, other shape paths keeping their copy, and `release.yaml`'s question. - **One existing test changed.** `test_a_file_kept_at_a_path_the_shape_writes_is_not_a_finding` in `tests/test_adopt_root_shape_paths.py` held that a kept file at any shape path is no finding. It now holds that for every path but the manifest, which is `plan-root-blocks-manifest`. - **Mutation probes:** 14 of 14 hunks reverted one at a time, each killed by at least one new test. - **Targeted runs.** The adopt list and the doctor ran on a local state of this branch (7a22a21) whose files were this head's but for `tests/test_adopt_root_shape_paths.py`. That file was re-run on this head, so the adopt list is two runs, not one sum. - The 15 adopt files (`test_adopt_empty_symlinks`, `_root_manifest`, `_root_shape_paths`, `_leg_path_collisions`, `_execute_refusals`, `_plan`, `_e2e`, `_submodules`, `_verify`, `_mount_ignored`, `_ignored_shape_files`, `_entry_paths`, `_finding_spelling`, `_history_gitlinks`, `_spec_only`) on 7a22a21: 837 passed, 1 failed, 1 skipped (a MedxEHR clone the plan test needs). The one failure is the #313 test this head changes. - `tests/test_adopt_root_shape_paths.py` alone, on this head: 120 passed. - `tests/test_shape_doctor.py`, as its own stream: 242 passed, 3 skipped (PowerShell lines only a Windows host prints). - `tests/test_scaffold_leg_path_collisions.py`, `tests/test_repo_hygiene.py` and `tests/test_cli_reference.py`: 167 passed. - This head merged with origin/main at 87373a6, which brings `tests/conftest.py`'s new `git()` identity: the two new files, `tests/test_adopt_root_shape_paths.py` and the three above, 342 passed. **The review.** An independent opus review at 69c54c9 approved with nits: 0 blocker, 0 should, 8 nit. It reproduced every row of both issues on c7c02fa and on this head, and killed 6 of 6 mutants: two of the author's, re-run, and four of its own. Its targeted runs on this head merged with origin/main ab623dd (`git merge-tree` clean, with #349's session `gh` guard in place): - the new files, `tests/test_adopt_root_shape_paths.py`, `tests/test_scaffold_leg_path_collisions.py`, `tests/test_adopt_execute_refusals.py`, `tests/test_repo_hygiene.py` and `tests/test_cli_reference.py`: 379 passed; - the other adopt files, in two streams: 303 passed, 1 skipped (the MedxEHR clone), and 323 passed; - the clone-template, literal-names, template-values, plan-escapes, stdout-encoding and Windows-path tests, which cover #268 and #270: 266 passed, 2 skipped (an operator `user.name` set; Windows only); - `tests/test_shape_doctor.py`: 254 passed, 3 skipped (PowerShell lines only a Windows host prints). c7c02fa's own `tests/test_adopt_root_shape_paths.py` against this head's code gave 1 failed (the `project.yaml` row, `:191`) and 119 passed, so no other #313 behaviour changed. - **A cross-lane composition, with lane 1's #183.** On the tree merged with lane 1's `fix/adopt-root-kept-symlinks` (#183, 6c44591), `tests/test_adopt_root_symlinks.py::test_a_link_with_an_empty_target_is_fixed_in_the_source` fails: `AssertionError: assert 'plan-empty-symlink' == 'plan-root-symlink'`. An empty link at or above a shape path is reported by both findings, and `execute` refuses with the first, which is this branch's. The two remediations agree (fix it in the source). Whichever of the two lands second removes the duplicate report. The same file also fails 3 other tests on origin/main with #183 and without this branch (`[a-link-at-the-copy-itself]` and `test_the_materializer_refuses_before_it_writes`, two cases), because #313's `plan-root-blocks-shape-copy` and `materialize-copy-blocked` answer first; those are not this branch's. - **Two known nits, left for a later round.** - An empty link AT `project.yaml` (or at a #313 copy path) gets `plan-root-blocks-manifest`, whose fix offers `drop`, and `plan-empty-symlink`, which says `drop` is no fix. - `plan` notes every empty link (25 links print 25 NOTEs), where `check` names 20 and counts the rest. - The other nits are smaller. A symlink target that begins with a NUL byte passes `check` and fails like #303, and the issue's `_verify` bullet, are follow-up issues; the case twin `Project.yaml` is under Left out. The new tests' `SYMLINKS_OFF` replaces the Windows job's own `GIT_CONFIG_COUNT` settings (`core.autocrlf=false`, `core.longpaths=true`) for those child processes, which is harmless while none of them checks anything out. **Merge-tree, as of 2026-10-12T15:14Z.** `git merge-tree --write-tree --name-only` of this head (69c54c9), against heads fetched then; every sha below is the one it had then, and main has moved since (see the merge paragraph below). It is a textual merge only; the semantic meeting with #183 was tested by the review (the first item above). - origin/main: clean at 0f67f7b. That includes #316 (34ecbc2), #268 (e43fa95), #270 (f795cf7) and #245 (161c6c9), which have landed since this branch was cut. It was clean at 161c6c9 and at ab623dd, the review's merge, too. - Open PRs touching `adopt-project.py`: #184 (f416e13), #353 (dcbc37b), #356 (5a5011d), #371 (34b9500), #373 (5ac03de), #377 (c35c896), #378 (2da8691), #382 (7e55d00) and #384 (a96297e) are all clean. #356 and #382 conflict with origin/main alone (2 and 5 blocks, from #245's landing), not with this head. - #314 (4b8da3d) was NOT clean: one block, in `_entry_findings`' docstring, where each side adds a paragraph after the same one (this head's "THEN THE SOURCE'S EMPTY SYMLINKS (#303)" and #314's "A VALUE NO FILE CAN HOLD"). Keep both. It had been clean at f45edf4, and it had 3 blocks against origin/main alone since #245 landed, so it needed a merge of main in any case. - Lane 1's branches without a PR: `fix/adopt-late-failure-notice` (9f1778d), `fix/adopt-record-submodule-placement` (70f4d29) and `fix/adopt-plan-quote-yaml-keywords` (d38e143) were clean. `fix/adopt-plan-quote-yaml-keywords` was 3 blocks against this head at e337f44, and conflicts with origin/main alone in 4. - `fix/adopt-root-kept-symlinks` (6c44591, #183) conflicts in 3 blocks of `adopt-project.py`: the module docstring's `check` paragraph, `_entry_findings`' docstring and its return line. They are the same 3 that c7c02fa itself has with that branch, and this branch's one added docstring line sits inside the first. It has 5 blocks against origin/main alone. - Lane 5's `fix/adopt-pin-writes-pin-file` (d806e8f): clean. `docs/adopt-execute-exits` is #371, above. - When #183 lands, an empty symlink kept at or above a shape path is reported twice, by `plan-empty-symlink` and by #183's `plan-root-symlink` (the first review item above). A dangling link kept at `project.yaml` is reported twice too, by `plan-root-blocks-manifest` and `plan-root-symlink`, and an empty one three times. Whichever lands second should keep one. **Brought onto main past #314 (bbbb541).** Since this branch was cut from c7c02fa, main took #245 (relative `.gitmodules` urls), #270 (`plan-unwritable-value`), #371 (`execute` verifies before it exits on a refused `gh pr create`), #377 (`plan` asks about every path `execute` will not extract) and #314 (`plan-manifest-invalid`, 3ef059c), all in `adopt-project.py`. The merge of origin/main 3ef059c has ONE conflict, in `_entry_findings`' docstring, where this branch adds "THEN THE SOURCE'S EMPTY SYMLINKS (#303)" and #314 adds "A VALUE NO FILE CAN HOLD IS SAID BEFORE THE VALIDATOR'S WORD ON IT (#255 before #261)" after the same paragraph: both are kept, this branch's first, because it describes the head of the list and #314's its tail. Nothing in code conflicted. `check` and `execute` read one list, and it composes as each feature says: a source's unsafe paths (#196), its empty symlinks (#303), coverage, `_leg_findings` with `plan-root-blocks-manifest` (#305), #166's split, #270's `plan-unwritable-value`, #314's `plan-manifest-invalid`, then #245's relative urls last. So an empty symlink beside an invalid value is still refused wearing `plan-empty-symlink`, with no leg created; probed on the merged tree with an empty link plus `visibility: secret`, plus a line break in `visibility`, and with `project.yaml` answered `root` plus `visibility: secret`, each listing the findings in that order. No test expectation or order changed: the merged tree was rehearsed before #314 landed (69c54c9 + 45e01f4 + c6f441b) and is the same tree object as the pushed merge, and `git diff 3ef059c bbbb541` is this PR's five files and nothing else (+1004/-15). Targeted runs on bbbb541: the new files, `test_adopt_root_shape_paths`, `test_adopt_manifest_values`, `test_scaffold_leg_path_collisions`, `test_agents_md_codes` and `test_cli_reference` 418 passed; `test_shape_doctor` and #377's, #245's and #270's adopt tests 595 passed, 4 skipped (PowerShell lines only a Windows host prints; one operator `user.name`); the other adopt files 234 passed, 1 skipped (the MedxEHR clone) and the guards, literal-names, clone-template, plan-escapes, gitmodules, leg-paths, stdout and Windows-path tests 1101 passed, 1 skipped. **Left out.** - `_verify` and the materializer do not themselves refuse a root `project.yaml` that is not the shape's manifest. This is the issue's second bullet. Once `execute` refuses the plan before any leg, through the same list `check` prints, nothing reaches them with one. It is a follow-up issue. - A top-level file named `project.yaml` only once case is ignored (`Project.yaml`) is still asked `ambiguous-top-level-yaml`'s question, which offers `root`. `check` refuses that answer with the case aside, before any leg. - #171 (a failed clone exits 2 silently) is lane 1's PR #184. This PR only keeps the empty-symlink run from reaching that clone. - Follow-up issues proposed by this PR (two since filed): - the source's own file kept in the root at one of the other shape paths the root's tools read (the pins, the validators, `scripts/repo_shape.py`, the naming policy, `scripts/bootstrap.py`, `contracts/shape-pin.yaml`), each measured above; - a symlink whose target begins with a NUL byte (`\0spec.md`, size 8), which `plan-empty-symlink`'s size test does not see: it fails like #303 (filed as #388); - `_verify` asking whether the split's root `project.yaml` is the shape's manifest (filed as #389); - an entry git cannot check out where `execute` runs, other than an empty symlink or a NUL-led one (a target longer than the platform allows, say), which is still found only by the first clone, after both legs exist. #303 leaves this as a note. Reviewed adversarially at 69c54c9 (APPROVE WITH NITS: 0 blockers, 0 should-fixes, 8 nits). The review reproduced every row of both issues on c7c02fa and on that head, and killed 6 of 6 mutants. No fix round followed: the branch's own commit is as reviewed, and what was added after it is the merge described above. CI is green on Linux, macOS and Windows, with SonarCloud passing, at fce86fa (main 7a8b981 merged in). Its merge with main a73b4e9 passed the targeted set (the three adopt test files, `tests/test_shape_doctor.py` and `tests/test_scaffold_leg_path_collisions.py`): 597 passed, 3 skipped. Closes #303 Closes #305 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 13, 2026
…-and-pr-tail One conflict, adopt-project.py's `_print_plan_report`: main's `_empty_symlink_notes` loop (#387) and this branch's `_tracking_branch_notes` loop, kept both in `_entry_findings`' order, the empty symlinks first. Everything else merged without conflict; docs/cli.md re-rendered byte-identical. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 13, 2026
…the tracking branch beside `master`, so `execute` reaches the leg push whose ruleset hint the test reads instead of refusing `plan-tracking-branch-missing` (#479, #480, #481) #373's `refused_run` built the adoption source on `master` alone and planned `--tracking-branch trunk` or `main`. With #479's `plan-tracking-branch-missing`, `execute` refuses that plan before any leg, so the four `adopt-*` cases of `test_an_adoption_hint_names_the_tracking_branch_the_run_was_made_with` found no hint (RV493's verification, N1). The source now gets `git branch <tracking>` beside `master`; it stays on `master`, so the test's `"master" not in printed` assertion still means what it says. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
…d `open_glass` share one `SHAPE_PIN_SOURCE_<PRODUCT>`, so `--pin` refuses the pair and `validate-pins.py` notes a declared one and reads the variable for neither (#275) (#409) `pin_source_env_name` turns every character outside `[A-Z0-9_]` into `_`, so `open-glass`, `open_glass` and `open.glass` are three legal product names, with three pin files (`contracts/<product lowercased>-pin.yaml`), and one variable, `SHAPE_PIN_SOURCE_OPEN_GLASS`. On ab623dd (this branch's main parent; the same rows reproduce on c7c02fa), with the issue's own probe against local bare remotes and a `gh` that exits 97: | step | ab623dd | this head (6494714) | |---|---|---| | `scaffold-project.py --pin open-glass@<A> --pin open_glass@<B>` | exit 0, three repositories | exit 2 `scaffold-pin-collides`, nothing created | | `validate-pins.py`, no source, on a project that already declares the pair | exit 0, both skips offer `SHAPE_PIN_SOURCE_OPEN_GLASS` | exit 0, one `note neutral-pin-source-collides`; the skips offer `--pin-source <product>=<path>` and a sibling checkout only | | `SHAPE_PIN_SOURCE_OPEN_GLASS=<open-glass>` | `neutral-pin-source-unreadable` for `open_glass`, exit 1 | exit 0, the note; the variable is read for neither pin, so each pin is read from its own source or skipped | | `SHAPE_PIN_SOURCE_OPEN_GLASS=<open_glass>` | `neutral-pin-source-unreadable` for `open-glass`, exit 1 | the same | | `--pin-source open-glass=... --pin-source open_glass=...` | exit 0 | both pins recompute, exit 0, the note | **What changed.** Every variable name is kept: an operator may have set one and the docs name them. What is done instead: - `scripts/repo_shape.py`: `pin_source_env_collisions(products)` is the one pure answer, `{variable: [products]}` for each variable two or more distinct products share. Distinct means distinct PIN FILES (`pin_product_key`, `lower()`, which is what `contracts/<product lowercased>-pin.yaml` is named by), so a product typed twice or in two cases is #241's file collision and never this one. `pin_source_env_shared` is the set of those keys. - `scaffold-project.py`: `_refuse_colliding_pins` (left where it is) names each variable's `--pin` arguments as typed after #241's file sentences, in the same `scaffold-pin-collides` refusal. Dry run and real run say the same no, before any process starts and ahead of the leg-path refusal. #241's wording and remediation are unchanged when only a file collides. A NEW pair is still refused, so the situation is not created anew. - `templates/assembly-root/scripts/validate-pins.py`: one NOTE per variable, `neutral-pin-source-collides`, naming every product and the variable, the keyed `--pin-source <product>=<path>` that reads each pin, and that which declaration stays is the human's decision. It is never a finding and never moves the exit code. A product whose variable is shared does not read it (rule 2 of `resolve_neutral_pin_source`), and its skip hint offers the keyed `--pin-source <product>=<path>` and not the variable or the bare form. A variable one product has to itself reads and is offered exactly as before. - `scripts/repo_shape.py`, the two chain readers: `link_pins_from_trees` does not read a variable two links of one chain share, and the `declared-unverified` hint stops naming it. Only a hand-written chain can have two such links, since scaffold takes every link but the first from the naming policy's alphanumeric `open<Product>` form. Each reader of `pin_source_env_name`, decided: | site | decision | |---|---| | `scaffold-project.py --pin` | refuse (above) | | `validate-pins.py` rule 2 and the skip hint | a note, and the variable is neither read nor offered for a shared product | | `repo_shape.py` `resolve_link_source` (via `link_pins_from_trees`) | skip the variable when another link of the chain shares it | | `repo_shape.py` `_chain_link_findings` hint | omit the variable for a shared link | | `adopt-project.py --pin`, `plan`, `check`, `execute` | not in this PR, see below | **Fix round 1 (from the review of 34dbd86).** **The design change: a declared pair is a NOTE and not a finding. This is the lane coordinator's decision, and Brett may reverse it.** The first round made `neutral-pin-source-collides` a FINDING. Main through ab623dd scaffolds such a project (`--pin open-glass@A --pin open_glass@B` exits 0), and `update-shape.py apply` runs the NEW `validate-pins.py` over the rewritten root, so for a project already declaring the pair the finding ended every apply as `REFUSED update-validators-red`, rolled back, for this update and every later one, until a person deleted a true declaration of descent. AGENTS.md tells an agent never to edit "a pinned file, a digest or a manifest to make a row go green", and the finding's remediation was exactly that, so the person was locked out. The coordinator chose the review's option (b): it is a NOTE now. The guard stays (the variable is read for neither pin), each pin still verifies from its own source, scaffold still refuses a new pair, and `validate-pins.py` says the rest in one line. To put the finding back, restore 34dbd86's `report.finding(...)` call in `_report_pin_source_collisions` (it is not a word swap for `report.advise`: `advise` takes one string, `finding` a code and a detail), give the finding the remediation 34dbd86 had, and turn the tests that expect the note (and `test_a_project_declaring_a_colliding_pair_takes_a_shape_update`) back into exit 1; the first round's versions of all of them are on 34dbd86. - **Proved on a real old project.** Main (ab623dd) scaffolded the pair into local bare remotes; `update-shape.py apply` to the first round's head 34dbd86 exited 2 `REFUSED update-validators-red` and rolled back (`git branch` shows only `main`); to this head it exits 0, the project's validator passes and prints the note, and the doctor's `pins` row is `ok` before and after. The doctor prints `passes` for a validator that exits 0 and does not echo its stdout, so the note itself is visible in `update-shape.py apply`'s output and in `validate-pins.py`'s, and the verdict moves for no reason but the update (`COMPLIANT, SHAPE BEHIND` to `COMPLIANT`). It is now a test: `test_a_project_declaring_a_colliding_pair_takes_a_shape_update`, over local stand-ins beside the project, so no `gh` is asked. - **Collisions by the variable.** Pairs were decided by `casefold()`, but the pin file is `lower()`. `openß`/`openss`, `openſ`/`opens`, `openfi`/`openfi` and `openς`/`openσ` (the letters are U+00DF, U+017F, U+FB01 and U+03C2 beside U+03C3) share a variable and have two pin files, and were never found. They are now, from the pin-file fold (`pin_product_key`). A pool of 31 names is checked pairwise against `neutral_pin_path` and `pin_source_env_name` themselves, and no single product is ever a collision. They are reachable only from a hand-written `project.yaml`. - **Windows.** The guard's only test needed the `#!` `gh` stub and was skipped on Windows. `test_a_shared_variable_answers_for_neither_pin_with_no_gh` puts a checkout of each pin beside the project, names one in the variable, and needs no `gh`: on ab623dd it is `neutral-pin-source-unreadable`, here each pin reads its own sibling. The hint and the non-ASCII rows are tested in-process, with no skip and no filesystem. - **The hint.** For a product two pins share, the skip hint offers the keyed `--pin-source <product>=<path>`: the bare form answers for every pin with no more specific source, so with two pins it gives the other the finding this issue is about. - **Trailers and title.** The trailers of the earlier commits (a30091f and 34dbd86 name Sonnet 5.5) are settled in this message, which carries Opus 5.5. The PR title now names `repo_shape.py`. - `scaffold-project.py` is not touched this round: `fix/adopt-pin-writes-pin-file` moves that code, and the change would only conflict with it. **Left out.** - `adopt-project.py` is untouched. Its half arrives with #273's PR, which moves scaffold's pin machinery into a shared module and routes adopt through `pin_source_env_collisions`. Once it lands, the one place that still keys by `casefold()` (`pin_variable_collisions` in `scripts/shape_materialize.py`) should read `pin_product_key`; it cannot differ for an ASCII `--pin`, which is all `PIN_ARG_RE` takes. - `scaffold-project.py`'s own `_colliding_pin_variables` also keys its argument lists by `casefold()` and one docstring there still says the project's validator reports the pair "as a finding"; both go with the code when #273 moves it. #312's `PinSources` keys `--pin-source` by `casefold()` too, so `openß=` and `openss=` are one key there; that is #312's. - A product in `neutral_product_pins:` and a chain link in `naming.referent_chain` that share a variable are read by two validators in one process, and neither sees both sets. Not detected here. - The doctor does not print a passing validator's stdout, so its `pins` row does not carry the note. Not changed: it would add a file to this PR. - No `--help` text changed, so `docs/cli.md` is untouched. `README.md` is at its line cap and its blob is pinned by `docs/handbook.html`, so the one README sentence this wants needs a regenerated handbook and waits for its own change. - `validate-pins.py` and `repo_shape.py` ship into projects, so every project's `shape-pin.yaml` reads `upstream-changed` for both until `update-shape.py apply`, which now succeeds for a project that declares such a pair. **Evidence.** All on the head (6494714), with git-filter-repo 2.47.0 on PATH, a `gh` that cannot be reached (a stub first on PATH, `GH_TOKEN` bogus, an empty `GH_CONFIG_DIR`), and the repository-local `user.name` blanked at command-line scope, as the standard's checkout sets one that two scaffold tests read. - `tests/test_pin_source_env_collisions.py` is 137 tests and passes on this head. Laid over 34dbd86, 50 fail and 87 pass: the note and update tests, the non-ASCII rows, the hint, the sibling guard, and `pin_product_key` itself (which 34dbd86 does not have); the scaffold refusals, the chain readers and the controls hold on both. - Targeted, run on ad6f569, whose tree differs from this head in `tests/test_pin_source_env_collisions.py` only (the last edits made one comparison read the committed blob and wrapped one line): the new file, `test_pin_validator`, `test_neutral_pin_validator`, `test_cli_reference`, `test_manifest_validator`, `test_naming_policy` and `test_repo_hygiene` **445 passed**; the scaffold pin, collision, referent-chain, leg-path and e2e files **245 passed**; `tests/test_update_shape*.py` **218 passed, 3 skipped** (junctions, NTFS only); `tests/test_shape_doctor.py` **254 passed, 3 skipped** (the PowerShell line, a Windows host only). - On this head itself: the new file, `test_repo_hygiene`, `test_cli_reference`, `test_pin_validator` and `test_neutral_pin_validator` **244 passed**. - This head merged with origin/main a983e6a, in a throwaway worktree: the new file, `test_pin_validator`, `test_neutral_pin_validator`, the scaffold pin, collision, referent-chain and e2e files, the whole doctor file and `test_cli_reference` **590 passed, 3 skipped** (the PowerShell line); the new file alone, in its final form, **137 passed**. origin/main had moved twice since; the merge-tree below is against it. - Twelve mutation probes, each hunk reverted alone: all caught. The note-to-finding one is caught by the update test (a committed mutant, since the test clones the repository's HEAD) and the guards by the sibling test, run with every test that needs a `#!` stub deselected as a Windows runner would; the hint one by an in-process test. - Nothing ran on macOS or Windows before this PR's CI. **Merge-tree, as computed during the fix round on 2026-10-12** (every sha and head is the one it had then; main and the other branches have moved since). `git merge-tree --write-tree` of this head (6494714) against origin/main 1f90a0f is clean, and so is the merge of origin/main a983e6a, which the merged-tree runs above used. #312, #309, #270, #268, #228, #245 and #351 have landed. The rest were computed on ad6f569 (the same tree but for one test file) against a983e6a. Open PRs touching this branch's files, all clean against the head: #314, #355 and #373 (`scaffold-project.py`), #343 (`validate-pins.py`), #390 (`repo_shape.py`) and #394 (`repo_shape.py`, which conflicts with origin/main by itself). The other 25 open PRs and the lane-5 branches `fix/adopt-check-link-and-manifest`, `fix/scaffold-free-text-values`, `fix/family-init-values`, `docs/adopt-execute-exits`, `fix/yaml-flow-brackets-surrogates` and `fix/adopt-pin-writes-pin-file` (#273's, 7a94b6b, also clean for this head) are clean too. #356, #382 and `fix/adopt-record-submodule-placement` conflict with origin/main and not with this head; #358, `fix/adopt-late-failure-notice` and `fix/adopt-plan-quote-yaml-keywords` conflict with both, with origin/main by themselves, so they are not this branch's. Neither are the five PRs below #200 that origin/main has not landed (#94, #96, #98, #100 and #161), which conflict with it too. The PR set is read from the refs under `refs/pull/` that origin/main's history has no `(#n)` subject for, not from GitHub, so a closed one may be in it. **One conflict, with `fix/link-source-values` (#338, 40762ca) in `scripts/repo_shape.py`, four hunks.** Not resolved in either branch; take both sides: (1) `resolve_link_source`'s signature, `default_source=None, env_shared: bool = False, *, tried: list | None = None`; (2) its docstring, both paragraphs; (3) `link_pins_from_trees`'s docstring, both paragraphs; (4) its call, `default_source, pin_product_key(name) in shared, tried=asked)` followed by #338's `if tried is not None: tried[name] = asked`. The auto-merged `resolve_link_source` body already composes: a shared variable is never an `env` candidate and so never appears in #338's `tried`. **Known limits after the re-verification (APPROVE WITH NITS, 0 blockers, 0 should-fixes, 3 nits, at 6494714).** For the four non-ASCII pairs this change now detects (`ß`/`ss`, `ſ`/`s`, `fi`/`fi`, `ς`/`σ`), the keyed `--pin-source <product>=<path>` form does NOT separate the two pins: #312's source lookup folds both keys into one, so one pin can get a false `unreadable` finding. A sibling checkout does separate them. Only a hand-written `project.yaml` can declare such a pair. The doctor's `pins` row still prints only "passes" for a validator that passes, so it does not carry the collision note; that needs `shape-doctor.py`, and is left for a follow-up. The wording for restoring the finding is under "Fix round 1" above: it is 34dbd86's `report.finding(...)` call, not a word swap. Reviewed adversarially at 34dbd86 (CHANGES NEEDED: 0 blockers, 2 should-fixes, 5 nits). The first should-fix was the finding itself, which locked a project that already declares the pair out of every `update-shape.py apply`; the second was a description that had gone stale. Fix round 1 (6494714) made the pair a NOTE, which is the coordinator's decision and one Brett may reverse (the way back is above), keyed collisions by the pin file's own fold, gave the guard a test that runs on Windows, and made a shared product's skip hint offer the keyed `--pin-source`. Its re-verification at 6494714 found every finding handled: APPROVE WITH NITS (0 blockers, 0 should-fixes, 3 nits: the keyed form and the non-ASCII pairs, the doctor's silence about the note, and the wording for restoring the finding, each stated above). CI is green on Linux, macOS and Windows, with SonarCloud passing, at b70a07d (main a73b4e9 merged in). Its merge with main 5a25387 passed the targeted set (the six pin and scaffold test files, `tests/test_validate_pins_unreadable_workflow.py`, `tests/test_ruleset_hint_tracking_branch.py` and `tests/test_shape_doctor.py`): 850 passed, 3 skipped. Closes #275 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 13, 2026
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
… is `PRODUCT=PATH` only when its key names a chain link, and a link source that cannot be entered is a WARNING with the system's reason, not a traceback (#338) (#453) **What was wrong.** Two faults, one in the parser and one in the lookup. - **The parser.** `_link_sources_from_args` split every `--link-source` value at its first `=`. A link tree under a directory named `a=b`, passed bare, became a source keyed to a link `a` that no chain names. Nothing asked for it, so the link stayed `declared-unverified`, exit 0. The WARNING then offered `--link-source openXdox=<path>` to the person who had passed exactly that. A mistyped key failed the same silent way. - **The lookup.** `repo_shape.link_manifest_path` asked `Path.is_dir()` and then `is_file()`. Before Python 3.14 those swallow only ENOENT, ENOTDIR, EBADF and ELOOP. So each of these ended the naming gate in a traceback, exit 1: - a keyed source under a parent this user cannot enter; - `SHAPE_PIN_SOURCE_<LINK>` pointing there; - a checkout `../openXdox` at mode 000, which every `--project` run asks for; - a 300-character name. **What changed.** **How a value is read** (`LinkSources` in `scripts/validate-repository-naming.py`). This mirrors #312's `PinSources`, with a chain link where that file has a declared pin: - The text before the first `=` is a LINK only when it casefolds to a link of a chain this run reads. Those are the entries of `--referent-chain`, and of each leg's `naming.referent_chain:` under `--project`. Both are read before any leg is classified. - The keyed reading wins even over a directory spelled like the whole value. `./` reads that directory. - Any other value is a bare PATH, taken whole. **What a lookup says** (`scripts/repo_shape.py`): - `link_manifest_lookup` asks `stat()` inside a guard, as `_digest_from_source` does in #312. It returns the system's reason when a source cannot answer, or says which of four plain cases it is (`no such file or directory`, `it holds no project.yaml`, `its project.yaml is not a file`, `it is neither a directory nor a file`), and it never raises. It replaces `link_manifest_path`, which is gone: nothing calls it. `link_declarations` gives the reason a manifest could not be read, and `declared_neutral_pins`, which it left with no caller, is removed (fix round 1). - `resolve_link_source` and `link_pins_from_trees` take a keyword-only `tried`. It records each source asked, in order, with its reason. Nothing in `repo_shape` prints. - `link_pins_from_trees` also takes a keyword-only `with_reasons` (fix round 2). Asked, it returns a `LinkPins`, a dict equal to the plain mapping that also remembers why a tree it reached could not be read. Only the scaffold asks. **What the validator prints** (stderr, after each name's WARNINGs): - `WARNING link-source-unreadable: <path> (<how>) could not answer for <link>: <reason>`. This covers a `--link-source` asked for a link that holds a pin, a `SHAPE_PIN_SOURCE_<LINK>` likewise, and a checkout beside the project that exists and still cannot answer. A keyed value that also names a directory gets a second line saying how to ask for that directory with `./`. A bare value holding `=` says it was read as a PATH. - `NOTE --link-source <typed>: not used — <why>.` for a value no such link asked for. The why is one of five: a later value replaced it; no name reads that link's own declaration (a keyed value); no name records a chain at all; no name reads any chain link's own declaration (a bare value, where chains exist and none is read); or every link that is read had a more specific source. - Three kinds of source are never named: a variable or a checkout asked only for a chain's LAST link (no one ever reads that link's own declaration); a variable or a checkout for a chain no classification reads (fix round 1: a spec leg's chain, a name with no referent, a chain whose shape fails; a `--link-source` value for such a chain is the NOTE); and a checkout that is absent or holds no manifest. - `--quiet` drops both, with the WARNINGs. So `make bootstrap` and the doctor's `naming` row print nothing new. `--help`, the module docstring and `docs/cli.md` (regenerated) say all of this. **Fix round 1 (lane 5's review of 40762ca: 0 blockers, 2 should, 4 nits).** Second commit, 4b21263. Each review finding, in order: - **A link manifest whose `neutral_product_pins:` is not a list of names is a tree that cannot answer** (SHOULD-1). `link_declarations` iterated `data.get(key) or []` as it stood, against the `link_pins_from_trees` docstring this change wrote ("nothing a source does raises out of here"). `5` ended the naming gate, the scaffold and the project's `validate-manifest.py` in `TypeError: 'int' object is not iterable`. A scalar `openDox` was read as the characters `D`, `e`, `n`, `o`, `p`, `x`, so the chain broke, and under `--project` that is a FINDING. A mapping was read as its keys, so `openDox: yes` DECLARED `openDox`. Each is now `(None, why)`, as `not a mapping` is: declared-unverified, with the reason named ("`neutral_product_pins` is not a list", "holds an entry that is not text"). An absent or null value, an empty list, and a blank entry (`null`, `''`) in a list, declare nothing, as ever. The fix is in the shared function, so the scaffold and `validate-manifest.py` are reached too. **Three behaviours change on purpose here, and the tests of fix round 2 pin each:** - **A falsy value is no longer an empty list.** `neutral_product_pins: 0`, `false` or `''` was read as no pin at all (`or []`), so a chain through the tree was BROKEN, and under `--project` that is a FINDING. It is now a tree that cannot answer: the link is declared-unverified, a WARNING, exit 0. - **An entry that is not text is no longer read as text.** `[openInk, 0]` and `[openInk, false]` dropped the entry as a blank, and `[openInk, true]` read it as the text `True`, so a chain through the list VERIFIED. Each is now declared-unverified, with a WARNING saying the list "holds an entry that is not text". That is stricter than the link tree's own `validate-manifest.py`, which takes `str(pin)` of an entry (`_declared_pins`, `templates/assembly-root/scripts/validate-manifest.py:85`): a chain is better left unverified than verified through a name nobody wrote. - **The scaffold no longer refuses `chain-not-declared` over a pins value that is a scalar.** With `--pin-source` a tree whose `neutral_product_pins:` is the scalar `openInk`, the scaffold used to refuse (`REFUSED chain-not-declared: ... openXdox declares I, e, k, n, o, p, not openInk`, exit 2, from the characters). It is now the declared-unverified WARNING and the scaffold goes on, exit 0, which is its documented rule for a tree that cannot answer: "a warning, never a refusal, because the check is offline". The warning says the tree was reached and could not be read (fix round 2, below). - **The reach wording is corrected** (SHOULD-2): "Who else this reaches" below, and the docstring of the test that made the claim. What stops crashing is the naming and manifest steps. `make validate`, `validate.yml` and `make bootstrap` still end in a traceback on the sibling row before Python 3.14, at a file this change does not touch (below). On 3.14 the sibling is passed over without a word there instead, which is #430. - **The mutation gaps** (nit). Six of the review's ten mutations survived. Five now die, each by a new test: a `project.yaml` that is a directory; a device or a FIFO as the source; the order A, B, A; a value holding `=` with no chain at all; and every failure of `SHAPE_PIN_SOURCE_<LINK>` for a link that holds a pin (a path that is not there, a directory with no manifest, a file that is no manifest), not only a permission. The sixth, the `typed in self.used` test in `unread_lines`, could not be killed because it was dead: a value is read as one link's keyed source or is the one bare PATH, so every lookup asks it of the same path and it is used or unread, never both. That was asserted over 293 tests and the test is removed. - **A chain no classification reads** (nit). `classify` consults a link's own declaration only for a name that claims a referent, with a recorded chain whose shape holds. For any other chain (one recorded on a spec leg; one on a name with no `open<Product>` to reach; one that does not begin at a held pin or does not end at a referent of the name) a source was still asked, so a path that was not there was reported as `link-source-unreadable` and a good tree was counted used, each for a question nobody put. `NamingPolicy.reads_link_declarations` now says when `resolve_referent` reads a chain, and `LinkSources.link_pins` asks it first: such a chain asks nothing, and a value for it is the NOTE that no name reads that link's declaration. A test runs `resolve_referent` with a mapping that records whether it was read, over every way a chain is and is not, and holds the new method to it. - **Dead code and stale sentences** (nit). `declared_neutral_pins` is removed: `git grep` over the tree, every pushed branch and every sibling worktree finds no caller. `link_manifest_lookup`'s docstring no longer says "replaces" over a name that is gone. This description no longer says `tests/test_naming_policy.py` takes `SEARCH_DENIED` from #312: it defines a mark of its own, with the same condition and its own reason text. **Fix round 2 (lane 5's re-review of 4b21263: 0 blockers, 0 should, 3 nits).** Third commit, 2ac911c. Each finding, in order: - **The three behaviours the body did not state are stated and tested** (nit 1). Round 1's not-a-list fix changed three behaviours on purpose, each listed under SHOULD-1 above, and the re-review found two of them had no test. R1 (`if declared is None` read as `if not declared`, which makes `0`, `false` and `''` declare nothing again) and R2 (`if pin is None or pin == ""` read as `if not pin`, which drops a `0` or `false` entry as a blank) both passed all 210 cases of `tests/test_naming_policy.py` at 4b21263. Six rows join `NOT_A_LIST_OF_NAMES`, which runs at the CLI and in process: `0`, `false` and `''` as the value, and `0`, `false` and `true` among the names. R1 now fails 9 cases and R2 fails 6. - **The scaffold's warning is true for a tree it reached** (nit 1, the third behaviour). Over a `--pin-source` whose `neutral_product_pins:` is not a list, round 1 turned the `chain-not-declared` refusal into a declared-unverified WARNING that says the tree "is not reachable here (a sibling checkout, SHAPE_PIN_SOURCE_<LINK>, or --link-source <LINK>=<path> would let it be read)". `--pin-source` did reach the tree, and only its declaration could not be read, so that blamed a path that was right. Now `link_pins_from_trees` takes a keyword-only `with_reasons`; asked, it returns a `LinkPins`, a dict equal to the plain mapping that also keeps `unreadable`, `{link: why}` for each link whose source answered and whose manifest could not be read. `_chain_link_findings` words such a link "which was reached but could not be read as a declaration (<why>)", in a branch of its own, and only the scaffold asks. The new text is asserted for each of the eleven not-a-list rows, with `scaffold-project.py --dry-run` run as a subprocess. - **Every other path's text is the bytes it was.** The sentence for a link no source reached is the same code in the same lines, and four tests hold it: the generic warning for a plain mapping, for an empty `LinkPins` and for an absent tree; the scaffold's own old warning for a clone that holds no `project.yaml`; the naming validator's two lines (its warning, and the `link-source-unreadable` line that names the reason); and `validate-manifest.py`'s. The naming validator and `validate-manifest.py` do not ask for reasons, so they print what they printed. - **One more scaffold path changes with it, on purpose.** A tree the scaffold reached and could not read as a declaration for any other reason (a manifest that does not parse, or is not a mapping) used to say "not reachable here" too, and was no truer. It now says what it is, with the system's reason, and a test for each of the two (a manifest that is not a mapping, one that does not parse) holds it, at the scaffold's command line and in process: narrowing the wording to the not-a-list reasons fails both. - `_declaration` still returns its four values, so nothing else in `scaffold-project.py` moved. - **A qualified chain link in the spy test** (nit 2). `_chain_read_cases` had no case whose chain link is qualified, so `reads_link_declarations` could take the chain as typed and not through `_chain_links` (R3) and pass every test. With `--referent-chain testorg/openXdox,openDox` that turns a verified chain into a declared-unverified one. `("codexDox", {"openXdox"}, ("testorg/openXdox", "openDox"), True)` is a case now, and R3 fails it. - **The description** (nit 3): the ten corrections of the re-review are applied (the reason a lookup returns, the fifth NOTE reason, the third kind of silent source, the SHOULD-1 bullet, "before Python 3.14", the `validate-pins.py` bullet, the 3.14.7 rows, the merge-tree section, the filed numbers, and the project's own `validate-manifest.py`). The mutation proofs: all three survivors were reproduced first at 4b21263 (210 passed each) and each dies at 2ac911c. Six mutations of the new code each die too: the warning ignoring the reasons (12 cases fail), the lookup recording none (12), the scaffold not asking (11), `with_reasons` on by default (3), an absent tree called reached (3), and the reached sentence used for a tree nothing reached (17). **Where it departs from #312, and why.** - **An explicit source that cannot answer stays a WARNING, exit 0.** #312 made an unreadable `--pin-source` a finding. Three reasons for the difference: - Exit 1 here means "a name is wrong", and this module has always said an unread link tree is a warning. - `SHAPE_PIN_SOURCE_<PRODUCT>` also answers `validate-pins.py`'s digest question, from a checkout that need not hold a manifest. - The lookup falls through to the next source, which may still verify the link. - **The fall-through is kept**, as it always was. With `--pin-source`, a keyed value is final. - **The lines go to stderr, spelled `WARNING`/`NOTE` as this file spells them.** #312's notes are ` note` lines on stdout, where that file's report lives. **Who else this reaches.** The guard is in `repo_shape.py`, so the same `link_pins_from_trees` lookup stops crashing for these callers too: - **The project's own `validate-manifest.py`.** It reaches the lookup through `SHAPE_PIN_SOURCE_<LINK>` or a sibling checkout, and now gives its usual declared-unverified WARNING instead of the traceback. - **`scaffold-project.py --referent-chain`.** A `SHAPE_PIN_SOURCE_<LINK>` under a locked parent no longer kills the plan, and `--pin-source` verifies the link. Neither of these prints the reason a source could not be entered; the scaffold says it only for a tree it reached and could not read (fix round 2, above). A `--pin-source` the scaffold cannot enter is still a traceback: that is #339 (`pin_digest_from_source`), and this PR does not touch it. **What this does not reach, and what the doctor says.** - **The project's gates still end in a traceback on the sibling row.** The NAMING and MANIFEST steps of `make validate`, `validate.yml` and `make bootstrap` stop crashing for a checkout `../openXdox` at mode 000. Those targets also run `validate-pins.py`, whose `resolve_neutral_pin_source` asks `(sibling / ".git").exists()` with no guard (`templates/assembly-root/scripts/validate-pins.py:463` on main), so before 3.14 a sibling at mode 000 still ends them in a `PermissionError`. That file is #312's, and `fix/pin-source-env-collisions` (PR #409, since landed), `fix/adopt-pin-writes-pin-file` and open PRs #343 and #419 touched it when this was written, so this PR does not edit it. The same line is listed for Python 3.14 by #430 (lane 2), where the helper that would cure both halves is asked for; the pre-3.14 traceback reproduction is added there as a comment, not filed apart. A `SHAPE_PIN_SOURCE_<LINK>` under a locked parent is different: `validate-pins.py` makes that a `neutral-pin-source-unreadable` FINDING (#312), and that stays. - **The doctor's `naming` row** runs the STANDARD's copy of the validator, so it stops crashing at once. **Its `manifest` row** runs the project's OWN `validate-manifest.py` on the project's own `repo_shape.py`, so it changes only once the project has applied this change. Over an existing project the doctor says `INVALID (manifest, pins)` until `update-shape.py apply` lands the two `upstream-changed` rows below. - **The verdict.** For `SHAPE_PIN_SOURCE_<LINK>` under a locked parent, on a project that carries these copies, `INVALID (naming, manifest, pins)` becomes `INVALID (pins)`, and `pins` is #312's finding for that locked variable, which is right. For a sibling at mode 000 the `pins` that remains is the traceback above, not a finding. The two copied scripts this change edits (`scripts/repo_shape.py` and `scripts/validate-repository-naming.py`) are copied into every project and pinned there. A project takes the change through `update-shape.py` (two `upstream-changed` rows), and a family holder takes the `repo_shape.py` row. Neither file has a second copy in this repository. **Evidence.** - **Issue rows, main ab623dd** (`repro338.sh`, a `gh` stub first on PATH). Under 3.10.21 and 3.12.3, rows 4-7 (keyed under a mode-000 parent, `SHAPE_PIN_SOURCE_OPENXDOX` there, a sibling at mode 000, a 300-character name) are a traceback, exit 1. Rows 2-3 (`a=b`, the typo) are a silent declared-unverified, exit 0. Under 3.14.7 row 1 verifies, and rows 2-7 are the silent declared-unverified WARNING, exit 0. - **Issue rows, this PR.** Under 3.10.21, 3.11.16, 3.12.3, 3.12.14, 3.13.15, 3.14.7 and 3.15.0rc2 (first commit), all 7 rows exit 0 with no traceback, and the output is byte-identical across versions. `a=b` verifies. The other five rows each name their source with its reason. Re-run on the fix-round head, 4b21263, under 3.10.21, 3.12.3 and 3.14.7: byte-identical to the first commit's run. Re-run on the fix-round-2 head, 2ac911c, under the same three: all 7 rows exit 0 with no traceback, the output is byte-identical across the three versions, and it is identical to the re-review's runs of the previous head over main. - **Reach on main.** Main gives a `PermissionError` traceback, exit 1, for `scaffold --dry-run`, the project's `validate-manifest.py`, `validate-repository-naming.py --project` and its `--quiet` form, each with `SHAPE_PIN_SOURCE_OPENXDOX` under a locked parent. This PR gives exit 0 for all four. `validate-pins.py` on the sibling row is the exception said above. - **New tests.** 17 cases (11 functions) in the first commit; 16 fail on ab623dd. The fix round adds 31 cases in `tests/test_naming_policy.py`, and 17 of them fail on 40762ca: the five shapes of pins that are not a list (each at the CLI, in-process and, for the scalar, under `--project`), and the chains nobody reads. The other 14 guard what a mutation could break (below) or the side the fix must not move (an absent, null or blank pin list still answers). - **Targeted run on 4b21263**, files `test_naming_policy`, `test_manifest_validator`, `test_scaffold_referent_chain_links`, `test_cli_reference`, `test_repo_hygiene`, `test_neutral_pin_validator`, `test_scaffold_e2e`, `test_shape_doctor`, `test_git_output_decoding` and `test_pin_validator`: **765 passed, 3 skipped**. The 3 skips are the PowerShell-only rows. (40762ca: 734 passed, 3 skipped.) `render-cli-reference.py --check` is ok. - **Mutation probes, first commit.** Reverting the stat guard, the parse, keyed-over-directory, the NOTE lines, the chain's-end restriction, the sibling-absence filter, the manifest reason, the `--quiet` drop, the "read as a PATH" sentence, the `./` line, the project-chain read, the WARNING lines or the env reporting each fails at least one new test. - **Targeted runs on 2ac911c (fix round 2).** Under 3.12.3, `test_naming_policy` (238 cases: 210 and 28 new), `test_cli_reference`, `test_manifest_validator`, `test_scaffold_referent_chain_links`, `test_repo_hygiene` and `test_setup_project_py`: **468 passed**; and `test_scaffold_e2e`, `test_scaffold_pin_and_reuse`, `test_scaffold_leg_path_collisions`, `test_scaffold_pin_collisions`, `test_neutral_pin_validator`, `test_git_output_decoding` and `test_pin_validator`: **312 passed**. Under 3.10.21 and under 3.14.7, `test_naming_policy`, `test_manifest_validator` and `test_scaffold_referent_chain_links`: **321 passed** each. `render-cli-reference.py --check` is ok, and `ast.parse(feature_version=(3, 9))` passes on the three changed files. - **Mutation probes, fix round.** The review's ten, re-run on 4b21263: MW1, MW2, MW3, MW5 and MW11 now die (they survived), and MW4, MW7, MW12 and MW13 still die; MW6 is the dead test, removed. (Fix round 2 kills the three the re-review found, R1 to R3, and six more: see above.) Nine more for the new code (the list check, the entry check, the null and blank tolerances, `reads_link_declarations` always true, without the shape check, without the held pins, and the validator ignoring it): eight die. The ninth, `reads_link_declarations` ignoring whether the name has a referent, survives as an equivalent mutant: with no referent `_chain_shape_problem`'s last question always reports a problem, so the explicit early return is redundant, and is kept because `resolve_referent` has the same first line. The spy test holds the two together. - **Windows.** The mode-000 rows skip on Windows and as root, using a `SEARCH_DENIED` mark this file defines, with the same condition as #312's. The new FIFO and `/dev/null` rows skip where those do not exist. `a=b` and the 300-character name run everywhere. On Windows the long-name row asserts the line, not POSIX's reason. None of it had been run on Windows or macOS when this was written; the last paragraph gives the CI result. **Merge-tree.** Recomputed at 19:04Z with `git merge-tree --write-tree` of 2ac911c against `origin/main` as it was then, 53bee5b, and against every one of the other 37 pushed branches that touches a file this PR changes (a scan of each head against its merge-base with main): - **Clean:** origin/main 53bee5b (it carries #314 and #390); #394 `fix/adopt-record-submodule-placement` 563ea16 (`scripts/repo_shape.py` and `validate-manifest.py`) and #406 `fix/validate-family-unnamed-mounts` 369af2e (`scripts/repo_shape.py`); #397 abb27e6, #400 7062a47, #401 409f10b, #416 7cca975, #452 e2df9a8 and #395 1ef3310 (`docs/cli.md`); #355 6fdbe3d, #373 741cbf2 and #395 (`scaffold-project.py`). - **Conflict, four hunks each, all in `scripts/repo_shape.py`:** `fix/pin-source-env-collisions` (PR #409) 6494714, and `fix/adopt-pin-writes-pin-file` 421e4e5 (no PR). Both also touch `scaffold-project.py`, and that merges clean with this PR's one edit there. Fix round 2's new hunks merge with neither: #409's change to the "not reachable here" sentence (the `offered` text, when a variable is shared) and the reached branch sit in different lines of `_chain_link_findings`. The four hunks, each taking both sides: 1. the `resolve_link_source` signature: `default_source=None, env_shared: bool = False, *, tried: list | None = None`; 2. its docstring: both paragraphs; 3. the `link_pins_from_trees` docstring and the lines after it: both paragraphs, then `found`, this PR's `unreadable` and #409's `links` and `shared`; 4. the call: `resolve_link_source(name, root, keyed_sources, default_source, pin_product_key(name) in shared, tried=asked)` (the same spelling on both branches), followed by `if tried is not None: tried[name] = asked`. Resolved that way in a throwaway and committed there, the merge with 6494714 passed **496 tests** (`test_naming_policy`, `test_pin_source_env_collisions`, `test_neutral_pin_validator`, `test_scaffold_referent_chain_links`, `test_manifest_validator`, `test_cli_reference`), and the merge with 421e4e5 passed **559 tests** (the same files and `test_pin_source_unreadable`). `render-cli-reference.py --check` is ok on both, pyflakes is clean, and the grammar parses as 3.9's. Neither branch carried the resolution when this was measured. #409 has since landed, and the merge of main into this branch (the last paragraph) takes those four hunks both ways. The earlier counts (468 and 434 from the first fix round, 430 from the re-review) are those runs' own, at their heads. **Left out.** Each of these was found by lane 5's reviews of this change, and each is filed or folded: - **A bare `--link-source` answers for every link with no more specific source** (#441). In a chain of three or more links, one bare tree is read as two links' declarations, and a chain that holds is reported broken, a FINDING for a name with no direct pin on its referent. Pre-existing, the same on main; `validate-pins.py`'s `--pin-source` has the same documented rule and was not examined. - **An empty path after a key (`--link-source openXdox=`) is `Path("")`, the current directory** (#442). Run from a project root, the project's own `project.yaml` is read as the link's declaration. Pre-existing; `validate-pins.py`'s `--pin-source openXdox=` has the same shape (there a finding naming `.`). - **`validate-pins.py`'s `(sibling / ".git").exists()` is unguarded** (the sibling row above), folded into lane 2's #430, which already covers the Python 3.14 half: the pre-3.14 traceback and its reproduction are added there as a comment. #312's file. - **The project's OWN manifest** with `neutral_product_pins: 5` ends in a `TypeError` in two gates before either can say anything about it (#443): `_pins_from_project` in the naming gate (`scripts/validate-repository-naming.py:302` on main), and `_declared_pins` in the project's own `validate-manifest.py` (`templates/assembly-root/scripts/validate-manifest.py:85`), which ends it before its own `manifest-neutral-product-pins` finding (`_neutral_product_pins_type_finding`, `:524` on main) can print. Reproduced on 3ef059c. That is the project's own declaration, read by the project's own gates, and not a link source, so it is outside this change; "read by the project's own gates" covers two crashing gates. Also not done here: - **Python 3.9 (the floor) was not run** because it is not installed here. The grammar of the three changed files was checked with `ast.parse(feature_version=(3, 9))`. Windows and macOS were not run here; CI ran them (the last paragraph). Reviewed adversarially at 40762ca, the branch's first commit (CHANGES NEEDED: 0 blockers, 2 should-fixes, 4 nits). The head stopped every crash in the issue's table, and the should-fixes were a docstring claim the code did not keep for a `neutral_product_pins:` that is not a list, and reach claims that overstated what the project's gate does on the sibling row. Fix round 1 (4b21263) resolved every finding, and the re-review of 4b21263 read APPROVE WITH NITS (0 blockers, 0 should-fixes, 3 nits: three behaviour changes this description had not stated, two of them untested; a spy-test case for a qualified chain link; and description corrections). Fix round 2 (2ac911c) resolved those three, and its re-verification of 2ac911c read APPROVE WITH NITS (0 blockers, 0 should-fixes, 2 nits: no test of the scaffold's wider wording for a manifest that is not a mapping or does not parse, now pinned by two tests that each fail when the wording is narrowed, and a cross-reference in this description that pointed the wrong way, now corrected). Main (9736424, #409's squash) was merged into the branch as ced8fac. `scripts/repo_shape.py` conflicted in four hunks; each takes both sides, as the review of 2ac911c resolved it: `resolve_link_source(..., default_source=None, env_shared: bool = False, *, tried=None)`, both docstrings kept, `link_pins_from_trees` opening with `found`, `unreadable`, `links` and `shared`, and the call passing `pin_product_key(name) in shared` with `tried=asked`. Nothing else conflicted. After the merge, `tests/test_pin_source_env_collisions.py` and the targeted set passed (667 and 312), and `render-cli-reference.py --check` printed `[ok]`. CI is green on Linux, macOS and Windows, with SonarCloud passing, at 938a131 (main 48585e6 merged in). Its merge with main 994be39 passed the targeted set (the naming, manifest, scaffold, pin and setup test files): 983 passed. Closes #338 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
… writes its pin file as the scaffold does, so an adopted root passes `validate-pins.py` (#273); a `--pin-source` that cannot answer is `pin-source-unreadable` (#339) (#508) **Brought up to date after #409 squash-landed** by merging origin/main (9736424, #409's squash) as f9ed46f. Its `scaffold-project.py` conflict was resolved to this branch's side once `git diff b70a07d 9736424 -- scaffold-project.py` (#409's final head against its squash) came out empty. So the file is the one tested on this branch plus main's own later changes to it, and holds nothing of #409's; details below. This branch carries #275's adopt half. It was built on `fix/pin-source-env-collisions`, PR #409 (#275's scaffold half and its fix round), merged at 6494714 as 583f647, over the earlier 5690235 (34dbd86). After 6494714, #409 only merged main, five times, up to its final head b70a07d, and none of those merges has a hand edit (`git show --remerge-diff` is empty for each). Adopt's refusal of two products that share a source variable goes through #409's function, so this branch lands after #409. It was never rebased. `git diff b70a07d 9736424` is empty for each file #409 touched: `scaffold-project.py`, `scripts/repo_shape.py`, `templates/assembly-root/scripts/validate-pins.py` and `tests/test_pin_source_env_collisions.py`. What f9ed46f resolves, four files with one hunk each: - **`scaffold-project.py`.** - The conflict hunk is main's block from `PIN_COLLISION_REMEDIATION` through `_pin_collision_remediation`, which #409's squash edits: it adds `PIN_VARIABLE_REMEDIATION`, `_colliding_pin_variables` and `_pin_collision_remediation` and changes `_refuse_colliding_pins`. This branch moved all of that to `scripts/shape_materialize.py`. The hunk takes this branch's side. (The merge commit's message says "re-adds"; this is the exact account.) - #409's auto-merged import of `PIN_SOURCE_ENV_PREFIX, pin_source_env_collisions` is dropped, because nothing in the file uses either name any more. - It is not `git checkout --ours` of the whole file. Main changed the file beyond #409 since this branch last merged it: #355's `--tracking-branch` check (6c2a371) and a73b4e9's `RULESET_HINT`. Those lines stay. - The result is the file `git merge-tree` gives for this branch with main before the squash (518010b). Its diff from the file tested here has the same patch-id as main's own 3ef059c..518010b change to it. - **`templates/assembly-root/scripts/validate-pins.py`.** Main's side, which is #409's final blob, with main's #359 workflow reader in it. This branch had no change of its own to the file beyond #409. - **`adopt-project.py`, `_commit_the_split`.** - Main: #445 wraps the function in its late-failure `try`, and #394 builds an `append` with the plan's submodule record. - This branch: its only edits there were a comment and `neutral_pins=_recorded_pins(plan)` on the materializer call. - Both now sit on main's call inside the `try`: `append=append, neutral_pins=_recorded_pins(plan)`. - **`docs/cli.md`.** It is regenerated with `scripts/render-cli-reference.py`, and `--check` is ok. It differs from main's only in the `--pin`, `--pin-owner` and `--pin-source` help of `adopt-project.py plan`. `scripts/repo_shape.py`, `templates/assembly-root/scripts/validate-pins.py` and `tests/test_pin_source_env_collisions.py` are main's blobs, which are #409's: this branch adds nothing to them. Nothing of #409 is lost. Its scaffold behaviour is `shape_materialize.refuse_colliding_pins` and `pin_variable_collisions`, keyed by #409's `pin_product_key`. **73d65c6, after the merge.** - **The change.** `_read_pins` reads `plan.get("pins")` where it read `plan.data.get("pins")`. - **Why.** #445's `tests/test_adopt_late_failure.py` hands `_commit_the_split` a `SplitPlan` as its plan, a `dict` that adds `entries`. On f9ed46f, three of its tests stopped at "committing the split" with `AttributeError: 'SplitPlan' object has no attribute 'data'` (3 failed, 83 passed in that file). - **Nothing else changes.** `Plan.get` with no default returns `plan.data.get(key)` exactly, so a loaded plan reads as before. `adopt-project.py plan --pin` took a NAME only (`--pin openGlass`) and refused a trailing `@<commit>`. `plan`, `check` and `execute` each exited 0, and `execute` printed `adoption verified`. The manifest then listed the name in `neutral_product_pins:` with no `contracts/<product>-pin.yaml` beside it, and on its first run the adopted root's own `validate-pins.py` refused it as `neutral-pin-missing`. #273's table, reproduced on c7c02fa with the issue's own commands (local bare remotes, and a stub `gh` that exits 97): | `--pin` | c7c02fa | this branch | |---|---|---| | `openGlass` | `execute` exit 0, `- openGlass`; clone's `validate-pins.py` exit 2 `neutral-pin-missing` | `plan` exit 2 `pin-malformed`, with the scaffold's remediation (`--pin openGlass@<40 hex>`); no plan is written | | `opensoft/openGlass` | exit 0, `- opensoft/openGlass`; exit 2 `neutral-pin-missing` for `contracts/opensoft/openglass-pin.yaml` | `plan` exit 2 `pin-malformed` | | `spec` | exit 0; `validate-pins.py` exit 0, reading the leg's `contracts/spec-pin.yaml` as the neutral pin | `spec@<sha>` is `adopt-pin-collides` at `plan` | | `shape` | exit 0; `contracts/shape-pin.yaml` read as the neutral pin | `shape@<sha>` is `adopt-pin-collides` at `plan` | | `openGlass@<sha>`, `opensoft/openGlass@<sha>` | `plan` exit 2 `adopt-pin-malformed` | `plan`, `check` and `execute` exit 0, and `gh` is called 0 times. `contracts/openglass-pin.yaml` is written and the manifest lists `- openGlass`. In the clone, `validate-manifest.py` exits 0, and `validate-pins.py` exits 0 with `openGlass: pin commit … digest recomputes` | Brett ruled for decision 1's first option ("go with your recommendation on #273"): adopt takes the scaffold's pin and writes the scaffold's pin file. That reverses the NAME-only `--pin` from #6, and the comment that described it is replaced with the new contract. - **One implementation.** The scaffold's pin code moved into `scripts/shape_materialize.py`, next to `neutral_pin_path` and the materializer that writes the file. What moved: - `PIN_ARG_RE`, `parse_pin`, `pin_digest_from_source` and `pin_digest_from_gh`; - `declared_pin_values`, `declared_pin_products` and `PIN_COLLISION_REMEDIATION` (in #309's wording); - from #275's branch, `PIN_VARIABLE_REMEDIATION` and the variable check; - `_refuse_colliding_pins`, split into the pure `pin_collisions` and `pin_variable_collisions`, and `refuse_colliding_pins(refusal=...)`, which raises one refusal for both. The code did not move verbatim. `_refuse_colliding_pins` became the three functions above; `pin_digest_from_source` was rewritten for #339 (below); and fix round 1 changed three of them (below): `pin_variable_collisions` keys each pin by #275's `pin_product_key`, `pin_digest_from_source` spells a path that is not printable, and `pin_digest_from_gh` refuses a missing `gh`. The scaffold's behaviour is otherwise unchanged. `scaffold-project.py` imports what it calls under its old names. Lane 5's review ran 34 scaffold cases (every pin refusal, the `gh` stub's four modes, dry and real successes) on 34dbd86 and on this branch at 7a94b6b. rc, stdout, stderr, the `gh` log and the files made were identical, apart from the shape's own commit and digest and #339's row. Its pin tests and #275's pass as they are. `tests/test_git_output_decoding.py`'s exemption for `pin_digest_from_gh` follows the function to its new file. - **`plan`.** `--pin`, `--pin-owner` and `--pin-source` mean what they mean for the scaffold. `plan` reads the digest from `--pin-source` or `gh api`, and records product, repository, commit, digest and `digest_source` in the plan, so `check` and `execute` read no network for a pin. `plan` refuses: - a name with no `@<commit>`, as `pin-malformed`. That includes an empty `--pin ''`, which adopt used to drop and now refuses, as the scaffold always has; - `spec`, `code` and `shape`, in any case, as `adopt-pin-collides` (#273 decision 4), in the scaffold's sentence and remediation (#241, #309); - one product given twice, the same way, whether it is a repeat or two names that share one file (decision 5); - two products that one `SHAPE_PIN_SOURCE_<PRODUCT>` names, such as `open-glass` and `open_glass`, the same way, through #275's check; - a leg path over a pin's file, as `adopt-leg-path-collides`; - a `--pin-source` path that `check` would refuse in `digest_source:` (fix round 1), as `adopt-unwritable-value`. It is asked only when a `--pin` records the path; - with no `gh` on PATH and no `--pin-source`, `gh-not-found` (fix round 1). - **`check` and `execute`.** These are new `check` findings, and `execute` refuses each one before any leg exists. They come from `_leg_findings`, the list `check` and `_refuse_what_check_finds` share: - `plan-pin-unrecorded`: a name alone (an old plan's `pins: [openGlass]`, or a hand edit), or an entry missing its commit or digest; - `plan-pin-malformed`; - `plan-pin-collides`, which covers two pins at one file, a pin at a file the shape writes, and two products that share one source variable; - `plan-root-holds-pin`: a source path kept in the root at the pin's file. The materializer would write the shape's pin beside it under `shape/`, and `validate-pins.py` would read the source's file instead. How each finding ends: - `plan-pin-unrecorded` and `plan-pin-malformed` end with the `plan --pin <product>@<40 hex commit>` that records the pin whole. - `plan-pin-collides` ends with re-running `plan` with each product once, or, for two products that share a source variable, with one product of each such group. - `plan-root-holds-pin` ends with the kept path's own remedy: send that entry to a leg, `drop` it, or rename it in the source and re-run `plan`. A leg moved over a pin's file in the plan is `plan-leg-path-collides`. - **Also part of #386: its `pins:` row.** A hand-edited `pins:` that is not a list (a name such as `pins: openGlass`, a number, a mapping, a quoted string) is now `plan-pin-malformed` in `check` (`` `pins:` is 'openGlass', not a list of pins ``), and `execute` refuses it before any leg exists. - Where: `_read_pins` in `adopt-project.py`; the test row is `plan-pin-malformed-not-a-list` in `tests/test_adopt_pins.py`. - Before: on main, `check` said `plan ok` for the name, the mapping and the string, and gave a traceback for the number (reproduced by lane 5's review on cb7063f). #386 records that `execute` then wrote the name as eight one-letter pins. - Not fixed here: #386's other keys, `allow_empty_legs:`, `paths:`, `legs:` and `source:`, so #386 stays open. Lane 2's #435 takes all of #386, `pins:` included, and its `pins:` rule keeps only text items where this branch writes mappings; whichever lands second reconciles them (#435 (comment), and "Where the meaning meets" below). - **`execute`.** It writes each pin through `materialize_assembly_root(neutral_pins=...)`. The manifest lists the product with the owner stripped (decision 3), so the entry and the file agree. - **Docs.** `--help` is updated and `docs/cli.md` regenerated (`render-cli-reference.py --check` ok). `pin_digest_from_source` caught only `Refusal`. A directory that is no clone is git's own failure, so it was always `pin-source-unreadable`, exit 2. A path `subprocess.run` cannot enter never reaches git: it is a `FileNotFoundError`, `NotADirectoryError` or `PermissionError` out of `cwd=`, and nothing caught it. `scaffold-project.py --pin ... --pin-source <that>` ended `--dry-run` and the real run alike in a traceback, exit 1. Since the move above, `adopt-project.py plan --pin` reads its digest through the same function and ended the same way. Both reach it through `declared_pin_values`, and `check` and `execute` read no source, so that call is the only reader. The issue's table, run on ab623dd for the scaffold and on d806e8f for `adopt-project.py plan` (adopt has no `--pin-source` on ab623dd), then on this head. `gh` is a stub that logs and exits 97, with `GH_TOKEN` bogus and an empty `GH_CONFIG_DIR`; it was called 0 times in every run: | `--pin-source` | before | this branch | |---|---|---| | a directory that is not a clone | `pin-source-unreadable`, exit 2 | unchanged | | a path that does not exist | `FileNotFoundError` traceback, exit 1 | `pin-source-unreadable`, exit 2: `<path> could not answer for opensoft/openGlass @ <40 hex>: no such directory` | | a file | `NotADirectoryError`, exit 1 | exit 2, `... it is a file, not a directory` | | under a parent this user cannot enter | `PermissionError`, exit 1 | exit 2, `... Permission denied` | | a symlink loop (not in the issue) | `RuntimeError` out of `Path.resolve()` on 3.12, exit 1 | exit 2, `... Too many levels of symbolic links` | The real run and `--dry-run` give the same stdout and stderr, and nothing exists afterwards (no remote directory, no work directory; for adopt, no plan file). What changed, in `scripts/shape_materialize.py` only: - `pin_digest_from_source` catches `OSError` as well as `Refusal` and raises the existing `pin-source-unreadable` with its remediation unchanged. The message names the path and the reason. - `stat()` judges the source inside the guard, as #312 does in `validate-pins.py`, and the sentences are that file's: a missing path, or one beneath a file, is "no such directory"; a file is "it is a file, not a directory"; any other `OSError`, from `stat()` or from git's `cwd=`, is the system's own reason (`strerror`). It is not `exists()`/`is_dir()`, which before 3.14 swallow only a missing path. A source `stat()` can read but `cwd=` cannot enter, a directory with no search bit, fails in `tree_digest` and the same guard answers it. - `declared_pin_values` resolves the path through a guarded `_resolved_pin_source`. `Path.resolve()` raises a `RuntimeError` on a symlink loop before 3.13 (an `OSError` since), before the digest was asked for. As typed, `stat()` meets the loop and says so. - A source that answers is read exactly as before: a relative path resolves against the working directory, a symlink to a clone is followed, `digest_source` reads `local-clone (<path as typed>)`, and the digest is `tree_digest`'s. `tests/test_pin_source_unreadable.py` (46 tests in the #339 commit; fix round 1 adds 17 for S1 and moves one, below) runs each case through `scaffold-project.py --dry-run`, its real run and `adopt-project.py plan`: missing, a file, beneath a file, relative and missing, a parent and a source with no search bit (skipped on Windows and as root, by a mark, so before any fixture builds a directory), a 300-character name, a symlink loop (skipped on Windows) and a directory that is no clone. Each asserts exit 2, the refusal naming the path and the reason, its remediation, no traceback and nothing created. It also holds `--dry-run` and the real run to the same stdout and stderr, checks the path is printed escaped under an ASCII stderr and as a surrogate when it is not UTF-8, and checks that a relative source and a symlink to a clone read as before. The session `gh` guard fails any test whose run called `gh`. origin/main ab623dd reached this branch through implQ's branch: 34dbd86 is a30091f merged with ab623dd. One file conflicted there, `templates/assembly-root/scripts/validate-pins.py`, in seven hunks, because #312 (8cdca5e) replaced `keyed_sources` and `default_source` with `PinSources` and #275 added `env_shared` and `shared`. #312's blob is the same at its PR head, at the squash and at ab623dd, so it landed the file implQ resolved against. The resolution keeps #312's side in all seven and adds #275's on top: | # | hunk | resolution | |---|---|---| | 1 | `resolve_neutral_pin_source` signature | #312's `sources: PinSources` and 3-tuple return, plus #275's `env_shared: bool = False` | | 2 | its docstring tail | #312's `Returns (path, how, typed)` text, plus #275's "Rule 2 is skipped when `env_shared`" paragraph | | 3 | `_recompute_neutral_pin_digest` signature | #312's `sources`, plus #275's `shared: frozenset[str]` after `report` | | 4 | the `resolve_neutral_pin_source(...)` call inside it | #312's 3-tuple unpack, plus `env_shared = product.casefold() in shared` and the extra argument | | 5 | `_check_one_neutral_product_pin` signature | #312's `sources`, plus #275's `shared` | | 6 | its call of `_recompute_neutral_pin_digest` | `sources, report, shared` | | 7 | `_check_neutral_product_pins` docstring and body | #312's `PinSources(...)` and `_note_unused_pin_sources`, plus #275's paragraph, `_report_pin_source_collisions(products, report)` and `shared = frozenset(pin_source_env_shared(products))` | This branch's own merge of 34dbd86 (5690235) had no conflict, and neither had its merge of #409's fix round, 6494714 (583f647). That fix round made `neutral-pin-source-collides` a note and keyed "one product" by `repo_shape.pin_product_key`. At 421e4e5, `templates/assembly-root/scripts/validate-pins.py`, `scripts/repo_shape.py` and `tests/test_pin_source_env_collisions.py` were byte-identical to the tree `git merge-tree` gives for 6494714 with origin/main 3ef059c (blobs 5604007, 091966b, 5220787), so this branch added nothing to those three files beyond #409. The 34dbd86 merge brought in #335, #312, #317, #270 (f795cf7), #344 and #349 with no conflict. Earlier merges of origin/main (87373a6, for #309's reworded constant, and e43fa95) are unchanged. Fix round 1 merged origin/main three more times: - **cd1fc79 (2bcfa69).** It brought #245, #351, #350, #345, #372, #371, #354, #380, #377 and the rest since ab623dd, with no conflict. - **45e01f4 (4098b61).** It brought #396's `tests/test_agents_md_codes.py`. That test finds `scaffold-pin-collides` as a string constant of `scripts/shape_materialize.py`, which `SHIPPED` lists. - **3ef059c (421e4e5), #314.** One conflict, one hunk in `adopt-project.py`. This branch's `_refuse_an_unrecordable_pin_source` and #314's section "The project must be able to hold what `execute` writes into it (#261)" were added at the same place; both are kept. Where the two meet, lane 5's earlier recipe applies: - `_flag_plan` builds each pin as the `PIN_FIELDS` mapping `plan` writes, not `list(pins)` of products. So the plan `plan` asks the validator about is the plan it writes. - #314's `test_plan_asks_what_check_asks_of_the_same_keys` plans `--pin openGlass@<commit> --pin-source <stand-in>` where it planned the name alone, which `plan` now refuses. Then f9ed46f, after #409 squash-landed, and 73d65c6 after it (top of this description). On the merged tree exactly one test failed: `tests/test_adopt_template_values.py::test_check_finds_and_execute_refuses_a_hand_edited_value[pins-surrogate]`. It plans, then hand-edits the plan to `pins: ["openX\ud800"]`, a name-only pin with a lone surrogate. Before this branch the refusal's headline was `REFUSED plan-unwritable-value`. A name-only pin in a plan is now `plan-pin-unrecorded` (#273), so `check` has a second, independent finding for that very plan, and the headline is the first finding's: ``` REFUSED plan-pin-unrecorded: 2 finding(s) ... FINDING plan-pin-unrecorded: `pins:` entry 1, 'openX\ud800', is a name with no commit and no digest ... FINDING plan-unwritable-value: a `pins:` item is 'openX\ud800', which `execute` writes into the assembly root's `project.yaml`, and it holds a lone surrogate ... ``` Nothing about #270's contract changed. The lone surrogate is still found as `plan-unwritable-value`, listed under the refusal, and `execute` still refuses before any leg exists. `check` builds `_leg_findings`, where `_pin_findings` sits, before it calls `_value_findings`, so the pin finding comes first. d806e8f changes one assertion for that row, `pins` only: the headline is `REFUSED plan-pin-unrecorded: ` where it was `PLAN_REFUSED`. The other 15 rows and the finding assertions, including the `plan-unwritable-value` line, are untouched. Lane 5's review of 7a94b6b found 3 should-fix and 5 nits. Each is answered here: - **#409's `pin_product_key` (review N1).** The merge of #409's fix round could not carry it, because this branch had moved the function that used it out of `scaffold-project.py`. `pin_variable_collisions` now keys each pin by that function, where it casefolded. `openß` and `openss` are two pin files and one `SHAPE_PIN_SOURCE_OPENSS`, and a casefold named each of the two pins twice. No `--pin` or recorded `pins:` entry `PIN_ARG_RE` admits can spell either, so the test asks the function directly. `refuse_colliding_pins`' docstring now says, in #409's words, that the root's `validate-pins.py` only notes such a pair. - **S1, a `--pin-source` path that is not printable.** `pin_digest_from_source` names the path as typed unless it is not printable (`str.isprintable`), and then as Python's `repr` spells it (`shown_source`). That covers the refusal's head and git's own reason, which names the directory it ran in. Printed raw, an ESC reached the terminal, and a line break or a carriage return began a line of the path's choosing, a forged `REFUSED` line among them, from the scaffold and from adopt. The traceback #339 replaced had shown a `repr`. A printable path is named byte for byte as before. - **S2, no `gh` on PATH.** `pin_digest_from_gh` refuses as `gh-not-found`, exit 2, as `repo_shape.tree_digest_from_gh` names it, and offers `--pin-source`. It was a `FileNotFoundError` traceback, exit 1, from the scaffold and, new with this branch, from `adopt-project.py plan --pin`. - **S3, a hand-edited `commit:` or `tree_sha256:`.** The lane's decision is to accept it and document it (option (c)); no behaviour changed. A pair that disagrees passes `check` and `execute` (`adoption verified`), because neither reads a network for a pin, by design. The adopted root's own `validate-pins.py` then exits 1 with `neutral-pin-digest-mismatch` wherever it can read the product. `check` now says so once, after the pins it will write: "each pin as `plan` read it, not read again here: the root's own `validate-pins.py` recomputes its digest wherever it can read the product". Options (a), `execute` recomputes before it creates the legs, and (b), `check` recomputes from a clone still on the machine, are filed as #449. - **N2, a `--pin-source` that `plan` cannot record.** `plan` asks `check`'s question (`one_printable_line`) of the path it records in each pin's `digest_source:`, before any digest is read. It refuses one that fails as `adopt-unwritable-value`, in #270's words (`unwritable_reason`), or as "an invisible formatting character" where those name nothing. Before, it was planned, exit 0, and printed raw in the report. `check` then refused it as `plan-pin-malformed` and said to re-run the same `plan`. The report's `pin` line and `check`'s `write` line now spell the digest's source with `_shown`. The scaffold half is pre-existing (a line break in the path is written into the pin file; a byte that is not UTF-8 dies after all three repositories exist) and is filed as #450. - **N3, six hunks no test guarded.** Each now has a test that fails when it alone is reverted: - the two name guards (a `pins:` name holding an ESC and a forged line); - the lower-casing of the commit and the digest (an upper-cased hand edit); - the `digest_source` printability check; - the not-text check (`commit: 1234567` unquoted); - the gitlink filter (a submodule kept where the pin is written is `plan-submodule-root` alone). - **N4.** This body. - **N5, #339's third decision.** A `--pin-source` that nothing reads is still silent, now in adopt too. This PR does not decide it. #451, filed, lays out refuse, note and leave, so closing #339 loses nothing. Lane 5's second review, of 421e4e5, found no blocking item, 2 should-fix items and 3 nits. The two should-fix items are this body: the landing after #409 (above) and #386 (above). The code needed no change for them. f63d1ed adds two tests and no code: - **n1. `plan` asks nothing of a `--pin-source` no `--pin` records.** With the `args.pin and` guard dropped, such a path holding an ESC or a zero-width joiner was refused in a sentence naming no file ("`execute` writes into , and ..."), and every test still passed. `test_a_pin_source_no_pin_records_is_not_asked_about` holds the plan to exit 0 and `pins: []`, with the path named nowhere in the plan or the report. The ESC row is POSIX-only by the file's `POSIX_NAMES` mark; the zero-width joiner row runs everywhere. It holds #451's answer today, silence, so a decision there changes this test on purpose. - **n2. A printable `--pin-source` is printed as it is.** With `_spelled` in place of `_shown` in `plan`'s `pin` line, every test still passed. `test_a_printable_pin_source_is_printed_as_it_is` holds `plan`'s `pin` line, and `check`'s `write` line for the plan `plan` wrote, to their exact text, ending `(local-clone (<path>))` with no backtick. - **n3** needs a code change and is left out (below). **Fix round 2, on f63d1ed.** Each run had a `gh` stub first on PATH (it logs and exits 97), `GH_TOKEN` bogus, an empty `GH_CONFIG_DIR`, `git-filter-repo` 2.47.0 next on PATH, and the identity blanked with `GIT_CONFIG_COUNT`; the session `gh` guard came first inside each run. - **The pin set: 812 passed, 2 skipped.** It covers adopt pins, adopt plan, #339's file, #275's tests, the pin and neutral-pin validators, git output decoding, adopt template values, CLI reference, scaffold pin and reuse, scaffold pin collisions, #314's two files and #396's file. The skips are `test_adopt_plan.py:403` (no MedxEHR clone) and `test_adopt_template_values.py:223` (this host reads `git config user.name` from the repository, not from `GIT_CONFIG_GLOBAL`). That is fix round 1's 809 passed, plus the three new rows. - **Shape doctor and scaffold leg-path collisions: 369 passed, 3 skipped** (the PowerShell line exists only on a Windows host). - **Mutation probes**, each one line changed in a throwaway worktree, over `tests/test_adopt_pins.py` and `tests/test_pin_source_unreadable.py` less the five end-to-end adoptions: - the `args.pin and` guard dropped: 2 failed, both n1 rows; - `plan`'s `pin` line `_spelled`: 1 failed, the n2 test; - `check`'s `write` line `_spelled`: 2 failed, the n2 test and `test_an_upper_cased_hand_edit_is_written_lower_cased`; - `plan`'s `pin` line raw: 1 failed, the no-break-space test that already guarded that hunk. **On the merged tree, 73d65c6** (the merge f9ed46f and the `_read_pins` commit), with the same harness: - **The pin set** above, with #445's `test_adopt_late_failure.py`, `test_adopt_git_reads.py` and `test_adopt_filter_repo_failure.py` and #387's `test_adopt_empty_symlinks.py` and `test_adopt_root_manifest.py`: **988 passed, 2 skipped**. The skips are the two above. - **Shape doctor, scaffold leg-path collisions and the adopt suites:** **967 passed, 3 skipped** (the PowerShell line). The adopt suites are e2e, execute refusals, verify, root shape paths (#387's too), leg-path collisions, clone template, ignored shape files and submodules. - **On the merge alone, f9ed46f** (the same tree as before the `_read_pins` commit): `tests/test_adopt_late_failure.py` gave 3 failed, 83 passed, the three `AttributeError` rows above. - `render-cli-reference.py --check` is ok on the merged tree. pyflakes reports the same four warnings on `adopt-project.py`, `scaffold-project.py` and `scripts/shape_materialize.py` as before the merge. **Fix round 1, on 421e4e5.** Each run had `git-filter-repo` 2.47.0 on PATH and the identity blanked with `GIT_CONFIG_COUNT`. A `gh` stub that logs and exits 97 was first on PATH, with `GH_TOKEN` bogus and an empty `GH_CONFIG_DIR`, and the session `gh` guard came first inside each run. - **The pin set:** 809 passed, 2 skipped (the same files and skips as above). - **Doctor and scaffold:** 782 passed, 3 skipped (the PowerShell line exists only on a Windows host). It covers shape doctor, scaffold leg-path collisions, scaffold e2e, referent chain links, naming policy, repo hygiene and #351's free-text values. - **The adopt suites:** 882 passed. It covers e2e, execute refusals, verify, spec-only, leg paths, root shape paths, plan escapes, leg-path collisions, clone template and submodules. - **Before the #314 merge,** on 74ce4d9's tree: the first two sets gave 577 passed, 2 skipped and 782 passed, 3 skipped. On 2bcfa69's tree (the merges alone, before the fix) they gave 542 passed, 2 skipped and 782 passed, 3 skipped. After the 45e01f4 merge, `tests/test_agents_md_codes.py`, repo hygiene and CLI reference: 110 passed. - **Lane 5's second review** also ran 421e4e5 merged with main cb7063f and with 53bee5b: the pin set, the manifest-value set, and the doctor set with #357's `tests/test_doctor_validator_rows.py`. All passed, with only the skips above. Proof that fix round 1's tests test something: - **On 2bcfa69's tree,** with this head's two test files laid over it, 31 of the new rows fail: - S1: 17; - N2 and its report spelling: 7; - S2: 3; - the `pin_product_key` port: 2; - `check`'s S3 line: 1; - the not-UTF-8 `adopt plan` row: 1. The N3 rows pass there, as they must: they guard hunks this branch already had. - **Mutation probes.** Each hunk was reverted alone, in a throwaway worktree, and both test files were run: 18 of 18 were caught. - **The port:** the casefold keying (2 fail); the casefold lookup alone (2). - **S1:** the path named raw (13); git's reason named raw (2); every path `repr`'d, printable or not (27). - **S2:** no `except` (3). - **N2:** no `plan` check (11); only #270's question asked (1); the formatting-character reason said otherwise (1); the report's `pin` line raw (1); `check`'s `write` line raw (1). - **S3:** the line dropped (1). - **N3:** the `_pin_fix` name guard (1); the `_read_pins` path guard (1); the lower-casing (1); the `digest_source` check (1); the not-text check (1); the gitlink filter (1). - **The issue rows reproduced.** S1, S2 and N2 were reproduced on 2bcfa69's tree and shown gone on the fix (a probe driving both tools; `gh` was called 0 times). **Before fix round 1.** Runs were on 7a94b6b, with `git-filter-repo` 2.47.0 first on PATH and the repository-local `user.name` blanked with `GIT_CONFIG_COUNT` (a workstation whose standard checkout sets one fails `test_the_scaffold_refuses_with_no_elector` on main itself; CI sets none). 7a94b6b differs from ffd1297 in `tests/test_pin_source_unreadable.py` only, so the three suites that do not read that file ran on ffd1297: - the new file: 46 passed; - adopt pins, adopt plan, git output decoding: 172 passed, 1 skipped (the MedxEHR clone is absent); - scaffold pin and reuse, pin collisions, e2e, referent chain links, neutral pin and pin validators, #275's tests, naming policy, repo hygiene, CLI reference, scaffold leg-path collisions: 571 passed; - shape doctor: 254 passed, 3 skipped (the PowerShell line exists only on a Windows host); - the new file with repo hygiene, git output decoding and CLI reference, once more after the last edit: 176 passed. Earlier on 5690235 and d806e8f, before the #339 commit: the adopt suites (e2e, execute refusals, verify, spec-only, leg paths, root shape paths, plan escapes, leg-path collisions, clone template) 740 passed, and `tests/test_adopt_template_values.py` with #273's other adopt tests 489 passed, 2 skipped. They were not re-run on 7a94b6b: the #339 diff touches `pin_digest_from_source` and the call in `declared_pin_values`, which those suites reach through `test_adopt_pins.py` only. Nothing was run on macOS or Windows. Proof the new tests test something: - **On d806e8f.** 38 of the 46 fail, each on the traceback and its exit 1. The 8 that pass are the unchanged behaviour: the not-a-clone refusal (4 rows) and the valid-source rows (4). - **Mutation probes.** Ten hunks reverted one at a time, each failing at least one new test: the `OSError` catch (16 fail), the catch narrowed to `OSError` only (21), the missing-path clause (9), the clause narrowed to `FileNotFoundError` (3), the `S_ISDIR` judgement (3), the loop guard (4), no resolve at all (3), the old `.resolve()` call (4), the reason as `str(exc)` (12), and the file sentence as the OS's (3). - #273's own 52 tests still fail on c7c02fa (51 fail or error; the one that passes checks the test's own table), and its 18 mutation probes were each caught. **Re-verified after the merge.** An independent re-verification of f9ed46f and 73d65c6 found the code correct: - #409 is carried whole. A 30,000-case fuzz of `--pin` lists through #409's final refusal and this branch's port differed in 0 cases. - `scaffold-project.py` equals the tested file plus main's own changes that are not #409's (same patch-id). - `_commit_the_split` keeps #445's `try` and #394's `append`. - `_read_pins` reads a loaded plan as before. Its findings were all in this description (the #506 bullet and three stale facts), now fixed. Runs on the head and on its merge with main 48585e6: 988 passed, 2 skipped; and 967 passed, 3 skipped. `git merge-tree --write-tree` was run at 01:48Z on 2026-10-13, comparing hunks per file. Each PR head, fetched from `refs/pull/<n>/head` with no `gh`, was merged with 73d65c6 and with origin/main 9736424 alone. Two sets were asked about: - every PR this description listed before that main does not now contain; - every PR numbered above #468 whose head main does not contain: #473, #474, #490, #491, #493, #494, #495, #496, #497, #503, #504 and #505. **Open state, read with `gh` by the coordinator at 02:00Z and refreshed at 02:25Z after a re-verification on main 48585e6.** Every pull request named below is open. #184, #94, #96, #98, #100 and #161 are closed, and #465 landed as 48585e6. `fix/adopt-plan-quote-yaml-keywords` has no pull request. #506 (`fix/scaffold-reuse-seeded-legs`) opened after this list was made, and the re-verification reads it. - **Landed since the last draft of this list:** #343, #346, #355, #358, #373, #378, #382, #387, #394, #397, #401, #404, #409, #419, #452, #456, #465, #466 and #484. #394's `_commit_the_split` conflict and #401's `docs/cli.md` conflict are both resolved in f9ed46f, above. - **Clean, 20:** #266, #347, #356, #374, #376, #384, #395, #453, #467, #468, #473, #474, #490, #491, #493, #494, #495, #497, #503 and #505. #453, #468 and #491 were updated onto main since the first read and now merge clean. #507 (lane 5's #486-#488) shares only the generated `docs/cli.md` with this branch and merges clean with it. - **Conflicting with main alone, and this branch adds no hunk:** - #402: `shape-doctor.py` 5; - #496: `shape-doctor.py` 10 and `tests/test_shape_doctor.py` 1. **Conflicts this branch adds:** - **#435** (3efcc7a, lane 2's `fix/adopt-plan-field-types`, for #386). - It adds 2 hunks in `adopt-project.py`: 5 here, against 3 with main alone. They are `render_plan`'s `pins:` line and `Plan.pins`. - Keep this branch's `_pin_lines` and `_pin_products`. The rest is under "Where the meaning meets". - **#504** (37ef29a, `fix/adopt-root-kept-symlinks`). - It adds 1 hunk in `adopt-project.py`: 2 here, against 1 with main alone. It is the materializer call in `_commit_the_split`. - Keep both arguments and both comments: `append=append, neutral_pins=_recorded_pins(plan), spell=_spelled`. - **#506** (119d8c4, `fix/scaffold-reuse-seeded-legs`). - It adds 1 hunk in `scaffold-project.py`. With main alone, #506 conflicts only in `docs/handbook.html`. - #506 inserts `LEG_TEMPLATES` right after `declared_pin_products`, which this branch moved out, and rewrites `_reusable_remotes` further down. Keeping both sides breaks the file with an unterminated string literal. - Keep only #506's `LEG_TEMPLATES` block. Drop #506's `declared_pin_values`/`declared_pin_products`, which moved here, and this branch's three-line `_reusable_remotes` opener, which #506 rewrites below. Resolved that way, pyflakes is clean and #506's scaffold tests pass (88). - **`fix/adopt-plan-quote-yaml-keywords`** (ed9968f; a branch with no pull request). - It adds 2 hunks in `adopt-project.py`: 3 here, against 1 with main alone. - One is `render_plan`'s `pins:` line. Keep `_pin_lines`, which already sends product, repository and `digest_source` through `y`. - The other is `_refuse_unholdable_values`' signature. Keep `pins: dict`, with that branch's `_validator_judged` and `_named_as_read` above it. - `fix/adopt-late-failure-notice` is no longer on origin. #445, the late-failure notice, is on main and merged here. **Where the meaning meets:** - **#435 and #386.** The coordinator has said this on #435: #435 (comment) - #435's `_plan_key_findings` (`plan-bad-pins`) keeps only text items in `pins:`, and its `PINS_REMEDIATION` tells a person to write `pins: [openGlass]`. - This branch writes each pin as a mapping, and it makes a name alone `plan-pin-unrecorded`. - Read from #435's code, not run: on a tree with both, the plan `plan --pin` writes is `plan-bad-pins` in `check`. - Whichever lands second reconciles them, with three changes: - `_plan_key_findings` takes this branch's mappings, or leaves `pins:` items to `_read_pins`; - a `pins:` that is not a list gives one finding, not both `plan-bad-pins` and `plan-pin-malformed`; - `PINS_REMEDIATION` says the mapping `plan` writes. - Until that is done, `tests/test_adopt_pins.py::test_a_plan_with_a_whole_pin_checks_ok` fails. - **#314.** It landed, and it is merged here (above). - **`fix/adopt-plan-quote-yaml-keywords`.** Its pin tests are written against the name-only `--pin`. They need the `name@commit` form, and their expected `pins: [...]` line becomes `_pin_lines`'s mapping. - **Two unwritable values are refused in two runs (lane 5's second review, n3).** `plan --elected-by 'A<LF>B' --pin openGlass@<sha> --pin-source <a path holding an ESC>` refuses the elector alone, exit 2. Re-run with the elector retyped, it refuses the source; #270's refusal names every unwritable value at once. Reproduced on 421e4e5, with `gh` called 0 times. One refusal would need a code change, not made here: - `_refuse_an_unrecordable_pin_source` would split into a function that returns the sentence; - `_checked_plan_inputs` would ask it before #270's `unwritable_values` raise, and raise one `adopt-unwritable-value` that names both, with both remediations; - a test would add the two-value row. N2's question is `one_printable_line`, not `unwritable_reason`, so it cannot simply join #270's list. - **Docs.** The README, AGENTS.md and `docs/handbook.html` are unchanged, because none of them calls adopt's `--pin` a name. A sentence saying adopt now takes the scaffold's `--pin` is a follow-up. It costs an AGENTS.md line-cap raise, and a README edit forces the handbook's README blob to change. - **A product spelled `null`, `true` or `false`.** The scaffold already accepts `--pin null@<sha>`: it exits 0 and builds a root whose `validate-pins.py` exits 1 `neutral-pin-entry-malformed`. Adopt's `plan` writes `product: null` through `y`, and `check` then finds `plan-pin-malformed` and refuses it before any leg exists. The one fix for both tools is in `parse_pin`, and it would change the scaffold's behaviour. - **Wording shared with the scaffold.** `digest_source` is written bare into the pin file, so a `--pin-source` path holding ` #` is cut short there. `templates/assembly-root/contracts/neutral-product-pin.yaml` and `validate-pins.py`'s `neutral-pin-missing` remediation name only the scaffold as the writer. Two comments in `scripts/repo_shape.py` still name the scaffold's `pin_digest_from_gh`, in a digest-pinned shipped copy. - **Filed as follow-ups:** - #449: S3, a hand-edited `commit:` or `tree_sha256:` that disagrees, with options (a) and (b); - #450: the scaffold half of N2, a `--pin-source` path no file can hold; - #451: #339's third decision (N5), a `--pin-source` that nothing reads. - **`validate-pins.py` prints a `--pin-source` path raw** in `neutral-pin-source-unreadable`, the twin of S1. It is a shipped template that #409 and other open PRs edit, so it is left for its own change. No issue is filed for it yet. - **Windows and macOS** were not run here; CI ran both (below). Two assertions that depend on what Windows answers for a path (a name past the filesystem's limit, a path beneath a file) check only the refusal and its remediation there. Fix round 1's rows that need a control character or a byte that is not UTF-8 in a path, or a symlink to `git`, are skipped on Windows by a mark, and so is fix round 2's ESC row. The rows for a no-break space and a zero-width joiner run everywhere, and so does the in-process `gh-not-found` test. Reviewed independently by lane 5 at 7a94b6b (3 should-fix, 5 nits: fix round 1) and at 421e4e5 (0 blockers, 2 should-fix, 3 nits: fix round 2). After #409's squash, the re-verification of f9ed46f and 73d65c6 found the code correct; its 1 should-fix and 3 nits were in this description and are fixed. GitHub's update-branch then merged main twice (33bae3f, 2170526), with no hand edit. CI's tests, tests-macos and tests-windows and SonarCloud all passed at 2170526 (main cac6bb2 merged in). On that head merged with main 57c85e9 (#490, #474 and #468, none of which touches this branch's files), the pin and adopt set with `tests/test_update_shape_landing.py` gave 1112 passed, 2 skipped, and the doctor, adopt end-to-end, unfinished-merge and bootstrap set gave 1056 passed, 3 skipped (the doctor's PowerShell line, which exists only on a Windows host). Closes #273 Closes #339 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
…default branch and `execute` asks the remote for it before any leg, the topic is set before the pull request with what is left said in one notice, and NEXT names only a bootstrap command that runs the shape's, so a `master` source's pull request is no longer refused after both legs are pushed, a refused one no longer leaves the topic unset, and NEXT no longer names a make target the root lacks or a script that is the source's own (#479, #480, #481) (#493) `tracking_branch` is the branch both legs are pushed to, the branch the root's `project.yaml` tracks, and the base `execute` opens the split's pull request against. `plan` defaulted it to `main` whatever the source used, so a `master` or `trunk` source was adopted all the way and then `gh pr create --base main` was refused (`Base ref must be a branch`), with the same command printed as the way out (#479). A refused pull request also ended the run before the `xf-project-<id>` topic was tried, and the way out never said so (#481). And a source with its own Makefile keeps it, the shape's going to `shape/Makefile`, while NEXT said `make bootstrap`, which exits 2 there (#480). - `plan --tracking-branch` defaults to the branch `plan` reads the source at (`source.default_branch:`); an explicit flag still wins. A defaulted branch `checked_value` refuses is named as the default, not as a flag. - `check` finds, and `execute` refuses before any leg exists, a tracking branch that is no branch of the source: `plan-tracking-branch-missing`, in the one list `_entry_findings` composes. The branches are the source's remote `origin`'s once it has fetched any (the repository the pull request goes to), and its own otherwise. `plan` and `check` ask nothing of the network. `plan` prints the same sentence as a NOTE for the value it records. - `execute` also asks the remote the split goes to, `git ls-remote <assembly url> refs/heads/<tracking>`, after `_refuse_an_occupied_work_dir` and before `_refuse_a_split_branch_left_behind`, the confirmation and any leg. A clone keeps `origin/<branch>` after GitHub deletes or renames the branch, and a clone made before GitHub renamed `master` to `main` keeps `origin/HEAD -> origin/master`, so `check` passed `master`. Absent, it is `plan-tracking-branch-missing`, whose remediation is `git fetch --prune origin`, `git remote set-head origin --auto`, the clone's own branch renamed to match, then `plan`, `check` and `execute` again; unreadable, `adopt-remote-unreadable`. Under `--local-remote-dir` the url is the source, which the split is pushed back into. - The plan's last follow-up names the pull request's base as the plan's `tracking_branch`, not by value, so a hand edit of `tracking_branch:` leaves no stale branch in `check` or the pull request's body. It said "the source's default branch". - `execute` sets the topic on each of the three repositories before the pull request, asking each whatever the one before it answered, and prints `set on all three` or the ones it set. - What a refused pull request and an unset topic leave is ONE notice after the table, only once the split verified: `The branch IS pushed.`, a `gh repo edit ... --add-topic ...` for each repository left without the topic, then the pull request's command, still last; with the pull request open, `The split IS pushed and the pull request IS open. Set the topic by hand:` and only the repositories left. A failed verification offers no topic command either. - `_commit_the_split` returns the materializer's collisions, and `_next_steps` reads them all. Where the shape's Makefile went to `shape/Makefile` and its `scripts/bootstrap.py` took its own name, NEXT names `python3 scripts/bootstrap.py` with a NOTE saying why. Where the source's own `scripts/bootstrap.py` stayed in the root, with or without its Makefile, both commands would run the source's script, so NEXT names none and says which merges come first. Elsewhere NEXT is byte for byte what it was. - `collision_follow_up` (scripts/shape_materialize.py) says of the Makefile that the shape's make targets (`bootstrap`, `validate`, `pins`, `park`, `resume`) arrive only with that merge, and what runs meanwhile; told the whole collision list, it names no meanwhile command where `scripts/bootstrap.py` went aside too. - `plan --help` says what `--tracking-branch` is and its default; `docs/cli.md` is regenerated. - #373's test `tests/test_ruleset_hint_tracking_branch.py` gives its adoption source the tracking branch beside `master`, which `execute` now requires of a source. Unchanged: `execute`'s exit codes, `_verify`, the leg pushes and `ADOPT_RULESET_HINT` (both already took the tracking branch), the pull-request-only posture, every other message (NEXT where nothing went aside compared byte for byte by a test), and AGENTS.md (490 of its 490-line cap; the tool's own output carries the change, and it names no `plan-*` or `adopt-*` code). Left out: a source that keeps a lowercase `makefile` or a `GNUmakefile`, which GNU make reads before the shape's `Makefile` on a case-sensitive disk; a `--tracking-branch` that names a branch other than the one the split is made on, whose pull request then carries that branch's difference too; `check` asking the remote (it stays offline, and `execute` asks before any leg); and follow-ups re-derived after a hand edit of a `leg:` answer, so a plan whose `scripts/` was answered `root` by hand keeps `plan`'s Makefile follow-up, while NEXT is right. Composition with #382 (adopt-project.py's split-branch check, now on main): `execute` now lists the assembly's remote first for the tracking branch (`_refuse_a_tracking_branch_the_remote_lacks`), so a remote `git ls-remote` cannot read is refused as `adopt-remote-unreadable` before `gh repo create`, with the failed command as the refusal's cause. #382's `test_execute_refuses_a_repository_it_cannot_list` is re-pinned to that refusal, with stronger assertions (the code, the detail, the remediation's "no leg repository was created", and the `git ls-remote` cause naming the URL). Lane: openreposhape-3 (openRepoShape-3) 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-3 (openRepoShape-3)
Claim: #324 (comment)
Refreshed 2026-10-12T22:17Z for head 9c7cef1, which merges main 6c2a371. Adoption's half is now main's code: #445's
ADOPT_RULESET_HINTalready names the tracking branch, soadopt-project.pyis main's byte for byte. The description below is the squash message this PR lands with.RULESET_HINTis what a person reads when an organisation's ruleset has justrefused a direct push: seed a branch, open a pull request. It hard-coded
mainas the local branch of
git -C <work> push -u origin main:seed/scaffoldand asgh pr create --base main. A run made with--tracking-branch trunkmakes itswork clones with
git init -b trunkand pushestrunk, so the first commanddied on
error: src refspec main does not match anyand the second named a basethe repository does not have.
Two fields name a branch, and they are not one value:
{tracking}is the remote's branch, the run's--tracking-branch, andstands in
--baseonly. Every caller passes it.{local}is the ref in{work}that the refused push carried, and stands inthe push command only. The scaffold's two pushes (
_seed_legs,_build_assembly_root) and the family holder's (_create_holder_worktree)push the tracking branch by name from a clone made on it, and pass
args.tracking_branchfor both.checked_valueby then (it admits only[A-Za-z0-9._/@+~-]and no leading-) and, since scaffold-project.py and family.py:--tracking-branchis asked ofgit check-ref-format --branchbefore anything exists, so~,a..b,x.lockanda/are refused under--dry-runand no longer die at the firstgit init -baftergh repo createhas made the repositories (#283) #355,git check-ref-format --branch, which must echo it aswritten, so it is printed as it stands, like
{repo}.An adoption does not print
RULESET_HINT. Since #445, both adopt legs printadopt-project.py's ownADOPT_RULESET_HINT, which already names the run'stracking branch (
<repo> has no <tracking> for that pull request to target)and prints no command.
adopt-project.pyis left asmainhas it, byte forbyte; the test pins #324's rule for it instead.
tests/test_ruleset_hint_tracking_branch.pyruns every caller against localbare remotes whose
pre-receivehook refuses everything butseed/*(aruleset in miniature, installed through
GIT_TEMPLATE_DIR), once with--tracking-branch trunkand once withmain:the printed block against a literal copy of the hint as it was, asserts that
the ref the hint names as local IS the commit the refused push carried, as
the remote's hook recorded it, then runs the printed push and sees
seed/scaffoldarrive at that commit;master: eachprints
ADOPT_RULESET_HINTfor that leg's repository and the run's trackingbranch, with no
mainfortrunkand nomaster, and the constant itselfholds no
main;RULESET_HINT.format(andADOPT_RULESET_HINT.format(for a call that leaves a field out, which would raise
KeyErrorinside thefailure path that exists to explain a refusal.
Unchanged: for the default
main, the scaffold's and the family holder's hintis byte-identical. The three callers'
mainruns pin it against the literalcopy and pass on
mainand on this change alike. What the hint advises is thesame, and so are its two exits (seed by pull request, or ask an operator with
the bypass right; never
--admin, per AGENTS.md); only the branches it namesmove.
Left out:
gh pr create --base <tracking>cannot run in any of the three places, formainas for any other branch: the repository the push was refused at isempty (
gh repo createis called without a README), so the pull request hasno base to land on. That is older than this change and is not touched here.
Closes #324
🤖 Generated with Claude Code
Summary by Sourcery
Make ruleset refusal hints follow the run's tracking branch so their suggested seed push and pull request target the correct branches.
Bug Fixes:
main, allowing seed instructions to work with non-default tracking branches.Enhancements:
Tests:
trunkandmain, including validation that printed seed commands target the refused commit and that all hint format calls provide required fields.