Skip to content

family.py: add never reads .gitmodules, so a section under ANOTHER name whose path is already members/<P> (a "ghost" section) does not stop it: it exits 0 and commits two sections for one path, validate-family.py says family ok, and a recursive clone takes the LATER section, so with the ghost second it dies fetching the ghost's URL #300

Closed

Description

@brettheap

Lane: openreposhape-4 (openRepoShape-4)

Found by this lane's adversarial review of #242's branch (head 289e542; its nit N6, which the branch's own writer had also found), and re-verified on main d94d155 (#232's squash) with a holder, local bare remotes and gh a shim that was never called. It is the mirror of the member-name-taken refusal #242 adds (NOT on main yet): that one refuses when .gitmodules already has a section whose NAME is members/<P> for another path; this is a section whose PATH is members/<P> under another name. It is pre-existing, and it is not --force's doing: the review found the same exit 0 on #236's code (cc83d15 then), on #242's head and on a force-only variant of #236. Not claimed: a git other than 2.43 (the first run below used it; a recursive clone's choice between two sections for one path is git's, not this tool's), and Windows.

What is wrong. cmd_add (scripts/family.py:823-912) guards the mount with member-already-present (:827-834), member-path-taken (the directory exists on disk, :835-844) and member-git-dir-cached (:851-862), then runs git submodule add (:868). None of them reads .gitmodules, and git submodule add registers the path under the new name without asking whether another section already holds it:

# $U: a clean checkout at d94d155. Atlas scaffolded into local bare remotes ($R/remotes); the holder $H made with `family.py init --local-remote-dir`.
# A section named `ghost` is committed in the holder: [submodule "ghost"] path = members/Atlas, url = /nowhere/ghost.git
python3 $U/scripts/family.py add --family-root $H --member testorg/Atlas --local-remote-dir $R/remotes
#   mounting testorg/Atlas at members/Atlas
#     Atlas            cc41816c22bb tree 2d49a3bad247… (project atlas)
#     committed 8d1fc0eb5c6c: .gitmodules, members/Atlas, family.yaml                     exit 0
cat $H/.gitmodules
#   [submodule "ghost"]
#   	path = members/Atlas
#   	url = /nowhere/ghost.git
#   [submodule "members/Atlas"]
#   	path = members/Atlas
#   	url = $R/remotes/Atlas.git
python3 $H/scripts/validate-family.py --root $H                                           # family ok: Fam (fam), 1 member(s)
# pushed to a bare repository of its own, then cloned:
git -c protocol.file.allow=always clone --recurse-submodules $R/remotes/Fam.git $R/clones/Fam
#   (the ghost first, as `add` leaves it)                                                  exit 0, members/Atlas holds the member: git took the later section
# the same .gitmodules with the two sections the other way round (the ghost second), committed and pushed:
git -c protocol.file.allow=always clone --recurse-submodules $R/remotes/Fam.git $R/clones2/Fam
#   fatal: repository '/nowhere/ghost.git' does not exist
#   fatal: clone of '/nowhere/ghost.git' into submodule path '$R/clones2/Fam/members/Atlas' failed
#   Failed to clone 'members/Atlas'. Retry scheduled
#   ...
#   Failed to clone 'members/Atlas' a second time, aborting                                 exit 1

So add commits a .gitmodules that registers one path twice; the holder passes its own validator; and which URL a recursive clone uses depends on which section is later in the file (in these two runs git took the later one). Because add appends, the member's own section is last at the moment of the commit and the clone works; the state is a landmine for the next edit, not a failure of this one. One more observation, from the sections the other way round: family.py remove --member Atlas then commits a .gitmodules that still holds a [submodule "members/Atlas"] section with no gitlink in the tree (once, in that order; not explored).

Why. member-path-taken asks the disk ((family.root / path).exists(), :836) and member-git-dir-cached asks .git/modules/members/<P> (:851-852); nothing asks the file that says which path belongs to which submodule name, and a section can exist, in .gitmodules and committed, with nothing on disk and no object store: a half-removed member, a hand edit, a merge that kept both sides. validate-family.py does not look either (the file has no reference to .gitmodules): it reads the rows of family.yaml and the gitlinks, and reports family ok for the committed result.

What it should do. add does not commit a second section for a path, and a holder that already has one is told. The decisions are the author's:

  1. Refuse before the mount. A section in .gitmodules whose path is members/<P>, whatever its name, is refused (the mirror of family.py add runs git submodule add without --force (and _commit_one a plain git add), so under an operator's global excludes naming members or the member's own name it exits 2 at the mount for every member, and the remediation it prints blames access (gh auth status, your SSH key) instead of the ignore rule #242's member-name-taken, which is its model: it reads the working-tree file and the index or HEAD copy, because the file can be gone from the work tree and still be committed), naming the section and its URL and saying what to do: git rm it, or family.py remove if it was a member. Or replace it: drop the ghost section as part of the add's one commit, which edits a registration somebody wrote and is the less safe reading.
  2. The validator. validate-family.py gains a finding for two sections with one path (and for a section whose path no row names), so a holder that already has one fails its own gate rather than passing it. It is a copied file, so that reaches holders through update-shape.py.
  3. remove. Whether remove also takes every section for the path out, or refuses when there are two (the observation above, which needs its own probe before it is decided).

How to test. The first probe as a test: commit a section named ghost for members/Atlas in a holder and run add --member <org>/Atlas; assert the refusal (exit 2, nothing mounted, the holder byte-identical), and, if decision 2 is taken, that validate-family.py on a holder with two sections for one path is a finding. A second row with the ghost section present only in HEAD and deleted from the work tree.

Related: #242 (its review's N6; member-name-taken is the other half), #236 (the rollback these refusals leave untouched by refusing first), #203, #299 (remove and bump taking path: from the row unchecked), and validate-family.py.

Activity

  1. brettheap commented on Oct 12, 2026

    @brettheap
    ContributorAuthor

    CLAIMED — lane openRepoShape-4, session 3431c370-df80-4c11-a041-41bce42d8b38@Eagle, 2026-10-12T04:01:59Z, for #300

    Logged in lanes/log/openRepoShape-4.md at 107bb60242dfebf348fb4a7a416e745115a857d3.

    The three reads (lane-collision-protocol Rule 1):

    1. existing `CLAIMED —` comments on opensoft/openRepoShape#300:
       none
    2. `gh pr list --repo opensoft/openRepoShape --state all --search "#300"`, kept where the row or its body names `#300`:
    315	family.py: `bump` and `remove` refuse `member-path` when a row's `path:` is not `members/<project>` or its `project:` is not a directory name, and hand git every path literally, so a hand-edited `path: members/*` or `project: .` no longer makes `remove` unmount every member in one commit while dropping one row (#299)	fix/family-member-path-literal	OPEN
    3. `git ls-remote --heads git@github.com:opensoft/openRepoShape` branches naming `300` as a whole token:
       none
    
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions