[brettheap]

Lane: openreposhape-5 (openRepoShape-5)
Claim: #407 (comment)

What was wrong

family.py remove commits .gitmodules as git rm leaves it in the work tree, and git rm edits only a .gitmodules the work tree has. With the work-tree copy deleted and the deletion not committed, git rm edited nothing. _commit_one then staged the deletion, and the commit took .gitmodules out of HEAD, with every other member's section in it. remove exited 0, and the holder it ran in passed its own gate. Once the commit was pushed, a recursive clone left every remaining member empty.

The issue's reproduction, run on bfed143 (I added one git switch -c, because #487 landed after the issue was filed):

registration keys before: Atlas 2, Nova 2
dry run: exit 0
gitmodules   none: git reads no section of .gitmodules in the work tree as giving members/Nova, so none is removed
remove: exit 0
  committed 766cfa532913: .gitmodules, members/Nova, family.yaml
 .gitmodules  |  6 ------
HEAD holds .gitmodules: no
registration keys after: Atlas 2, Nova 0
family ok: Fam (fam), 1 member(s)
recursive clone: exit 0
the clone's members/Atlas holds 0 entries
REFUSED member-uninitialized: members/Atlas is not an initialized submodule checkout, ...
fatal: No url found for submodule path 'members/Atlas' in .gitmodules

What changed

One check, _checked_gitmodules_as_staged, runs before any git command that writes. The dry run makes it too, in the same words. It reads the index's records for .gitmodules (stage, blob and git's -v flag letter) in one git ls-files -s -v -z, which writes nothing.

• gitmodules-missing (remove): the work tree has no .gitmodules, while the index (at any stage) or HEAD has one. The remediation names the restore git takes in that state:

  • git checkout -- .gitmodules (the index's copy);
  • git checkout HEAD -- .gitmodules, where the deletion is staged;
  • for an unmerged file, by the stages the index holds: git checkout --merge -- .gitmodules where it has both sides' copies; git checkout --ours where only this side's is left (the other side deleted it); git checkout --theirs where only the other side's is; git checkout-index -f --stage=1 where both sides deleted it. Each is followed by git add .gitmodules.

• gitmodules-unstaged (remove and add): the work tree's copy is not the index's. That means one of:

  • git lists it as modified (git ls-files -m). Where git add would still stage the index's own blob from it (git hash-object --path=.gitmodules says so, and it is a regular file), the refusal adds that the difference may be its line endings alone;
  • it is unmerged;
  • its deletion is staged while the file is still on disk;
  • the index marks it assume-unchanged, which ls-files -m does not look past, and its content is not the index's blob;
  • it is untracked: neither the index nor HEAD holds one. This includes the first member's add in a new holder.

  The remediation offers both ways out: stage the work tree's copy, or put back the index's (or HEAD's) copy. Behind assume-unchanged, staging first takes the mark off (git update-index --no-assume-unchanged -- .gitmodules), because git add stages nothing while it is on. An untracked file is staged with git add -f .gitmodules, since an operator's global excludes may match it (and, if wanted, committed on its own first), or moved out of the holder.

• gitmodules-sparse: the index marks .gitmodules skip-worktree, as a sparse checkout that leaves it out does. git neither compares nor stages that file, and git checkout -- .gitmodules matches nothing. The remediation is git sparse-checkout disable.

  • add refuses it whatever the file's state, since git submodule add cannot stage it.
  • remove refuses it where the file is missing or is not the index's copy, and commits a clean copy on disk rightly, as it did on bfed143.

Every refusal ends with "No git command that writes was run and nothing was written.", and every remediation ends with "run the same command again". Typed as printed, each one leads to the right commit (the tests do exactly that).

remove asks this first, before #360's member-path-registered-twice. add asks it after its mount-path refusals (#242, #300), which already name what they find in the same file. Two of those, member-name-taken and gitmodules-unreadable, have the person mend the file by hand and run add again. They now say "then git add .gitmodules", as #300's and #360's remediations already did. Without that step, the run again was this PR's own gitmodules-unstaged. The holder's validate-family.py keeps its words: it reads the work tree's copy, so re-running it needs no stage.

Round 2: the review's points and Codex's

Point	What changed
S1	member-name-taken and GITMODULES_UNREADABLE say "then git add .gitmodules". New test: each one's state, mended by hand, the printed stage typed, and add then mounts the member. The text tests are updated (test_family_add_excludes.py's member-name-taken test, and test_refusal_default_remediation.py, which now holds add's remedy equal to the gate's but for the one step). test_family_add_refusals.py::test_a_gitmodules_git_cannot_read_is_a_refusal_not_a_pass pins the validator's text, which is unchanged, so it is untouched, and so is the :518 test, which asserts only the REFUSED line.
N1	The line-endings clause, only where hash-object gives the index's blob from a regular file. Tested with a CRLF copy (clause) and a link to the very same bytes (no clause: hash-object reads through the link, but git would stage the link).
N2, sparse	gitmodules-sparse, with git sparse-checkout disable. Tested by a real git sparse-checkout set --no-cone '/*' '!/.gitmodules', for both verbs, typed as printed.
N2, modify/delete	The per-stage restores above. Tested for stages 1+2 (--ours), 1+3 (--theirs) and 1 alone (checkout-index), each typed as printed.
N3	test_a_holder_whose_head_has_no_gitmodules_is_not_asked asserts only that no gitmodules- refusal is made. Its docstring says the real run fails there, a known case from before #407 that goes to #459.
N4	The base split below is corrected.
N5	unmerged() and typed_as_printed() run git through raw(root, env, ...), under the module's isolated environment. raw() gained a stdin argument for update-index --index-info.
N6	held() and assert_held() are gone. Refusals are read with test_family_dry_run.state() and assert_as_it_was(); state() now reads a missing .gitmodules as None (three lines).
Codex, :4476	The untracked .gitmodules is gitmodules-unstaged, for both verbs, dry run alike. Tested: the first add in a holder with no member, and remove in the holder #407 leaves behind with the old bytes put back untracked. Staged as printed, each commits.
Codex, :4519	assume-unchanged: compared by content (hash-object against the stage-0 blob) and refused. skip-worktree: gitmodules-sparse, as for N2. Tested for both verbs: the assume-unchanged edit kept through the printed update-index and add, and the skip-worktree edit through git sparse-checkout disable, then the restore the next refusal prints.

Round 3: the re-review's nits

Nit	What changed
1	remove again commits a clean .gitmodules the index marks skip-worktree, as bfed143 and b1120c3 did: gitmodules-sparse is asked for add whatever the file's state, and for remove only where the file is missing or is not the index's copy. add, an edited file and a missing one stay refused. The function's and the module's docstrings say which verb refuses what. New test: remove with the mark on and the file clean commits exactly .gitmodules, family.yaml and the gitlink.
2	Two cases the round-2 tests left open. X1: add over a file marked both skip-worktree and assume-unchanged (two update-index calls, ls-files -v tag s) is gitmodules-sparse. X2: a clean assume-unchanged file passes, and both verbs commit the right tree. Each set-up asserts the ls-files -v letter it expects.
3	The untracked branch prints git add -f .gitmodules. The untracked add test also runs under an operator's core.excludesFile holding * (set as #242's tests set it), asserts that the file is ignored there, types the remedy, and add commits.
4	Decision 5's wording, and the note for the squash message below. This round's commit subject says only what it does.

Decisions and evidence

1. Refuse, not rebuild (the issue's first option). The holder is left byte for byte as it was, and the person picks the copy they mean. Building this member's section out of the index's blob would commit a .gitmodules the work tree does not have. The holder would then show .gitmodules as deleted, and the person's next git commit -a would delete it after all. For a modified copy it would also have to guess which copy the person means.
2. A staged change still goes in. The remediations of family.py remove, when .gitmodules gives the member's path to two sections, takes out only the one git finds by that path (the later in the file) and exits 0, so its commit still registers members/<P> with no gitlink and no row, validate-family.py passes the result (after #317 it reported the doubled file and stops), and the next add of that member is refused member-path-registered or silently reuses the leftover section and the .git/config registration deinit never touched #360 and 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 tell the person to edit .gitmodules, git add it, and run the same command again. So the rule is "the work tree's copy must be the index's", not "must be HEAD's".
3. "Modified" is git's own answer. It comes from git ls-files -m, the comparison git status uses and the one git rm refuses an unstaged change with. On bfed143, an unstaged edit, a CRLF-only copy and a symlink were each refused by git rm only after git submodule deinit -f had emptied the member (then rolled back), and each dry run exited 0. The CRLF case is still refused (as git rm refuses it), and now says why it may look like no change: its hash-object is the index's blob (probe: both 26f54704).
4. Assume-unchanged is compared by content; skip-worktree is refused outright. Probes against round 1's check (states-wip.log):
   • assume-unchanged, clean: both verbs commit correctly, so it is not refused. Edited: ls-files -m is empty and both verbs committed the edit. git add .gitmodules with the mark on exits 0 and stages nothing (the hash stays a2d4d491 against the index's 26f54704), so the keep-remedy takes the mark off first. git checkout -- .gitmodules restores it with the mark on.
   • skip-worktree, file on disk: remove committed a hidden edit; add failed at its mount even for a clean file (git add refuses a path outside the sparse-checkout definition, then rollback). git add and git checkout -- both fail while the mark is on. So add refuses any skip-worktree .gitmodules, on disk or not. remove refuses one that is missing or is not the index's copy, and commits a clean one on disk rightly, as it did on bfed143 and b1120c3 (round 3; round 2 refused that one too).
5. git sparse-checkout disable is the one command that works as typed both in a sparse checkout and where only the mark is left. git sparse-checkout add /.gitmodules is narrower in a real non-cone sparse checkout, but fails for a mark set by hand (fatal: no sparse-checkout to add to). With sparse checkout on, git checkout --ignore-skip-worktree-bits -- .gitmodules and git update-index --no-skip-worktree + git checkout -- both put the file back, but remove then failed at its commit's git add ("outside of your sparse-checkout definition"). disable worked with sparse checkout on and with only the mark left, and keeps a copy already on disk as it is. The refusal names the state in its own words. It is a dozen lines, not a sparse-definition reader.
6. An untracked file is staged, not committed by the printed command, and staged with -f. add's branch check (family.py add, bump and remove commit straight onto the holder's tracking branch, and add's NEXT pushes a <branch> that does not exist, though AGENTS.md says to land each as a pull request #487) comes after this refusal, so a printed git commit could land on the tracking branch. Staged, the file goes into the verb's commit; the remediation says to commit it on its own first if that is not wanted. An operator's global excludes may match the untracked file (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 *), and a plain git add then refuses it, so this branch prints git add -f .gitmodules. The tracked branches keep the plain git add, which stages a tracked file under the same excludes.
7. The dry run (N9). In the issue's state, remove --dry-run prints the same refusal, exit 2. Past the refusal, the work tree's file is the index's copy, or no copy exists anywhere. So deinit reads the same name the dry run reads.
8. bump is not affected. Its commit is [path, MANIFEST] and never stages .gitmodules. Probe: bump with the work-tree .gitmodules deleted committed family.yaml and members/Atlas only.

Tests

tests/test_family_remove_gitmodules.py, 35 tests:

• remove over each state the work tree lacks (6): deleted (the issue), deletion staged, and unmerged with the file deleted at stages 1+2+3, 1+2, 1+3 and 1. Both runs refuse alike, exit 2, nothing on stdout, and the holder unchanged: test_family_dry_run.state(), which is HEAD, ls-files -s -z, status --porcelain=v1 -z, family.yaml, .gitmodules, .git/config, the member's HEAD, every work-tree byte and the holder's objects. The remedy is typed as printed; remove then exits 0, HEAD's .gitmodules gives only the other member's path, and the gate passes;
• the issue's clone: after the restore and remove, a push and a fresh clone --recurse-submodules populate the other member, and the clone's gate exits 0;
• each work-tree copy that is not the index's, for both verbs (12): edited, CRLF only, a link to the same bytes (skipped on Windows), assume-unchanged and edited, unmerged, deletion staged with the file kept. One printed remedy is typed, and the verb then commits;
• sparse (4): a real sparse checkout for both verbs, and add over a clean file marked skip-worktree by hand and over one marked both ways (ls-files -v tag s); skip-worktree with an edit (2), both verbs;
• a clean file behind a mark, committed with exactly .gitmodules, family.yaml and the member's gitlink (3): skip-worktree (remove), assume-unchanged (remove and add);
• untracked (3): add in a new holder, with no excludes and under an operator's *; remove in a holder family.py remove, in a holder whose work-tree .gitmodules has been deleted and the deletion not committed, commits that deletion with the member's removal, so every other member's section leaves HEAD while remove exits 0 and the holder's validate-family.py passes, and once pushed a recursive clone exits 0 with each remaining member's directory empty and its own validate-family.py refusing member-uninitialized #407 already damaged;
• add over a missing file is still git's own refusal, with its dry run's note;
• a holder whose HEAD lost .gitmodules: no gitmodules- refusal (N3);
• S1 (2): member-name-taken and gitmodules-unreadable, mended by hand, the stage typed as printed, add then mounts.

Round 2's 29 tests on round 1's head b1120c3: 15 failed, 14 passed in 80.15s. The 15 are exactly round 2's cases: CRLF's clause (2), sparse (2), skip-worktree (2), untracked (2), S1 (2), the --ours, --theirs and checkout-index restores (3), and assume-unchanged (2). On bfed143 with them: 27 failed, 2 passed in 59.67s; the 2 that pass pin behaviour the fix keeps (add leaves a missing file to git, and N3's holder is not asked).

Round 1's base proof, corrected (the review's N4): on bfed143 with round 1's 14 tests, 12 failed, 2 passed. Of the 12, 8 failed on "HEAD changed" (the verb committed; the clone test is among them), 3 on ".git/config changed" (lacks[deletion staged], [edited-remove] and [crlf-remove]: the run failed after deinit and the rollback re-wrote the registration) and 1 on "status changed" ([unmerged-remove]). In lacks[deletion staged] it was not git rm that failed after deinit but _commit_one's git --literal-pathspecs add -f -- .gitmodules family.yaml (pathspec '.gitmodules' did not match any files).

On this head (ebe6b51), Linux, /usr/bin/python3 -m pytest, the files round 3 touches and the importers of the changed helpers: tests/test_family_remove_gitmodules.py tests/test_refusal_default_remediation.py tests/test_family_add_excludes.py tests/test_family_dry_run.py tests/test_family_member_path.py: 182 passed in 224.37s (0:03:44). The new file alone: 35 passed in 46.50s.

Round 2's targeted runs, on cb159b1 (round 3 changes family.py's skip-worktree condition and one remedy, and the new test file):

• tests/test_family_remove_gitmodules.py tests/test_family_bump_remove_rollback.py tests/test_family_dry_run.py tests/test_family.py tests/test_cli_reference.py tests/test_repo_hygiene.py tests/test_family_member_path.py tests/test_family_tracking_branch.py tests/test_family_printed_quoting.py tests/test_family_member_name.py tests/test_family_bump_to_a_tag.py tests/test_family_init_landing.py tests/test_family_init_values.py tests/test_template_text_placeholders.py: 706 passed, 19 skipped in 1013.94s (0:16:53) (the skips are test_family_init_values.py's template-quoting cases, as on main);
• tests/test_family_add_refusals.py tests/test_validate_family_duplicates.py tests/test_validate_family_member_path_glob.py tests/test_validate_family_unnamed_mounts.py tests/test_validate_family_without_git.py tests/test_family_add_excludes.py tests/test_family_add_member_id.py tests/test_refusal_default_remediation.py tests/test_agents_md_codes.py tests/test_shape_doctor.py: 632 passed, 3 skipped in 1482.84s (0:24:42) (the skips are the Windows-only PowerShell line).

Windows: every file the tests write is written with write_bytes, git's stdin gets bytes, and nothing compares against a file a helper wrote with write_text. The holder's attributes keep .gitmodules LF in the work tree (i/lf w/lf attr/text=auto eol=lf). The symlink case skips on Windows, where git checks a link out as a plain file. macOS: no case variants are involved.

Reproductions

The issue's script, with local bare remotes, a gh that exits 97, no global or system git configuration, and the registrations read from .git/config by grep:

• on round 2's head (cb159b1): dry run: exit 2 and remove: exit 2, both REFUSED gitmodules-missing: ... with the same stderr, and a full snapshot of the holder (every file, .git included) identical after each. The registration sections stay 1 and 1. After a push of the unchanged holder, the recursive clone exits 0, members/Atlas holds 15 entries, and the clone's gate prints family ok.
• round 1 typed the remedy as printed (git checkout -- .gitmodules): remove: exit 0, a commit with .gitmodules | 3 --- (Nova's section only), the holder's gate exits 0, and after a push the recursive clone populates Atlas and its gate exits 0. The lacks and clone tests repeat that on this head.
• round 2's states, on cb159b1: for remove and add, a real sparse checkout, a skip-worktree edit, an assume-unchanged edit (kept, and dropped), CRLF and a link; for remove, an assume-unchanged deletion, the unmerged file deleted at stages 1+2, 1+3, 1 and 2+3, and the untracked file in a damaged holder; for add, the untracked file in a new holder. Each is refused alike by both runs (identical stderr), each printed command exits 0 as typed, and the run again commits what the remedy chose. Round 3 changes two of these paths (a clean skip-worktree file for remove, and the untracked remedy's -f), and its tests on ebe6b51 hold both.

Nothing here needs real GitHub or a cloud tool.

Mutation probes

Round 2's code hunks, each reverted alone in a throwaway worktree at cb159b1, with the new file run against it:

hunk reverted	result	new tests that fail
M1 S1, GITMODULES_UNREADABLE's stage	1 failed, 28 passed	S1 [gitmodules-unreadable]
M2 S1, member-name-taken's stage	1 failed, 28 passed	S1 [member-name-taken]
M3 the untracked refusal (Codex, :4476)	2 failed, 27 passed	both untracked tests
M4 gitmodules-sparse	4 failed, 25 passed	sparse [remove], [add]; skip-worktree [remove], [add]
M5 the per-stage restores (always --merge)	3 failed, 26 passed	lacks[our side only, deleted], [their side only, deleted], [neither side, deleted]
M6 the line-endings clause (N1)	2 failed, 27 passed	[crlf-remove], [crlf-add]
M7 _stages_as's regular-file guard	2 failed, 27 passed	[a link to the same bytes-remove], [...-add]
M8 the assume-unchanged branch (Codex, :4519)	2 failed, 27 passed	[assume-unchanged, edited-remove], [...-add]
M9 the flags read as H (_gitmodules_in_index)	6 failed, 23 passed	the 2 assume-unchanged, 2 sparse and 2 skip-worktree cases
M10 the return where nothing holds a .gitmodules	29 errors	every test, at the family fixture's first add into a holder with no .gitmodules anywhere, which is then refused as untracked

Round 1's hunks are still held by the same tests (round 1's probes M1 to M8). The other hunks are docstrings and comments.

Round 3's hunks, each reverted alone at ebe6b51, with the new file run against it:

hunk reverted	result	new tests that fail
R1 the narrowing (always refuse skip-worktree)	1 failed, 34 passed	[skip-worktree-remove] of the clean-behind-a-mark test
R2 its verb == "add" arm	2 failed, 33 passed	sparse [marked by hand, clean-add], [marked both ways, clean-add]
R3 X1, tag == "S" without .upper()	1 failed, 34 passed	sparse [marked both ways, clean-add]
R4 X2, every assume-unchanged copy refused	3 failed, 32 passed	[assume-unchanged-remove], [assume-unchanged-add], sparse [marked both ways, clean-add]
R5 plain git add in the untracked remedy	3 failed, 32 passed	the three untracked tests (the pinned text)
R5b plain git add in the remedy AND the test's expected text	1 failed, 2 passed (-k untracked)	[*]: typing git add .gitmodules fails, "The following paths are ignored by one of your .gitignore files ... Use -f"

Merge checks

git merge-tree --write-tree of this head (ebe6b51) with each of these is clean:

• origin/main dd3f23a. Since bfed143, main took tests/test_repo_hygiene.py and docs/handbook.html: each handbook section records the hash of every README section it reads, and README.md's line cap is 1422 plus one file per raise in tests/readme-cap.d/, so two README pull requests no longer conflict on the footer's one blob or the cap's one line, and a stale page names exactly which sections to re-read (#519) #551, shape-doctor.py: the machine row of a machine that is not ready names the preflight checks that failed, in the [!!] lines the preflight printed, so its reason no longer ends on this machine is not ready yet (see above). with nothing above it (#549) #552, tests: every test that clones a just-pushed local remote by its plain path now clones it with clone_just_pushed, so the macOS failed to copy file race of #440 cannot reach it; load_without_bytecode keeps the doctor's and the Windows-path tests' loads of the family root's scripts from writing .pyc into the templates; the not-a-list doctor cases hold INVALID, exit 1; and GOOD_KEYS holds th #554, shape-doctor.py: a git repository is told of both ways in, the adoption and the scaffold, and the adoption line ends --org <your-org> where its origin names no organisation plan can read, so pasting it is no longer refused adopt-no-org (#545) #553, bump-leg.py: --dry-run gives the real run's answer, so --to an annotated tag's own id is peeled to the commit it names in both runs, and a pin layout the rewrite refuses is refused by the dry run too (#534, #438) #561, update-shape.py: apply refuses to keep an --accept-local copy of a helper that lacks a name a copy it writes imports from it, before anything is written, so a kept scripts/repo_shape.py no longer leaves make bootstrap dying on an ImportError after the validators passed (#536) #562, README.md and the handbook: the by-hand setup commands dry-run the scaffold with the --visibility and --elected-by setup passes, and apply --add names all five refusals it gives by their codes, each held by a test (#550, #429) #579, adopt-project.py: plan names an elector it took from git config user.name by where it came from, so with no --elected-by and no git config user.name the refusal reads --elected-by (not given, so git config user.name) is '' and no longer --elected-by is '', a flag nobody passed (#529) #567, update-shape.py: a files: or shape: header with a trailing comment is rewritten and keeps it, --branch is judged the same from any directory, and apply's messages say what is true (#437, #431, #535) #558, bootstrap.py: a leg detached off the pin is left where it is and said, so a commit made there is no longer stranded when its branch is already at the pin, the not moving an existing branch line names where the checkout is, and bump-leg.py and AGENTS-shape.md say "or says why not" (#533) #556 and openRepoShape --doctor says it writes nothing into the repository but a --placement-plan file you name, keeps its clone for the NEXT lines that name it, shows its synopsis description again, and refuses a second --placement-plan or a second path (#546) #580; none touches scripts/family.py or a test file this branch changes. This branch does not merge main;
• fix/family-bump-to-checks 8024f9c (family.py: bump --to asks add's duplicate-id question, peels an annotated tag's own id as add --at does, and says to push first a commit the member does not hold after its fetch (#541) #568), fix/family-members-case-variants 8a95345 (family.py: add refuses, before the member is read and on every disk, any path under another spelling of members/, in its index, in HEAD or on disk, and names a remedy per entry that never moves the members' checkouts (#362, #363, #364) #574), fix/link-source-chain-defaults a707d4b (validate-repository-naming.py, repo_shape.py, validate-pins.py: a bare --link-source is the first chain link's tree alone, and a source that names no path is refused (#441, #442) #576), fix/family-siblings-tracking-branch 2156a01 (siblings.py and family.py: make siblings asks about the family folder before it clones, clones each member on its tracking branch, and family.py siblings runs the holder's own copy (#543) #578);
• lane 2's fix/537-dry-run-asks-remotes 69c58aa (family.py and scaffold-project.py: --dry-run asks the real run's own question about existing remotes, so family-remote-exists and scaffold-remote-exists come at the plan, and a missing gh is gh-not-found, not a traceback (#537) #573), and validate-pins.py and validate-family.py: THE LOCKSTEP RULE closes the count only where a leg or member is out of step, a lone digest or workflow-reference mismatch names its bump-leg.py line, and holder remedies branch first (#539) #575's head 0680799 (lane 2, which changes validate-family.py; its unreadable-file remedy still ends "then run this again.", which test_refusal_default_remediation.py expects).

Left out, and checklist candidates

• (a) family.yaml hand edits are committed by add, bump and remove. Each commits family.yaml as the work tree has it, so an unstaged hand edit outside members: (a comment line) goes into the commit: the review reproduced +# a hand note nobody staged in a commit of each verb. It is family.py remove, in a holder whose work-tree .gitmodules has been deleted and the deletion not committed, commits that deletion with the member's removal, so every other member's section leaves HEAD while remove exits 0 and the holder's validate-family.py passes, and once pushed a recursive clone exits 0 with each remaining member's directory empty and its own validate-family.py refusing member-uninitialized #407's rule for a different file, and a staged-only rule there would chain into every remedy that says "edit family.yaml, then re-run", as S1 did here. It needs its own issue.

• (b) A HEAD that already lost .gitmodules fails after the first write. With no .gitmodules anywhere, this check asks nothing. remove --dry-run exits 0 and says no section is removed; the real remove runs deinit and git rm, then dies at git --literal-pathspecs add -f -- .gitmodules family.yaml (pathspec '.gitmodules' did not match any files) and puts back only part of the unmount (members/Nova is left empty). Same on bfed143. Once the old file is put back in the work tree, this PR now refuses it as untracked, and staged as printed remove commits it back into HEAD; but nothing tells the person to do that.

• (c) The gate passes a row with no .gitmodules section (Tracking: setup, scaffold, family and the shared helpers, creating projects and families #459, the issue's decision 3). validate-family.py should find a members: row whose path no section of HEAD's .gitmodules gives. A holder already carrying a family.py remove, in a holder whose work-tree .gitmodules has been deleted and the deletion not committed, commits that deletion with the member's removal, so every other member's section leaves HEAD while remove exits 0 and the holder's validate-family.py passes, and once pushed a recursive clone exits 0 with each remaining member's directory empty and its own validate-family.py refusing member-uninitialized #407 commit passes its own gate, and only a fresh recursive clone shows it.

• (d) A sparse checkout whose rules leave .gitmodules out, with the file put back on disk by hand. git then clears the skip-worktree mark, so this check passes, and remove's commit fails at git add ("outside of your sparse-checkout definition") and is rolled back. Nothing wrong is committed; it is a late failure, and reading the sparse rules is not this PR's.

• (e) A verb run during a merge, cherry-pick or rebase fails after its first writes. Any family.py verb run while one is in progress fails at its commit, git commit -- <paths> ("fatal: cannot do a partial commit during a merge."), and is rolled back. It is the same on bfed143. It touches this PR because its --ours and --theirs restores name modify/delete conflicts, whose commonest source is git merge: typed as printed the restore works, then remove fails late. The tests build those stages with update-index --index-info and no MERGE_HEAD, the state a conflicted git stash pop leaves. The candidate: refuse an in-progress merge, cherry-pick or rebase before the first write, beside the existing up-front refusals.

For the squash message

• Round 2's commit subject (cb159b1) says "no printed command leads into another refusal". That is inexact: for an edit hidden by skip-worktree, git sparse-checkout disable leads, by design, into gitmodules-unstaged, and test_an_edit_skip_worktree_hides_is_refused_then_named_as_modified pins that chain. The squash message must not repeat it.

Closes #407

🤖 Generated with Claude Code

Summary by Sourcery

Make family.py add and remove verify .gitmodules consistency before writing so they commit only staged changes and cannot remove or corrupt unrelated submodule registrations.

Bug Fixes:

• Prevent family.py add and remove from committing missing, unstaged, unmerged, untracked, assume-unchanged, or sparse .gitmodules changes that were not staged, avoiding corruption of other submodule registrations.
• Provide actionable refusals and restore commands for missing, conflicted, modified, ignored, and sparse .gitmodules states, consistently in dry runs and real executions.

Enhancements:

• Ensure staged .gitmodules edits remain eligible for the command’s commit and update related refusal guidance to stage hand-edited files before retrying.
• Add comprehensive coverage for .gitmodules work-tree/index mismatches, sparse checkouts, index flags, unmerged stages, untracked files, recursive clones, and remediation behavior.

Documentation:

• Document the .gitmodules consistency requirements and refusal behavior for add and remove.

Tests:

• Add regression tests covering all affected .gitmodules states and verifying dry-run parity, repository preservation, printed remediations, and successful retries.

[brettheap]

(lane openRepoShape-5, 2026-10-14T23:19Z) @codex review

[sourcery-ai]

Reviewer's Guide

The PR hardens family.py add and remove by validating that the work-tree .gitmodules matches the intended index state before any write, preventing accidental deletion or inclusion of unrelated changes while providing consistent dry-run refusals and remediation guidance, with extensive regression and clone-integrity tests.

Sequence diagram for safe family remove validation

sequenceDiagram
    actor User
    participant CLI as family.py
    participant Git as Git repository
    participant Commit as Commit staging

    User->>CLI: remove or remove --dry-run
    CLI->>Git: _index_records()
    CLI->>Git: _head_oid()
    CLI->>Git: ls-files -m -z
    alt .gitmodules missing or differs from index
        CLI-->>User: Refusal with remediation
        CLI-->>User: No git command that writes was run
    else work-tree copy matches index
        CLI->>Git: deinit / git rm
        CLI->>Commit: _commit_one()
        Commit-->>User: Removal committed
    end

Loading

Sequence diagram for safe family add validation

sequenceDiagram
    actor User
    participant CLI as family.py
    participant Git as Git repository
    participant Commit as Commit staging

    User->>CLI: add or add --dry-run
    CLI->>Git: _checked_mount_path()
    CLI->>Git: _checked_gitmodules_as_staged()
    alt .gitmodules differs from index
        CLI-->>User: Refusal with remediation
        CLI-->>User: No git command that writes was run
    else file state is acceptable
        CLI->>Git: git submodule add
        CLI->>Commit: _commit_one()
        Commit-->>User: Addition committed
    end

Loading

Flow diagram for gitmodules validation outcomes

flowchart TD
    Start[add or remove begins] --> Held{Index or HEAD holds .gitmodules?}
    Held -->|No| Proceed[Continue without validation refusal]
    Held -->|Yes| Exists{Work-tree .gitmodules exists?}
    Exists -->|No and verb is remove| Missing[Refuse gitmodules-missing]
    Exists -->|No and verb is add| GitAdd[Let git submodule add refuse]
    Exists -->|Yes| Match{Work-tree copy matches index?}
    Match -->|No| Unstaged[Refuse gitmodules-unstaged]
    Match -->|Yes| Proceed
    Missing --> Remedy[Restore or resolve .gitmodules, then rerun]
    Unstaged --> Remedy
    Proceed --> Write[Run the command's git writes]

Loading

File-Level Changes

Change	Details	Files
Add a shared preflight check that prevents remove and add from writing an unintended .gitmodules state.	Detect missing, modified, unmerged, and staged-deletion states by inspecting the index, HEAD, work tree, and Git's modified-file result. Refuse before any write operation, consistently in real and dry-run modes, with actionable remediation commands and exit status 2. Allow already-staged changes to proceed, while preserving add's existing delegation for a missing work-tree file and doing nothing when neither index nor HEAD contains .gitmodules. Invoke the check at the appropriate points in remove and add, and document the interaction with dry-run and submodule deinitialization.	scripts/family.py
Add comprehensive regression coverage for .gitmodules consistency and preservation of repository state.	Cover missing, staged-deletion, unmerged, edited, CRLF, and deletion-staged-but-present states for both real and dry-run commands. Verify refusals leave HEAD, index, status, configuration, work-tree bytes, and objects unchanged, then validate printed remediations and resulting commits. Reproduce the recursive-clone failure scenario and confirm the repaired commit preserves remaining submodules. Preserve and test existing behavior for missing .gitmodules during add and for holders whose HEAD no longer contains the file.	tests/test_family_remove_gitmodules.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#407	Prevent remove from committing a .gitmodules deletion or other unstaged change it did not make; refuse before any write when the work-tree copy is missing or differs from the index.	✅
#407	Make remove --dry-run accurately reflect the real command in the affected state, including reporting the refusal rather than implying no registration will be removed.	✅
#407	Have validate-family.py detect a member row whose path has no corresponding .gitmodules section in HEAD.	❌	The PR explicitly leaves this validator change out of scope. A holder whose HEAD already lacks .gitmodules can still pass its gate despite having a member row that cannot be populated by a recursive clone.

Possibly linked issues

• family.py remove, in a holder whose work-tree .gitmodules has been deleted and the deletion not committed, commits that deletion with the member's removal, so every other member's section leaves HEAD while remove exits 0 and the holder's validate-family.py passes, and once pushed a recursive clone exits 0 with each remaining member's directory empty and its own validate-family.py refusing member-uninitialized #407: The PR implements the issue’s proposed refusal: remove stops before writes when .gitmodules is missing or differs from the index, preventing deletion of other members’ registrations.

---

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.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-14T23:22:51.695232Z	b1120c3	Manual request

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b1120c3b81

ℹ️ 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".

[chatgpt-codex-connector]

Reject an untracked .gitmodules before add

When adding the first member to an empty holder, neither the index nor HEAD contains .gitmodules; if an untracked .gitmodules already exists, this early return skips the new protection. git submodule add then appends its section and stages the entire file, so _commit_one silently commits any pre-existing comments or unrelated submodule sections that the user never staged. Distinguish a genuinely absent work-tree file from an untracked one and refuse the latter.

AGENTS.md reference: AGENTS.md:L392-L398

Useful? React with 👍 / 👎.

[brettheap]

(lane openRepoShape-5, 2026-10-15T12:35Z) Fixed in cb159b1, with round 3 in ebe6b51. An untracked .gitmodules, one that neither the index nor HEAD holds, is now refused gitmodules-unstaged by add and remove. The refusal comes before the first write, and the dry run refuses it too.

The remedy says to stage it with git add -f .gitmodules. The -f is there because an operator's global excludes may match the file (#242). The remedy also says to commit it on its own first if the command's commit should carry only its own change. It prints no git commit, because #487's branch check runs later and a printed commit could land on the tracking branch.

Tests in tests/test_family_remove_gitmodules.py:

• test_add_refuses_an_untracked_gitmodules_in_a_holder_with_none covers your case: the first add into an empty holder. It runs once with no excludes and once with core.excludesFile holding *. Each time it types the remedy as printed and checks that add then commits.
• test_remove_refuses_an_untracked_gitmodules_in_a_holder_head_lost_it covers remove.

On b1120c3 the same state committed the file silently, with exit 0.

[chatgpt-codex-connector]

Do not trust ls-files for assume-unchanged entries

If a tracked .gitmodules is marked with git update-index --assume-unchanged and then edited, git ls-files -m returns no path—the local update-index -h describes this flag as “mark files as not changing.” The guard therefore accepts the file, while the later explicit-path commit records its working-tree contents, silently including the unstaged edit alongside the member change. Inspect the index flag and refuse it, or compare the work-tree content and mode directly against the stage-0 entry rather than relying solely on ls-files -m.

AGENTS.md reference: AGENTS.md:L392-L398

Useful? React with 👍 / 👎.

[brettheap]

(lane openRepoShape-5, 2026-10-15T12:35Z) Fixed in cb159b1, with round 3 in ebe6b51. The check no longer trusts ls-files -m alone. It reads the index flags with git ls-files -s -v -z -- .gitmodules.

• Assume-unchanged (tag h): the work-tree copy is compared with the stage-0 blob by content, through git hash-object --path=.gitmodules. An edited copy is refused gitmodules-unstaged, before the first write and in the dry run too.

  • To keep the edit: git update-index --no-assume-unchanged -- .gitmodules, then git add .gitmodules. git add stages nothing while the mark is on.
  • To drop it: git checkout -- .gitmodules.
  • A clean assume-unchanged copy passes, and both verbs commit the right tree.

• Skip-worktree (tag S, or s with both marks) is refused gitmodules-sparse, with git sparse-checkout disable as the remedy that works as typed:

  • by add always, since git submodule add cannot stage it;
  • by remove where the file is missing or is not the index's copy.

  remove still commits a clean, hand-marked skip-worktree file, as it did before family.py remove, in a holder whose work-tree .gitmodules has been deleted and the deletion not committed, commits that deletion with the member's removal, so every other member's section leaves HEAD while remove exits 0 and the holder's validate-family.py passes, and once pushed a recursive clone exits 0 with each remaining member's directory empty and its own validate-family.py refusing member-uninitialized #407.

Tests in tests/test_family_remove_gitmodules.py:

• test_a_work_tree_copy_that_is_not_the_indexs_is_refused, whose assume-unchanged, edited case runs for both verbs and types each printed remedy;
• test_a_clean_gitmodules_behind_a_mark_is_committed_as_before;
• test_an_edit_skip_worktree_hides_is_refused_then_named_as_modified;
• test_a_gitmodules_a_sparse_checkout_leaves_out_is_refused.

Mutants that drop the flag read, the content compare or the .upper() are each caught.

[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

[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 4 hours by commenting @sourcery-ai review. Upgrade to get a review now.