Repository navigation
tests.yml: the Windows job installs git-filter-repo 2.47.0, so the 23 test files that skip without it run there and an adoption is executed on Windows for the first time, and the comments and docstrings that said that job installs none now say why it does (#471) - #497
Closed
brettheap wants to merge 3 commits into
Closed
brettheap wants to merge 3 commits into
brettheap wants to merge 3 commits into
Conversation
… test files that skip without it run there and an adoption is executed on Windows for the first time, and the comments and docstrings that said that job installs none now say why it does (#471) v1.0.0 says that every AGENTS.md procedure, adopt included, works end to end on Linux, macOS and Windows. `tests-windows` left `git-filter-repo` out on purpose, as a job for the entry point and the materializer alone, so every test that executes an adoption skipped there and the Windows half of that sentence was a claim no runner had checked. - `tests-windows` installs `git-filter-repo` at the ubuntu job's pin, 2.47.0, on ONE line: PowerShell is that runner's default shell, and the ubuntu job's `\` continuation is bash's. pip puts `git-filter-repo.exe` in Python's `Scripts` directory, which setup-python puts on PATH; probe run 38312924968 found it there and `git filter-repo --version` answered. - The suite still runs whole, as before. - The comment that explained leaving it out now says why it is installed; the job header no longer names it as a reason for a second job, and the macOS job's step no longer says "unlike the windows job". - Seven test docstrings said the Windows job installs none, as the reason their filter-repo-free tests run there. They now say a machine that has none runs them, which stays true. `tests/test_shape_doctor.py`'s `no_adoption_offered` said the missing-tool case was "the Windows CI job, by design"; no CI runner lacks the tool now, and its assertion holds either way. The 23 files are the 21 that spell `shutil.which("git-filter-repo")`, `test_adopt_filter_repo_failure.py` (`shutil.which(FILTER_REPO_TOOL)`) and `test_adopt_git_reads.py`, which imports `needs_filter_repo`. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…that keeps a `.gitattributes` of its own is a strict xfail on Windows, because the shape's `eol=lf` rule lands in `shape/` and a Windows checkout writes CRLF into every pinned copy at the root, until the fix that appends that rule to the source's own file lands (#471) Probe run 38312924968 ran the adoption suites on Windows for the first time: 1696 passed, 34 skipped, and this one case failed. The source tracks `* text=auto` at its root, so `execute` writes the shape's `* text=auto eol=lf` beside it at `shape/.gitattributes`, where it governs only `shape/`. The nine copies `contracts/shape-pin.yaml` pins at their own paths are then governed by the source's rule alone, and git's checkout on Windows (`core.eol=native`, or Git for Windows' default `core.autocrlf=true`) writes them with CRLF. `validate-pins.py` digests the bytes on disk (#51), so the first bootstrap reports shape-copy-drift on all nine. Each digest Windows reported is the sha256 of the CRLF form of the pinned LF bytes, and the same nine findings, byte for byte, come back on Linux under `core.eol=crlf` or `core.autocrlf=true`. Brett Heap ruled at 2026-10-12T22:48Z that the split appends a marked block of `/<copy> text eol=lf` lines to the source's own root `.gitattributes`. That fix is a separate pull request on #471. Until it lands this case is a STRICT xfail on Windows only, applied to this parameter alone through `request.applymarker`: the other test of the same fixture parameter passes there, and a mark on the fixture's param would make it a strict XPASS. The fix turns the xfail red, so it removes the mark. Lane: openreposhape-4 (openRepoShape-4) 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 10 minutes by commenting |
Reviewer's GuideThe PR installs git-filter-repo 2.47.0 in the Windows CI job, causing the previously skipped adoption tests to execute there for the first time, and updates related comments and docstrings. It also adds a strict Windows-only expected failure for the known CRLF/shape-copy-drift defect so the follow-up fix will become a CI failure when the xfail is removed or no longer expected. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
… writes its pin file as the scaffold does, so an adopted root passes `validate-pins.py` (#273); a `--pin-source` that cannot answer is `pin-source-unreadable` (#339) (#508) **Brought up to date after #409 squash-landed** by merging origin/main (9736424, #409's squash) as f9ed46f. Its `scaffold-project.py` conflict was resolved to this branch's side once `git diff b70a07d 9736424 -- scaffold-project.py` (#409's final head against its squash) came out empty. So the file is the one tested on this branch plus main's own later changes to it, and holds nothing of #409's; details below. This branch carries #275's adopt half. It was built on `fix/pin-source-env-collisions`, PR #409 (#275's scaffold half and its fix round), merged at 6494714 as 583f647, over the earlier 5690235 (34dbd86). After 6494714, #409 only merged main, five times, up to its final head b70a07d, and none of those merges has a hand edit (`git show --remerge-diff` is empty for each). Adopt's refusal of two products that share a source variable goes through #409's function, so this branch lands after #409. It was never rebased. `git diff b70a07d 9736424` is empty for each file #409 touched: `scaffold-project.py`, `scripts/repo_shape.py`, `templates/assembly-root/scripts/validate-pins.py` and `tests/test_pin_source_env_collisions.py`. What f9ed46f resolves, four files with one hunk each: - **`scaffold-project.py`.** - The conflict hunk is main's block from `PIN_COLLISION_REMEDIATION` through `_pin_collision_remediation`, which #409's squash edits: it adds `PIN_VARIABLE_REMEDIATION`, `_colliding_pin_variables` and `_pin_collision_remediation` and changes `_refuse_colliding_pins`. This branch moved all of that to `scripts/shape_materialize.py`. The hunk takes this branch's side. (The merge commit's message says "re-adds"; this is the exact account.) - #409's auto-merged import of `PIN_SOURCE_ENV_PREFIX, pin_source_env_collisions` is dropped, because nothing in the file uses either name any more. - It is not `git checkout --ours` of the whole file. Main changed the file beyond #409 since this branch last merged it: #355's `--tracking-branch` check (6c2a371) and a73b4e9's `RULESET_HINT`. Those lines stay. - The result is the file `git merge-tree` gives for this branch with main before the squash (518010b). Its diff from the file tested here has the same patch-id as main's own 3ef059c..518010b change to it. - **`templates/assembly-root/scripts/validate-pins.py`.** Main's side, which is #409's final blob, with main's #359 workflow reader in it. This branch had no change of its own to the file beyond #409. - **`adopt-project.py`, `_commit_the_split`.** - Main: #445 wraps the function in its late-failure `try`, and #394 builds an `append` with the plan's submodule record. - This branch: its only edits there were a comment and `neutral_pins=_recorded_pins(plan)` on the materializer call. - Both now sit on main's call inside the `try`: `append=append, neutral_pins=_recorded_pins(plan)`. - **`docs/cli.md`.** It is regenerated with `scripts/render-cli-reference.py`, and `--check` is ok. It differs from main's only in the `--pin`, `--pin-owner` and `--pin-source` help of `adopt-project.py plan`. `scripts/repo_shape.py`, `templates/assembly-root/scripts/validate-pins.py` and `tests/test_pin_source_env_collisions.py` are main's blobs, which are #409's: this branch adds nothing to them. Nothing of #409 is lost. Its scaffold behaviour is `shape_materialize.refuse_colliding_pins` and `pin_variable_collisions`, keyed by #409's `pin_product_key`. **73d65c6, after the merge.** - **The change.** `_read_pins` reads `plan.get("pins")` where it read `plan.data.get("pins")`. - **Why.** #445's `tests/test_adopt_late_failure.py` hands `_commit_the_split` a `SplitPlan` as its plan, a `dict` that adds `entries`. On f9ed46f, three of its tests stopped at "committing the split" with `AttributeError: 'SplitPlan' object has no attribute 'data'` (3 failed, 83 passed in that file). - **Nothing else changes.** `Plan.get` with no default returns `plan.data.get(key)` exactly, so a loaded plan reads as before. `adopt-project.py plan --pin` took a NAME only (`--pin openGlass`) and refused a trailing `@<commit>`. `plan`, `check` and `execute` each exited 0, and `execute` printed `adoption verified`. The manifest then listed the name in `neutral_product_pins:` with no `contracts/<product>-pin.yaml` beside it, and on its first run the adopted root's own `validate-pins.py` refused it as `neutral-pin-missing`. #273's table, reproduced on c7c02fa with the issue's own commands (local bare remotes, and a stub `gh` that exits 97): | `--pin` | c7c02fa | this branch | |---|---|---| | `openGlass` | `execute` exit 0, `- openGlass`; clone's `validate-pins.py` exit 2 `neutral-pin-missing` | `plan` exit 2 `pin-malformed`, with the scaffold's remediation (`--pin openGlass@<40 hex>`); no plan is written | | `opensoft/openGlass` | exit 0, `- opensoft/openGlass`; exit 2 `neutral-pin-missing` for `contracts/opensoft/openglass-pin.yaml` | `plan` exit 2 `pin-malformed` | | `spec` | exit 0; `validate-pins.py` exit 0, reading the leg's `contracts/spec-pin.yaml` as the neutral pin | `spec@<sha>` is `adopt-pin-collides` at `plan` | | `shape` | exit 0; `contracts/shape-pin.yaml` read as the neutral pin | `shape@<sha>` is `adopt-pin-collides` at `plan` | | `openGlass@<sha>`, `opensoft/openGlass@<sha>` | `plan` exit 2 `adopt-pin-malformed` | `plan`, `check` and `execute` exit 0, and `gh` is called 0 times. `contracts/openglass-pin.yaml` is written and the manifest lists `- openGlass`. In the clone, `validate-manifest.py` exits 0, and `validate-pins.py` exits 0 with `openGlass: pin commit … digest recomputes` | Brett ruled for decision 1's first option ("go with your recommendation on #273"): adopt takes the scaffold's pin and writes the scaffold's pin file. That reverses the NAME-only `--pin` from #6, and the comment that described it is replaced with the new contract. - **One implementation.** The scaffold's pin code moved into `scripts/shape_materialize.py`, next to `neutral_pin_path` and the materializer that writes the file. What moved: - `PIN_ARG_RE`, `parse_pin`, `pin_digest_from_source` and `pin_digest_from_gh`; - `declared_pin_values`, `declared_pin_products` and `PIN_COLLISION_REMEDIATION` (in #309's wording); - from #275's branch, `PIN_VARIABLE_REMEDIATION` and the variable check; - `_refuse_colliding_pins`, split into the pure `pin_collisions` and `pin_variable_collisions`, and `refuse_colliding_pins(refusal=...)`, which raises one refusal for both. The code did not move verbatim. `_refuse_colliding_pins` became the three functions above; `pin_digest_from_source` was rewritten for #339 (below); and fix round 1 changed three of them (below): `pin_variable_collisions` keys each pin by #275's `pin_product_key`, `pin_digest_from_source` spells a path that is not printable, and `pin_digest_from_gh` refuses a missing `gh`. The scaffold's behaviour is otherwise unchanged. `scaffold-project.py` imports what it calls under its old names. Lane 5's review ran 34 scaffold cases (every pin refusal, the `gh` stub's four modes, dry and real successes) on 34dbd86 and on this branch at 7a94b6b. rc, stdout, stderr, the `gh` log and the files made were identical, apart from the shape's own commit and digest and #339's row. Its pin tests and #275's pass as they are. `tests/test_git_output_decoding.py`'s exemption for `pin_digest_from_gh` follows the function to its new file. - **`plan`.** `--pin`, `--pin-owner` and `--pin-source` mean what they mean for the scaffold. `plan` reads the digest from `--pin-source` or `gh api`, and records product, repository, commit, digest and `digest_source` in the plan, so `check` and `execute` read no network for a pin. `plan` refuses: - a name with no `@<commit>`, as `pin-malformed`. That includes an empty `--pin ''`, which adopt used to drop and now refuses, as the scaffold always has; - `spec`, `code` and `shape`, in any case, as `adopt-pin-collides` (#273 decision 4), in the scaffold's sentence and remediation (#241, #309); - one product given twice, the same way, whether it is a repeat or two names that share one file (decision 5); - two products that one `SHAPE_PIN_SOURCE_<PRODUCT>` names, such as `open-glass` and `open_glass`, the same way, through #275's check; - a leg path over a pin's file, as `adopt-leg-path-collides`; - a `--pin-source` path that `check` would refuse in `digest_source:` (fix round 1), as `adopt-unwritable-value`. It is asked only when a `--pin` records the path; - with no `gh` on PATH and no `--pin-source`, `gh-not-found` (fix round 1). - **`check` and `execute`.** These are new `check` findings, and `execute` refuses each one before any leg exists. They come from `_leg_findings`, the list `check` and `_refuse_what_check_finds` share: - `plan-pin-unrecorded`: a name alone (an old plan's `pins: [openGlass]`, or a hand edit), or an entry missing its commit or digest; - `plan-pin-malformed`; - `plan-pin-collides`, which covers two pins at one file, a pin at a file the shape writes, and two products that share one source variable; - `plan-root-holds-pin`: a source path kept in the root at the pin's file. The materializer would write the shape's pin beside it under `shape/`, and `validate-pins.py` would read the source's file instead. How each finding ends: - `plan-pin-unrecorded` and `plan-pin-malformed` end with the `plan --pin <product>@<40 hex commit>` that records the pin whole. - `plan-pin-collides` ends with re-running `plan` with each product once, or, for two products that share a source variable, with one product of each such group. - `plan-root-holds-pin` ends with the kept path's own remedy: send that entry to a leg, `drop` it, or rename it in the source and re-run `plan`. A leg moved over a pin's file in the plan is `plan-leg-path-collides`. - **Also part of #386: its `pins:` row.** A hand-edited `pins:` that is not a list (a name such as `pins: openGlass`, a number, a mapping, a quoted string) is now `plan-pin-malformed` in `check` (`` `pins:` is 'openGlass', not a list of pins ``), and `execute` refuses it before any leg exists. - Where: `_read_pins` in `adopt-project.py`; the test row is `plan-pin-malformed-not-a-list` in `tests/test_adopt_pins.py`. - Before: on main, `check` said `plan ok` for the name, the mapping and the string, and gave a traceback for the number (reproduced by lane 5's review on cb7063f). #386 records that `execute` then wrote the name as eight one-letter pins. - Not fixed here: #386's other keys, `allow_empty_legs:`, `paths:`, `legs:` and `source:`, so #386 stays open. Lane 2's #435 takes all of #386, `pins:` included, and its `pins:` rule keeps only text items where this branch writes mappings; whichever lands second reconciles them (#435 (comment), and "Where the meaning meets" below). - **`execute`.** It writes each pin through `materialize_assembly_root(neutral_pins=...)`. The manifest lists the product with the owner stripped (decision 3), so the entry and the file agree. - **Docs.** `--help` is updated and `docs/cli.md` regenerated (`render-cli-reference.py --check` ok). `pin_digest_from_source` caught only `Refusal`. A directory that is no clone is git's own failure, so it was always `pin-source-unreadable`, exit 2. A path `subprocess.run` cannot enter never reaches git: it is a `FileNotFoundError`, `NotADirectoryError` or `PermissionError` out of `cwd=`, and nothing caught it. `scaffold-project.py --pin ... --pin-source <that>` ended `--dry-run` and the real run alike in a traceback, exit 1. Since the move above, `adopt-project.py plan --pin` reads its digest through the same function and ended the same way. Both reach it through `declared_pin_values`, and `check` and `execute` read no source, so that call is the only reader. The issue's table, run on ab623dd for the scaffold and on d806e8f for `adopt-project.py plan` (adopt has no `--pin-source` on ab623dd), then on this head. `gh` is a stub that logs and exits 97, with `GH_TOKEN` bogus and an empty `GH_CONFIG_DIR`; it was called 0 times in every run: | `--pin-source` | before | this branch | |---|---|---| | a directory that is not a clone | `pin-source-unreadable`, exit 2 | unchanged | | a path that does not exist | `FileNotFoundError` traceback, exit 1 | `pin-source-unreadable`, exit 2: `<path> could not answer for opensoft/openGlass @ <40 hex>: no such directory` | | a file | `NotADirectoryError`, exit 1 | exit 2, `... it is a file, not a directory` | | under a parent this user cannot enter | `PermissionError`, exit 1 | exit 2, `... Permission denied` | | a symlink loop (not in the issue) | `RuntimeError` out of `Path.resolve()` on 3.12, exit 1 | exit 2, `... Too many levels of symbolic links` | The real run and `--dry-run` give the same stdout and stderr, and nothing exists afterwards (no remote directory, no work directory; for adopt, no plan file). What changed, in `scripts/shape_materialize.py` only: - `pin_digest_from_source` catches `OSError` as well as `Refusal` and raises the existing `pin-source-unreadable` with its remediation unchanged. The message names the path and the reason. - `stat()` judges the source inside the guard, as #312 does in `validate-pins.py`, and the sentences are that file's: a missing path, or one beneath a file, is "no such directory"; a file is "it is a file, not a directory"; any other `OSError`, from `stat()` or from git's `cwd=`, is the system's own reason (`strerror`). It is not `exists()`/`is_dir()`, which before 3.14 swallow only a missing path. A source `stat()` can read but `cwd=` cannot enter, a directory with no search bit, fails in `tree_digest` and the same guard answers it. - `declared_pin_values` resolves the path through a guarded `_resolved_pin_source`. `Path.resolve()` raises a `RuntimeError` on a symlink loop before 3.13 (an `OSError` since), before the digest was asked for. As typed, `stat()` meets the loop and says so. - A source that answers is read exactly as before: a relative path resolves against the working directory, a symlink to a clone is followed, `digest_source` reads `local-clone (<path as typed>)`, and the digest is `tree_digest`'s. `tests/test_pin_source_unreadable.py` (46 tests in the #339 commit; fix round 1 adds 17 for S1 and moves one, below) runs each case through `scaffold-project.py --dry-run`, its real run and `adopt-project.py plan`: missing, a file, beneath a file, relative and missing, a parent and a source with no search bit (skipped on Windows and as root, by a mark, so before any fixture builds a directory), a 300-character name, a symlink loop (skipped on Windows) and a directory that is no clone. Each asserts exit 2, the refusal naming the path and the reason, its remediation, no traceback and nothing created. It also holds `--dry-run` and the real run to the same stdout and stderr, checks the path is printed escaped under an ASCII stderr and as a surrogate when it is not UTF-8, and checks that a relative source and a symlink to a clone read as before. The session `gh` guard fails any test whose run called `gh`. origin/main ab623dd reached this branch through implQ's branch: 34dbd86 is a30091f merged with ab623dd. One file conflicted there, `templates/assembly-root/scripts/validate-pins.py`, in seven hunks, because #312 (8cdca5e) replaced `keyed_sources` and `default_source` with `PinSources` and #275 added `env_shared` and `shared`. #312's blob is the same at its PR head, at the squash and at ab623dd, so it landed the file implQ resolved against. The resolution keeps #312's side in all seven and adds #275's on top: | # | hunk | resolution | |---|---|---| | 1 | `resolve_neutral_pin_source` signature | #312's `sources: PinSources` and 3-tuple return, plus #275's `env_shared: bool = False` | | 2 | its docstring tail | #312's `Returns (path, how, typed)` text, plus #275's "Rule 2 is skipped when `env_shared`" paragraph | | 3 | `_recompute_neutral_pin_digest` signature | #312's `sources`, plus #275's `shared: frozenset[str]` after `report` | | 4 | the `resolve_neutral_pin_source(...)` call inside it | #312's 3-tuple unpack, plus `env_shared = product.casefold() in shared` and the extra argument | | 5 | `_check_one_neutral_product_pin` signature | #312's `sources`, plus #275's `shared` | | 6 | its call of `_recompute_neutral_pin_digest` | `sources, report, shared` | | 7 | `_check_neutral_product_pins` docstring and body | #312's `PinSources(...)` and `_note_unused_pin_sources`, plus #275's paragraph, `_report_pin_source_collisions(products, report)` and `shared = frozenset(pin_source_env_shared(products))` | This branch's own merge of 34dbd86 (5690235) had no conflict, and neither had its merge of #409's fix round, 6494714 (583f647). That fix round made `neutral-pin-source-collides` a note and keyed "one product" by `repo_shape.pin_product_key`. At 421e4e5, `templates/assembly-root/scripts/validate-pins.py`, `scripts/repo_shape.py` and `tests/test_pin_source_env_collisions.py` were byte-identical to the tree `git merge-tree` gives for 6494714 with origin/main 3ef059c (blobs 5604007, 091966b, 5220787), so this branch added nothing to those three files beyond #409. The 34dbd86 merge brought in #335, #312, #317, #270 (f795cf7), #344 and #349 with no conflict. Earlier merges of origin/main (87373a6, for #309's reworded constant, and e43fa95) are unchanged. Fix round 1 merged origin/main three more times: - **cd1fc79 (2bcfa69).** It brought #245, #351, #350, #345, #372, #371, #354, #380, #377 and the rest since ab623dd, with no conflict. - **45e01f4 (4098b61).** It brought #396's `tests/test_agents_md_codes.py`. That test finds `scaffold-pin-collides` as a string constant of `scripts/shape_materialize.py`, which `SHIPPED` lists. - **3ef059c (421e4e5), #314.** One conflict, one hunk in `adopt-project.py`. This branch's `_refuse_an_unrecordable_pin_source` and #314's section "The project must be able to hold what `execute` writes into it (#261)" were added at the same place; both are kept. Where the two meet, lane 5's earlier recipe applies: - `_flag_plan` builds each pin as the `PIN_FIELDS` mapping `plan` writes, not `list(pins)` of products. So the plan `plan` asks the validator about is the plan it writes. - #314's `test_plan_asks_what_check_asks_of_the_same_keys` plans `--pin openGlass@<commit> --pin-source <stand-in>` where it planned the name alone, which `plan` now refuses. Then f9ed46f, after #409 squash-landed, and 73d65c6 after it (top of this description). On the merged tree exactly one test failed: `tests/test_adopt_template_values.py::test_check_finds_and_execute_refuses_a_hand_edited_value[pins-surrogate]`. It plans, then hand-edits the plan to `pins: ["openX\ud800"]`, a name-only pin with a lone surrogate. Before this branch the refusal's headline was `REFUSED plan-unwritable-value`. A name-only pin in a plan is now `plan-pin-unrecorded` (#273), so `check` has a second, independent finding for that very plan, and the headline is the first finding's: ``` REFUSED plan-pin-unrecorded: 2 finding(s) ... FINDING plan-pin-unrecorded: `pins:` entry 1, 'openX\ud800', is a name with no commit and no digest ... FINDING plan-unwritable-value: a `pins:` item is 'openX\ud800', which `execute` writes into the assembly root's `project.yaml`, and it holds a lone surrogate ... ``` Nothing about #270's contract changed. The lone surrogate is still found as `plan-unwritable-value`, listed under the refusal, and `execute` still refuses before any leg exists. `check` builds `_leg_findings`, where `_pin_findings` sits, before it calls `_value_findings`, so the pin finding comes first. d806e8f changes one assertion for that row, `pins` only: the headline is `REFUSED plan-pin-unrecorded: ` where it was `PLAN_REFUSED`. The other 15 rows and the finding assertions, including the `plan-unwritable-value` line, are untouched. Lane 5's review of 7a94b6b found 3 should-fix and 5 nits. Each is answered here: - **#409's `pin_product_key` (review N1).** The merge of #409's fix round could not carry it, because this branch had moved the function that used it out of `scaffold-project.py`. `pin_variable_collisions` now keys each pin by that function, where it casefolded. `openß` and `openss` are two pin files and one `SHAPE_PIN_SOURCE_OPENSS`, and a casefold named each of the two pins twice. No `--pin` or recorded `pins:` entry `PIN_ARG_RE` admits can spell either, so the test asks the function directly. `refuse_colliding_pins`' docstring now says, in #409's words, that the root's `validate-pins.py` only notes such a pair. - **S1, a `--pin-source` path that is not printable.** `pin_digest_from_source` names the path as typed unless it is not printable (`str.isprintable`), and then as Python's `repr` spells it (`shown_source`). That covers the refusal's head and git's own reason, which names the directory it ran in. Printed raw, an ESC reached the terminal, and a line break or a carriage return began a line of the path's choosing, a forged `REFUSED` line among them, from the scaffold and from adopt. The traceback #339 replaced had shown a `repr`. A printable path is named byte for byte as before. - **S2, no `gh` on PATH.** `pin_digest_from_gh` refuses as `gh-not-found`, exit 2, as `repo_shape.tree_digest_from_gh` names it, and offers `--pin-source`. It was a `FileNotFoundError` traceback, exit 1, from the scaffold and, new with this branch, from `adopt-project.py plan --pin`. - **S3, a hand-edited `commit:` or `tree_sha256:`.** The lane's decision is to accept it and document it (option (c)); no behaviour changed. A pair that disagrees passes `check` and `execute` (`adoption verified`), because neither reads a network for a pin, by design. The adopted root's own `validate-pins.py` then exits 1 with `neutral-pin-digest-mismatch` wherever it can read the product. `check` now says so once, after the pins it will write: "each pin as `plan` read it, not read again here: the root's own `validate-pins.py` recomputes its digest wherever it can read the product". Options (a), `execute` recomputes before it creates the legs, and (b), `check` recomputes from a clone still on the machine, are filed as #449. - **N2, a `--pin-source` that `plan` cannot record.** `plan` asks `check`'s question (`one_printable_line`) of the path it records in each pin's `digest_source:`, before any digest is read. It refuses one that fails as `adopt-unwritable-value`, in #270's words (`unwritable_reason`), or as "an invisible formatting character" where those name nothing. Before, it was planned, exit 0, and printed raw in the report. `check` then refused it as `plan-pin-malformed` and said to re-run the same `plan`. The report's `pin` line and `check`'s `write` line now spell the digest's source with `_shown`. The scaffold half is pre-existing (a line break in the path is written into the pin file; a byte that is not UTF-8 dies after all three repositories exist) and is filed as #450. - **N3, six hunks no test guarded.** Each now has a test that fails when it alone is reverted: - the two name guards (a `pins:` name holding an ESC and a forged line); - the lower-casing of the commit and the digest (an upper-cased hand edit); - the `digest_source` printability check; - the not-text check (`commit: 1234567` unquoted); - the gitlink filter (a submodule kept where the pin is written is `plan-submodule-root` alone). - **N4.** This body. - **N5, #339's third decision.** A `--pin-source` that nothing reads is still silent, now in adopt too. This PR does not decide it. #451, filed, lays out refuse, note and leave, so closing #339 loses nothing. Lane 5's second review, of 421e4e5, found no blocking item, 2 should-fix items and 3 nits. The two should-fix items are this body: the landing after #409 (above) and #386 (above). The code needed no change for them. f63d1ed adds two tests and no code: - **n1. `plan` asks nothing of a `--pin-source` no `--pin` records.** With the `args.pin and` guard dropped, such a path holding an ESC or a zero-width joiner was refused in a sentence naming no file ("`execute` writes into , and ..."), and every test still passed. `test_a_pin_source_no_pin_records_is_not_asked_about` holds the plan to exit 0 and `pins: []`, with the path named nowhere in the plan or the report. The ESC row is POSIX-only by the file's `POSIX_NAMES` mark; the zero-width joiner row runs everywhere. It holds #451's answer today, silence, so a decision there changes this test on purpose. - **n2. A printable `--pin-source` is printed as it is.** With `_spelled` in place of `_shown` in `plan`'s `pin` line, every test still passed. `test_a_printable_pin_source_is_printed_as_it_is` holds `plan`'s `pin` line, and `check`'s `write` line for the plan `plan` wrote, to their exact text, ending `(local-clone (<path>))` with no backtick. - **n3** needs a code change and is left out (below). **Fix round 2, on f63d1ed.** Each run had a `gh` stub first on PATH (it logs and exits 97), `GH_TOKEN` bogus, an empty `GH_CONFIG_DIR`, `git-filter-repo` 2.47.0 next on PATH, and the identity blanked with `GIT_CONFIG_COUNT`; the session `gh` guard came first inside each run. - **The pin set: 812 passed, 2 skipped.** It covers adopt pins, adopt plan, #339's file, #275's tests, the pin and neutral-pin validators, git output decoding, adopt template values, CLI reference, scaffold pin and reuse, scaffold pin collisions, #314's two files and #396's file. The skips are `test_adopt_plan.py:403` (no MedxEHR clone) and `test_adopt_template_values.py:223` (this host reads `git config user.name` from the repository, not from `GIT_CONFIG_GLOBAL`). That is fix round 1's 809 passed, plus the three new rows. - **Shape doctor and scaffold leg-path collisions: 369 passed, 3 skipped** (the PowerShell line exists only on a Windows host). - **Mutation probes**, each one line changed in a throwaway worktree, over `tests/test_adopt_pins.py` and `tests/test_pin_source_unreadable.py` less the five end-to-end adoptions: - the `args.pin and` guard dropped: 2 failed, both n1 rows; - `plan`'s `pin` line `_spelled`: 1 failed, the n2 test; - `check`'s `write` line `_spelled`: 2 failed, the n2 test and `test_an_upper_cased_hand_edit_is_written_lower_cased`; - `plan`'s `pin` line raw: 1 failed, the no-break-space test that already guarded that hunk. **On the merged tree, 73d65c6** (the merge f9ed46f and the `_read_pins` commit), with the same harness: - **The pin set** above, with #445's `test_adopt_late_failure.py`, `test_adopt_git_reads.py` and `test_adopt_filter_repo_failure.py` and #387's `test_adopt_empty_symlinks.py` and `test_adopt_root_manifest.py`: **988 passed, 2 skipped**. The skips are the two above. - **Shape doctor, scaffold leg-path collisions and the adopt suites:** **967 passed, 3 skipped** (the PowerShell line). The adopt suites are e2e, execute refusals, verify, root shape paths (#387's too), leg-path collisions, clone template, ignored shape files and submodules. - **On the merge alone, f9ed46f** (the same tree as before the `_read_pins` commit): `tests/test_adopt_late_failure.py` gave 3 failed, 83 passed, the three `AttributeError` rows above. - `render-cli-reference.py --check` is ok on the merged tree. pyflakes reports the same four warnings on `adopt-project.py`, `scaffold-project.py` and `scripts/shape_materialize.py` as before the merge. **Fix round 1, on 421e4e5.** Each run had `git-filter-repo` 2.47.0 on PATH and the identity blanked with `GIT_CONFIG_COUNT`. A `gh` stub that logs and exits 97 was first on PATH, with `GH_TOKEN` bogus and an empty `GH_CONFIG_DIR`, and the session `gh` guard came first inside each run. - **The pin set:** 809 passed, 2 skipped (the same files and skips as above). - **Doctor and scaffold:** 782 passed, 3 skipped (the PowerShell line exists only on a Windows host). It covers shape doctor, scaffold leg-path collisions, scaffold e2e, referent chain links, naming policy, repo hygiene and #351's free-text values. - **The adopt suites:** 882 passed. It covers e2e, execute refusals, verify, spec-only, leg paths, root shape paths, plan escapes, leg-path collisions, clone template and submodules. - **Before the #314 merge,** on 74ce4d9's tree: the first two sets gave 577 passed, 2 skipped and 782 passed, 3 skipped. On 2bcfa69's tree (the merges alone, before the fix) they gave 542 passed, 2 skipped and 782 passed, 3 skipped. After the 45e01f4 merge, `tests/test_agents_md_codes.py`, repo hygiene and CLI reference: 110 passed. - **Lane 5's second review** also ran 421e4e5 merged with main cb7063f and with 53bee5b: the pin set, the manifest-value set, and the doctor set with #357's `tests/test_doctor_validator_rows.py`. All passed, with only the skips above. Proof that fix round 1's tests test something: - **On 2bcfa69's tree,** with this head's two test files laid over it, 31 of the new rows fail: - S1: 17; - N2 and its report spelling: 7; - S2: 3; - the `pin_product_key` port: 2; - `check`'s S3 line: 1; - the not-UTF-8 `adopt plan` row: 1. The N3 rows pass there, as they must: they guard hunks this branch already had. - **Mutation probes.** Each hunk was reverted alone, in a throwaway worktree, and both test files were run: 18 of 18 were caught. - **The port:** the casefold keying (2 fail); the casefold lookup alone (2). - **S1:** the path named raw (13); git's reason named raw (2); every path `repr`'d, printable or not (27). - **S2:** no `except` (3). - **N2:** no `plan` check (11); only #270's question asked (1); the formatting-character reason said otherwise (1); the report's `pin` line raw (1); `check`'s `write` line raw (1). - **S3:** the line dropped (1). - **N3:** the `_pin_fix` name guard (1); the `_read_pins` path guard (1); the lower-casing (1); the `digest_source` check (1); the not-text check (1); the gitlink filter (1). - **The issue rows reproduced.** S1, S2 and N2 were reproduced on 2bcfa69's tree and shown gone on the fix (a probe driving both tools; `gh` was called 0 times). **Before fix round 1.** Runs were on 7a94b6b, with `git-filter-repo` 2.47.0 first on PATH and the repository-local `user.name` blanked with `GIT_CONFIG_COUNT` (a workstation whose standard checkout sets one fails `test_the_scaffold_refuses_with_no_elector` on main itself; CI sets none). 7a94b6b differs from ffd1297 in `tests/test_pin_source_unreadable.py` only, so the three suites that do not read that file ran on ffd1297: - the new file: 46 passed; - adopt pins, adopt plan, git output decoding: 172 passed, 1 skipped (the MedxEHR clone is absent); - scaffold pin and reuse, pin collisions, e2e, referent chain links, neutral pin and pin validators, #275's tests, naming policy, repo hygiene, CLI reference, scaffold leg-path collisions: 571 passed; - shape doctor: 254 passed, 3 skipped (the PowerShell line exists only on a Windows host); - the new file with repo hygiene, git output decoding and CLI reference, once more after the last edit: 176 passed. Earlier on 5690235 and d806e8f, before the #339 commit: the adopt suites (e2e, execute refusals, verify, spec-only, leg paths, root shape paths, plan escapes, leg-path collisions, clone template) 740 passed, and `tests/test_adopt_template_values.py` with #273's other adopt tests 489 passed, 2 skipped. They were not re-run on 7a94b6b: the #339 diff touches `pin_digest_from_source` and the call in `declared_pin_values`, which those suites reach through `test_adopt_pins.py` only. Nothing was run on macOS or Windows. Proof the new tests test something: - **On d806e8f.** 38 of the 46 fail, each on the traceback and its exit 1. The 8 that pass are the unchanged behaviour: the not-a-clone refusal (4 rows) and the valid-source rows (4). - **Mutation probes.** Ten hunks reverted one at a time, each failing at least one new test: the `OSError` catch (16 fail), the catch narrowed to `OSError` only (21), the missing-path clause (9), the clause narrowed to `FileNotFoundError` (3), the `S_ISDIR` judgement (3), the loop guard (4), no resolve at all (3), the old `.resolve()` call (4), the reason as `str(exc)` (12), and the file sentence as the OS's (3). - #273's own 52 tests still fail on c7c02fa (51 fail or error; the one that passes checks the test's own table), and its 18 mutation probes were each caught. **Re-verified after the merge.** An independent re-verification of f9ed46f and 73d65c6 found the code correct: - #409 is carried whole. A 30,000-case fuzz of `--pin` lists through #409's final refusal and this branch's port differed in 0 cases. - `scaffold-project.py` equals the tested file plus main's own changes that are not #409's (same patch-id). - `_commit_the_split` keeps #445's `try` and #394's `append`. - `_read_pins` reads a loaded plan as before. Its findings were all in this description (the #506 bullet and three stale facts), now fixed. Runs on the head and on its merge with main 48585e6: 988 passed, 2 skipped; and 967 passed, 3 skipped. `git merge-tree --write-tree` was run at 01:48Z on 2026-10-13, comparing hunks per file. Each PR head, fetched from `refs/pull/<n>/head` with no `gh`, was merged with 73d65c6 and with origin/main 9736424 alone. Two sets were asked about: - every PR this description listed before that main does not now contain; - every PR numbered above #468 whose head main does not contain: #473, #474, #490, #491, #493, #494, #495, #496, #497, #503, #504 and #505. **Open state, read with `gh` by the coordinator at 02:00Z and refreshed at 02:25Z after a re-verification on main 48585e6.** Every pull request named below is open. #184, #94, #96, #98, #100 and #161 are closed, and #465 landed as 48585e6. `fix/adopt-plan-quote-yaml-keywords` has no pull request. #506 (`fix/scaffold-reuse-seeded-legs`) opened after this list was made, and the re-verification reads it. - **Landed since the last draft of this list:** #343, #346, #355, #358, #373, #378, #382, #387, #394, #397, #401, #404, #409, #419, #452, #456, #465, #466 and #484. #394's `_commit_the_split` conflict and #401's `docs/cli.md` conflict are both resolved in f9ed46f, above. - **Clean, 20:** #266, #347, #356, #374, #376, #384, #395, #453, #467, #468, #473, #474, #490, #491, #493, #494, #495, #497, #503 and #505. #453, #468 and #491 were updated onto main since the first read and now merge clean. #507 (lane 5's #486-#488) shares only the generated `docs/cli.md` with this branch and merges clean with it. - **Conflicting with main alone, and this branch adds no hunk:** - #402: `shape-doctor.py` 5; - #496: `shape-doctor.py` 10 and `tests/test_shape_doctor.py` 1. **Conflicts this branch adds:** - **#435** (3efcc7a, lane 2's `fix/adopt-plan-field-types`, for #386). - It adds 2 hunks in `adopt-project.py`: 5 here, against 3 with main alone. They are `render_plan`'s `pins:` line and `Plan.pins`. - Keep this branch's `_pin_lines` and `_pin_products`. The rest is under "Where the meaning meets". - **#504** (37ef29a, `fix/adopt-root-kept-symlinks`). - It adds 1 hunk in `adopt-project.py`: 2 here, against 1 with main alone. It is the materializer call in `_commit_the_split`. - Keep both arguments and both comments: `append=append, neutral_pins=_recorded_pins(plan), spell=_spelled`. - **#506** (119d8c4, `fix/scaffold-reuse-seeded-legs`). - It adds 1 hunk in `scaffold-project.py`. With main alone, #506 conflicts only in `docs/handbook.html`. - #506 inserts `LEG_TEMPLATES` right after `declared_pin_products`, which this branch moved out, and rewrites `_reusable_remotes` further down. Keeping both sides breaks the file with an unterminated string literal. - Keep only #506's `LEG_TEMPLATES` block. Drop #506's `declared_pin_values`/`declared_pin_products`, which moved here, and this branch's three-line `_reusable_remotes` opener, which #506 rewrites below. Resolved that way, pyflakes is clean and #506's scaffold tests pass (88). - **`fix/adopt-plan-quote-yaml-keywords`** (ed9968f; a branch with no pull request). - It adds 2 hunks in `adopt-project.py`: 3 here, against 1 with main alone. - One is `render_plan`'s `pins:` line. Keep `_pin_lines`, which already sends product, repository and `digest_source` through `y`. - The other is `_refuse_unholdable_values`' signature. Keep `pins: dict`, with that branch's `_validator_judged` and `_named_as_read` above it. - `fix/adopt-late-failure-notice` is no longer on origin. #445, the late-failure notice, is on main and merged here. **Where the meaning meets:** - **#435 and #386.** The coordinator has said this on #435: #435 (comment) - #435's `_plan_key_findings` (`plan-bad-pins`) keeps only text items in `pins:`, and its `PINS_REMEDIATION` tells a person to write `pins: [openGlass]`. - This branch writes each pin as a mapping, and it makes a name alone `plan-pin-unrecorded`. - Read from #435's code, not run: on a tree with both, the plan `plan --pin` writes is `plan-bad-pins` in `check`. - Whichever lands second reconciles them, with three changes: - `_plan_key_findings` takes this branch's mappings, or leaves `pins:` items to `_read_pins`; - a `pins:` that is not a list gives one finding, not both `plan-bad-pins` and `plan-pin-malformed`; - `PINS_REMEDIATION` says the mapping `plan` writes. - Until that is done, `tests/test_adopt_pins.py::test_a_plan_with_a_whole_pin_checks_ok` fails. - **#314.** It landed, and it is merged here (above). - **`fix/adopt-plan-quote-yaml-keywords`.** Its pin tests are written against the name-only `--pin`. They need the `name@commit` form, and their expected `pins: [...]` line becomes `_pin_lines`'s mapping. - **Two unwritable values are refused in two runs (lane 5's second review, n3).** `plan --elected-by 'A<LF>B' --pin openGlass@<sha> --pin-source <a path holding an ESC>` refuses the elector alone, exit 2. Re-run with the elector retyped, it refuses the source; #270's refusal names every unwritable value at once. Reproduced on 421e4e5, with `gh` called 0 times. One refusal would need a code change, not made here: - `_refuse_an_unrecordable_pin_source` would split into a function that returns the sentence; - `_checked_plan_inputs` would ask it before #270's `unwritable_values` raise, and raise one `adopt-unwritable-value` that names both, with both remediations; - a test would add the two-value row. N2's question is `one_printable_line`, not `unwritable_reason`, so it cannot simply join #270's list. - **Docs.** The README, AGENTS.md and `docs/handbook.html` are unchanged, because none of them calls adopt's `--pin` a name. A sentence saying adopt now takes the scaffold's `--pin` is a follow-up. It costs an AGENTS.md line-cap raise, and a README edit forces the handbook's README blob to change. - **A product spelled `null`, `true` or `false`.** The scaffold already accepts `--pin null@<sha>`: it exits 0 and builds a root whose `validate-pins.py` exits 1 `neutral-pin-entry-malformed`. Adopt's `plan` writes `product: null` through `y`, and `check` then finds `plan-pin-malformed` and refuses it before any leg exists. The one fix for both tools is in `parse_pin`, and it would change the scaffold's behaviour. - **Wording shared with the scaffold.** `digest_source` is written bare into the pin file, so a `--pin-source` path holding ` #` is cut short there. `templates/assembly-root/contracts/neutral-product-pin.yaml` and `validate-pins.py`'s `neutral-pin-missing` remediation name only the scaffold as the writer. Two comments in `scripts/repo_shape.py` still name the scaffold's `pin_digest_from_gh`, in a digest-pinned shipped copy. - **Filed as follow-ups:** - #449: S3, a hand-edited `commit:` or `tree_sha256:` that disagrees, with options (a) and (b); - #450: the scaffold half of N2, a `--pin-source` path no file can hold; - #451: #339's third decision (N5), a `--pin-source` that nothing reads. - **`validate-pins.py` prints a `--pin-source` path raw** in `neutral-pin-source-unreadable`, the twin of S1. It is a shipped template that #409 and other open PRs edit, so it is left for its own change. No issue is filed for it yet. - **Windows and macOS** were not run here; CI ran both (below). Two assertions that depend on what Windows answers for a path (a name past the filesystem's limit, a path beneath a file) check only the refusal and its remediation there. Fix round 1's rows that need a control character or a byte that is not UTF-8 in a path, or a symlink to `git`, are skipped on Windows by a mark, and so is fix round 2's ESC row. The rows for a no-break space and a zero-width joiner run everywhere, and so does the in-process `gh-not-found` test. Reviewed independently by lane 5 at 7a94b6b (3 should-fix, 5 nits: fix round 1) and at 421e4e5 (0 blockers, 2 should-fix, 3 nits: fix round 2). After #409's squash, the re-verification of f9ed46f and 73d65c6 found the code correct; its 1 should-fix and 3 nits were in this description and are fixed. GitHub's update-branch then merged main twice (33bae3f, 2170526), with no hand edit. CI's tests, tests-macos and tests-windows and SonarCloud all passed at 2170526 (main cac6bb2 merged in). On that head merged with main 57c85e9 (#490, #474 and #468, none of which touches this branch's files), the pin and adopt set with `tests/test_update_shape_landing.py` gave 1112 passed, 2 skipped, and the doctor, adopt end-to-end, unfinished-merge and bootstrap set gave 1056 passed, 3 skipped (the doctor's PowerShell line, which exists only on a Windows host). Closes #273 Closes #339 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This was referenced Oct 13, 2026
brettheap
pushed a commit
that referenced
this pull request
Oct 13, 2026
…attributes-lf-block, so the Windows job installs git-filter-repo and runs the adoption suites over #471's fix, with #497's strict Windows xfail removed by hand because the merge does not conflict (#471) #497 is folded into #511 (decision of lane openRepoShape-4's coordinator after #511's review round was verified at 4aba2bd), so all of #471 lands in one pull request and one CI cycle, and #511's own Windows job runs the adoption suites with the fix. The two branches merge CLEANLY, so git keeps #497's `WINDOWS_CRLF_COPIES = pytest.mark.xfail(sys.platform == "win32", strict=True, ...)` and its `request.applymarker` in `tests/test_adopt_ignored_shape_files.py`. Over this branch's fix that case passes on Windows, and a strict xfail that passes is a failure (`XPASS(strict)`). Resolved by hand, as #511's review gave it: - #497's module docstring change (lines 26-27) is kept; - the `#: KNOWN ON WINDOWS, AND STRICT` comment and the `WINDOWS_CRLF_COPIES` constant are deleted; - `test_the_assembly_bootstraps_where_it_failed_with_shape_copy_missing` is back to `(adopted, tmp_path)`, with no `request` and no `applymarker`. The file is this branch's version plus #497's docstring lines; #497 touches nothing else in it. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
(lane openRepoShape-4, 2026-10-13T12:35Z) Superseded by #511. To cover all of #471 in one CI cycle, #511 merged this branch at 6824181 (merge 30922cd). This PR's two reviewed commits, a528006 (the Windows job installs git-filter-repo 2.47.0, and the comments and docstrings are corrected) and a8b5064, are in #511's history and land in its squash. a8b5064's strict Windows xfail is removed in #511 by hand, per the review of #511, because #511 fixes the case it marked. #511's own Windows job is therefore the first run of the adoption suites on Windows over the fix. The branch is kept until #511 lands. |
brettheap
pushed a commit
that referenced
this pull request
Oct 13, 2026
…itory with conftest's `rmtree`, which clears the read-only bit git gives its objects, so the Windows job, which refuses to unlink them, no longer ends 12 adoption-pin tests in an error at setup (#471) #471 folds in #497's `git-filter-repo` install, so the adoption suites ran on a Windows runner for the first time, and the run was green but for 12 errors, all at the module fixture `adopted` of tests/test_adopt_pins.py. The fixture copies a repository with `shutil.copytree` to give the scaffold a pin source, and deletes the copy with `shutil.rmtree` before `check` so the digest is proved to have been read from it. Git writes its loose objects and packs READ-ONLY, and Windows refuses to unlink a read-only file, so the delete raised `PermissionError: [WinError 5]` there and succeeded everywhere else. `tests/conftest.py`'s `rmtree` clears the read-only bit and retries the one call that failed, and is what every other test that deletes a tree holding a `.git` already uses. The fixture now calls it. Test-only: no tool changes. The other `shutil.rmtree` calls under tests/ were each read and none needs the same: test_cli_reference.py:391 and test_setup_sh.py:496 pass `ignore_errors=True` over a scratch directory of generated text and over a checkout the Windows job never runs, test_family.py:1580 removes a submodule's working directory, which holds a `.git` FILE and no objects, and test_update_shape_landing.py:1284 removes a plain directory the test made. Checked against main's f391e58 (#435), which asks the keys of a plan file their types before anything is read: nothing this PR adds reads `source:`, `legs:`, `paths:`, `allow_empty_legs:` or `pins:` of a plan, and its one plan-writing change, the `.gitattributes` follow-up, is a string in the same `follow_ups:` list, which the new checks accept (`plan ok`). A plan holding `source: 5`, `legs: []`, `paths: {}` and `pins: true` is named by key by `check` (exit 1) and refused by `execute` (exit 2) before a work directory is made. Nothing changed for that. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 14, 2026
…epo 2.47.0, so the adoption test files that skip without it run there, and a source that keeps its own root `.gitattributes` gets one marked block of `/<copy> text eol=lf` lines appended to it in the split, so a Windows clone writes the pinned copies at the root with the bytes the pin records; the verification counts that file as the source's bytes followed by exactly that block, and `update-shape.py apply --add` extends it (#471) (#511) v1.0.0 says that every AGENTS.md procedure, adopt included, works end to end on Linux, macOS and Windows. `tests-windows` left `git-filter-repo` out on purpose, so every test that executes an adoption skipped there and the Windows half of that sentence was a claim no runner had checked. `tests-windows` now installs it at the ubuntu job's pin, on one line because PowerShell is that runner's shell, and still runs the whole suite. At #497's base 23 files skipped without it (the 21 that spell `shutil.which("git-filter-repo")`, `test_adopt_filter_repo_failure.py` and `test_adopt_git_reads.py`), 178 of their 1731 items. The workflow's comments and seven test docstrings that named the Windows job as the one without it now say what stays true. The tool adds about 8 to 11 minutes to the Windows job. Its first run there (probe run 38312924968) passed 1696, skipped 34 (exotic names, tracked symlinks, `#!` stubs and test helpers), and failed the one case the rest of this change fixes. Adoption's first run on a Windows runner failed one ordinary case: a source that tracks its own root `.gitattributes`. The shape's `* text=auto eol=lf` went beside it at `shape/.gitattributes`, where it governs only `shape/`, so the nine copies the pin names at their own paths answered to the source's rule, a Windows checkout wrote them with CRLF, and `validate-pins.py`, which digests the bytes on disk (#51), reported shape-copy-drift on each. Under Git for Windows' default `core.autocrlf=true` any root `.gitattributes` but the shape's does it. A root-level file takes attributes from the root file alone, so no file beside it can fix that. Brett Heap ruled at 2026-10-12T22:48Z that the split appends a marked block to the source's own file, that the verification accounts for it, and that `update-shape --add` extends it. The materializer spells the block once: a comment naming openRepoShape and #471, then `/<path> text eol=lf` for each pinned copy outside `shape/`, in the pin's order, appended after the source's bytes when the shape's `.gitattributes` collided, and never through a symlink. `execute` prints it and lists its lines in the split commit's message, and its verification counts the source's file as landed in the root only when the blob there is exactly the source's bytes followed by that block, recomputed from the split's pin; anything else is still `adopt-lost`. `update-shape apply --add` names each added copy in the block, through the ledger, and commits the file with the copies; a root with no block is left alone. The follow-up says what the block protects. A source with no root `.gitattributes` keeps today's behaviour. The two `git-filter-repo` failure tests whose stub is a `#!` script now run on Windows, where git starts the stub with its own `sh`. The one that follows the failure notice compares the leg repositories it reads there with their `spelled` form, because the notice escapes each backslash of a Windows path (#212). The test that a `.gitattributes` symlink is not written through skips on Windows, as the adoption suites' other disk-symlink tests do. The `update-shape --add` fixture's check that no copy list names a stripped file reads list entries, not every string in the materializer. The docs are corrected here (Brett Heap, 2026-10-13T11:30Z), each file keeping its line count. README.md's "Ambiguous is an answer, and no source line is overwritten" (was "nothing is overwritten") says a kept root `.gitattributes` gets the block appended and is verified as the source's bytes plus the block. Its "Why `.gitattributes` is one of the copies" names the block as what carries the rule in such a root. AGENTS.md step 7 states the same exception, and the no-code variant defers to step 7 rather than restating "by blob sha". docs/handbook.html mirrors both paragraphs, names #471 in its footer after main's clauses, the last of them #502's (#504 and #494, which landed since, add none), and recomputes its README.md blob over the merged README.md. Review round 1. A `.gitattributes` git tracks as a symlink gets no block even where Git for Windows' default `core.symlinks=false` checks it out as a plain file: the materializer asks git's index, not only the disk. Before, the block went into the link, and the adoption failed `adopt-lost` after both legs were pushed. The plan reads the link from the source tree. Its follow-up says nothing was appended and git reads no `.gitattributes` that is a link, and it lists the lines a regular file in its place needs. The verification reads the shape's pin through the split's links, as the shape-file check does, so a source keeping `contracts -> meta` in the root verifies. `--add` extends the last block. README.md's `--add` sentence names the block, at an unchanged line count. Merged with main at d41cc29, at 89c6a6c and at 8beb3d9. Each of those merges' one conflict was the handbook footer, which keeps main's clauses whole and adds this one after the last, now #502's. Two of what main brought meet this change, #513 and #512. #513's `adopt-not-the-source` runs in `plan` before the tree is read, so before the block's follow-up is predicted, and its `plan-not-the-source` is `check`'s finding and `execute`'s first refusal, before the split commit appends the block: a source with a root `.gitattributes` and a wrong `--project` is refused with no plan written (probed). #512's `merged-away` takes the `.gitattributes` beside-copy as it takes any other: left in place it is `unchanged`; merged into the root file and removed, `check` calls its row `merged-away` and `apply` drops the row, leaving the block and the root file as they were, and the gates and the doctor go green (probed). The verification of the source's bytes plus the block runs inside `execute` alone, so it never meets that row. A `.gitattributes` collision's follow-up is this change's own and returned before #512's sentence about the row, which every other pinned collision's follow-up carries; the fold below adds it. The rest do not meet it. #402 rewrote the doctor's NEXT lines, in a file this change does not touch. #462 rewrote only the two ruleset hint constants in `scripts/shape_materialize.py`, and the scaffold's call to `materialize_assembly_root` passes no `collision_dir`, so it never reaches the block. #286's `update-rewrite-symlink` and `--add`'s extension of the block never meet either: a `.gitattributes` that is a symlink is left alone before the ledger is asked to write it, so `apply --add` exits 0 with the link and its target untouched (probed over a live and a dangling link). #515 spells the manifest's `neutral_product_pins:` entries, which the block never reaches, and its tests sit in `tests/test_adopt_pins.py` beside this change's fixture fix and merged clean. #507 is `family.py` and the quoting of the commands it prints, #517 is `scaffold-project.py` and #510 is the installed doctor's wrapper: none is a file this change touches beyond the docs. Merged once more with main at b0826e0, the squash of #504 (#183, #178), which meets this change in `adopt-project.py` and `scripts/shape_materialize.py`; the handbook footer did not conflict, because #504 does not touch it. Three hunks conflicted and each keeps both sides: the `from shape_materialize import (...)` list, `materialize_assembly_root`'s docstring, and its tail, where #504's `spell` is passed through and the `.gitattributes` block is appended after it. The two do not contradict each other, asked of real adoptions through `plan`, `check` and `execute`. (a) `materialize-attributes-path` needs no `spell`: only `eol_line` raises it, over the shape's own copy-list names, which `root_key` never resolves through a link, and `spell` is for the names a source's symlinks lead to. (b) Where #504's `check` passes, a root `.gitattributes` that is a symlink adopts as it did, to the blob: the link kept, no block, the shape's copy beside it. Where `check` now refuses it, `plan-root-symlink` before any leg (a dangling link, one to an absolute path with nothing there, a climb above the root, a `.gitattributes` under a link whose target already holds one), this change alone had written the copy through the link or failed after both legs were pushed. (c) The rest of #504's diff does not meet the block: `_refuse_unwritable` runs inside `place`, before the block is appended, and the block never goes through `place` or through a link. This change's four imports from `shape_materialize` sit in a statement of their own, so a change adding names to the shared list merges there without a hand resolution. Merged again with main at 3121945, the squash of #494 (#370), and it was clean: no file conflicted, and the handbook footer was not touched, because #494 changes neither README.md nor docs/handbook.html. It meets this change in `adopt-project.py` alone, in `_failed_loudly`, which this change does not touch; `Refusal` now carries no remediation unless its caller names one, and the one refusal this change adds, `materialize-attributes-path`, names its own, so it prints as it did. - The follow-up for a source's own `.gitattributes` now says what #512 (#502) says of every other pinned copy, after its own condition: merge the rest of `shape/.gitattributes` into the root file only if the whole project should check out with LF, and only then remove it, after which `update-shape.py check` reports its pin row `merged-away` and `apply` drops the row. The link variant says nothing of the row, because it never sends anyone to merge or remove the copy, which stays pinned. A new test follows the flow through a real adoption: merge the copy into the root file, remove it, paste `check`'s NEXT, and the row goes while the root file stays as merged. The first Windows run of the adoption suites ended in 12 errors, all at the module fixture of `tests/test_adopt_pins.py`: it deleted a `shutil.copytree` copy of a repository with `shutil.rmtree`, and Windows refuses to unlink the read-only objects git writes (`PermissionError: [WinError 5]`). It now deletes it with `rmtree` from `tests/conftest.py`, which clears the read-only bit and retries, as the other suites do. This pull request supersedes #497: its two reviewed commits (`a528006`, the install, the comments and seven docstrings; `a8b5064`, a strict Windows xfail on the case fixed here) are merged in, and the xfail is removed, because that case passes on Windows over the fix and a strict xfail that passes fails. Closes #471 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Sonnet 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)
What changes, and why
v1.0.0 says that every AGENTS.md procedure, adopt included, works end to end on Linux, macOS and Windows.
tests-windowsleftgit-filter-repoout on purpose, so every test that executes an adoption skipped there, and the Windows half of that sentence was a claim no runner had checked (#471).tests-windowsinstallsgit-filter-repo==2.47.0, the ubuntu job's pin, on one line: PowerShell is that runner's default shell, and the ubuntu job's\continuation is bash's. pip putsgit-filter-repo.exein Python'sScriptsdirectory, which setup-python puts on PATH; the probe found it there andgit filter-repo --versionanswered.test_adopt_clone_template.py,test_adopt_entry_paths.py,test_adopt_ignored_shape_files.py,test_adopt_mount_ignored.py,test_adopt_submodules.py,test_adopt_verify.py, andtest_shape_doctor.py'sno_adoption_offered). They now say what stays true.The suites that skip without
git-filter-repoare 23 files, not the 8 #471 names: the 21 that spellshutil.which("git-filter-repo"),test_adopt_filter_repo_failure.py(shutil.which(FILTER_REPO_TOOL)), andtest_adopt_git_reads.py, which importsneeds_filter_repo. They hold 1731 test items, 178 of them gated on the tool.The probe, and the one failure
Probe run 38312924968 (throwaway branch
ci/w471-probe-1, Windows only, the 23 files): 1696 passed, 1 failed, 34 skipped.The failure is
test_adopt_ignored_shape_files.py::test_the_assembly_bootstraps_where_it_failed_with_shape_copy_missing[its-own-attributes], an ordinary repository, so a class-1 defect: the source tracks* text=autoin its own root.gitattributes, soexecutewrites the shape's* text=auto eol=lfBESIDE it atshape/.gitattributes, where it governs onlyshape/. The nine copiescontracts/shape-pin.yamlpins at their own paths (Makefile,AGENTS-shape.md, five underscripts/,contracts/repository-naming.yaml,.github/workflows/validate.yml) are then governed by the source's rule alone, a Windows checkout writes them with CRLF, andvalidate-pins.py, which digests the bytes on disk (#51), reportsshape-copy-drifton all nine.core.eol=crlf, or undercore.autocrlf=true, gives the same nine findings with byte-identical digests.core.autocrlf=true, Git for Windows' installer default, ANY root.gitattributesbut the shape's own lets this happen,*.png binaryincluded. So every adopted source that tracks a root.gitattributesis exposed on a default Windows install, not only atext=autoone.Root-level copies such as
Makefiletake attributes from the root.gitattributesalone, so no fix leaves the source's file untouched. Brett Heap ruled at 2026-10-12T22:48Z: the split appends a marked block of/<copy> text eol=lflines to the source's own root.gitattributes, with the verification accounting for it andupdate-shape.py --addextending it. That fix is a separate pull request on #471. Until it lands, this one case is a STRICT xfail on Windows only, applied to that parameter alone throughrequest.applymarker(the other test of the same fixture parameter passes on Windows, and a mark on the fixture's param would make it a strict XPASS). The fix turns it red, so it removes the mark.The 34 skips, by class
entry_paths:357,:841x2b'caf\xe9')git_reads:363,plan_escapes:406,:467,template_values:300submodules:868ignored_shape_files:219,:651,:675,leg_path_collisions:913,mount_ignored:309,:487,:660,relative_submodule_url:515,root_shape_paths:1002core.symlinks, Developer Mode): #457ghorgit-filter-repois a#!scriptfilter_repo_failure:350,:379,github_exits:342x2,387,412,441,460,488,505,template_values:595execute's post-pushghexits and a failed extraction's notice are not executed on Windows; the code is platform-neutral and runs on Linux and macOSgit_reads:162,:181,github_exits:530Timing (probe run 38312924968)
git-filter-repotook 1.4s.test_adopt_relative_submodule_url.py::test_a_relative_url_its_leg_resolves_alike_adopts_and_clones[dotdot-dotdot]12.12s,[relative-origin]10.37s,[dotdot]9.55s;test_each_relative_url_is_resolved_on_its_own8.78s;test_adopt_late_failure.py::test_a_failed_verification_is_flushed_and_its_way_out_runs8.62s;test_adopt_relative_submodule_url.py::test_the_remediation_adopts_and_clones8.60s;test_adopt_submodules.py::test_adopt_moves_existing_gitmodules_with_its_dependency[spec]8.35s;test_adopt_relative_submodule_url.py::test_check_finds_a_relative_url_no_github_leg_resolves_alike[too-many-dotdots]8.05s;test_adopt_submodules.py::test_adopt_moves_existing_gitmodules_with_its_dependency[code]7.96s;test_adopt_git_reads.py::test_elected_by_given_adopts_under_a_user_name_that_is_not_utf87.82s.Tests run (Linux, local)
At the head
a8b5064, on437a360:git-filter-repo2.47.0 on PATH: 1731 passed.tests/test_shape_doctor.pyandtests/test_repo_hygiene.py: 350 passed, 3 skipped (the PowerShell line, which is printed only on a Windows host).tests.yml: clean.request.applymarkerand a true condition, a failing body reports XFAIL and a passing one a strict XPASS failure; with a false condition the mark does nothing.Part of #471
🤖 Generated with Claude Code
Summary by Sourcery
Verify end-to-end adoption behavior on Windows by enabling git-filter-repo in its CI job and documenting the remaining platform-specific limitation.
New Features:
Enhancements:
CI:
Tests: