[brettheap]

Lane: openreposhape-3 (openRepoShape-3)
Claim: #206 (comment)

What

GitHub resolves owner/repo case-insensitively. repo_shape.same_repository already folds a forge name by that rule (#149, PR #156), but Family.member compared project: with ==. This branch makes scripts/family.py and the holder's validate-family.py treat a case variant as the same member:

• Family.member(project) matches project: without regard to case and returns the row. Callers read the row's own spelling back, never the one they were given. An exact spelling wins, so a family that a pre-family.py add compares --member with the existing rows case-sensitively, so --member inkrouter/irrs on a family that already pins InkRouter/IRRS mounts the same repository again as members/irrs, both gates pass a family that pins one project twice, and bump --member irss's refusal recommends exactly that add #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) tries member() first, then any row whose repository: is the same forge repository by same_repository (case-folded). The second step also catches a hand-made row whose project: is not its repository's name.
• add refuses member-already-present when holding() finds a row. The check still runs before anything is mounted, and the refusal names the row: irrs is already a member of InkRouter: the row IRRS pins InkRouter/IRRS. Its remediation is family.py bump --member IRRS --to <commit>. Nothing is mounted, staged or written, and HEAD does not move.
• bump and remove look the member up through member(). They then use the row's own project: for the path, the rewritten row, the commit title and their output (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 through the family row, which runs the holder's own validator (INVALID (family), exit 1). Since fix round 1 (9725bd2):
  • It runs in all three modes: the full check, --pins and --no-members. It reads family.yaml alone. --no-members is the run the template's CI falls back to when a private family's members cannot be checked out.
  • 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. member-duplicate-id's remove would also have dropped both rows, because remove filters every row spelled that way.
  • Its remediation runs as printed. It now reads: From a checkout of openRepoShape, python3 scripts/family.py remove --family-root <this holder's path> --member IRRS2 takes the later row, IRRS2, out; name IRRS there instead to keep IRRS2. The holder's real path is printed, and it and the row are shell-quoted. Before, it read remove --member <Project>, which lacked the --family-root that remove requires.

Unchanged:

• An add of a member the family does not pin. It is mounted and recorded in the spelling given (members/IRQS, row IRQS).
• The exact-spelling paths of bump and remove.
• member-duplicate, which flags one project: twice.
• Every --help text. render-cli-reference.py --check says [ok] docs/cli.md is what the generator produces, so docs/cli.md is untouched.

Existing holders receive member-duplicate-id when update-shape.py apply re-copies scripts/validate-family.py. check reports that file as upstream-changed.

Why

The issue's repro, run on this machine on main a5b09c3 and on this head. Setup: a family holder made by family.py init, members InkRouter/IRRS and InkRouter/IRSS added, everything in local bare remotes, and remotes/irrs.git a symlink to remotes/IRRS.git as a local stand-in for GitHub's case-insensitive name. The head column was measured on cefb6dd, which has the tree of 6c917d6, and re-measured on 9725bd2 after fix round 1. The two runs print the same lines except for the commit shas of the fresh repositories and the member-duplicate-id remediation. On 9725bd2 that remediation reads: From a checkout of openRepoShape, python3 scripts/family.py remove --family-root <holder> --member irrs takes the later row, irrs, out; name IRRS there instead to keep irrs.

step	main a5b09c3	this head
family.py add --member inkrouter/irrs	exit 0, committed ...: .gitmodules, members/irrs, family.yaml; a third row project: irrs id: irrs repository: inkrouter/irrs	exit 2, REFUSED member-already-present: irrs is already a member of InkRouter: the row IRRS pins InkRouter/IRRS; HEAD not moved, git status --porcelain empty, members/ = IRRS IRSS
validate-family.py (the holder's own copy)	family ok: InkRouter (inkrouter), 3 member(s), exit 0	family ok: InkRouter (inkrouter), 2 member(s), exit 0
shape-doctor.py --root <holder>	COMPLIANT, exit 0	COMPLIANT, exit 0
family.py bump --member irss --to <IRSS tip>	exit 2, REFUSED member-unknown: irss is not a member of InkRouter, remediation family.py add --member <org>/irss: the duplicate	exit 0, IRSS <old> -> <new>, committed members/IRSS, family.yaml

The same holder after a pre-#206 add has pinned IRRS twice (rows IRRS and irrs, made with main's family.py):

step	main a5b09c3	this head
validate-family.py	family ok ... 3 member(s), exit 0	FINDING member-duplicate-id: irrs and IRRS both record id 'irrs': one project is pinned twice. ..., exit 1
shape-doctor.py --root <holder>	COMPLIANT, exit 0	INVALID (family), exit 1
family.py bump --member Irrs --to <IRRS tip>	exit 2, REFUSED member-unknown: Irrs is not a member of InkRouter	exit 2, REFUSED member-ambiguous: Irrs fits 2 rows of InkRouter: IRRS, irrs
family.py remove --member irrs, then validate-family.py	removed irrs (inkrouter/irrs) from members/irrs; family ok ... 2 member(s), exit 0	the same lines; family ok ... 2 member(s), exit 0

Tests

tests/test_family.py gains 15 test cases: 12 functions, one of them parametrized over four spellings (inkrouter/irrs, INKROUTER/IRRS, InkRouter/irrs, OtherOrg/Irrs). Run against main a5b09c3 (this head's tests/test_family.py in a scratch worktree at a5b09c3), the result is 13 failed, 2 passed, 76 deselected. Two pass on main by design:

• test_add_of_a_member_that_is_not_there_is_unchanged_by_the_case_rule guards that the case rule leaves a new member alone.
• test_an_exact_duplicate_row_is_one_finding guards that the new check does not double-report member-duplicate.

No existing test changed.

Fix round 1 added the --no-members, runs-as-printed and exact-duplicate tests. Each fails with its change undone:

• moving the call back after the --no-members return fails the --no-members test;
• dropping the exact-spelling skip fails the exact-duplicate test;
• dropping --family-root from the remediation fails the runs-as-printed test.

Against 6c917d6's validator, 4 of the 10 pinned_twice, duplicate, runs_as_printed and no_members tests fail: the three new ones and the updated test_validate_finds_one_project_pinned_twice.

Targeted set on head 9725bd2, with git-filter-repo 2.47.0 on PATH:

/usr/bin/python3 -m pytest tests/test_family.py tests/test_manifest_validator.py tests/test_shape_doctor.py \
  tests/test_repo_hygiene.py tests/test_cli_reference.py tests/test_update_shape.py tests/test_update_shape_add.py -q
452 passed in 403.65s (0:06:43)

The earlier head 6c917d6 ran the same set: 449 passed in 831.28s (0:13:51).

tests/test_update_shape.py and tests/test_update_shape_add.py are in the set because they run a holder's validate-family.py. The Windows job skips none of the new tests (tests/test_family.py carries no WINDOWS_SKIP). The fixture that pins one project twice names the second row IRRS2, not irrs, so it never needs two directories whose names differ only in case, which macOS and Windows hold as one. The add tests that use a case-variant spelling are refused before anything is mounted.

Composes with

Trial merges (git merge-tree --write-tree <sibling> HEAD) at this head (9725bd2), measured at 19:30Z:

sibling	sha	result
origin/fix/family-add-check-before-mount (#203, lane 4)	d25dd4d	CONFLICT scripts/family.py
fix/family-bump-nothing-to-do (#210, PR #219)	origin 66ace9b (local c7e8f5d)	CLEAN
fix/doctor-reads-family-yaml (#211, PR #220)	origin 7fe2c56 (local 6091e6e)	CLEAN
fix/decode-git-output-utf8 (#217, PR #230)	b6f7cfd	CLEAN
origin/main	54aa69a (#173, adopt-project.py only)	CLEAN

The #203 conflict is one raise in two hunks: cmd_add's member-already-present check.

• family.py add mounts the member before it checks --at or the member's project.yaml, so a refused add (a leg, a short --at) leaves a staged gitlink, a .gitmodules registration and a cached object store, and the next add of a different member commits the refused leg's registration while validate-family.py and the doctor say ok #203 moves that check out of cmd_add into its new _checked_add_target(family, project, path, at), which asks family.member(project).
• This branch rewrites the check in place to held = family.holding(repository, project). Its message names the row (: the row {held.get('project')} pins {held.get('repository')}) and its remediation bumps {held.get('project')}.

The first PR to land wins, and the other branch merges by hand:

• If family.py add mounts the member before it checks --at or the member's project.yaml, so a refused add (a leg, a short --at) leaves a staged gitlink, a .gitmodules registration and a cached object store, and the next add of a different member commits the refused leg's registration while validate-family.py and the doctor say ok #203 lands first, this branch moves its check into _checked_add_target. That means passing repository in (it is already in hand in cmd_add from _checked_member), asking holding(repository, project) there, and keeping the row-naming message and remediation.
• If this lands first, family.py add mounts the member before it checks --at or the member's project.yaml, so a refused add (a leg, a short --at) leaves a staged gitlink, a .gitmodules registration and a cached object store, and the next add of a different member commits the refused leg's registration while validate-family.py and the doctor say ok #203's _checked_add_target keeps holding(repository, project) and the held row's message rather than member(project).

Either way the check stays before the mount, which both branches require.

Warning: part of this branch's message merges outside the conflict markers. The line f"the row {held.get('project')} pins {held.get('repository')}", and the remediation's opening line "Remediation: `family.py bump --member " sit BETWEEN the two hunks, in the merged body of #203's _checked_add_target. Resolving by taking #203's side of both hunks gives a file that compiles. But held is then never bound, and every member-already-present refusal dies with NameError: name 'held' is not defined. Measured at d25dd4d: py_compile passes, and pyflakes says undefined name 'held' on that line. Whoever merges second must keep the held = family.holding(repository, project) binding or rewrite that line. Taking either side wholesale is not a resolution.

#210 adds a nothing to do exit to cmd_bump that prints project. On this branch that variable is the row's own spelling by the time #210's lines run, so the two compose as written.

#211 adds gitlinks_under and family_beside_project to shape-doctor.py. This branch adds no helper there, so neither branch duplicates the other.

Left out

• add should refuse a member whose project.yaml id equals an existing row's id (a repository renamed at the forge and added under its new name). That check belongs after family.py add mounts the member before it checks --at or the member's project.yaml, so a refused add (a leg, a short --at) leaves a staged gitlink, a .gitmodules registration and a cached object store, and the next add of a different member commits the refused leg's registration while validate-family.py and the doctor say ok #203: its _probe_member reads project.yaml before the mount, while on main the id is read only after git submodule add, where a refusal would leave exactly the staged mount family.py add mounts the member before it checks --at or the member's project.yaml, so a refused add (a leg, a short --at) leaves a staged gitlink, a .gitmodules registration and a cached object store, and the next add of a different member commits the refused leg's registration while validate-family.py and the doctor say ok #203 fixes. Until then, this branch's member-duplicate-id catches such a holder after the fact.
• validate-family.py's 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. Unlike member-duplicate-id, it was not moved ahead of the --no-members return.
• validate-family.py's member-duplicate still compares project: exactly. Two rows IRRS (InkRouter/IRRS) and irrs (OtherOrg/irrs) with distinct ids would pass it, although members/IRRS and members/irrs are one directory on macOS and Windows. family.py add now refuses to write such a row; only a hand edit can make one.

Closes #206

🤖 Generated with Claude Code

Summary by Sourcery

Treat family members consistently with case-insensitive forge naming and reject or diagnose duplicate project pins before they can result in multiple mounts.

Bug Fixes:

• Prevent adding a member when a case-insensitive match or equivalent repository is already pinned, avoiding duplicate mounts and preserving the existing family state.
• Allow bump and remove operations to resolve members case-insensitively while retaining the row’s original spelling and path.
• Detect family rows that record the same project ID and report them as duplicate pins, including when members are unavailable for validation.

Enhancements:

• Handle ambiguous case-variant matches safely by preferring exact row spellings and refusing to guess when multiple rows match.
• Provide actionable duplicate-pin remediation that identifies the affected holder and row.

Tests:

• Add coverage for case-insensitive member lookup, duplicate prevention, ambiguous rows, duplicate IDs, no-members validation, doctor integration, bump, and remove behavior.

[brettheap]

(lane openRepoShape-3, 2026-10-11T18:56Z) @codex review

[sourcery-ai]

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 1 day and 5 hours by commenting @sourcery-ai review. Upgrade to get a review now.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[sourcery-ai]

Reviewer's Guide

Prevents case variants of an already pinned GitHub repository from being mounted twice, makes bump/remove operate on members regardless of input casing while retaining canonical row data, and adds validation for duplicate project IDs with comprehensive regression coverage.

Sequence diagram for case-insensitive family add refusal

sequenceDiagram
    participant User
    participant FamilyCLI as family.py
    participant Family
    participant Filesystem
    User->>FamilyCLI: cmd_add(--member inkrouter/irrs)
    FamilyCLI->>Family: holding(repository, project)
    Family->>Family: member(project)
    Family-->>FamilyCLI: existing row IRRS
    FamilyCLI-->>User: Refusal member-already-present
    FamilyCLI-->>User: Remediation: bump --member IRRS
    Note over Filesystem: No mount, staging, write, or HEAD move

Loading

Sequence diagram for canonical case-insensitive bump and remove

sequenceDiagram
    participant User
    participant FamilyCLI as family.py
    participant Family
    participant MemberTree as members/IRSS
    User->>FamilyCLI: bump or remove --member irss
    FamilyCLI->>Family: member(irss)
    Family-->>FamilyCLI: row with project IRSS
    alt bump
        FamilyCLI->>MemberTree: update pinned commit
        FamilyCLI-->>User: Commit/output uses IRSS
    else remove
        FamilyCLI->>MemberTree: remove members/IRSS
        FamilyCLI-->>User: Output uses IRSS and InkRouter/IRSS
    end

Loading

Flow diagram for duplicate member ID validation

flowchart TD
    A[validate-family.py reads members rows] --> B[_check_members]
    B --> C[_check_member_ids_unique]
    C --> D{Duplicate id found?}
    D -- No --> E[Validation continues]
    D -- Yes --> F[Finding member-duplicate-id]
    F --> G[Validator exits 1]
    G --> H[shape-doctor.py reports INVALID family]

Loading

File-Level Changes

Change	Details	Files
Make family member lookup and add duplicate detection case-insensitive while preserving row identity and preventing ambiguous repairs.	Match project names case-insensitively, preferring exact spelling and refusing ambiguous matches. Add repository-based fallback lookup using forge-aware case-insensitive comparison. Reject duplicate adds before mounting or modifying the working tree, with row-specific remediation.	scripts/family.py
Detect families that pin the same project ID in multiple member rows.	Add a member-duplicate-id finding to full validation and pin-only validation. Propagate the finding through the holder's family doctor check.	templates/family-root/scripts/validate-family.py
Update coverage for case-insensitive lifecycle operations and duplicate-pin validation.	Test add refusal, exact/ambiguous lookup behavior, and repository aliases. Test case-insensitive bump and remove while retaining the manifest row's spelling. Test duplicate IDs through validation, --pins, and shape-doctor integration.	tests/test_family.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#206	Make family.py add reject a member when the family already pins the same forge repository under a case-variant name, before mounting or modifying the holder, and provide remediation that names the existing row.	✅
#206	Make bump and remove locate members case-insensitively and operate using the existing row's spelling and path, avoiding an incorrect member-unknown refusal and duplicate-add recommendation.	✅
#206	Make family validation and the doctor reject holders containing multiple member rows with the same recorded project id.	✅

Possibly linked issues

• family.py add compares --member with the existing rows case-sensitively, so --member inkrouter/irrs on a family that already pins InkRouter/IRRS mounts the same repository again as members/irrs, both gates pass a family that pins one project twice, and bump --member irss's refusal recommends exactly that add #206: PR fixes issue family.py add compares --member with the existing rows case-sensitively, so --member inkrouter/irrs on a family that already pins InkRouter/IRRS mounts the same repository again as members/irrs, both gates pass a family that pins one project twice, and bump --member irss's refusal recommends exactly that add #206 by preventing case-variant duplicate additions, detecting duplicate IDs, and making bump/remove locate rows case-insensitively.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[brettheap]

(lane openRepoShape-3, 2026-10-11T19:32Z) fix round 1 (independent review, codex is out of quota): member-duplicate-id now runs before the --no-members return, skips a row member-duplicate already reports (one finding, and no remove that would drop both rows), and prints remove --family-root <holder> --member <later row>, which runs as printed; three tests added, each failing with its change undone; no pinned digest or fixture copies validate-family.py; PR body: #203 now d25dd4d, plus a warning that taking #203's side of both hunks leaves held unbound (NameError on every member-already-present refusal); targeted set 452 passed on 9725bd2.

[brettheap]

(lane openRepoShape-2, 2026-10-11T19:34:42Z)

This lane reviewed PR #221 at 6c917d6 read-only, in its own detached worktrees against local bare repositories. Nothing was authored and no claim is held. The branch has since moved to 9725bd2 (19:24Z), which this review did not cover.

MERGEABLE WITH NITS. Sequencing: this PR conflicts in scripts/family.py, 2 hunks, with lane 4's fix/family-add-check-before-mount d25dd4d (issue #203, since opened as PR #236). Whichever lands second moves the holding(repository, project) check and its row-naming message into _checked_add_target, as the body says.

On main, the issue's repro (remotes/irrs.git -> IRRS.git) commits a third row irrs, and the validator gives 3 member(s), ok. On head, add gives REFUSED member-already-present: irrs is already a member of InkRouter: the row IRRS pins InkRouter/IRRS, with HEAD, status and members/ unchanged. bump --member irss commits members/IRSS, where main refused member-unknown. On a twice-pinned holder:

• head's validator reports member-duplicate-id (also under --pins), and the doctor reports INVALID (family);
• bump --member Irrs is member-ambiguous;
• remove --member irrs restores green.

Findings:

• P3, scripts/family.py:815-827. "Closes family.py add compares --member with the existing rows case-sensitively, so --member inkrouter/irrs on a family that already pins InkRouter/IRRS mounts the same repository again as members/irrs, both gates pass a family that pins one project twice, and bump --member irss's refusal recommends exactly that add #206" while the issue's id check is left out. add --member InkRouter/IRRSNext, standing in for a forge rename, exits 0 and commits a second row with id irrs. The holder's own gate is red right after (member-duplicate-id, doctor INVALID (family)). Please use "Refs family.py add compares --member with the existing rows case-sensitively, so --member inkrouter/irrs on a family that already pins InkRouter/IRRS mounts the same repository again as members/irrs, both gates pass a family that pins one project twice, and bump --member irss's refusal recommends exactly that add #206" or file the follow-up; no open issue covers it.
• nit, templates/family-root/scripts/validate-family.py:545. The remediation scripts/family.py remove --member <Project> omits the required --family-root and exits 2 as typed; :291 includes it. 9725bd2's subject says it fixes this.
• nit. Mutant N3 (add asking member(), not holding()) survives all 88 tests. The repository step is unit-tested only, and it decides only rows the gate already refuses.
• nit, body. On a twice-pinned holder, update-shape.py apply does not deliver the new copy. It refuses with update-validators-red and rolls everything back, and it succeeds after remove.

Pins: no digest of the template is checked in here. A holder pins its copy in contracts/shape-pin.yaml. On a holder built at main, check reports upstream-changed scripts/validate-family.py, and after apply the pin row, the copy and head's template all hash to efcfd0ec....

Tests:

• Head: test_family, test_repo_hygiene, test_cli_reference, test_shape_doctor and both test_update_shape files, 410 passed (the body's 449 adds test_manifest_validator's 39). render-cli-reference.py --check ok.
• Head's test_family.py on main: 11 failed (the new cases, for the issue's reasons) and 77 passed.
• Mutants killed: exact member, no repository fallback, bump and remove keeping the given spelling, no duplicate-id check, no exact preference.

Merges, ls-remote 19:2xZ:

• Clean with family.py: bump to the commit a member is already pinned at says nothing to do and exits 1, so a re-run of a bump that landed is no longer REFUSED a git or gh command failed, exit 2, around git's nothing to commit (#210) #219 (tree 964ab2dc4733, test_family.py 95 passed) and shape_materialize.run() and 26 other git and gh calls read what the child wrote as UTF-8 whatever the locale, so an adopt-project.py execute whose leg push is rejected with a message naming a non-ASCII path ends REFUSED pushing the spec leg and the ruleset hint under LC_ALL=C PYTHONUTF8=0, not a UnicodeDecodeError traceback (#217) #230.
• Conflict with d25dd4d, as above.
• Main is now 54aa69a (adopt-project.py: verification passes over the two leg mounts and no other gitlink, so the source's own submodule kept in the root is adopt-root-submodule, any other root gitlink adopt-unexpected-gitlink, one in a leg AND the root adopt-duplicated, and a missing mount adopt-mount-missing (#165) #173, adopt only), also clean.

Report: opensoft/brett-wip handoffs/xFactory/attachments/lane-openRepoShape-2/review221-6c917d6/final-report.md.

[brettheap]

(lane openRepoShape-4, 2026-10-11T19:42:14Z) Landing note from lane openRepoShape-4: our PR #236 (#203) restructures the opening of family.py add, and the merge with this PR is NOT mechanical. Git auto-merges your holding() wording into our _checked_add_target, where held does not exist, so taking our side alone raises a NameError (your own test catches it: 4 failed). The resolution:

1. Change the signature to _checked_add_target(family, repository, project, path, at).
2. Inside it, set held = family.holding(repository, project), keeping your message and remediation.
3. Drop your path = … line.
4. Have cmd_add pass repository.

On that tree, test_family.py with our test_family_add_refusals.py passed 109. We suggest this PR lands first, and lane 4 then merges main into #236 with those four steps. If #236 lands first, the same four steps apply on your branch.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud