Repository navigation
scaffold-project.py: a --pin whose file the shape writes itself (spec, code, shape, any case, owner or not) or another --pin writes (openGlass and OpenGlass, or one product twice) is refused before anything is created, under --dry-run alike, so --pin spec@<sha> no longer plans and then makes three repositories to die in the materializer, and a repeated pin is no longer dropped without a word (#241) - #256
Merged
Merged
Conversation
…e moves out of adopt-project.py to sit beside `assembly_root_paths`, so scaffold-project.py can ask the same question from one definition, and adopt's sentences stay byte for byte what they were (#197) `scaffold-project.py --spec-path contracts` creates all three repositories, pushes both legs and only then dies at `git submodule add`, because the shape has already written `contracts/` into the assembly root (#197). The question that catches it is #172's shape half, which #198 wrote inside `adopt-project.py`, where the scaffold cannot import it. A second copy in the scaffold is how the two tools would start refusing different paths, so this commit moves the code and changes no behaviour. What moved, with #198's docstrings kept: `CASE_ASIDE`, `MOVE_THE_LEG`, `_overlap` (now `path_overlap`), `_first_and_more` (now `first_and_more`), and the half of `_shape_collisions` that reads `assembly_root_paths` (now `shape_path_collisions`). The functions are public because two modules import them. `shape_path_collisions` takes the `collision_dir` the caller hands `materialize_assembly_root`, so the two callers differ here by the argument they differ by there. Adopt passes its `COLLISION_DIR`, and its sentence, which names `execute` and `shape/`, is produced unchanged. What stayed in adopt-project.py: - `_shape_collisions`, which now asks the shared half and then its own question about `shape/`. That half reads #198's `SHAPE_DIR_RESERVED`, built from adopt's `COLLISION_DIR` and also said by `LEG_COLLISION_REMEDIATION`. Only an adoption writes copies there, and moving the sentence would have meant a second spelling of the phrase or moving a constant adopt's remediation reads. So it stays, verbatim. - `leg_path_collisions`, unchanged. - The root-kept half (`_kept_collisions`, `_kept_in_root`, `_leg_collision_findings`), since only an adoption reads a source tree. - `LEG_COLLISION_REMEDIATION`, which speaks of `check` and `shape/`. The moved names come in through one separate import statement, under the private names `_kept_collisions` and `_shape_collisions` already call. The statement sits after `import shape_advisory`, away from the materializer import list that #198 and #179 each edit. Checked: for every path `assembly_root_paths()` lists plus 24 other values (the `shape/` variants among them), asked as both legs, plus seven root-kept cases, the remediation and `SHAPE_DIR_RESERVED`, adopt's output is identical before and after (78 lines, `cmp` against #198's head 163173e, whose tree main's squash 20e13f0 holds byte for byte). tests/test_adopt_leg_path_collisions.py is unchanged and passes, with test_adopt_leg_paths.py and test_shape_doctor.py (622 passed). Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the shape writes into the assembly root is refused before anything is created, under `--dry-run` alike, so `--spec-path contracts` no longer creates three repositories and pushes both legs to die at `git submodule add` (#197) `_build_assembly_root` writes the shape into the root FIRST and mounts the legs after it, so a leg path the shape had just written met only `git submodule add`. Measured on #198's head 163173e, whose tree main holds byte for byte since its squash 20e13f0, against a local remote directory: - `--spec-path contracts`, `--code-path project.yaml` and `--code-path Makefile/code` each planned under `--dry-run` with exit 0. - The real run then created all three bare repositories, pushed a commit to each leg, left the assembly root with no commit and exited 2 (`'contracts' already exists and is not a valid git repo`, `destination path ... project.yaml already exists`, `could not create leading directories of ... Makefile/code`). - `--spec-path Contracts` exited 0 with a gitlink beside the shape's `contracts/`, which a macOS or Windows disk keeps as one path. Against GitHub, each left three real repositories for a run that made no project. THE QUESTION IS ADOPT'S (#172), through the one definition the previous commit moved: `shape_path_collisions` over `assembly_root_paths()`, with `path_overlap`'s relation (is, contains, is inside) and `CASE_ASIDE`. A file the shape gains is refused here without anybody editing this tool. The refusal is `scaffold-leg-path-collides`, beside adopt's `adopt-leg-path-collides`. It names each leg's overlap in #198's words, then a remediation that lists the materializer's paths and says nothing was created. `shape_path_collisions` gains `collision_dir=None`, the scaffold's case. The scaffold hands `materialize_assembly_root` no collision directory, so the sentence says what happens here instead of what adopt's `execute` would do: the shape writes the path into the root before the legs are mounted, and Git cannot mount a leg at a path the root already holds, or beyond a file it holds on the way there. Adopt passes `shape`, and its 78 dumped lines are still byte-identical to 163173e's, which are main's. WHY `shape` IS NOT REFUSED HERE. Adopt refuses a mount at or under `shape/` because `execute` writes the shape's copy of a file the source already holds there, and #198 reserves the directory for those copies. The scaffold materializes into a directory it made itself, where a collision raises rather than landing under `shape/`. `update-shape.py` reads `shape/` only for a pin that names a path under it, which only an adoption writes. So nothing the scaffold or its project's tools write lands there, and refusing it would be a rule with no collision behind it. `shape` and `shape/spec` scaffold, and a test holds that. WHERE THE REFUSAL SITS. `_scaffold` calls `_refuse_colliding_leg_paths` straight after the last `checked_value`, before any process can start: - before `_identity`, which may read git's config; - before `_declaration`, which asks `gh api` for a `--pin` with no `--pin-source`; - before `_leg_visibility`, which asks `gh repo view` about a reused root; - before the plan is printed and the `--dry-run` return; - before `mkdtemp` or `--work-dir`, the local remote directory and `_create_remotes`. A dry run therefore refuses with the real run's exit code and its exact stderr. `main()` does nothing but parse arguments before `_scaffold`. tests/test_scaffold_leg_path_collisions.py (89 tests): - Every path `assembly_root_paths()` lists is refused as either leg. So are a directory that holds them (`contracts`, `scripts`, `.github`, `.github/workflows`) and a path beyond a file (`Makefile/code`, `contracts/shape-pin.yaml/spec`). The case-only variants say the aside. - The refused files are exactly what `materialize_assembly_root` writes into an empty directory, and each directory written into is refused too. - Both tools hold `shape_materialize`'s own objects, and a spy shows both call them, adopt with `shape` and the scaffold with none. - `--dry-run` and the real run give the same exit code and the same stderr. - The real run leaves the remote directory's listing unchanged and makes no temporary work directory. - A stub `gh` records no call before the refusal, against a control that reaches it. - A non-colliding pair (`project`, `scripts/code`) still scaffolds. Most cases run `main()` in-process with `subprocess.Popen` refused, so a pass proves no git or gh process started. Out of scope, left: `--pin` also writes `contracts/<product>-pin.yaml`, which `assembly_root_paths()` does not list (`contracts` is refused through the other files under it, a pin's exact path is not). Non-canonical spellings such as `contracts/` and `./contracts` slip past this check until #179 (#201) lands its canonical check before `checked_value`. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-pin.yaml` a `--pin` writes is refused before anything is created, from the one `neutral_pin_path` the materializer writes it at, so `--pin openGlass@<sha> --spec-path contracts/openglass-pin.yaml` no longer plans and then makes three repositories (#197) #197's review of aabfae0 (S1) found the one path the materializer writes that the refusal did not know. `_build_assembly_root` hands `materialize_assembly_root` the declared pins and `_materialize` writes each at `contracts/<product lowercased>-pin.yaml`, but `assembly_root_paths()` described a call with no pin. On aabfae0, and on main a5b09c3, `--pin openGlass@<sha> --pin-source <clone> --spec-path contracts/openglass-pin.yaml` planned under `--dry-run` with exit 0, and the real run created all three repositories, pushed both legs and died at `git submodule add`. `Contracts/OpenGlass-pin.yaml` exited 0 with a gitlink beside the pin, one path on a macOS or Windows disk. ONE DEFINITION. `neutral_pin_path(product)` in scripts/shape_materialize.py is now the tools' only spelling of that path: `_materialize` writes the pin there, the scaffold's plan line and `descendant_note` name it from there, and `assembly_root_paths(neutral_pins=())` lists it between the copies and `contracts/shape-pin.yaml`, where the materializer writes it. `shape_path_collisions` takes the same keyword and asks about that list, and the scaffold passes its `--pin` products. A parameter on the list, rather than the scaffold appending pin paths to it, keeps "every path this call writes, in the order it writes them" in the one function beside the materializer, so the collision set, the remediation's list and the test that holds the list to what was written read one answer. The products come from `declared_pin_products`: `parse_pin` alone, the parser `declared_pin_values` uses, keyed and ordered as its dict is, with no digest and so no process. Adopt hands the materializer no pin and passes none, so its sentences are byte for byte what they were: the review's dump of adopt's collision sentences, re-run on these bytes, has sha256 5eb70cfb684a, the same as aabfae0's, and tests/test_adopt_leg_path_collisions.py is unchanged. The refusal runs where it did: after `checked_value`, before `_identity`, `_declaration`, `_shape_revision`, `_leg_visibility`, the dry-run return, the work directory, the remotes and `git init`. A malformed `--pin` is now refused there, by `parse_pin`'s own `pin-malformed`, rather than in `_declaration`; it was refused before anything was created either way. The remediation lists the pins' files with the rest. With no `--pin`, `contracts/openglass-pin.yaml` lies inside a directory the shape writes into, not a path it writes, and it still plans. Two ordering claims are now tests that run on every OS (review N3). A run without `--elected-by`, with GIT_AUTHOR_NAME unset because CI sets it, refuses with no process started, where its control starts `git config user.name`. With `_declaration` replaced by a recorder, a colliding `--pin` run never calls it, where its control does. Adopt's sentence tail, "`execute` would write it into the mounted leg", is pinned word for word. The `shape_path_collisions` docstring now says adopt passes its `COLLISION_DIR`, `shape` with no slash, and that `plan`, `check` and `execute` all reach it through `leg_path_collisions` (N4). Mutants, each against tests/test_scaffold_leg_path_collisions.py (104 tests): the scaffold passing no pins, 11 failed; `assembly_root_paths` dropping the pins, 10 failed; `neutral_pin_path` not lowercasing, 9 failed; the refusal moved after `_identity`, 1 failed, the new no-elector test alone; moved after `_declaration`, 13 failed, the new recorder test among them. Runs on these bytes: test_scaffold_leg_path_collisions, test_adopt_leg_path_collisions and test_scaffold_pin_and_reuse, 216 passed. On their merge with main a5b09c3, those three plus test_scaffold_e2e, test_scaffold_ignored_shape_files, test_adopt_leg_paths, test_shape_doctor, test_cli_reference, test_commit_trailers, test_adopt_mount_ignored and test_scaffold_leg_paths, 999 passed. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pec`, `code`, `shape`, any case, owner or not) or another `--pin` writes (`openGlass` and `OpenGlass`, or one product twice) is refused before anything is created, under `--dry-run` alike, so `--pin spec@<sha>` no longer plans and then makes three repositories to die in the materializer, and a repeated pin is no longer dropped without a word (#241) `--pin <product>@<40 hex>` is written at `neutral_pin_path(product)`, after the shape's own files and before the copy pin. Nothing asked whether that file was one the shape writes itself or one another `--pin` writes. So on main and on #197's 87fd767, `--pin spec@<sha>` (`Spec@`, `code@`, `<owner>/code@`, `shape@`) and `--pin openGlass@<sha> --pin OpenGlass@<sha>` exited 0 under `--dry-run`, then created all three repositories, pushed both legs and died with `materialize-collision`, the root never committed, and the corrected re-run was `scaffold-remote-exists`. `--pin openGlass@<A> --pin openGlass@<B>` exited 0 having written `<B>` and dropped `<A>`. `_refuse_colliding_pins` asks the materializer's own two definitions: a pin's file is `neutral_pin_path(product)`, and the shape's own files are `assembly_root_paths()` with no pins. `neutral_pin_path` lowercases, so the capital cases need no case rule of the scaffold's. It reads the RAW `--pin` list, never `declared_pin_products`, which collapses a repeat, and refuses a repeat whether or not its commits agree: either way it is a typing error and one pin would be lost. Each argument is named as typed, one sentence per file, under `scaffold-pin-collides` beside `scaffold-leg-path-collides`, with a remediation saying a neutral product is named for the repository it pins. It runs before `_refuse_colliding_leg_paths`, so that refusal never counts a pin path twice (#197's re-review, N1: "and 4 more" where there were three), and like it before `_identity`, `_declaration` and `--dry-run`'s return, so no `git` or `gh` has run. `_refuse_colliding_leg_paths`' docstring now says `assembly_root_paths(neutral_pins)`, which is what it is called with (N3). `--help` is byte-identical, so docs/cli.md is unchanged. Stacked on #234 (87fd767), whose `neutral_pin_path` and `assembly_root_paths(neutral_pins=...)` this reuses. tests/test_scaffold_pin_collisions.py: each refused case in this process with every process start refused, dry and real, and end to end against a local remote directory with a `--pin-source` clone and a `gh` that fails if called, dry and real saying the same words and creating nothing; one pin and two different products still scaffold, each pin at its own file with its own commit; the pin refusal comes before the leg-path refusal, `_identity` and `_declaration`. Mutants, against this file and tests/test_scaffold_leg_path_collisions.py: the check removed, 30 failed; run over `declared_pin_products`, 6 failed (the two repeats); moved after the leg-path refusal, 2 failed (the two ordering cases). Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… `opensoft/openGlass` "by default" since `--pin-owner` changes it, its docstrings say "On main through 39ac59c" now that "as on main" is no longer true, and a pin's file is compared with the shape's files case-folded on both sides, as the leg-path check does (#241) Fix round 1 for #241's review (a25c2e1, MERGEABLE WITH NITS); the merge of origin/main (S1) is the commit beneath this one, and its changed lines are exactly #241's own, `git diff 87fd767 a25c2e1`. N1: `PIN_COLLISION_REMEDIATION` said `--pin openGlass@<40 hex commit>` "names opensoft/openGlass" as a fixed fact. Under `--pin-owner MedxSoft` it names MedxSoft/openGlass, so the sentence was false for that run. It now says "by default", as the neighbouring `pin-malformed` remediation does ("resolved under opensoft/openGlass by default"). The test's `REMEDIATION` constant, which asserts the message word for word, says the same. N2: the `_refuse_colliding_pins` docstring and the test module docstring said "On #197's 87fd767, as on main" (and "On main and on #197's 87fd767"), which stops being true once this lands: main then refuses what they describe. They say "On main through 39ac59c" now, the file's own style (`_refuse_colliding_leg_paths`: "On main through 20e13f0"). The test docstring of `test_a_dry_run_refuses_exactly_as_the_real_run`, "On main the dry run said yes", gets the same words, being the same sentence. N3: `_refuse_colliding_pins` asked `path not in shape_files`, exact, while #234's `path_overlap` compares the leg paths case-folded for the case- folding disk of macOS and Windows. Both sides are `casefold()`ed now, with a comment saying why: a future mixed-case file the shape writes under `contracts/` would otherwise be passed by a pin whose lowercased path collides with it only on that disk, and die in `place()`. The change is EQUIVALENT TODAY and no test is added for it: `neutral_pin_path` lowercases, and every `contracts/*-pin.yaml` in `assembly_root_paths()` (`spec-pin`, `code-pin`, `shape-pin`) is lowercase, so the review's mutant that case-folds this comparison passes every test and no test could tell the two apart. N4 and O1-O3 of the review are follow-up issues, not this change. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 1 day and 2 hours by commenting |
Reviewer's GuideThe PR introduces an early Flow diagram for early pin collision validationflowchart TD
A[_scaffold] --> B[unsafe-value and scaffold-bad-leg-path checks]
B --> C[_refuse_colliding_pins]
C --> D{parse_pin each raw --pin}
D --> E[neutral_pin_path product]
E --> F{Path overlaps assembly_root_paths or another pin?}
F -- Yes --> G[Raise scaffold-pin-collides]
G --> H[No git, gh, or files created]
F -- No --> I[_refuse_colliding_leg_paths]
I --> J[_identity and _declaration]
J --> K{--dry-run?}
K -- Yes --> L[Return without materializing]
K -- No --> M[Create and materialize project]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This was referenced Oct 11, 2026
…16ae) and this branch both add a sentence to scaffold-project.py's WHAT IT DOES, resolved as their union; the two refusals' code merged without conflict (#241) Main took 23e16ae (#271, issue #251: a `--referent-chain` link between the first and the last that is not a neutral product's name is refused as `chain-link-malformed`) after this pull request was green, and GitHub then reported it DIRTY. The one conflict was the module docstring's WHAT IT DOES item 1, where each side added a sentence after "just as early, for the same reason (#197),". Resolved as the union, both refusals named in one sentence in landing order: a `--pin` whose file the shape writes itself or another `--pin` writes (#241), and a `--referent-chain` link between the first and the last that is not a neutral product's name (#251). Every other hunk of scaffold-project.py auto-merged, and docs/cli.md is main's own: this branch changed no help text, and `render-cli-reference.py --check` says it is what the generator produces. THE TWO REFUSALS' ORDER in the merged `_scaffold`, which no conflict marker showed: `_refuse_colliding_pins` (#241) first, then `_refuse_colliding_leg_paths`, `_identity`, then `_refuse_malformed_chain_links` (#251), `_declaration` and `_check_declared_names`, with `--dry-run` returning after all of them. Both branches' tests keep holding: #256's pin refusal still comes before the leg path check, `_identity` and `_declaration`, and #251's still comes before `_declaration` reads a pin's tree. Neither branch's tests pin an order between the two refusals. BOTH AT ONCE, probed once on the merged tree with local remotes and a `gh` that exits 97 (never called): `--pin spec@<sha> --pin openXdox@<sha> --referent-chain 'openXdox,a #b,openDox'` is ONE refusal, `scaffold-pin-collides`, exit 2, nothing created, under `--dry-run` alike and whichever order the arguments are typed in. The `chain-link-malformed` refusal is not shown until the colliding pin is fixed; the same chain with only a clean pin is refused as `chain-link-malformed` on its own. The result is deterministic, and the operator meets the two mistakes one re-run apart. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
This was referenced Oct 12, 2026
brettheap
added a commit
that referenced
this pull request
Oct 12, 2026
…opt-record-submodule-placement Brings in c2ff2b0 (#215) and c3a9645 (#241) on top of d94d155. No rebase. c2ff2b0 changes adopt-project.py, as this branch does, which is why main is merged again so soon after d94d155. c3a9645 changes scaffold-project.py alone. No conflicts. Checked: - adopt-project.py: #215 makes a `.gitmodules` value that git's submodule reader refuses (`update = bogus`, `shallow = maybe`, ...) #195's `plan-gitmodules-unreadable` finding, which `execute` refuses before any leg exists. It changes `_registrations`, `Source` and `_where_git_fails`, and adds `_refused_value`, `_who_refuses_a_boolean` and their constants. This branch's `submodule_record`, `_resolution_text` and the record's preamble are unchanged by the merge, and so are `_answered_legs`, `_kept_in_a_leg`, `_commit_the_split`, `cmd_execute`, `_refuse_what_check_finds`, `_entry_findings`, `_submodule_findings`, `submodule_plan_problems`, `Plan`, `y`, `emit` and `walk`. So, as for #195, a source #215 refuses gets no leg, no split and no record, and the record is still spelled by `double_quoted`. - No top-level name is bound twice in adopt-project.py, scaffold-project.py, scripts/repo_shape.py, scripts/shape_materialize.py, shape-doctor.py, validate-manifest.py or update-shape.py. Each merged module's names are the base's plus what either side added. No definition was changed by both sides. - The branch's diff against main is still #189 alone: 12 files, +1670 -157, file by file the same as against d94d155. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 12, 2026
…, #205, #215, #241 and #249/#250 merge with no conflict, and `_entry_findings` still hands `_leg_findings` the tree and ends with the root-symlink findings origin/main moved 60588d5 -> b30595c while the targeted runs of 8f624aa went: 018ce0d (#207 via #222), 67a398a (#272 via #274), d94d155 (#205 via #232), c2ff2b0 (#215 via #240), c3a9645 (#241 via #256) and b30595c (#249 and #250 via #284). It is merged again because c2ff2b0 changes adopt-project.py and b30595c changes scripts/shape_materialize.py's `render`. Conflicts: none. `git merge-tree --write-tree` gave tree be093a5, and the merge stages the same tree. Checked on that tree: - `_entry_findings` still calls `_leg_findings(plan, tree)` and ends with `_root_symlink_findings(plan, source, tree)`. - No top-level name is bound twice in adopt-project.py, scaffold-project.py, scripts/repo_shape.py or scripts/shape_materialize.py, none is loaded but bound nowhere (AST), and main changed the signature of no function this branch calls. - Both sides are intact by +/- line sets: each side's change against the merge base 60588d5 reappears, line for line, against the other side. - c2ff2b0 adds helpers to tests/conftest.py (`planned`, `GITLINK`, `findings` and others) and no fixture. tests/test_adopt_root_symlinks.py imports its conftest names one by one and none of these, so its own `planned` and `GITLINK` are unaffected. - b30595c's `render` spells a double-quoted YAML value with `double_quoted`; this branch's walker reads `assembly_root_paths()` and never renders. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 12, 2026
Main brings three commits since d94d155: - c2ff2b0 (#215 via #240): adopt-project.py refuses a `.gitmodules` value that git's submodule reader dies on. It adds helpers to tests/conftest.py, adds tests/test_adopt_gitmodules_values.py, and moves helpers out of tests/test_adopt_history_gitlinks.py; - c3a9645 (#241 via #256): scaffold-project.py's `--pin` collisions; - b30595c (#249, #250 via #284): repo_shape.py's quoted scalars, and shape_materialize.py's `render()` spells a value between a YAML template's double quotes. One conflict, in tests/conftest.py. Both sides append after `resolve()`: main its `.gitmodules` helpers, this branch `spelled()`. Both are kept, main's block first. Against main, the merged file differs by exactly this branch's own diff. Against this branch, it differs by main's own diff, but for where git places two added blank lines. adopt-project.py merged without a conflict, and each top-level definition comes whole from one side. c2ff2b0 changes `_registrations`, `_where_git_fails` and `Source.registered_submodules`, and adds `_refused_value`, `_who_refuses_a_boolean` and their constants. This branch changes none of them, and its own `Source.__init__` and `Source.open` are unchanged. The new refusal comes through `_entry_findings`. `cmd_execute` calls that in `_refuse_what_check_finds`, before `_refuse_an_occupied_work_dir` and `_create_leg_remotes`, so the refusal is raised before any leg exists and no notice is due. Main's `refused_before_any_leg` asserts that the remotes directory is never made. No top-level name is bound twice in adopt-project.py, scaffold-project.py, scripts/repo_shape.py, scripts/shape_materialize.py or tests/conftest.py. None of them reads a global it does not bind. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 12, 2026
…#241, #249/#250, #204, #203 and #238 merge with no conflict, and a `.gitmodules` value git refuses is answered before #190's relative-url check, which reads no url from a file git cannot read Main since d94d155, the main GitHub's update-branch merged at 8ce5614: c2ff2b0 (#215 via #240), c3a9645 (#241 via #256), b30595c (#249/#250 via #284), be2790e (#204 via #227), 6d8d88b (#203 via #236) and ede15ce (#238 via #287). No conflict hunk. Three files were changed on both sides; `git merge` auto-merged each, and every hunk of each side lies apart from the other's. Each side's +/- lines are identical before and after the merge in all three (`git diff -U0` d94d155..8ce5614 against ede15ce..merge, and d94d155..ede15ce against 8ce5614..merge). Line numbers are the merged file's. adopt-project.py - main, c2ff2b0 (#215): 384-506, `SUBMODULE_TYPED_KEY_RE` to `_refused_value` and `_who_refuses_a_boolean`; 510-515 and 522-524, `_registrations`' docstring and its return of `_refused_value`'s words; 791-797, `Source.registered_submodules`' docstring; 2181-2185, `_where_git_fails`' docstring. - #190: 36-38 and 51-53, the module docstring; 102-103, the `repo_shape` import list; 334-339, `Registrations.url`; 2252-2501, `RELATIVE_URL` to `_relative_url_findings`; 2503-2548, `_submodule_findings` and `_entry_findings` with `local_remote_dir` and the NOTEs; 2608 and 2630, `cmd_check`; 3070-3092, `_repository_urls` and `_remote_urls`; 3111-3216, `_refuse_what_check_finds` and `_as_resolved_here`; 3443, `cmd_execute`. scripts/repo_shape.py - main, b30595c (#249/#250): 173-535, the YAML reader's quote boundaries, from the comment after `YamlError` to `_split_flow`. - #190: 1975-1985, `REMOTE_PORT_RE` and `REMOTE_HOST_ALIASES`; 2099-2116, `remote_key`'s docstring and body. tests/test_shape_doctor.py - #190: 2662-2693, `test_a_port_and_githubs_ssh_host_are_not_part_of_a_remotes_key`. - main, ede15ce (#238): 5209-5808, the validator-PYTHONPATH tests after `test_a_drifted_roots_next_command_runs_when_it_is_pasted`. Every other file is one side's blob: main's for the 17 main changed, and 8ce5614's for tests/test_adopt_relative_submodule_url.py and tests/test_adopt_submodules.py. The merge's diff against main is #190's five files, with the stat it had against d94d155 (1097+, 41-). No function or class is bound twice at the top level of adopt-project.py, scaffold-project.py, scripts/repo_shape.py, scripts/shape_materialize.py, shape-doctor.py, scripts/family.py or the three test files. The helpers c2ff2b0 moved into tests/conftest.py (`planned`, `findings`, ...) are plain functions, and tests/test_adopt_relative_submodule_url.py imports seven named helpers from conftest and defines its own `planned`. HOW #215 AND #190 MEET. `_submodule_findings` lists `submodule_plan_problems` first, which holds #215's `plan-gitmodules-unreadable`, then `_relative_url_findings`. Where git's submodule reader refuses a value of the source's `.gitmodules`, `registered_submodules` answers with git's words, a string, and `_kept_relative_urls` reads no url from a string. So a source with both problems gets #215's finding alone, and no NOTE either. #190's finding is not lost: once the value is fixed, `check` reports it, and `execute` refuses it through the same list. #190 adds lines only after #215's, so it can hide nothing of #215's. A relative url kept in a leg is always beside a gitlink kept in a leg, where `_where_git_fails` names a failure, so the unreadable finding is always printed whenever #190's would be skipped. Measured on this tree with a source holding `upstream/dependency`, `url = ../dependency` and `update = bogus`, with `.gitmodules` and `upstream/` kept in code for testorg: - `origin` in otherorg: `check` 1 with `plan-gitmodules-unreadable` alone; `execute --local-remote-dir` 2, `REFUSED plan-gitmodules-unreadable`, with no remotes directory made. With `update = checkout`: `check` 1, `plan-submodule-relative-url`. - `origin` in testorg: the same `check` 1 and `execute` 2, with no NOTE. With `update = checkout`: `check` 0 and the NOTE. - At 8ce5614, before #215, the first source was `plan-submodule-relative-url`. The second was `check` 0 with the NOTE, over a file `git fast-export` dies on. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lane: openreposhape-4 (openRepoShape-4)
Claim: #241 (comment)
What
scaffold-project.pyrefuses, asscaffold-pin-collides, a--pinwhose pin file the shape writes into the assembly root itself (a product that lowercases tospec,codeorshape, owner-qualified or not) and two--pins whose files are one path (openGlassandOpenGlass, or one product twice, whether or not the commits agree). Nothing has been created when it refuses,--dry-runrefuses the same words, and nogitorghhas run.Why
--pin <product>@<40 hex>is written atneutral_pin_path(product),contracts/<product lowercased>-pin.yaml, after the shape's own files and before the copy pin. Nothing asked whether that file was one the shape writes itself or one another--pinwrites. On main through 39ac59c,--pin spec@<sha>(Spec@,code@,<owner>/code@,shape@) and--pin openGlass@<sha> --pin OpenGlass@<sha>exited 0 under--dry-run, then created all three repositories, pushed both legs and died withmaterialize-collision, the root never committed, and the corrected re-run wasscaffold-remote-exists.--pin openGlass@<A> --pin openGlass@<B>exited 0 having written<B>and dropped<A>without a word.The refusal and its ordering
_refuse_colliding_pinsasks the materializer's own two definitions: a pin's file isneutral_pin_path(product), and the shape's own files areassembly_root_paths()with no pins.neutral_pin_pathlowercases, so no case rule of the scaffold's own is added. The product is read byparse_pin, so a malformed--pinis stillpin-malformed.--pinlist, neverdeclared_pin_products, which collapses a repeat. Each argument is named as it was typed, one sentence per file, with one remediation (nothing was created, a neutral product is named for the repository it pins, each product once)._scaffold:unsafe-valueandscaffold-bad-leg-path(scaffold-project.py accepts a non-canonical leg path such as--spec-path spec/, writes a straysubmodule.spec/.urlbeside a registration that points at the work directory, and the new repository's recursive clone fails once the work directory is gone: #169's bug on the scaffold side #179) as before, then THIS refusal, then_refuse_colliding_leg_paths, then_identity,_declarationand the--dry-runreturn. It runs before the leg-path refusal so that refusal never counts a pin path twice (scaffold-project.py accepts a leg path that collides with a path the shape writes (--spec-path contracts,--code-path project.yaml), creates all three repositories and pushes both legs, then dies: #172's shape half on the scaffold side #197's re-review, N1: "and 4 more" where there were three).--helpis byte-identical, sodocs/cli.mdis unchanged.Tests
tests/test_scaffold_pin_collisions.py(new): each refused case in this process with every process start refused, dry and real, and end to end against a local remote directory with a--pin-sourceclone and aghthat fails if called, dry and real saying the same words and creating nothing; one pin and two different products still scaffold, each pin at its own file with its own commit; the pin refusal comes before the leg-path refusal,_identityand_declaration. The tests feed git no stdin and write only ASCII names, and the case pairs are refused in this process before anything is written, so they hold on macOS and Windows.Mutants against that file and
tests/test_scaffold_leg_path_collisions.py: the check removed, 30 failed; run overdeclared_pin_products, 6 failed; moved after the leg-path refusal, 2 failed; the reviewer added nine more (pin path spelled by hand, no copy pin in the shape files, an identical repeat accepted, only the first collision reported, a malformed pin skipped, moved after_identity,_declarationand the--dry-runreturn, the owner kept in the product): each caught. The case-folded comparison is an equivalent mutant today and is not tested (see below).Over the merged head 2ab4dae,
tests/test_scaffold_pin_collisions.py,test_scaffold_pin_and_reuse.py,test_scaffold_leg_path_collisions.py,test_scaffold_mount_ignored.py,test_scaffold_e2e.py,test_adopt_leg_path_collisions.pyandtest_cli_reference.py: 351 passed.Review and the fix round
Adversarial review of a25c2e1 (lane openRepoShape-4): MERGEABLE WITH NITS, no MUST. Each rule in the issue held, dry and real runs were byte-identical, and nothing was left on disk after any refusal.
--pin'scontracts/<product>-pin.yamlincluded, is refused before anything is created, under--dry-runalike, through the one definition adopt-project.py asks, so--spec-path contractsno longer creates three repositories and pushes both legs to die atgit submodule add(#197) #234 was squash-merged (b538015), so the stacked head conflicted with main inscaffold-project.py(5 hunks, each scaffold-project.py: a leg path that is, holds or lies inside a path the shape writes into the assembly root, a--pin'scontracts/<product>-pin.yamlincluded, is refused before anything is created, under--dry-runalike, through the one definition adopt-project.py asks, so--spec-path contractsno longer creates three repositories and pushes both legs to die atgit submodule add(#197) #234's squashed lines against the same lines plus scaffold-project.py--pin spec@<sha>(orcode@,shape@, or two--pins whose products differ only by case) passes--dry-run, then creates all three repositories, pushes both legs and dies withREFUSED materialize-collision, because the pin'scontracts/<product>-pin.yamlis a file the shape already writes #241's edit). Resolved as a plain merge of origin/main with-X ours(this branch's side in the conflicting hunks only; main's other changes, scaffold-project.py: the leg mount passes--forcetogit submodule add, so a leg path an ignore rule matches (__pycache__,.DS_Store, or the defaultspecunder an operator's own excludes) scaffolds instead of dying at the mount after all three repositories exist and both legs are pushed (#229) #235's--forcemount among them, are kept).git diff origin/main HEADhas the same changed lines asgit diff 87fd767 a25c2e1.--pin openGlass@<sha>names opensoft/openGlass; under--pin-ownerthat is false. It says "by default", aspin-malformeddoes.--spec-path contracts,--code-path project.yaml), creates all three repositories and pushes both legs, then dies: #172's shape half on the scaffold side #197's 87fd767, as on main" (and the test module docstring) now say "On main through 39ac59c".--pin'scontracts/<product>-pin.yamlincluded, is refused before anything is created, under--dry-runalike, through the one definition adopt-project.py asks, so--spec-path contractsno longer creates three repositories and pushes both legs to die atgit submodule add(#197) #234's leg-path check does. Equivalent today (everycontracts/*-pin.yamlthe shape writes is lowercase), so no test was added.--spec-path contracts,--code-path project.yaml), creates all three repositories and pushes both legs, then dies: #172's shape half on the scaffold side #197's and this one lack a doc line), O1 (two tests intest_scaffold_pin_and_reuse.pyreach a realgh apiwhenghis on PATH), O2 (adopt's--pin specand repeated--pin), O3 (pin_source_env_namemaps products differing only in.,-,_to one variable).Closes #241
🤖 Generated with Claude Code
Summary by Sourcery
Refuse colliding or repeated pins before scaffolding begins, while retaining valid pin behavior and providing consistent diagnostics across dry-run and real execution.
Bug Fixes:
Enhancements:
Tests: