Repository navigation
adopt-project.py: check reports the split branch an earlier run left as execute's adopt-branch-exists, and a branch in git's way of it as plan-split-branch-taken, which execute now refuses before any leg exists, so a source branch adopt no longer has both legs made before git filter-repo fails (#320) - #382
Merged
Merged
Conversation
…eg exists, a branch at or in git's way of the plan's split branch where the split is pushed, so a re-run over an earlier run's `adopt/three-repo-shape` no longer makes both legs and dies at the push of the split (#320) `execute` pushes its one split commit to `refs/heads/<adopt_branch>` of the repository the split goes to: the source itself under `--local-remote-dir`, the assembly repository on GitHub otherwise. An earlier run leaves that branch, verified or failed after the push, and nothing in `plan`, `check` or `execute` looked for it. Measured on main 71b58a1 with git 2.43 and `git filter-repo` 2.47.0: the same plan re-run into fresh legs after a verified run passed `check` (exit 0), and `execute` exited 2 with both leg repositories made, at the push of the split (`! [rejected] HEAD -> adopt/three-repo-shape (non-fast-forward)`); a source branch `adopt` passed `check`, and `execute` exited 2, saying nothing, at `git filter-repo` after both legs were made; a branch at the plan's own commit was fast-forwarded with the split and verified. Over a tip `.gitmodules` git's submodule reader cannot read, #226's history scan refused the re-run as `plan-gitmodules-unreadable`, naming the old split commit, which the extraction never exports. - A branch AT the plan's split branch, or one git cannot hold beside it (its name a directory of the split branch's, or under it), in the repository the split is pushed to, is the new finding `plan-split-branch-taken`. It names the place, the branch and its commit, the run that pushes there, and the two exits: delete that branch there, or set `adopt_branch:` to a branch git can create there. A remediation is printed once after the findings. - `check` asks without the network: the source's own branches (where `execute --local-remote-dir` pushes), and, where the source's `origin` is the plan's `<org>/<assembly>`, that remote's branches as the source last fetched them (where `execute` without it pushes). Its findings come after `plan-stale` and before every finding about the entries. - `execute` asks the one repository it pushes to, with `git ls-remote --heads`, after `_checked_plan_values` and before `_confirm` and `_create_leg_remotes`. Its findings lead `_refuse_what_check_finds`'s list, so the refusal wears `plan-split-branch-taken` and still carries every other finding of `check`'s; a `--local-remote-dir` run's finding is `check`'s word for word. A repository it cannot list is refused as `adopt-split-target-unreadable`, before any leg exists. - #226's history scan is unchanged, and still reads the branch. With this finding first the re-run is refused for the branch, and a run that gets past it reads what the scan reads: with the branch deleted neither holds it, and with another `adopt_branch:` `git filter-repo` exports it (measured: `_extract_leg` fails on the tip `.gitmodules` the scan's finding names under another name, and passes under the same one, which its `checkout -B` shadows). Unchanged: a plan whose split branch is new where it is pushed checks and executes as before, and `plan` is untouched. `tests/test_adopt_e2e.py`'s `leg-remote-exists` re-run now names another split branch, because the same plan is refused first for the first run's; a new test beside it covers that refusal. Left out: a branch in git's way in a local source, for a run without `--local-remote-dir`, still fails the extraction after both legs exist (`check` reports it, `execute` asks only GitHub); a case-only twin of the split branch is not compared; the notices after a failed run (`_verify`'s footer and the mount remediation still say to re-run into fresh legs) are #188's. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 22 hours and 10 minutes by commenting |
Reviewer's GuideAdds preflight detection of an existing or Git-incompatible adoption split branch in the actual push destination, causing Sequence diagram for split-branch preflight refusalsequenceDiagram
participant User
participant Check as check_or_execute
participant Source as Source_repository
participant Target as Split_push_target
participant Legs as Leg_creation
User->>Check: check or execute
alt check
Check->>Source: inspect local branches
opt origin is assembly repository
Check->>Source: read origin remote-tracking branches
end
else execute
Check->>Target: git ls-remote --heads
alt target cannot be listed
Target-->>Check: failure
Check-->>User: refuse adopt-split-target-unreadable
else target branches returned
Target-->>Check: branch names and commits
end
end
alt split branch is taken or in git's way
Check-->>User: report/refuse plan-split-branch-taken
Check-->>User: print remediation
else split branch is available
Check->>Legs: create and push leg repositories
Legs-->>User: continue adoption
end
Flow diagram for split-branch collision handlingflowchart TD
A[Start check or execute] --> B[Resolve adopt_branch]
B --> C{Which target is inspected?}
C -->|check| D[Read source branches and eligible origin tracking branches]
C -->|execute| E[git ls-remote --heads on actual split target]
E -->|Cannot list| F[Refuse adopt-split-target-unreadable]
D --> G[Find branches at or in the way of adopt_branch]
E --> G
G -->|Collision found| H[Report or refuse plan-split-branch-taken]
H --> I[Print remediation once]
G -->|No collision| J[Run existing entry findings]
J --> K{Any findings?}
K -->|Yes| L[Refuse before leg creation]
K -->|No| M[Create legs and continue adoption]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Author
|
(lane openRepoShape-3, 2026-10-12T14:54Z) @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
…#190's five overlaps with #320 kept on both sides #190 (161c6c9) and this branch changed the same lines of adopt-project.py in five places. Both are kept: - the module docstring's `check` bullet names the relative url and the split branch; - `cmd_check` asks the split branch first and keeps `_entry_findings`'s `notes`; - `_refuse_what_check_finds(plan, source, local_remote_dir=None, taken=((), ()))` keeps both docstring paragraphs, and its findings are `taken`'s first, then `_entry_findings(plan, source, local_remote_dir)`'s; - `cmd_execute` passes `args.local_remote_dir`, then `_execute_split_branch(...)`. The resolved adopt-project.py is byte-identical to the composition of 7e55d00 with a0272b2 that ran 432 tests green before #190 landed, and a0272b2's adopt-project.py and tests are 161c6c9's. d82a83e, 0f67f7b and 3ae0109 merge with no conflict. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rce's own branches for one git cannot hold beside the split branch, and `check` says that finding is about both runs, so a local source holding `adopt` no longer has both GitHub leg repositories made before `git filter-repo` fails (#320) The review of 7e55d00 measured a run without `--local-remote-dir`, from a local source holding the branch `adopt`, with `gh` recorded: `check` reported `plan-split-branch-taken` as a finding about `execute --local-remote-dir` alone, and `execute` asked only GitHub, which held no such branch, created both leg repositories, and exited 2 with nothing on stderr. `_extract_leg` clones the source, checks the split branch out in the clone and runs `git filter-repo`, which makes each of the clone's remote-tracking branches a branch and cannot make `adopt` beside `adopt/three-repo-shape`, whichever repository the split is then pushed to. - A source branch in git's way of the split branch (its name a directory of the split branch's, or under it) is now said as the extraction's failure, about `execute` with `--local-remote-dir` or without it. - `execute` without `--local-remote-dir` asks the source's own branches (`git for-each-ref refs/heads/`, no network) before GitHub, and refuses those names before any repository is created. The split branch's own name there is not refused for that run: the clone's `checkout -B` shadows it, and the push goes to GitHub. - A plan that seeds both legs extracts nothing, so neither command asks the source for the extraction; a branch in git's way there is then the push's finding, about `execute --local-remote-dir`, as before. - The remediation says the extraction meets the source's branches, and that a branch named as the source's `origin` last fetched it leaves `check`'s findings once `git fetch --prune origin` finds it gone. Unchanged: the split branch at its own name, and GitHub's branches, are asked and worded as before; a `--local-remote-dir` run's refusal still carries `check`'s findings word for word. Tests: `tests/test_adopt_split_branch.py` gains eight. Run against 7e55d00's `adopt-project.py`, five fail: the two runs without `--local-remote-dir` over a source branch `adopt` and `adopt/three-repo-shape/old` reach `gh repo create`, the two `check` sentences are the push's, and the remediation names no prune. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 12, 2026
Contributor
Author
|
(lane openRepoShape-3, 2026-10-12T16:13Z) fix round 1 (independent review, codex is out of quota): New head: 1. should-fix: conflict with main after #245 (#190) landed. Done by merge, not rebase, because the PR is open.
2. should-fix: a run without
3. nit: a stale 4. nit: refusing the fast-forward case. Kept as is, as the review accepts. Targeted set on 33c6dee: all 21 Trial merges against 33c6dee (
Squash message: rewritten for the final state; the merge of main is not part of it. Its subject now adds "or in git's way of it in the source the legs are extracted from". The PR title still carries the first subject. The last "Left out" is: a case-only twin of the split branch, and #188's notices. |
This was referenced Oct 12, 2026
brettheap
added a commit
that referenced
this pull request
Oct 12, 2026
… is refused and a name holding a lone surrogate is a finding, so `pins: [a, b]]` is not the pin `b]` and `name: "a\ud83db"` no longer kills `make validate` (#301, #280) (#390) Two reader-side defects the lane filed, in one commit on `fix/yaml-flow-brackets-surrogates`, cut from main c7c02fa. Below, "this head" is that commit, 21fbea4, the one the review approved; main was merged into the branch afterwards (last paragraph). **#301, `scripts/repo_shape.py`.** In YAML a plain scalar inside a flow collection ends at any of `[ ] { } ,`. `_split_flow` counts brackets to find its commas and never asked whether the count went below zero or ended above it, so a stray bracket was kept as text: - `pins: [a, b]]` read `['a', 'b]']`; - `k: [x [a, b]]` read `['x [a, b]']`; - since #284, `k: {a"]:}` read the key `a"]`. PyYAML refuses each. A new `_refuse_flow_indicator` refuses a PLAIN item or value (called from `_flow_value`) and a PLAIN key (called from `_split_key_flow`) of a flow collection that holds `[`, `]`, `{` or `}`. The refusal is a `YamlError`, so `load_yaml` turns it into `REFUSED yaml-unparsable` and names the scalar: ``a plain scalar in a flow collection holds `]`, which ends it in YAML: 'b]'``. These still read as before: - a quoted scalar's brackets (`k: ["a]", {"b}": "c]"}]`); - nested and empty flow collections; - a bracket in a BLOCK plain scalar (`k: a[b]`, `- see [1]`); - a bracket in a comment after a flow. No other function changed. **#280, `templates/assembly-root/scripts/validate-manifest.py`.** `_name_finding` now reports a display name holding any of U+D800 to U+DFFF as `manifest-name`: ``` FINDING manifest-name: name is 'a\ud83db', which holds U+D83D, a lone surrogate: no UTF-8 file, commit message or console can hold it. Write each character as itself, or as one `\U` escape (`\U0001F600`, not the pair `\uD83D\uDE00`). ``` The finding goes to stderr with the name escaped by `repr`, before the `manifest ok` line could raise. **Why the #280 refusal is in the validator and not in the reader.** Two places could refuse a lone surrogate, the reader and the validator. The evidence rules out the reader: - **The writer emits a lone-surrogate escape on purpose.** The escape is `\uDXXX`, a `\u` and four hex digits from D800 to DFFF. `double_quoted('caf\udce9')` is `"caf\udce9"`. That is how `adopt-project.py`'s plan keeps a path byte that is not UTF-8 (#199), and `check` must read it back. `tests/test_adopt_entry_paths.py` holds that round trip (120 passed with `test_adopt_plan.py` on this head). Lane 1's pending #189 branch (70f4d29, `submodule_record`) also writes `\udce9` escapes into `project.yaml`'s `submodules:` block, and its docstring relies on the reader resolving them. A reader refusal would break both. - **PyYAML 6.0.3 agrees with the reader.** The pure-Python `SafeLoader` reads `name: "a\ud83db"` as the lone surrogate. Only libyaml (`CSafeLoader`) refuses it, and it refuses even the escape pair `"\ud83d\ude00"`, which the Python loader reads as two surrogates, as this reader does. - **The defect is in a value, not in the YAML.** A display name is text a person chose, and no UTF-8 stream holds a surrogate. On c7c02fa and on this head, with local bare remotes only: | launcher | c7c02fa | 21fbea4 | |---|---|---| | `scripts/validate-manifest.py` | exit 1, `UnicodeEncodeError` traceback | exit 1, the `manifest-name` finding | | `make validate` | exit 2 on that traceback | exit 2 on the finding | | `shape-doctor.py --root .` | `INVALID (manifest)`, the row quotes `UnicodeEncodeError: ...` | `INVALID (manifest)`, the row quotes `FINDING manifest-name: ...` | | a real `bump-leg.py` | exit 0, prints `manifest ok: a\ud83db`, and COMMITS the subject `Bump spec leg to <sha> in a?b` (`errors="replace"`) | exit 2 `bump-leg-validators-red`, every byte rolled back, 0 commits | | `update-shape.py apply` | exit 0, prints `manifest ok: a\ud83db`, commits | exit 2 `update-validators-red`, restored, 0 commits | So all four launchers now agree, and each names the problem. **Readers and callers this changes.** - **#301:** every `parse_yaml`/`load_yaml` caller, and only for a document whose flow collection holds a plain scalar with a bracket. Such a document was read, or misread, before and is refused now. Every tracked YAML file loads the same on c7c02fa and on this head (14/14 raw, 18/18 rendered templates). So do the 6 YAML files of a scaffolded project (`project.yaml`, four `contracts/*.yaml` and `validate.yml`): 6/6 on a c7c02fa scaffold and 6/6 on a 21fbea4 scaffold. No writer emits such a scalar: - every flow sequence written in the tree is bare names or `[]` (adopt's `pins: [...]` and `allow_empty_legs: [...]`, `naming_block`'s `also_matches` and `referent_chain`); - `double_quoted` quotes any value holding a bracket. - **#280:** the project's own `scripts/validate-manifest.py` (a digest-pinned shape copy; it reaches existing projects through `update-shape.py apply`, which reports it `upstream-changed`). Every caller of that copy now stops at the finding: - `make validate`; - the CI step in `validate.yml`; - `shape-doctor.py`'s `manifest` row; - `bump-leg.py` and `update-shape.py apply` (their red-validator refusal and rollback); - a family holder's `make validate`, which runs each member's. `bootstrap.py` runs the same copy in its step (b), but on such a name it dies at its own first `print`, before step (b) (see Left out). With #314 merged as well, the scaffold's pre-check asks this validator about `--name`. In the tree of this head and #314 alone, `scaffold-project.py --name $'caf\xe9' --dry-run` is refused `scaffold-manifest-invalid` with this finding before anything is created (checked at #314's head 4b8da3d). With #351 (landed as d82a83e) and without #314, that `--name` is already refused, as `scaffold-unwritable-value`. **Evidence.** - **Issue rows.** All ten #301 documents reproduce on c7c02fa and are refused on this head. 8 are documents PyYAML refuses. 2 (`k: {a:}#}: b}`, `k: {}#":}`) PyYAML reads only by taking a `#` with no space before it for a comment, and they are refused by design, as `k: "a"#c` is. Every #280 row is in the table above. - **Seeded oracle fuzz.** Corpus: an earlier review's, seed 2490250, 785,646 documents in six contexts. PyYAML 6.0.3 is the oracle only. | | c7c02fa | 21fbea4 | |---|---|---| | read the way PyYAML reads it | 254,629 | 254,629 | | read, where PyYAML refuses | 179,999 | 112,562 | | refused, where PyYAML reads | 48,434 | 49,492 | | read, but not as PyYAML reads it | 2,330 | 1,272 | | the flow contexts alone: read where PyYAML refuses / refused where PyYAML reads | 77,073 / 15,413 | 10,306 / 16,465 | Only two kinds of document change: - 67,437 invalid documents c7c02fa read are refused; - 1,058 documents c7c02fa MISREAD are refused. 1,037 of these are PyYAML's `#` leniency, and 21 are a single-pair mapping in a flow sequence (`k: [a:[]]`), which the subset never read as a mapping. Nothing c7c02fa read correctly changes, nothing is newly read, and there are 0 crashes. Against the issue's two-line `depth < 0` sketch, run on the same corpus, this head refuses everything the sketch refuses, plus 21,631 more: 21,608 invalid documents (the opener class, such as `k: [a[]` and `k: [x [a, b]]`) and 23 misreads. The fuzz ran on 38738cf, whose `scripts/` and `templates/` are byte-identical to 21fbea4's. - **Tests fail on c7c02fa.** With the four test files copied onto c7c02fa: 45 failed, 191 passed. The failures are 31 bracket rows, 10 validator rows and the 4 launchers. - **Mutation probes.** Each hunk was reverted or weakened on its own, and each mutant fails new tests: - M1, no check on items and values: 23 failed; - M2, no check on keys: 8; - M3, no quoted-scalar exemption: 14; - M4, no quoted-key gate: 11; - M5, closers only: 3; - M6, no surrogate finding: 10; - M7, upper bound excludes U+DFFF: 2; - M8, lower bound excludes U+D800: 2. **Tests.** - New: `tests/test_yaml_flow_brackets.py`, with the ten issue rows, sixteen more positions, the refusal's wording, a file refusal, fifteen still-read rows and the writer round trip. - In `tests/test_manifest_validator.py`: five surrogate spellings, each under a pipe and under `PYTHONIOENCODING=utf-8`, plus four still-ok controls on either side of the range. - In `tests/test_validator_child_encoding.py`: a real bump, `update-shape.py apply`, the doctor and `make validate`, each on `name: "a\ud83db"`. - `tests/test_yaml_quote_boundaries.py`: its docstring said `k: "a"#c` was the only PyYAML disagreement. There are eight; seven are `BRACKET_IN_A_KEY` rows refused by design. Targeted runs on 21fbea4, Linux, Python 3.12.3, git-filter-repo 2.47.0, each in the background: - the reader and validator files (`test_yaml_flow_brackets`, `test_yaml_flow_quotes`, `test_yaml_flow_mappings`, `test_yaml_quote_boundaries`, `test_yaml_block_scalar_keys`, `test_load_yaml_refusals`, `test_adopt_plan_escapes`, `test_manifest_validator`, `test_pin_validator`, `test_neutral_pin_validator`, `test_template_quoted_values`): **462 passed**; - `test_validator_child_encoding`, `test_stdout_encoding`, `test_repo_hygiene`, `test_cli_reference`: **165 passed, 1 skipped** (`test_stdout_encoding.py:286`, a Windows-only default); - `test_adopt_plan`, `test_adopt_entry_paths`: **120 passed, 1 skipped** (`test_adopt_plan.py:393`, `SHAPE_MEDXEHR_CLONE` unset); - `test_shape_doctor`: **242 passed, 3 skipped** (`test_shape_doctor.py:5330`, PowerShell on a Windows host only); - this head merged with origin/main 86da808, which carries #316: `test_template_quoted_values`, `test_template_values_single_pass`, `test_yaml_flow_brackets`, `test_yaml_quote_boundaries`, `test_manifest_validator`: **412 passed**; - this head merged with #314 (ab66bc0): `test_scaffold_manifest_values`, `test_adopt_manifest_values`, `test_manifest_validator`, `test_yaml_flow_brackets`: **293 passed**. The full suite and the macOS and Windows legs did not run before this PR's CI. **Merge-tree, as of 2026-10-12T15:12Z** (`git merge-tree --write-tree --name-only`), against origin/main d82a83e and the heads named here; each sha is the one it had then, and main has moved since. - **Clean with this commit:** - origin/main d82a83e, which carries #316 (34ecbc2), #245 (head a0272b2, landed at 14:55Z as 161c6c9) and #351 (landed as d82a83e). #245's one change to `scripts/repo_shape.py` is in `remote_key`, away from the YAML reader, and #351 touches `scaffold-project.py` only; - #314 (4b8da3d). With it merged, the `--name` refusal above still holds (checked at 4b8da3d); - lane 1's `fix/adopt-late-failure-notice` (9f1778d) and `fix/adopt-plan-quote-yaml-keywords` (d38e143); - the sibling `fix/pin-source-env-collisions` (34dbd86, #275); - every other open PR head but one: 28 of the 29 open, all but #358. - **Conflicts that are not this commit's.** Each conflicts the same way with origin/main alone, in a file this commit does not touch: - #358 (ebffb08), in `shape-doctor.py`; - lane 1's `fix/adopt-root-kept-symlinks` (6c44591), in `adopt-project.py`. - **Five heads already conflict with origin/main itself, and merge clean with this commit:** #314, #356 (5a5011d), #382 (7e55d00) and lane 1's `fix/adopt-plan-quote-yaml-keywords` in `adopt-project.py`, and lane 1's `fix/adopt-late-failure-notice` in `tests/conftest.py`. - **One conflict is this commit's:** `fix/adopt-record-submodule-placement` (70f4d29, lane 1, #189), in one hunk, the import block of `tests/test_manifest_validator.py`. Both branches add to it: that branch adds `import os` and `import sys`, and this one adds `import subprocess` between them, so the resolution is all three. Its `repo_shape.py` and `validate-manifest.py` changes auto-merge. Not resolved in either branch. **The review.** An independent opus review at 21fbea4 approved it with nits: 0 blocker, 0 should-fix, 5 nit. All five are in text (this description, the author's report and the commit message), none in the code or the tests. Four were fixed in the description; the fifth, the commit message's spelling of the surrogate escape as a six-digit one, is fixed in this message: the escape is `\uDXXX`, a `\u` and four hex digits, D800 to DFFF. Its own seeded structured fuzz (seed 7301280: 120,000 nested flows in five block contexts, 600,000 documents, PyYAML 6.0.3 as the oracle only) found 0 changed reads: no document c7c02fa read the way PyYAML reads it changed, and nothing crashed. An instrumented reader returned no plain scalar holding `[ ] { }` inside a flow on this head, against 76,955 on c7c02fa. The author's seed 2490250 counts reproduced row for row, and all 13 mutants it tried were killed. **664 passed** on this head merged with origin/main ab623dd (16 test files; 361 on the head alone, 8 files). The one conflict, with lane 1's #189 branch, is the `tests/test_manifest_validator.py` import block, and resolves to all three imports. Three follow-ups it confirmed are not in this change: the bootstrap's raw name print and the flow entries still read (both under Left out), and a block value that opens with `]` or `}`, which c7c02fa and this head both read where PyYAML refuses. **Left out.** - **A plain flow entry that opens with an indicator, and an empty flow entry.** Most of what this head still reads where PyYAML refuses is one of two classes: - an entry opening with `:`, `#`, `?` or `-` (`k: {:":}` reads `{':"': None}`, `k: [:]`; 6,286 flow documents); - an empty entry from a leading or doubled comma (`k: [,a]` reads `['a']`, `k: [a,,b]` reads `['a', 'b']`; 3,858). Neither is a bracket, and #301 does not claim them. - **A single-pair mapping in a flow sequence.** `k: [a: b]` still reads as the string `'a: b'`, where PyYAML reads `[{'a': 'b'}]`. Only the ones holding a bracket are now refused. - **`templates/assembly-root/scripts/bootstrap.py`.** It prints the display name raw (`bootstrap: <name> (<root>)`) before its validators run. On a project named `"a\ud83db"`, `make bootstrap` still dies there with `UnicodeEncodeError`, on c7c02fa and on this head (filed as #391). - **`elected_by` and `reference`.** A surrogate in either is still accepted. Neither is printed by the validator, so nothing raises; the scaffold side of such values is #282. - **The escape pair `"\ud83d\ude00"`.** It is still two surrogates, not one character, as PyYAML's Python loader reads it. The finding asks for the character itself or its one `\U` escape instead of the reader inventing a joining rule. Reviewed adversarially at 21fbea4 (APPROVE WITH NITS: 0 blockers, 0 should-fixes, 5 nits, all in text and none in the code or the tests). The code and the tests needed no fix round; the nits are fixed in this message. Main was merged into the branch after the review with no conflict: at that merge (c1a119d, main 45e01f4 merged in) the branch's own diff against main is the one reviewed, the same 6 files with 422 lines added and 6 removed. CI is green on Linux, macOS and Windows, with SonarCloud passing, at c1a119d (main 45e01f4 merged in). The combined tree with main 3ef059c (#314 landed) was run locally before landing. Closes #301 Closes #280 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 12, 2026
36538fd..437a360 is #355, #401, #456 and #343; #401 adds `_leg_path_help` and the `--spec-path`/`--code-path` help to adopt-project.py, away from every line this branch changes. The merge has no conflict. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n left under `execute`'s own `adopt-branch-exists`, and `execute` refuses only a branch in git's way of it, after #445's refusals #445 (75052cb) refuses in `execute` the split branch an earlier run left, as `adopt-branch-exists`, before any leg exists, and only where the push cannot fast-forward it. Its tests pin its order: an occupied work directory first, then that branch, and a live leg repository under `--local-remote-dir` keeps `leg-remote-exists`. This branch's refusal of the same branch came first, in `_refuse_what_check_finds`, and preempted all three: 8 tests of `tests/test_adopt_late_failure.py` failed on 973d0fe. - `check` keeps the finding main lacks. For the split branch itself where the split is pushed it is now `adopt-branch-exists`, the code `execute` refuses it under and `_left_by_the_split` tells the re-run, asked by #445's rule (`_in_the_history_of`), so a branch at the source commit or behind it, which `execute` pushes over and verifies, is no finding. - A branch in git's way of the split branch (`adopt`, `adopt/three-repo-shape/old`), which main still meets only after both legs exist (measured on ec4a7a0: at `git filter-repo`, or at the split push), stays `plan-split-branch-taken` in `check` and in `execute`. - `execute` asks for it in `_refuse_a_branch_in_the_split_branchs_way`, right after `_refuse_a_split_branch_left_behind`, skipped like it while a leg repository is still there under `--local-remote-dir`. `_refuse_what_check_finds` is main's again. - Dropped: `execute`'s own refusal of the split branch itself, and `adopt-split-target-unreadable`, which #445's `git ls-remote` of the same repository now always meets first. Unchanged: #445's functions and `cmd_execute`'s order before the new call are byte-identical to main's; `tests/test_adopt_late_failure.py` and `tests/test_adopt_e2e.py` are main's, unedited. `execute` over a split branch whose commit #226's history scan names is refused for the scan's finding first, as on main; `check` names the branch first. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 12, 2026
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a1ffd5271
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Comment on lines
+3644
to
+3645
| args = ["git", *FILE_PROTOCOL, "ls-remote", "--heads", "--", url] | ||
| listed = subprocess.run(args, cwd=cwd, capture_output=True, check=False) |
There was a problem hiding this comment.
When a local source configures uploadpack.hideRefs (for example, hiding refs/heads/adopt), this git ls-remote --heads call omits that branch even though receive-pack still rejects a push to refs/heads/adopt/three-repo-shape because of the ref directory/file collision. Consequently both check and execute --local-remote-dir pass this new guard, then execute creates and pushes both leg repositories before the split push fails—the exact partial-adoption state this change is intended to prevent. For a local assembly/source path, enumerate its refs directly (as the non-local extraction check already does) rather than relying on upload-pack's advertised view.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
(lane openRepoShape-3, 2026-10-12T23:49Z) Confirmed by an independent verification (git 2.43, a source with uploadpack.hideRefs = refs/heads/adopt): check says ok, execute --local-remote-dir pushes both legs, then the split push is refused loudly by #445's REFUSED pushing the split branch, quoting git's lock error. Under v1.0.0's bar it is a known limit, not a blocker: hideRefs/transfer.hideRefs are server-side settings that an ordinary working repository does not carry, and #445's own git ls-remote <source> refs/heads/<branch> (main's code, kept byte-identical here) has the same blind spot. It is recorded in the PR's Left out and the squash message. Reading a local source's branches with git for-each-ref instead goes on the adopt parent #457's checklist.
brettheap
changed the title
check reports, and execute refuses before any leg exists, a branch at or in git's way of the plan's split branch where the split is pushed, so a re-run over an earlier run's adopt/three-repo-shape no longer makes both legs and dies at the push of the split (#320)check reports the split branch an earlier run left as execute's adopt-branch-exists, and a branch in git's way of it as plan-split-branch-taken, which execute now refuses before any leg exists, so a source branch adopt no longer has both legs made before git filter-repo fails (#320)
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>
Contributor
Author
|
(lane openRepoShape-3, 2026-10-13T00:44Z) @codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
…d `open_glass` share one `SHAPE_PIN_SOURCE_<PRODUCT>`, so `--pin` refuses the pair and `validate-pins.py` notes a declared one and reads the variable for neither (#275) (#409) `pin_source_env_name` turns every character outside `[A-Z0-9_]` into `_`, so `open-glass`, `open_glass` and `open.glass` are three legal product names, with three pin files (`contracts/<product lowercased>-pin.yaml`), and one variable, `SHAPE_PIN_SOURCE_OPEN_GLASS`. On ab623dd (this branch's main parent; the same rows reproduce on c7c02fa), with the issue's own probe against local bare remotes and a `gh` that exits 97: | step | ab623dd | this head (6494714) | |---|---|---| | `scaffold-project.py --pin open-glass@<A> --pin open_glass@<B>` | exit 0, three repositories | exit 2 `scaffold-pin-collides`, nothing created | | `validate-pins.py`, no source, on a project that already declares the pair | exit 0, both skips offer `SHAPE_PIN_SOURCE_OPEN_GLASS` | exit 0, one `note neutral-pin-source-collides`; the skips offer `--pin-source <product>=<path>` and a sibling checkout only | | `SHAPE_PIN_SOURCE_OPEN_GLASS=<open-glass>` | `neutral-pin-source-unreadable` for `open_glass`, exit 1 | exit 0, the note; the variable is read for neither pin, so each pin is read from its own source or skipped | | `SHAPE_PIN_SOURCE_OPEN_GLASS=<open_glass>` | `neutral-pin-source-unreadable` for `open-glass`, exit 1 | the same | | `--pin-source open-glass=... --pin-source open_glass=...` | exit 0 | both pins recompute, exit 0, the note | **What changed.** Every variable name is kept: an operator may have set one and the docs name them. What is done instead: - `scripts/repo_shape.py`: `pin_source_env_collisions(products)` is the one pure answer, `{variable: [products]}` for each variable two or more distinct products share. Distinct means distinct PIN FILES (`pin_product_key`, `lower()`, which is what `contracts/<product lowercased>-pin.yaml` is named by), so a product typed twice or in two cases is #241's file collision and never this one. `pin_source_env_shared` is the set of those keys. - `scaffold-project.py`: `_refuse_colliding_pins` (left where it is) names each variable's `--pin` arguments as typed after #241's file sentences, in the same `scaffold-pin-collides` refusal. Dry run and real run say the same no, before any process starts and ahead of the leg-path refusal. #241's wording and remediation are unchanged when only a file collides. A NEW pair is still refused, so the situation is not created anew. - `templates/assembly-root/scripts/validate-pins.py`: one NOTE per variable, `neutral-pin-source-collides`, naming every product and the variable, the keyed `--pin-source <product>=<path>` that reads each pin, and that which declaration stays is the human's decision. It is never a finding and never moves the exit code. A product whose variable is shared does not read it (rule 2 of `resolve_neutral_pin_source`), and its skip hint offers the keyed `--pin-source <product>=<path>` and not the variable or the bare form. A variable one product has to itself reads and is offered exactly as before. - `scripts/repo_shape.py`, the two chain readers: `link_pins_from_trees` does not read a variable two links of one chain share, and the `declared-unverified` hint stops naming it. Only a hand-written chain can have two such links, since scaffold takes every link but the first from the naming policy's alphanumeric `open<Product>` form. Each reader of `pin_source_env_name`, decided: | site | decision | |---|---| | `scaffold-project.py --pin` | refuse (above) | | `validate-pins.py` rule 2 and the skip hint | a note, and the variable is neither read nor offered for a shared product | | `repo_shape.py` `resolve_link_source` (via `link_pins_from_trees`) | skip the variable when another link of the chain shares it | | `repo_shape.py` `_chain_link_findings` hint | omit the variable for a shared link | | `adopt-project.py --pin`, `plan`, `check`, `execute` | not in this PR, see below | **Fix round 1 (from the review of 34dbd86).** **The design change: a declared pair is a NOTE and not a finding. This is the lane coordinator's decision, and Brett may reverse it.** The first round made `neutral-pin-source-collides` a FINDING. Main through ab623dd scaffolds such a project (`--pin open-glass@A --pin open_glass@B` exits 0), and `update-shape.py apply` runs the NEW `validate-pins.py` over the rewritten root, so for a project already declaring the pair the finding ended every apply as `REFUSED update-validators-red`, rolled back, for this update and every later one, until a person deleted a true declaration of descent. AGENTS.md tells an agent never to edit "a pinned file, a digest or a manifest to make a row go green", and the finding's remediation was exactly that, so the person was locked out. The coordinator chose the review's option (b): it is a NOTE now. The guard stays (the variable is read for neither pin), each pin still verifies from its own source, scaffold still refuses a new pair, and `validate-pins.py` says the rest in one line. To put the finding back, restore 34dbd86's `report.finding(...)` call in `_report_pin_source_collisions` (it is not a word swap for `report.advise`: `advise` takes one string, `finding` a code and a detail), give the finding the remediation 34dbd86 had, and turn the tests that expect the note (and `test_a_project_declaring_a_colliding_pair_takes_a_shape_update`) back into exit 1; the first round's versions of all of them are on 34dbd86. - **Proved on a real old project.** Main (ab623dd) scaffolded the pair into local bare remotes; `update-shape.py apply` to the first round's head 34dbd86 exited 2 `REFUSED update-validators-red` and rolled back (`git branch` shows only `main`); to this head it exits 0, the project's validator passes and prints the note, and the doctor's `pins` row is `ok` before and after. The doctor prints `passes` for a validator that exits 0 and does not echo its stdout, so the note itself is visible in `update-shape.py apply`'s output and in `validate-pins.py`'s, and the verdict moves for no reason but the update (`COMPLIANT, SHAPE BEHIND` to `COMPLIANT`). It is now a test: `test_a_project_declaring_a_colliding_pair_takes_a_shape_update`, over local stand-ins beside the project, so no `gh` is asked. - **Collisions by the variable.** Pairs were decided by `casefold()`, but the pin file is `lower()`. `openß`/`openss`, `openſ`/`opens`, `openfi`/`openfi` and `openς`/`openσ` (the letters are U+00DF, U+017F, U+FB01 and U+03C2 beside U+03C3) share a variable and have two pin files, and were never found. They are now, from the pin-file fold (`pin_product_key`). A pool of 31 names is checked pairwise against `neutral_pin_path` and `pin_source_env_name` themselves, and no single product is ever a collision. They are reachable only from a hand-written `project.yaml`. - **Windows.** The guard's only test needed the `#!` `gh` stub and was skipped on Windows. `test_a_shared_variable_answers_for_neither_pin_with_no_gh` puts a checkout of each pin beside the project, names one in the variable, and needs no `gh`: on ab623dd it is `neutral-pin-source-unreadable`, here each pin reads its own sibling. The hint and the non-ASCII rows are tested in-process, with no skip and no filesystem. - **The hint.** For a product two pins share, the skip hint offers the keyed `--pin-source <product>=<path>`: the bare form answers for every pin with no more specific source, so with two pins it gives the other the finding this issue is about. - **Trailers and title.** The trailers of the earlier commits (a30091f and 34dbd86 name Sonnet 5.5) are settled in this message, which carries Opus 5.5. The PR title now names `repo_shape.py`. - `scaffold-project.py` is not touched this round: `fix/adopt-pin-writes-pin-file` moves that code, and the change would only conflict with it. **Left out.** - `adopt-project.py` is untouched. Its half arrives with #273's PR, which moves scaffold's pin machinery into a shared module and routes adopt through `pin_source_env_collisions`. Once it lands, the one place that still keys by `casefold()` (`pin_variable_collisions` in `scripts/shape_materialize.py`) should read `pin_product_key`; it cannot differ for an ASCII `--pin`, which is all `PIN_ARG_RE` takes. - `scaffold-project.py`'s own `_colliding_pin_variables` also keys its argument lists by `casefold()` and one docstring there still says the project's validator reports the pair "as a finding"; both go with the code when #273 moves it. #312's `PinSources` keys `--pin-source` by `casefold()` too, so `openß=` and `openss=` are one key there; that is #312's. - A product in `neutral_product_pins:` and a chain link in `naming.referent_chain` that share a variable are read by two validators in one process, and neither sees both sets. Not detected here. - The doctor does not print a passing validator's stdout, so its `pins` row does not carry the note. Not changed: it would add a file to this PR. - No `--help` text changed, so `docs/cli.md` is untouched. `README.md` is at its line cap and its blob is pinned by `docs/handbook.html`, so the one README sentence this wants needs a regenerated handbook and waits for its own change. - `validate-pins.py` and `repo_shape.py` ship into projects, so every project's `shape-pin.yaml` reads `upstream-changed` for both until `update-shape.py apply`, which now succeeds for a project that declares such a pair. **Evidence.** All on the head (6494714), with git-filter-repo 2.47.0 on PATH, a `gh` that cannot be reached (a stub first on PATH, `GH_TOKEN` bogus, an empty `GH_CONFIG_DIR`), and the repository-local `user.name` blanked at command-line scope, as the standard's checkout sets one that two scaffold tests read. - `tests/test_pin_source_env_collisions.py` is 137 tests and passes on this head. Laid over 34dbd86, 50 fail and 87 pass: the note and update tests, the non-ASCII rows, the hint, the sibling guard, and `pin_product_key` itself (which 34dbd86 does not have); the scaffold refusals, the chain readers and the controls hold on both. - Targeted, run on ad6f569, whose tree differs from this head in `tests/test_pin_source_env_collisions.py` only (the last edits made one comparison read the committed blob and wrapped one line): the new file, `test_pin_validator`, `test_neutral_pin_validator`, `test_cli_reference`, `test_manifest_validator`, `test_naming_policy` and `test_repo_hygiene` **445 passed**; the scaffold pin, collision, referent-chain, leg-path and e2e files **245 passed**; `tests/test_update_shape*.py` **218 passed, 3 skipped** (junctions, NTFS only); `tests/test_shape_doctor.py` **254 passed, 3 skipped** (the PowerShell line, a Windows host only). - On this head itself: the new file, `test_repo_hygiene`, `test_cli_reference`, `test_pin_validator` and `test_neutral_pin_validator` **244 passed**. - This head merged with origin/main a983e6a, in a throwaway worktree: the new file, `test_pin_validator`, `test_neutral_pin_validator`, the scaffold pin, collision, referent-chain and e2e files, the whole doctor file and `test_cli_reference` **590 passed, 3 skipped** (the PowerShell line); the new file alone, in its final form, **137 passed**. origin/main had moved twice since; the merge-tree below is against it. - Twelve mutation probes, each hunk reverted alone: all caught. The note-to-finding one is caught by the update test (a committed mutant, since the test clones the repository's HEAD) and the guards by the sibling test, run with every test that needs a `#!` stub deselected as a Windows runner would; the hint one by an in-process test. - Nothing ran on macOS or Windows before this PR's CI. **Merge-tree, as computed during the fix round on 2026-10-12** (every sha and head is the one it had then; main and the other branches have moved since). `git merge-tree --write-tree` of this head (6494714) against origin/main 1f90a0f is clean, and so is the merge of origin/main a983e6a, which the merged-tree runs above used. #312, #309, #270, #268, #228, #245 and #351 have landed. The rest were computed on ad6f569 (the same tree but for one test file) against a983e6a. Open PRs touching this branch's files, all clean against the head: #314, #355 and #373 (`scaffold-project.py`), #343 (`validate-pins.py`), #390 (`repo_shape.py`) and #394 (`repo_shape.py`, which conflicts with origin/main by itself). The other 25 open PRs and the lane-5 branches `fix/adopt-check-link-and-manifest`, `fix/scaffold-free-text-values`, `fix/family-init-values`, `docs/adopt-execute-exits`, `fix/yaml-flow-brackets-surrogates` and `fix/adopt-pin-writes-pin-file` (#273's, 7a94b6b, also clean for this head) are clean too. #356, #382 and `fix/adopt-record-submodule-placement` conflict with origin/main and not with this head; #358, `fix/adopt-late-failure-notice` and `fix/adopt-plan-quote-yaml-keywords` conflict with both, with origin/main by themselves, so they are not this branch's. Neither are the five PRs below #200 that origin/main has not landed (#94, #96, #98, #100 and #161), which conflict with it too. The PR set is read from the refs under `refs/pull/` that origin/main's history has no `(#n)` subject for, not from GitHub, so a closed one may be in it. **One conflict, with `fix/link-source-values` (#338, 40762ca) in `scripts/repo_shape.py`, four hunks.** Not resolved in either branch; take both sides: (1) `resolve_link_source`'s signature, `default_source=None, env_shared: bool = False, *, tried: list | None = None`; (2) its docstring, both paragraphs; (3) `link_pins_from_trees`'s docstring, both paragraphs; (4) its call, `default_source, pin_product_key(name) in shared, tried=asked)` followed by #338's `if tried is not None: tried[name] = asked`. The auto-merged `resolve_link_source` body already composes: a shared variable is never an `env` candidate and so never appears in #338's `tried`. **Known limits after the re-verification (APPROVE WITH NITS, 0 blockers, 0 should-fixes, 3 nits, at 6494714).** For the four non-ASCII pairs this change now detects (`ß`/`ss`, `ſ`/`s`, `fi`/`fi`, `ς`/`σ`), the keyed `--pin-source <product>=<path>` form does NOT separate the two pins: #312's source lookup folds both keys into one, so one pin can get a false `unreadable` finding. A sibling checkout does separate them. Only a hand-written `project.yaml` can declare such a pair. The doctor's `pins` row still prints only "passes" for a validator that passes, so it does not carry the collision note; that needs `shape-doctor.py`, and is left for a follow-up. The wording for restoring the finding is under "Fix round 1" above: it is 34dbd86's `report.finding(...)` call, not a word swap. Reviewed adversarially at 34dbd86 (CHANGES NEEDED: 0 blockers, 2 should-fixes, 5 nits). The first should-fix was the finding itself, which locked a project that already declares the pair out of every `update-shape.py apply`; the second was a description that had gone stale. Fix round 1 (6494714) made the pair a NOTE, which is the coordinator's decision and one Brett may reverse (the way back is above), keyed collisions by the pin file's own fold, gave the guard a test that runs on Windows, and made a shared product's skip hint offer the keyed `--pin-source`. Its re-verification at 6494714 found every finding handled: APPROVE WITH NITS (0 blockers, 0 should-fixes, 3 nits: the keyed form and the non-ASCII pairs, the doctor's silence about the note, and the wording for restoring the finding, each stated above). CI is green on Linux, macOS and Windows, with SonarCloud passing, at b70a07d (main a73b4e9 merged in). Its merge with main 5a25387 passed the targeted set (the six pin and scaffold test files, `tests/test_validate_pins_unreadable_workflow.py`, `tests/test_ruleset_hint_tracking_branch.py` and `tests/test_shape_doctor.py`): 850 passed, 3 skipped. Closes #275 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 13, 2026
…409) into fix/adopt-default-branch-and-pr-tail Conflict: adopt-project.py module docstring, the check paragraph; both clauses kept (the tracking branch, then the split branch). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 13, 2026
…the assembly's remote fails a repository git cannot list as a `CommandFailed`, as #382's `_refuse_a_split_branch_left_behind` does, and says itself as `REFUSED adopt-remote-unreadable`, so the two refusals compose and `test_execute_refuses_a_repository_it_cannot_list` and `test_a_remote_that_cannot_be_asked_is_refused_before_any_repository` both hold (#479, #320) `RemoteUnreadable` is a `CommandFailed` whose `loudly`, the one place `main` asks how a failed command is said, returns the `adopt-remote-unreadable` refusal. The `OSError` case keeps the plain `Refusal`: it has no command. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 13, 2026
…it_cannot_list` expects #493's `adopt-remote-unreadable` for a remote git cannot list, with the failed `git ls-remote` as its cause, and adopt-project.py drops `RemoteUnreadable`, so #382's test and #493's lookup no longer need a refusal that is also a `CommandFailed` (#479, #320) The first lookup to list the assembly's remote is now `_refuse_a_tracking_branch_the_remote_lacks`, not `_refuse_a_split_branch_left_behind`. The test asserts a `Refusal` with code `adopt-remote-unreadable`, a detail that names the url, the "no leg repository was created" remediation, and a `CommandFailed` cause whose `args_list` starts `git ls-remote` and holds the url. adopt-project.py is back to its 433a1b4 text. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
pushed a commit
that referenced
this pull request
Oct 13, 2026
…plit-branch clause and this branch's symlink clause both kept in the module docstring's `check` paragraph Merged by sha 48585e6 (17e7449, 518010b, 9736424, 48585e6 past 5a25387). PR #504 conflicted with it, so GitHub could build no merge ref and CI could not run. One conflict, in adopt-project.py's module docstring: both sides end the `check` paragraph's list after the source's submodules' relative urls, this branch with "no symlink kept in the root that the shape would write one of its files ... through where it cannot", #320 (518010b, via #382) with "the split branch one `execute` can make". Both kept, this branch's first, then #320's, then the one closing sentence. How #320 meets this branch's code: `cmd_check` asks `_check_split_branch` before `_entry_findings`, so `check` prints `adopt-branch-exists` and `plan-split-branch-taken` before every entry finding, `plan-root-symlink` included; `cmd_execute` refuses `plan-split-branch-taken` in `_refuse_a_branch_in_the_split_branchs_way`, right after `_refuse_a_split_branch_left_behind`, so after `_refuse_what_check_finds` and this branch's `_refuse_symlinks_checked_out_as_files` (`adopt-symlinks-off`), and before any leg. Every other line of both sides' changes reappears as each side made it (sides_intact.py: only the conflict hunk differs). names.py (base 5a25387): no top-level name bound twice or loaded and bound nowhere in adopt-project.py, scaffold-project.py or scripts/repo_shape.py; scripts/shape_materialize.py binds `YamlError` and `parse_yaml` twice, as main does; problems 0. 9736424 changed `repo_shape.resolve_link_source`'s signature, which this branch does not call. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
… 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>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lane: openreposhape-3 (openRepoShape-3)
Claim: #320 (comment)
What
#445 (75052cb) landed after this PR was written. It refuses in
execute, asadopt-branch-existsand before any leg exists, the split branch an earlier run left, but only where the push cannot fast-forward it. Its tests pin its order: an occupied work directory, then that branch, and a live leg under--local-remote-dirkeepsleg-remote-exists. This PR is now composed with it:checkreports the split branch itself, where the split is pushed, asadopt-branch-exists. That isexecute's own code, and the one_left_by_the_splittells the re-run. It usesexecute's rule, so a branch at the source commit or behind it (whichexecutepushes over and verifies) is no finding. One situation keeps one code in both commands.A branch in git's way of the split branch (
adopt,adopt/three-repo-shape/old) isplan-split-branch-taken:checkreports it for the source, as the extraction's failure for both runs, or as the push's alone when the plan seeds both legs;checkreports it for the assembly repository, as the source'soriginlast fetched it;executerefuses it before any leg exists, in_refuse_a_branch_in_the_split_branchs_way, right after_refuse_a_split_branch_left_behindand skipped like it while a leg repository is still there under--local-remote-dir.Dropped from the reviewed head 33c6dee:
execute's own refusal of the split branch itself, which is adopt-project.py: every failure once a leg repository exists flushes stdout, then printsREFUSED <step>., what failed and one notice of what the run left, so a failedgit filter-repono longer exits 2 with an empty stderr (#171, #188, #177) #445's;adopt-split-target-unreadable, because adopt-project.py: every failure once a leg repository exists flushes stdout, then printsREFUSED <step>., what failed and one notice of what the run left, so a failedgit filter-repono longer exits 2 with an empty stderr (#171, #188, #177) #445'sgit ls-remoteof the same repository always meets an unlistable one first._refuse_what_check_findsis main's again.Why
Measured with git 2.43 and
git filter-repo2.47.0. Every repository is local; a GitHub run is pointed at bare repositories byurl.<base>.insteadOf, with a recordingghstub,GH_TOKEN=bogusand an emptyGH_CONFIG_DIR.adopt, localcheck0plan ok;execute2 atgit filter-repo, both legs madecheck1plan-split-branch-taken;execute2REFUSED plan-split-branch-taken, no legadopt/three-repo-shape/old, localadopt, localcheck0;execute2 at the split push, both legs madecheck1;execute2REFUSED plan-split-branch-taken, no legadopt, GitHubcheck0;execute2 atgit filter-repoafter twogh repo createcheck1;execute2REFUSED plan-split-branch-taken, nogh repo createadopt(source has noorigin)check0;execute2 at the split push after twogh repo createcheck0 (nothing it can read holds it);execute2REFUSED plan-split-branch-taken, nogh repo createcheck0plan ok;execute2REFUSED adopt-branch-exists, no legcheck1adopt-branch-exists;executeunchangedoriginlast fetched itcheck0;execute2REFUSED adopt-branch-existscheck1adopt-branch-exists;executeunchangedcheck0;execute0adoption verifiedcheck0;execute0 verifiedTests
The head is now 5a1ffd5: GitHub's merge of 32abe48 with main 7a8b981 (#397, #394), which leaves this PR's diff against main unchanged (merge-tree of 32abe48 and 7a8b981 gives 5a1ffd5's tree). The coordinator's gate on 5a1ffd5, over every
tests/test_adopt_*.py,tests/test_shape_doctor.py,tests/test_scaffold_leg_path_collisions.pyand the guards: 2669 passed, 5 skipped.On 32abe48,
tests/test_adopt_late_failure.py,tests/test_adopt_split_branch.py,tests/test_adopt_e2e.py,tests/test_agents_md_codes.py,tests/test_git_output_decoding.py,tests/test_repo_hygiene.pyandtests/test_cli_reference.py: 328 passed.tests/test_adopt_late_failure.py(adopt-project.py: every failure once a leg repository exists flushes stdout, then printsREFUSED <step>., what failed and one notice of what the run left, so a failedgit filter-repono longer exits 2 with an empty stderr (#171, #188, #177) #445) andtests/test_adopt_e2e.pyare main's, unedited. The 8 that failed on 973d0fe pass.tests/test_adopt_split_branch.pyhas 37 tests:adopt-work-dir-occupiedandleg-remote-exists, andadopt-branch-existsfirst.check, 3 tests fail;_refuse_an_occupied_work_dir, 3 fail.No test is skipped on any OS beyond the existing
git filter-repoguard.Composes with
REFUSED <step>., what failed and one notice of what the run left, so a failedgit filter-repono longer exits 2 with an empty stderr (#171, #188, #177) #445 (on main): its functions andcmd_execute's order before the new call are byte-identical to main's.executerecords inproject.yamlwhich leg the plan gave each submodule a leg keeps, and shape-doctor.py holds the submodule to that row, so one in the other leg is MISPLACED and a real adoption's placement readsok(#189) #394 changes only_commit_the_split'sproject.yamlrecord, away from every line this PR touches.Left out
.gitmodulesgit's submodule reader cannot read is acheckfinding and anexecuterefusal when the tip holds no submodule but the history of the source's branches and tags does, and so isignore,updateorbranchwith no value beside any submodule, soexecuteno longer dies ingit fast-exportafter both leg repositories exist (#195) #226's history scan names,executestill refuses the scan's finding first, as on main.checknames the branch first, and its remediation says that finding stands until the branch is deleted.git ls-remote(uploadpack.hideRefs,transfer.hideRefs) is not seen, here or by adopt-project.py: every failure once a leg repository exists flushes stdout, then printsREFUSED <step>., what failed and one notice of what the run left, so a failedgit filter-repono longer exits 2 with an empty stderr (#171, #188, #177) #445's refusal; the split push then fails loudly, after the legs (codex's P2 on 5a1ffd5, a known limit).Closes #320
🤖 Generated with Claude Code
Summary by Sourcery
Detect and refuse split-branch conflicts before leg creation while making check accurately report stale and incompatible branches.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: