Repository navigation
The two copies of "is this the same repository" become ONE definition in scripts/repo_shape.py, and #149's case rule lands with the move (#155) - #156
Merged
Conversation
…e spellings name the same repository", and #149's case rule lands in it (#155) The rule was written TWICE: `templates/family-root/scripts/siblings.py` is what `make siblings` REFUSES a working clone by (`WRONG ORIGIN`) and `scripts/shape_materialize.py` is what `shape-doctor.py`'s `members` row REPORTS by (#146, #147). The second copy's own section comment says why it was written and what ends it — `repo_shape.py` is the home a rule shared by every tool belongs in, and the price is that it is digest-pinned, so every estate reads `upstream-changed` for it until it re-pins. That price is owed either way now, and #155 is the change that was told to ride it. This commit only ADDS the section; the callers move onto it next, and the two copies go with them. It is placed beside the other shared vocabulary this module owns, because `checked_value` and `SAFE_PATH_RE` already draw the same drive-letter-versus-scp-colon distinction for a value on a git command line. ONE BEHAVIOUR CHANGES WITH THE MOVE, AND ONLY ONE (#149). `same_repository` lowercased the WHOLE normalised remote. For a remote that names a HOST that is right — GitHub treats an owner and a name case-insensitively, and a report that called `https://GitHub.com/Org/Repo` and `https://github.com/org/repo` two repositories would send somebody to delete a good clone over a capitalisation. For a FILESYSTEM PATH it is wrong: `/srv/mirrors/IRRS.git` and `/srv/mirrors/irrs.git` are two different bare repositories on a case-sensitive host, and the estates that mount members from bare repositories on disk are exactly the ones where a remote is a path. So `_remote_names_a_host` asks that question once, and the case is folded only for a host — `<scheme>://host/…`, git's scp `[user@]host:path`, and the bare `Org/Repo` a `family.yaml` row is written as. Strict on every platform, deliberately: Windows and macOS filesystems are case-INsensitive by default, and a report that changed its answer with the machine it ran on would be worse than one that is strict everywhere. Proven against both copies over a corpus of 38 remote spellings and every one of the 1444 ordered pairs: 12 pairs change answer, every one of them two filesystem paths differing only in case, and `join_remote`, `resolved_remote`, `remote_is_a_path` and `redacted` answer identically for every input. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…out of the ONE definition now, and the parity test that held two copies together is an identity assertion (#155) `templates/family-root/scripts/siblings.py` keeps `resolve_relative` — the half that reads the holder's own `origin`, which is the only part of this that touches a machine — and imports the arithmetic from `repo_shape`, which `family.py init` copies in beside it and `contracts/shape-pin.yaml` carries with it. `shape-doctor.py` takes `same_repository`, `resolved_remote` and `redacted` out of its EXISTING guarded `repo_shape` import and keeps `lane_trailer_argument` in the `shape_materialize` one. `scripts/shape_materialize.py` never called any of them — it held them for the doctor while the pin could not move — so what is left there is a pointer to where they went, for the next reader of #146 or #147, and no code. Every message `make siblings` prints is byte-identical and so is every row `shape-doctor.py` reports: the functions moved unchanged except for #149's case rule, which the previous commit argues. THE PARITY TEST WAS THE RIGHT INSTRUMENT FOR TWO COPIES AND THE WRONG ONE FOR A RULE THAT SHOULD HAVE ONE. It ran 14 url pairs and 10 relative-url pairs through both implementations and asserted they answered the same, which can only ever say the two have not drifted YET — never that either is right. It now asserts the doctor's name, the holder's name and `repo_shape`'s are the SAME OBJECT, and then runs both tables against that one function with the answer each row must get, including the rows #149 asks for: a host folds case through every spelling, a filesystem path does not, and a path still matches itself however it is spelled. `tests/test_windows_paths.py` asks the holder's `join_remote` the Windows questions it used to ask `join_relative` — so a holder that stopped importing the shared rule fails there rather than quietly answering out of a second copy — and asks `repo_shape.remote_is_a_path` the path-versus-scp-url question directly. PINNED FILES: `scripts/repo_shape.py` (previous commit) and `templates/family-root/scripts/siblings.py`. Every project and every holder reads `upstream-changed` for them in `update-shape.py check` until its estate re-pins, which is the price #147 named and deferred and this change was told to ride. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 3 days and 6 hours by commenting @sourcery-ai review. Upgrade to get a review now.
Contributor
Author
|
@codex review |
This was referenced Sep 12, 2026
Reviewer's GuideThe PR consolidates repository identity and remote URL arithmetic into the digest-pinned scripts/repo_shape.py, updates shape-doctor and family siblings to import that single definition, and changes comparison semantics so host remotes fold case while filesystem paths remain case-sensitive. Tests now verify function identity and explicit expected behavior, including Windows and relative-URL handling. Sequence diagram for shared remote comparison and sibling resolutionsequenceDiagram
participant Doctor as shape-doctor.py
participant Siblings as siblings.py
participant RepoShape as repo_shape.py
participant Git as git
Doctor->>RepoShape: resolved_remote(url, base)
RepoShape->>RepoShape: join_remote(base, url)
Doctor->>RepoShape: same_repository(one, two)
RepoShape->>RepoShape: remote_key(one)
RepoShape->>RepoShape: remote_key(two)
Siblings->>Git: git_text(remote get-url origin)
Git-->>Siblings: holder origin
Siblings->>RepoShape: join_remote(base, url)
Siblings->>RepoShape: same_repository(one, two)
Flow diagram for host versus filesystem remote comparisonflowchart TD
Start[Compare two remote spellings] --> Normalize[remote_key normalizes scheme credentials separators and suffix]
Normalize --> HostCheck{_remote_names_a_host?}
HostCheck -->|yes| Fold[Lowercase normalized host remote]
HostCheck -->|no| Preserve[Preserve filesystem path case]
Fold --> Compare[same_repository compares identities and repository tail]
Preserve --> Compare
Compare --> Result[Return repository identity result]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate issues remain with relative-path classification and mirror-origin matching.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Centralizes repository identity and remote handling in scripts/repo_shape.py, with host/path-aware case comparison.
Changes:
- Updates sibling and doctor tooling to use shared helpers.
- Removes duplicated remote-handling logic.
- Expands tests for shared identity and path behavior.
File summaries
| File | Summary |
|---|---|
tests/test_windows_paths.py |
Tests shared Windows path handling. |
tests/test_shape_doctor.py |
Tests shared identity and expected results. |
templates/family-root/scripts/siblings.py |
Uses shared repository helpers. |
shape-doctor.py |
Imports shared comparison and redaction helpers. |
scripts/shape_materialize.py |
Removes obsolete duplicate logic. |
scripts/repo_shape.py |
Adds shared remote handling and identity comparison. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7038429aa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+1793
to
+1795
| if not text or text.startswith("/"): | ||
| return False | ||
| return REMOTE_DRIVE_RE.match(text.split("/", 1)[0]) is None |
There was a problem hiding this comment.
When a holder or sibling uses a relative local remote such as ../mirrors/Repo.git, this predicate classifies it as host-based because it is neither absolute nor drive-qualified, so remote_key lowercases it and treats ../mirrors/repo.git as the same repository on a case-sensitive filesystem. verify_sibling can therefore accept a case-distinct repository—especially a fork carrying the same project id—and then fetch or dispatch park/resume inside the wrong checkout instead of reporting and skipping it. Relative ./ and ../ filesystem spellings need the same case-preserving treatment as absolute paths.
AGENTS.md reference: AGENTS.md:L376-L380
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Lane: openreposhape-2 (openRepoShape-2)
Taken as a P1, in 0cc60d9 — same fix as the Copilot thread on this line, and
your framing is the one that decided it: verify_sibling is about to FETCH
into that directory, or dispatch make park inside it, so a case-distinct
repository accepted there is not a cosmetic wrong answer.
./ and ../ are now in the path branch beside the absolute path, the
file:// url with an empty authority and the Windows drive, so a relative
local remote keeps its case; remote_is_a_path, which decides the separator
question for the same strings, has always called them paths, and the two
halves of the rule now agree about every spelling. The docstring lists the
four path cases and argues the one value that cannot be decided from the
string — a bare Org/Repo stays a forge name, because that is what every
manifest here writes and git requires a ./ of anybody who means otherwise.
Rows: ../IRRS.git != ../irrs.git and ../mirrors/Repo.git !=
../mirrors/repo.git in the identity table, plus remote_is_a_path on both
dot forms in tests/test_windows_paths.py. Whole suite green: 1154 passed,
1 skipped.
…epo` fallback is a forge question, so a mirror clone is no longer called WRONG ORIGIN over a capitalisation (#155) Both halves came out of the review on PR #156 (Copilot twice, Codex the first one as a P1), and both are places where #149's rule as first written either did not reach or reached too far. A RELATIVE PATH IS A PATH. `_remote_names_a_host` called anything that was not absolute and not a Windows drive a host, so `../mirrors/Fam.git` — which is what `git remote get-url origin` answers in a holder cloned from a relative local remote — was lowercased, and `../IRRS.git` and `../irrs.git` were one repository again on a case-sensitive disk. That is the exact false positive #149 is about, so `./` and `../` join the absolute path, the `file://` url with an empty authority and the Windows drive in the path branch, and `remote_is_a_path` — the separator half of the rule — has always called them paths, so the two halves now agree about every spelling. THE BARE TWO-SEGMENT FORM STAYS A FORGE NAME, deliberately and as the one thing here that cannot be decided from the string: `mirrors/Fam.git` might be a directory under wherever the process is standing, but `Org/Repo` is what every manifest in this standard writes, and git requires the `./` of anybody who means a path from somewhere. The docstring says so where the decision is. THE TAIL IS A FORGE QUESTION UNLESS BOTH SIDES ARE PATHS. `same_repository` falls back to the trailing `owner/repo` of the reference, and preserving the case of a path broke that for a normal mirror layout: a clone whose origin is `/srv/mirrors/InkRouter/IRRS.git` was matched against the row's bare `InkRouter/IRRS`, which folds — so the suffix check failed and `make siblings` would have reported the InkRouter estate's own layout as `WRONG ORIGIN`. The tail now compares case-insensitively whenever either side names a host or a forge repository, and strictly when both sides are paths, where the filesystem is the only authority there is. The rows for all three findings are in the table: `../IRRS.git` != `../irrs.git`, `../mirrors/Repo.git` != `../mirrors/repo.git`, `/srv/mirrors/InkRouter/IRRS.git` == `InkRouter/IRRS` (and with the case swapped, and in either argument order, and from a relative mirror), `/srv/mirrors/InkRouter/Other.git` != `InkRouter/IRRS` so the fold cannot make everything match, and `/srv/mirrors/InkRouter/IRRS.git` != `/srv/mirrors/inkrouter/irrs.git` because two paths compare strictly through the tail as well. Re-proven against the copy on `main` over 45 remote spellings and all 2025 ordered pairs: 29 pairs differ, every one of them main=True head=False with TWO FILESYSTEM PATHS on it, and every one of them answers True again when both sides are lowercased — the case fold and nothing else. No pair gained a True, and `join_remote`, `resolved_remote`, `remote_is_a_path` and `redacted` answer identically to main for every input. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Comment on lines
+2013
to
+2015
| if _remote_names_a_host(one) or _remote_names_a_host(two): | ||
| return left.lower().endswith(suffix.lower()) | ||
| return left.endswith(suffix) |
Contributor
Author
There was a problem hiding this comment.
Lane: openreposhape-2 (openRepoShape-2)
Two claims here; one is real and PRE-EXISTING, the other does not reproduce.
Both checked against the copy on main at 5c63116 and against this head.
The path/path tail match is real, and it is older than this PR — filed as
#157 rather than changed here. /srv/a/IRRS.git vs /other/a/IRRS.git
answers True on main and True here; so does
/srv/mirrors/InkRouter/IRRS.git vs /elsewhere/InkRouter/IRRS.git. The
fallback is loose by construction — its docstring says it is there because "a
mirror or an enterprise host spells the same repository under a different
one", which is also why https://github.com/Org/Repo.git and
https://gitlab.example/Org/Repo.git are one repository. #155's law is that
behaviour is byte-identical apart from #149's case rule, and this answer is
the same before and after, so tightening it belongs in its own change: #157,
unclaimed, with the two candidate shapes and the one thing that must not
break — a clone whose origin is spelled under a different PREFIX from the
mount (/private/var/folders/… versus /var/folders/… on macOS is exactly
this, and the tail match is what makes the doctor and make siblings agree
about that directory today).
The reverse mirror row is not false. same_repository("InkRouter/IRRS", "/srv/mirrors/InkRouter/IRRS.git") returns True here, and the table asserts
that row — ("InkRouter/IRRS", "/srv/mirrors/InkRouter/IRRS.git", True) — in
test_the_doctor_and_siblings_run_ONE_definition_of_one_repository, which
passes. The keys are left='inkrouter/irrs',
right='srv/mirrors/InkRouter/IRRS', so the tail taken from the second
argument is 'InkRouter/IRRS', and because the LEFT side names a forge
repository the comparison folds case: 'inkrouter/irrs'.endswith('inkrouter/irrs').
That is the "either direction when at least one side is a host" behaviour you
asked for — it is already what the or in the guard does.
No code change on this round. Suite green at this head: 1154 passed, 1
skipped, and ubuntu, windows and macos all pass in CI.
brettheap
added a commit
that referenced
this pull request
Sep 12, 2026
…ory") into feat/apply-rewrites-readme-shape-line (#148) NO CONFLICT, AND THE TWO BRANCHES MEET IN THREE FILES. #156 moved the remote-url block out of `scripts/shape_materialize.py` into `scripts/repo_shape.py` and left a pointer comment; this branch added the README `Shape:` line's pattern and its two readers to the same file, several hundred lines above. `shape-doctor.py` takes #156's imports and keeps this branch's `readme_shape_note`; `tests/test_shape_doctor.py` takes both sets of tests. Git merged all three by itself, and the whole suite is the check that it merged them correctly. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Sep 12, 2026
brettheap
added a commit
that referenced
this pull request
Sep 13, 2026
…srv/a/IRRS.git and /other/a/IRRS.git stop being one repository, and the symlinked-prefix case moves to the two callers that own a disk (#157) (#160) * same_repository compares two filesystem paths by EXACT equality, so /srv/a/IRRS.git and /other/a/IRRS.git stop being one repository, and the symlinked-prefix case moves to the two callers that own a disk (#157) The trailing `owner/repo` fallback matched ANY two remotes whose last two path components agreed, whatever stood in front of them, so two bare repositories under different roots were one repository. It reached `templates/family-root/scripts/siblings.py::verify_sibling`, which FETCHES into a directory it accepts and dispatches `make park`/`make resume` inside it, and `shape-doctor.py`'s `members` row, which reported it as the member's working clone. Copilot raised it on PR #156 and it was declined there on purpose: #155's law was that behaviour is byte-identical apart from #149's case rule, and this answer was the same before and after it. THE RULING, recorded on the issue before this was written (#157 (comment)): shape 2, exact equality when both sides are paths. * The tail is a FORGE question and only a forge question. In a forge name the two trailing components ARE the whole identity -- `Org/Repo` is an owner and a repository with nothing in front of it to lose -- while in a path everything in front of the tail is part of the identity. So a host against a host, a path against a host, and a path against the bare `Org/Repo` a `family.yaml` row is written as all answer exactly what they answered before; two paths must now be the SAME path. * `/srv/mirrors/InkRouter/IRRS.git` is still matched against the row's bare `InkRouter/IRRS`, case-folded. Dropping that would call every clone in a mirror-based estate `WRONG ORIGIN` -- the InkRouter estate's own layout -- and it is safe for the same reason the path case is not: a forge name has no prefix to throw away. * ONE DIRECTORY REACHED THROUGH A SYMLINK is `/var/folders/...` through one path and `/private/var/folders/...` through the other on macOS, which is how this suite's own fixtures reach one, and the tail match is what made the doctor and `make siblings` agree about it. Only a machine can answer that, and `scripts/repo_shape.py` may not touch a disk -- the promise that lets `tests/test_windows_paths.py` ask the Windows questions from Linux. So the module gains the PURE half, `remote_local_path` (which spelling names a path, and what path it names, None for a host remote and None for a relative one that `realpath` would resolve against wherever the process is standing), and `siblings.py::local_spelling` and `shape-doctor.py::local_spelling` are the three lines of `os.path.realpath` on the side that has the disk. Proven against the copy on `main` at b83ba5c over every ordered pair of 54 remote spellings: 62 of 2916 differ, every one of them `main=True head=False` with TWO FILESYSTEM PATHS on it, none gained a True, and `remote_key`, `remote_is_a_path`, `redacted`, `join_remote` and `resolved_remote` answer identically to `main` for every input. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * The disk half of the rule becomes one `same_repository_here` per caller that asks the STRING rule first, so resolving a symlink can add an answer and never take one away, and `file:///D:/mirrors/Fam.git` folds to the Windows path it names (#157) Three findings from the first review round, all taken. CODEX (P2, `siblings.py`): a mirror is allowed to BE a symlink onto a storage path whose own tail is a serial number -- `/mirrors/InkRouter/IRRS.git` -> `/storage/uuid-123.git` -- and the row's `repository: InkRouter/IRRS` is matched BY the origin's tail. Resolving the origin before that fallback threw the name away and called a good clone `WRONG ORIGIN`. The two callers now hold ONE function, `same_repository_here`, which asks `same_repository` first and the disk only when it has said no; the `repository:` comparison beside it is asked of the origin exactly as git reported it. Resolution can therefore only ADD an answer, which is the property that makes it safe to apply at all. COPILOT (moderate, both callers): on POSIX `os.path.exists("D:/mirrors/x.git")` asks about a directory named `D:` under whatever the process is standing in, so a `realpath` there could resolve an unrelated local path. Only a path that `os.path.isabs` calls absolute ON THIS PLATFORM is inspected now. COPILOT (moderate, `repo_shape.py`): `file:///D:/mirrors/Fam.git` is the file url RFC 8089 blesses and git accepts, and stripping `file://` left `/D:/mirrors/Fam.git` -- a leading `/` that belongs to the url, not to the path. It was the one spelling of a Windows remote that did not fold to `D:\mirrors\Fam.git`'s key. `_remote_body` drops it, so every question below it -- `_remote_names_a_host`, `remote_key`, `remote_local_path` -- answers about the same path, and `same_repository`'s answer for every pair is unchanged (`main` reached the same True through the tail). COPILOT (nit, tests): the regression test called the helper and never the callers. `test_working_clone_and_verify_sibling_ACCEPT_a_symlinked_mirror` drives `shape-doctor.py::working_clone` and `siblings.py::verify_sibling` over both shapes a symlink makes -- a mirror reached through a symlinked PREFIX, which only the disk can accept, and a mirror that IS a symlink onto a storage path, which only the unresolved `repository:` fallback can accept -- and asserts both callers refuse a stranger at the same path. Mutation-checked: with the disk half removed both cases fail, and with the `repository:` fallback asked of a resolved origin the second one fails. Differentially against `main` at b83ba5c over every ordered pair of 59 remote spellings: 66 of 3481 differ, every one `main=True head=False` with TWO FILESYSTEM PATHS on it, none gained a True. `remote_key` differs for exactly the four empty-authority-drive spellings and for nothing else; `remote_is_a_path`, `redacted`, `join_remote` and `resolved_remote` are identical for every input. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Only a `file://` url's own leading slash comes off in front of a Windows drive, so a POSIX directory called `D:` stays the path it is rather than folding into the drive of the same three characters (#157) Copilot, round 2: the normalisation `_remote_body` gained for `file:///D:/mirrors/Fam.git` was applied to EVERY body that began `/<drive>:`, not only to one a scheme had just been taken off. `/D:/mirrors/IRRS.git` written as a path is a directory called `D:` under a POSIX root -- absurd, and somebody's -- and folding it into the Windows drive `D:/mirrors/IRRS.git` would have reported two paths that no single machine could even both hold as one repository. The `/` that comes off is the URL's, so it comes off only when there was a scheme to leave it behind. Two rows say so: `/D:/mirrors/IRRS.git` is neither `D:/mirrors/IRRS.git` nor `file:///D:/mirrors/IRRS.git`. The differential against `main` at b83ba5c over every ordered pair of 59 spellings now reads: 74 of 3481 differ, every one `main=True head=False` with TWO FILESYSTEM PATHS on it, 72 of them a TAIL match `main` made over unequal keys and the other 2 the `/D:/` versus `file:///D:/` pair in both orders, which `main` called equal keys. No pair gained a True. `remote_key` differs for exactly the three `file:///<drive>:/` spellings and for nothing else; `remote_is_a_path`, `redacted`, `join_remote` and `resolved_remote` are identical for every input. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * The caller test builds its file url with the THIRD slash, so Windows actually exercises the `file:///C:/...` fold it claims to, and `_remote_body` asks `startswith` rather than a slice (#157) Copilot, round 3, and it is right about what the assertion was worth on Windows: `"file://" + through_link` builds `file:///tmp/...` on POSIX and `file://C:/...` on Windows -- a url with `C:` for an AUTHORITY, which is not the empty-authority spelling 8a7c042's fold is about. That body reaches `_remote_body` with no leading slash at all, so it took the other road through the same function and the assertion passed on `windows-latest` without exercising the line it exists for. The url is built as `"file:///" + <path with any leading slash stripped>` now, which is `file:///tmp/...` on POSIX and `file:///C:/Users/...` on Windows -- both the empty-authority form -- and the test asserts the three slashes are there before it asks the question, so a future edit to that expression fails here rather than quietly stopping covering anything. And SonarCloud's python:S6659 on the PR (MINOR, `scripts/repo_shape.py`): the scheme guard asks `text.startswith("/")` rather than slicing one character off it. Same answer, one open issue fewer, and the PR-scoped query is clean again. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * The three symlinks these tests build are DIRECTORY symlinks on Windows too, which is the only kind either fixture can be walked through (#157) Copilot, round 4 (two suppressed findings, both real). `os.symlink` defaults `target_is_directory=False`, and on Windows that writes a FILE symlink: a mirror tree reached as `link/remotes/IRRS.git` and a bare repository reached as `mirrors/InkRouter/IRRS.git` are then not reachable at all, so a runner that HAS the privilege would fail these tests rather than ask their question. The argument is ignored on POSIX, where a symlink is a symlink, so this costs the two legs that run today nothing. Today both tests SKIP on `windows-latest` -- creating one needs Developer Mode or an elevated shell, and the skip says so -- which is why this has never shown. It is still what the tests claim, and a claim that is only true on two of three platforms is the thing this repository's suite is not allowed to make. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Oct 11, 2026
brettheap
added a commit
that referenced
this pull request
Oct 11, 2026
…t-project.py writes a recorded submodule's unprintable character as one, so a submodule named with a no-break space, U+3000, a zero-width space, NEL or U+2028 is held to its row and moving it to the other leg is no longer COMPLIANT (#189) P3-1 of lane openRepoShape-2's review of e167d8b. `_quoted` spelled every character `str.isprintable` refuses as `ascii()` does (`\xa0`, ` `) and a byte that is not UTF-8 as `\xe9`, and `repo_shape._unescape` resolved only `\n \t \" \\`. So the row for `vendor/nb\xa0sp` read back with a literal backslash, named no gitlink, and the doctor called the submodule `kept`; a real adoption of each of six such names, moved to the other leg, stayed COMPLIANT (scratchpad/fix189e/logs/probe-names-dbf8887.log). - scripts/repo_shape.py: `_unescape` also resolves `CODE_POINT_ESCAPES`, YAML's `\xNN`, `\uNNNN` and `\UNNNNNNNN`, with YAML's meaning: the code point, never a byte. One with too few digits, or past U+10FFFF, keeps its backslash, and `\\x41` is still a backslash and `x41`. - adopt-project.py: `_quoted` writes only the escapes those two tables resolve: the named ones for a backslash, a quote, a newline and a tab, and the shortest code-point escape for every other unprintable character. A byte that is not UTF-8 is now `\udce9`, which reads back as the surrogate the path holds, where `\xe9` read back as U+00E9, another name. The doctor reads `project.yaml` with the standard's own `repo_shape` (shape-doctor.py imports it from SHAPE_ROOT/scripts), and the project's own validator with its copy, which `execute` writes from the same standard. PINNED FILE: scripts/repo_shape.py. Every assembly root and every family holder reads `upstream-changed scripts/repo_shape.py` in `update-shape.py check` until its estate re-pins with `apply`, as for #156 and #193; this repository has no digest of it to update. `apply` copies every upstream-changed row together and rolls back if the project's validators go red, so the record's validator never runs beside an older reader. HOW THIS MEETS #199 (fix/adopt-plan-linebreak-names, not on main). #199 adds the same reader to repo_shape.py and more: it also resolves `\r`, and it adds the writer `double_quoted` from the same tables. Whichever lands second takes #199's side of repo_shape.py's escape section, deletes `_quoted`, `_escaped` and `_NAMED_ESCAPES` here, and spells both `submodule_record` fields with `double_quoted`. A record this branch writes reads back unchanged under #199's reader, because both spell by YAML's code points; the one difference is a carriage return, `\x0d` here and `\r` there. Tests: the reader's escape table gains the code points and the cases that must keep their backslash (tests/test_windows_paths.py); the record's round-trip names gain nine unprintable ones and a literal `\x41`, and the non-UTF-8 name now reads back as itself (tests/test_adopt_submodules.py); and the doctor holds six such names, written by `submodule_record`, to their rows: `recorded` in code, MISPLACED (6 paths) once staged in spec (tests/test_shape_doctor.py). 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 11, 2026
…y pins as `member-already-present` naming the row that pins it, `bump` and `remove` find a member in any case, and validate-family.py refuses two rows with one `id`, so `--member inkrouter/irrs` beside `InkRouter/IRRS` no longer mounts one project twice past both gates and `bump --member irss` no longer recommends that add (#206) (#221) GitHub resolves `owner/repo` case-insensitively, the rule `repo_shape.same_repository` already folds a forge name by (#149, PR #156), but `Family.member` compared `project:` with `==`. So on main a5b09c3 `family.py add --member inkrouter/irrs` on a family that pins `InkRouter/IRRS` exited 0, mounted the same repository again at `members/irrs` and wrote a third row with the same `id: irrs`; `validate-family.py` said `family ok ... 3 member(s)` and the doctor said COMPLIANT; and `bump --member irss` refused `member-unknown` with the remediation `family.py add --member <org>/irss`, which is that duplicate. - `Family.member(project)` matches `project:` case-insensitively and returns the row, whose own spelling is what every caller reads back. An exact spelling wins, so a family a pre-#206 `add` pinned twice (`IRRS` and `irrs`) can still be mended by naming the row to touch; a spelling that fits two rows is `REFUSED member-ambiguous` naming both, never a guess. - `Family.holding(repository, project)`: `member()` first, then any row whose `repository:` is the same forge repository by `same_repository` (case-folded), which also catches a hand-made row whose `project:` is not its repository's name. - `add` refuses `member-already-present` on `holding()`, still before anything is mounted, and names the row: `irrs is already a member of InkRouter: the row IRRS pins InkRouter/IRRS`, with `family.py bump --member IRRS --to <commit>` in the remediation. Nothing is mounted, staged or written; HEAD does not move. - `bump` and `remove` look the member up through `member()` and then use the row's own `project:` for the path, the rewritten row, the commit title and what they print (`Bump IRSS to ...`, `removed IRSS (InkRouter/IRSS) from members/IRSS`). - `validate-family.py` gains `member-duplicate-id`: two `members:` rows that record one `id:` (the member's own `project.yaml` id) are one project pinned twice, however each row spelled its repository. It reaches the doctor's `family` row, which runs the holder's own validator (`INVALID (family)`, exit 1). - It runs in the full check, in `--pins` and under `--no-members`: it reads `family.yaml` alone, and `--no-members` is the run the template's CI falls back to when a private family's members cannot be checked out, so that run does not pass one project pinned twice either. - A row whose `project:` an earlier row already spells exactly is skipped. That row is `member-duplicate`'s finding, so one fault is one finding, and `member-duplicate-id`'s `remove` would have dropped both rows, because `family.py remove` filters every row spelled that way. - Its remediation runs as printed: from a checkout of openRepoShape, `python3 scripts/family.py remove --family-root <this holder> --member IRRS2` takes the later row, IRRS2, out; name IRRS there instead to keep IRRS2. The holder's path and the row are shell-quoted, and the `--family-root` that `remove` requires is there. Tests: `tests/test_family.py` gains 15 cases; 13 of them fail against main a5b09c3, and the two that pass there are the guards for an `add` of a new member and for an exact duplicate row being one finding. Each change, undone, fails its own test. The fixture that pins one project twice names the second row `IRRS2`, so no test needs two directories whose names differ only in case, and none carries a Windows or macOS skip. Unchanged: an `add` of a member the family does not pin (mounted and recorded in the spelling given, `members/IRQS`, row `IRQS`), the exact-spelling paths of `bump` and `remove`, `member-duplicate` (one `project:` twice), every `--help` text (`render-cli-reference.py --check` ok, so `docs/cli.md` is untouched). No pinned digest or fixture copies validate-family.py: the only copy in the tree is the template, and a holder's `contracts/shape-pin.yaml` row is computed at `init`. An existing holder receives `member-duplicate-id` when `update-shape.py apply` re-copies `scripts/validate-family.py`, which `check` reports as `upstream-changed`. Left out: - `add` refusing a member whose `project.yaml` `id` equals an existing row's (a repository renamed at the forge and added under its new name) belongs after #203, whose `_probe_member` reads `project.yaml` before the mount; on main the id is read only after `git submodule add`, where a refusal would leave the staged mount #203 fixes. `member-duplicate-id` already catches that holder after the fact. - `member-duplicate` (one `project:` written twice) still runs only with the members checked, so `--no-members` passes a `family.yaml` that names one row twice, as on main. - `member-duplicate` still compares `project:` exactly: two hand-made rows `IRRS` (`InkRouter/IRRS`) and `irrs` (`OtherOrg/irrs`) with distinct ids pass it, although their mounts are one directory on macOS and Windows. `family.py add` now refuses to write such a row. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lane: openreposhape-2 (openRepoShape-2)
Claim: #155 (comment)
Closes #155.
Closes #149.
#149's own claim is
#149 (comment) —
taken exactly where that issue's last paragraph asks for it, at the moment the
two copies become one.
What was wrong
ONE rule — "do these two remote spellings name the same repository?" — was
written TWICE, and the second copy was written on purpose as a stopgap three
commits ago.
templates/family-root/scripts/siblings.pyheldSCHEME_RE,SCHEME_ONLY_RE,DRIVE_RE,base_is_a_path,_one_level_up,join_relativeandsame_repository. That is the definitionmake siblingsREFUSES a working clone by (WRONG ORIGIN).scripts/shape_materialize.pyheldREMOTE_SCHEME_RE,REMOTE_SCHEME_ONLY_RE,REMOTE_DRIVE_RE,remote_key,remote_is_a_path,_up_one,join_remote,resolved_remote,CREDENTIAL_RE,redactedandsame_repository. That is the definitionshape-doctor.py'smembersrow REPORTS by (shape-doctor.py: the members row calls a beside-the-holder repository the working clone on its project id alone, where the family's own verifier also requires the origin #146, shape-doctor.py: the members row asks the origin question too, so a fork beside the holder is no longer called the member's working clone (#146) #147).The second copy's own section comment said why it was there and what would end
it:
scripts/repo_shape.pyis where a rule shared by every tool belongs, andit is digest-pinned into every project, so a line added to it leaves every
estate
upstream-changeduntil it re-pins — "a price this defect does notjustify on its own… can ride the next change that has to move the pin anyway".
What held the two copies together was a PARITY TEST that ran 14 url pairs and
10 relative-url pairs through BOTH and asserted they answered the same. That
is the right instrument for two copies and the wrong one for a rule that
should have one: it can only ever say the two have not drifted YET, never that
either is right, and every fix to the rule had to be written twice or it
re-created the disagreement #147 removed. #149 is that fix, and this is it
written once.
What landed
scripts/repo_shape.py(DIGEST-PINNED) gains one section, "Is this theSAME REPOSITORY?", holding the three regexes,
_remote_body,_remote_names_a_host,remote_key,remote_is_a_path,_up_one,join_remote,resolved_remote,CREDENTIAL_RE,redactedandsame_repository. It is the one file both tools may read: the doctorimports nothing out of
templates/(those files are the standard'spayload, and a report that imported them would be reading the copy it is
meant to compare against), and
family.py initcopiesrepo_shape.pyintoevery family holder, where it lands beside
siblings.pyinscripts/andis reached by the same
sys.pathlinebootstrap.pyandvalidate-family.pyalready use.templates/family-root/scripts/siblings.py(DIGEST-PINNED) imports itand keeps only
resolve_relative— the half that reads the holder's ownorigin, the only part of this that touches a machine — andclone_url.Every message
make siblingsprints is byte-identical.shape-doctor.pytakessame_repository,resolved_remoteandredactedout of its EXISTING guardedrepo_shapeimport and keepslane_trailer_argumentin theshape_materializeone. One stale docstringsentence — "the module the parity test holds the two copies of this rule
together in" — says what is true now.
scripts/shape_materialize.pykeeps a pointer to where the block went,for the next reader of shape-doctor.py: the members row calls a beside-the-holder repository the working clone on its project id alone, where the family's own verifier also requires the origin #146 or shape-doctor.py: the members row asks the origin question too, so a fork beside the holder is no longer called the member's working clone (#146) #147, and NO code: it never called any of
these itself, it held them for the doctor while the pin could not move.
The pinned-copy consequence, named as the issue names it. The two pinned
files above are
scripts/repo_shape.pyandtemplates/family-root/scripts/siblings.py. Every project and every holderreads
upstream-changedfor them inupdate-shape.py checkuntil its estatere-pins. That is the price #147 named and deferred; this is the change it was
deferred to.
The one behaviour change (#149)
Everything else moved unchanged.
same_repositorylowercased the WHOLEnormalised remote. For a remote that names a HOST that is right — GitHub
treats an owner and a name case-insensitively, and a report that called
https://GitHub.com/Org/Repoandhttps://github.com/org/repotworepositories would send somebody to delete a good clone over a capitalisation.
For a FILESYSTEM PATH it is wrong:
/srv/mirrors/IRRS.gitand/srv/mirrors/irrs.gitare two different bare repositories on acase-sensitive host, and the estates that mount members from bare repositories
on disk — every fixture in this suite, and any mirror-based family — are
exactly the ones where a remote is a path.
A remote is a PATH when it is one of these four, which
_remote_names_a_hostdecides and argues:/srv/mirrors/Repo.git;file:///srv/mirrors/Repo.git, the same answer by a different road — thatspelling carries an EMPTY authority, so the scheme leaves an absolute path
and no host behind it;
D:\a\remotes\Fam.git, the one colon that is not scpsyntax;
./or../(Copilot and Codex on this PR): aholder whose own
originis../mirrors/Fam.gitis as much on a disk asone whose origin is absolute,
remote_is_a_pathhas always called thosepaths for the separator question, and lowercasing them put the same_repository folds the case of a filesystem path, so two bare repositories on a case-sensitive host that differ only in case are called one repository #149 false
positive straight back.
Everything else names a host:
<scheme>://<host>/…, git's scp[user@]host:path, and the bareOrg/Repoafamily.yamlrepository:rowis written as. The bare two-segment form is read as a FORGE NAME,
deliberately — it is the one value that cannot be decided from the string,
since
mirrors/Fam.gitmight be a directory under wherever the process isstanding;
Org/Repois what every manifest in this standard writes and whatclone_urlturns intohttps://github.com/Org/Repo.git, and git requires the./of anybody who means a path from somewhere.And the trailing
owner/repofallback is a FORGE question unless both sidesare paths (Copilot on this PR). A mirror at
/srv/mirrors/InkRouter/IRRS.gitis matched against the row's bareInkRouter/IRRS— a name somebody typed into a manifest, case-insensitive atthe forge — so comparing that tail case-sensitively would have reported the
InkRouter estate's own layout as
WRONG ORIGINover a capitalisation, whichis the fault #149 is against rather than for. When both sides are paths the
filesystem is the only authority there is and the comparison stays strict.
STRICT ON EVERY PLATFORM, DELIBERATELY, which is what #149 argues: Windows and
macOS filesystems are case-INsensitive by default, so a platform probe would
make a report change its answer with the machine it ran on.
D:\remotes\Fam.gitandd:\remotes\fam.gitare therefore two repositorieshere, drive letter included — the drive is case-insensitive on Windows but the
rest of the string is a path, and one rule that is strict everywhere beats two
answers.
How it was proven
tests/test_shape_doctor.py::test_the_doctor_and_siblings_run_ONE_definition_of_one_repositoryreplaces the parity test. It asserts IDENTITY first —
shape-doctor.py'ssame_repository,siblings.py's, andrepo_shape.same_repositoryare the same object, and the same forresolved_remote,join_remoteandredacted— and then runs both tablesagainst that one function with the ANSWER each row must get, rather than a
comparison between two implementations that could both be wrong.
for both review findings: a host folds case through every spelling
(
https://GitHub.com/Org/Repo==https://github.com/org/repo,git@GitHub.com:Org/Repo.git==ssh://git@github.com/org/repo,Org/Repo==org/repo); a path does not, absolute,file://, Windows orrelative (
/srv/mirrors/IRRS.git!=/srv/mirrors/irrs.git,file:///srv/mirrors/IRRS.git!=file:///srv/mirrors/irrs.git,D:\remotes\Fam.git!=d:\remotes\fam.git,../IRRS.git!=../irrs.git,../mirrors/Repo.git!=../mirrors/repo.git); a mirror isstill matched against the manifest's bare row
(
/srv/mirrors/InkRouter/IRRS.git==InkRouter/IRRS, with the caseswapped, with the arguments reversed, and from a relative mirror), while
/srv/mirrors/InkRouter/Other.git!=InkRouter/IRRSso the fold cannotmake everything match and
/srv/mirrors/InkRouter/IRRS.git!=/srv/mirrors/inkrouter/irrs.gitbecause two paths compare strictly throughthe tail as well; and a path still matches ITSELF however it is spelled.
tests/test_windows_paths.pyasks the HOLDER'sjoin_remotethe Windowsquestions it used to ask
join_relative, so a holder that stopped importingthe shared rule fails there rather than quietly answering out of a second
copy, and asks
repo_shape.remote_is_a_paththe path-versus-scp-urlquestion directly, including both dot forms.
main, over 45 remote spellings and all 2025 orderedpairs of them: 29 pairs differ, every one of them
main=True head=Falsewith TWO FILESYSTEM PATHS on it, and every one of them answers True again
when both sides are lowercased — the case fold and nothing else. No pair
gained a True.
join_remote,resolved_remote,remote_is_a_pathandredactedanswer identically tomainfor every input.passed, 1 skipped, 0 failed (
tests/test_[a-m]*.py373 passed 1 skipped,tests/test_[n-r]*.py349 passed,tests/test_[s-z]*.py432 passed), andgreen on ubuntu, windows and macos in CI at 7038429 before the review round.
No tool's
--helpchanged, sodocs/cli.mdis untouched.The review, and the one thing left open
Round 1 (Copilot twice, Codex once as a P1) found two places where #149's rule
as first written either did not reach or reached too far — a relative remote
classified as a host, and the trailing
owner/repofallback broken for amirror layout. Both are taken in 0cc60d9 and answered on their threads; the
rule they produced is the one written above.
Round 2 (Copilot) raised the tail fallback again, in a form that is older than
this PR: it matches any two remotes whose last two components agree, so
/srv/a/IRRS.gitand/other/a/IRRS.gitare one repository — onmainat5c63116 exactly as here. #155's law is that behaviour is byte-identical apart
from #149's case rule, so it is filed as #157, unclaimed, with the two
candidate shapes and the thing that must not break (a clone whose
originisspelled under a different prefix from the mount:
/private/var/folders/…versus
/var/folders/…on macOS is exactly that, and the tail match is whatmakes the doctor and
make siblingsagree about it today). The same comment'ssecond claim — that
InkRouter/IRRSversus/srv/mirrors/InkRouter/IRRS.gitreturns false — does not reproduce: it returns true, and the table asserts that
row.
🤖 Generated with Claude Code