[brettheap]

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

Stacked on #819 (fix/768-gitmodules-values, 233a334e); retarget to main once it lands.

#839's seven lines, in the family holder's two pinned scripts, templates/family-root/scripts/bootstrap.py and templates/family-root/scripts/validate-family.py. Each line was probed on 233a334e first; one already held there (5).

The lines

1. The credential advice beside "not for want of a credential" (fetch_members).

• Before: "If the members are PRIVATE, set SHAPE_LEGS_TOKEN ..." was printed for every failed git submodule update, and then "NOT CHECKED OUT, and not for want of a credential" for every row unfetchable named.
• Now credential_advice prints it only where git asked for a credential. It is not printed where git's init died at a gitlink it has no url for (stopper), or where git read nothing from .gitmodules: a file a merge left unmerged, a value git dies on, or a file it cannot parse. Git fetched nothing in those states.
• Beside a denial, the advice reads "If the members the next line does not name are PRIVATE". This is the Direction's "both kinds" case.
• One step past the Direction's literal rule: where every member is checked out and a leg's fetch failed for want of a credential, no row is denied and the advice stands. The token's name is SHAPE_LEGS_TOKEN, and the legs are what it is for.
• Tests: test_no_advice_is_printed_beside_its_denial covers nine states, every reason unfetchable gives where git exits non-zero (no section, the variant, no url, unmerged, a refused value, an unparsable file, a stray, a hand mount, a gitlink outside members/). It asserts bootstrap's own stderr lines exactly: the fetch line, the denial, the failure, and no advice. test_the_credential_s_skip_prints_the_advice_alone (three states), test_a_leg_the_credential_s_keeps_the_advice_with_every_member_out and test_beside_a_denial_the_advice_is_for_the_members_it_does_not_name pin the other side.

2. A member's refusal and its finding both read make bootstrap exited 2 (run_member_target). Chosen: tell them apart, by reading make's own Error <code> line.

• Measured on GNU Make 4.3: make exits 2 for a command exiting 1, 2, 3 or 126, and 2 again for a failure of its own (No rule to make target). It prints make: *** [Makefile:N: bootstrap] Error <code> (make[1]: under another make) for a failing command, and no such line for its own failure.
• make siblings runs the member's scripts/bootstrap.py directly and prints "its own bootstrap exited N". The direct run was not chosen: --members-make validate, which the holder's own make validate runs, has no script to call, and the docs say the holder runs each member's own make bootstrap.
• So make still runs. run_make passes make's stderr on as it comes and reads the code from the last Error <code> line.
• What the person reads now, at the end of the run:
  • a member's finding: IRRS (make bootstrap: the member's command exited 1), under the member's own bootstrap FAILED: ... and make's Error 1;
  • a member's refusal: IRRS (make bootstrap: the member's command exited 2), under its REFUSED ... and make's Error 2;
  • make's own failure: IRRS (make bootstrap exited 2 and printed no `Error <code>` line for a command of the member's: make's own message above says what failed), under make's own message.
  • A make speaking another language, or a command killed by a signal, gets the third line too, which is still true there.
• Test: test_a_finding_a_refusal_and_make_s_own_failure_are_told_apart, for bootstrap and for validate: four members, a finding, a pass, a refusal and no rule, with the final line asserted exactly and the members' and make's own lines asserted on stderr. tests/test_bootstrap_spelling.py's make line follows the new text.

3. The git rm --cached that refuses over a checkout of its own.

• Reproduced on the gate's own state, as the Direction asks: mkdir Members && git mv members/IRRS Members/IRRS, staged, then a commit (or only an untracked file) in the moved checkout.
• In that state the gate does not print the wrong-case move. It prints three findings in every mode:
  • member-gitlink-unnamed for Members/IRRS, a second path to the member, whose line is git rm --cached -- Members/IRRS;
  • gitmodules-path-unnamed, which prints no command;
  • member-gitlink-removal-staged for the row's own gitlink.
• Typed as printed, the git rm refused on git 2.43: error: the following file has staged content different from both the file and the HEAD, exit 1. It refused with the commit, with the untracked file alone, and with both.
• _forced reads the state git refuses: the index stages the gitlink at a commit HEAD does not record there, and a .git entry is on disk at it. There the line takes -f, and the finding says what -f does, in family.py add: the move back of a member's gitlink points its section of .gitmodules back, sets aside first what the index holds in its way and puts back a member whose removal is staged, its closing mv is guarded and leaves a checkout to delete by hand, and --sparse follows the sparse rules, so no printed line leaves a section naming another spelling or un-registers a member (#761) #828's words: "-f takes out a staged gitlink whose checkout has moved on or holds a file git does not track, which git rm otherwise refuses, and with --cached touches no file".
• Where HEAD records nothing at the variant, the finding also says that nothing of it is left to commit. The row's own removal-staged lines put the member back, and a commit between the two would have un-registered it.
• The wrong-case move's own git rm -r --cached --ignore-unmatch takes -f the same way. That state is a committed variant whose checkout moved on, staged, with an untracked file in it.
• Tests: test_a_git_mv_staged_with_work_in_the_checkout_is_undone_as_printed (commit, untracked) types every printed line as printed, then finds nothing left to commit, the gate passing in every mode and HEAD recording the member at its pin. test_a_checkout_moved_on_at_the_variant_is_moved_with_f pins the move's -f. test_a_staged_second_path_with_nothing_on_disk_takes_no_f checks there is no -f where nothing is on disk.

4. The guard that follows a link, and its missing nothing-there clause (_wrong_case_move, now _guarded_move). Comment 6237599648 is the same fault and is fixed here too.

• The guard is now [ ! -L V ] && { [ ! -e V ] || [ V -ef R ] || [ -e V/.git ]; } || mv -- V ../<V with - for />, the issue's form. The sentence says it "moves nothing where nothing is there, where the two names are one directory or where it holds a .git entry, a checkout, and moves a symbolic link there wherever it points, the link and never what it points at", typed "in a POSIX shell (Git Bash on Windows)" as family.py add: a section astray beside its gitlink, a variant of a row's path under members/, a link or checkout in a move's way, an unmerged entry and a staged or unmerged .gitmodules are named first, so each remedy types clean (#840) #865's text says.

• Typed in dash (this host's /bin/sh) and bash 5.2 on the issue's seven layouts:

  at Members/IRRS	base guard	family.py add: a section astray beside its gitlink, a variant of a row's path under members/, a link or checkout in a move's way, an unmerged entry and a staged or unmerged .gitmodules are named first, so each remedy types clean (#840) #865's guard	this guard
  nothing	mv: cannot stat, exit 1	kept, exit 0	kept, exit 0
  a link to nowhere	moved	moved	moved
  a link to the row's directory	kept	kept	moved
  a link to a clone (a .git file)	kept	kept	moved
  Members a link to members	kept	kept	kept
  an empty directory apart	moved	moved	moved
  a checkout apart	kept	kept	kept

• family.py add: a section astray beside its gitlink, a variant of a row's path under members/, a link or checkout in a move's way, an unmerged entry and a staged or unmerged .gitmodules are named first, so each remedy types clean (#840) #865's family.py guard still keeps a link at the variant. scripts/family.py at family.py add: a section astray beside its gitlink, a variant of a row's path under members/, a link or checkout in a move's way, an unmerged entry and a staged or unmerged .gitmodules are named first, so each remedy types clean (#840) #865's f7072d08 (_closing_sentences) prints [ ! -e V ] && [ ! -L V ] || [ V -ef R ] || [ -e V/.git ] || mv ..., which answers as base does on the two link rows above. The validator's text and family.py's therefore differ in the link clause until family.py add: a section astray beside its gitlink, a variant of a row's path under members/, a link or checkout in a move's way, an unmerged entry and a staged or unmerged .gitmodules are named first, so each remedy types clean (#840) #865 takes the same form. This pull request does not edit scripts/family.py; family.py add: a section astray beside its gitlink, a variant of a row's path under members/, a link or checkout in a move's way, an unmerged entry and a staged or unmerged .gitmodules are named first, so each remedy types clean (#840) #865's re-review is asked to rule on it.

• Tests: test_a_link_or_nothing_at_the_variant_is_typed_as_printed covers link-to-the-row, link-to-a-clone, a dangling link and nothing, each in sh and in bash, in w840's state. After the move's lines and their commit, the guard is typed as printed: a link is moved with its target untouched, nothing exits 0, the gate passes in every mode and family.py add of SPARE goes through. test_the_guard_moves_a_link_always_and_nothing_where_nothing_is types the gate's own _guarded_move on all seven layouts in both shells. The N-2 test's three layouts in tests/test_validate_family_remedies_typed.py pass unchanged. variant_refused in tests/test_family_head_lost_gitmodules.py follows the new text.

5. Comment 6237354577 (a), bootstrap's gitmodules_sections reading only the work tree's file. This already held at 233a334e: #819's gitmodules_source reads the work tree's file, then :.gitmodules, then HEAD:.gitmodules. #819's own test_where_the_work_tree_has_no_gitmodules_git_reads_the_index_s_or_head_s pins it, and test_the_credential_s_skip_prints_the_advice_alone's index-read and HEAD-read cases ask it again here. No code change.

6. Comment 6237354577 (b), member-gitmodules-absent's git config -f lines where the work tree's .gitmodules is gone.

• Measured on 233a334e: OTHER's section was taken out of HEAD's file and the work tree's file deleted (or its deletion staged). The finding's two lines made a file holding OTHER's section alone, and staged and committed, HEAD lost MEMBER's section: a second round.
• Now the first of the url-absent and absent findings (member-gitmodules-url-absent, member-gitmodules-absent) puts it back first, from where git reads it (git checkout -- .gitmodules, or git checkout HEAD -- .gitmodules where the index holds none), as validate-family.py: the wrong-case move back mends .gitmodules, adds --sparse and moves an entry in the way aside, a row's staged removal and a url-less section are findings, and two refusals name a cure that works (#762) #820's member-gitmodules-url-absent does.
• No later url-absent or absent finding puts it back. Measured on 233a334e: with two url-less sections, the second member-gitmodules-url-absent put the file back over the url the first had written, and an absent section before a url-less one was lost the same way.
• Other findings that write or put back the file are not ordered with these two. rv876 measured two such states: the wrong-case move's git config -f .gitmodules line, and member-gitlink-removal-staged's git checkout HEAD -- .gitmodules. Each behaves the same on 233a334e, and both are filed in the family root's validate-family.py and bootstrap.py: with the work tree's .gitmodules gone, three remedies undo each other or refuse; a git mv'd checkout is never named; bootstrap says ok beside git's exit 128 #879 (C1, C2).
• No finding puts it back after a gitmodules-value-refused that already put HEAD's back (refused).
• Tests: test_a_section_absent_from_head_is_written_into_the_file_put_back (deleted, deletion staged; every mode; typed and committed, HEAD keeps MEMBER's section). test_two_findings_that_write_the_file_put_it_back_once (two url-less, absent then url-less, url-less then absent). test_a_value_finding_that_put_head_s_back_is_not_put_back_over.

7. Comment 6237599648, the move printed unguarded for an absent variant. Line 4's nothing-there clause: the nothing rows above.

Tests and runs

• run9s.sh with SHARDS=3 at 77f8a6c8, on 95 test files: the issue's Verify files, every test file that runs or reads either script (git grep -l 'bootstrap.py\|validate-family' -- tests), tests/test_family.py and the trio (test_repo_hygiene.py, test_agents_md_codes.py, test_cli_reference.py):

  • git 2.43.0: 6335 passed, 59 skipped, rc 0 (shards 1596 + 5, 2203 + 27, 2536 + 27).
  • git 2.47.1: 6335 passed, 59 skipped, rc 0, the same in each shard.
  • The 59 skips are the same on both versions, and none is in the new file. 27 wait on shape-doctor.py and siblings.py: a .gitmodules section git ignores ([submodule ""], a .. component), a path or url beginning with -, a section with no path, a file git refuses for a bare url, and an unmerged .gitmodules name no mount url, NEXT line or clone, where git says No url found and exits 128 (#733) #775 (siblings.py's shape-doctor.py and siblings.py: a .gitmodules section git ignores ([submodule ""], a name with a .. component, a url beginning with -) is still read as a member, where git says No url found and exits 128 #733 rules). The rest are templates that already quote a value (22), Windows or a disk that ignores case (5), and empty parameter sets (5).

• On the predicted squash of bootstrap.py and validate-family.py: a .gitmodules value git dies on as it reads the file fails make bootstrap naming the key, and the family gate names it in every mode, so update = None no longer ends family bootstrap ok (#768) #819 and this branch onto main 33419925 (9aca0f4d; see the merge checks), git 2.43.0, run9.sh. Main has changed tests/test_bootstrap_spelling.py and tests/test_family.py since 233a334e.

  • tests/test_family.py: 112 passed.
  • tests/test_bootstrap_spelling.py, the new file, test_family_head_lost_gitmodules.py, test_family_docs_one_spelling.py, test_bootstrap_after_bump.py, test_validate_family_remedies_typed.py, test_family_bootstrap_gate_order.py, test_pinned_copy_text.py and the trio: 656 passed.

• New tests: tests/test_family_holder_bootstrap_texts.py, 48 cases, all passing at 77f8a6c8.

• On 233a334e's two scripts, with this branch's tests: 27 fail and 14 error. The 14 are the seven-layout guard test, which calls the gate's _guarded_move, a function 233a334e does not have. 7 pass there by design: the three credential-skip cases (line 1's other half, and comment (a), which held), the leg case, url-less-then-absent, the value-refused order and the no--f case. Each guards the fix against going too far.

• Mutants, one or more per new hunk, all 20 caught:

  • the advice always printed;
  • the advice never printed;
  • the advice's "does not name" wording dropped;
  • the advice kept only for rows the credential leaves out (the Direction's literal rule);
  • the exit code always make's;
  • make's stderr not passed on;
  • the first Error line read in place of the last;
  • the STRAY -f dropped;
  • the STRAY's "nothing left to commit" turned round;
  • the move's -f dropped;
  • _forced ignoring HEAD;
  • _forced ignoring whether a checkout is there;
  • the -L dropped;
  • the nothing-there clause dropped;
  • the -f clause dropped from the text;
  • the absent finding's put-back dropped;
  • every url-less finding putting back;
  • every absent finding putting back;
  • the value-refused flag ignored;
  • _check_members passing no flag.

  _forced ignoring HEAD is caught by the existing test_a_checkout_at_the_variant_is_kept_and_handled_as_the_text_says.

Pinned copies, docs and codes

• Both files are in FAMILY_COPIED_VERBATIM, so each holder takes one upstream-changed row per file.
• No committed digest or byte comparison names either file: templates/family-root/contracts/shape-pin.yaml holds only placeholders, and git grep finds no sha256 of them in tests/.
• docs/cli.md takes only validate-family.py's first docstring line, which is unchanged.
• No finding code or refusal code is new or renamed. AGENTS.md, templates/family-root/AGENTS-shape.md and both READMEs say nothing these lines change. Each script's module docstring says what its lines now do.
• Cognitive complexity, counted by hand:
  • fetch_members drops from 15 to 14, since its failure report moved to fetch_incomplete.
  • _check_gitlinks_registered was over 15 on base (18) and drops to 17; its def line keeps its first line.
  • _check_members stays at 14.
  • Every new function is at 6 or under.
• The new tests share their helpers within the file, and no assert joins conditions with and.

Merge checks

git merge-tree --write-tree of 77f8a6c8, fetched 2026-10-18 at 05:02Z:

• origin/main 33419925: clean. Main has moved past d7befb91, and family-root README.md and AGENTS-shape.md, and AGENTS.md: the holder's docs say its parent folder is named EXACTLY after its repository, case included, on every disk, and the never to hand-edit the members: block names the row repair validate-family.py asks for, whose commit is the person's #822 landed in it as 6fbf3301.
• family.py add: a section astray beside its gitlink, a variant of a row's path under members/, a link or checkout in a move's way, an unmerged entry and a staged or unmerged .gitmodules are named first, so each remedy types clean (#840) #865 f7072d08: clean.
• shape-doctor.py, family.py and the family root's bootstrap.py and validate-family.py: a holder's members: that is not a list, or holds an entry that is not a mapping, is said in the gate's words, so the doctor's members row waits on the family row instead of calling the family empty, add, bump, remove, make bootstrap and make siblings refuse it by name where each ended in a TypeError, read no members or committed the entry away, and --no-members finds the entry (#864) #870 c0d8dd16 (lane 4's round 1; it edits both scripts): clean, with no conflicting hunk.
• family-root README.md and AGENTS-shape.md, and AGENTS.md: the holder's docs say its parent folder is named EXACTLY after its repository, case included, on every disk, and the never to hand-edit the members: block names the row repair validate-family.py asks for, whose commit is the person's #822's head 0519f9fd: conflicts in both scripts, and 233a334e (bootstrap.py and validate-family.py: a .gitmodules value git dies on as it reads the file fails make bootstrap naming the key, and the family gate names it in every mode, so update = None no longer ends family bootstrap ok (#768) #819's head) conflicts with it in validate-family.py too.
  • This is a criss-cross with two merge bases, b9b70692 and 5afb24b9. Since b9b70692, family-root README.md and AGENTS-shape.md, and AGENTS.md: the holder's docs say its parent folder is named EXACTLY after its repository, case included, on every disk, and the never to hand-edit the members: block names the row repair validate-family.py asks for, whose commit is the person's #822 changes neither script.
  • family-root README.md and AGENTS-shape.md, and AGENTS.md: the holder's docs say its parent folder is named EXACTLY after its repository, case included, on every disk, and the never to hand-edit the members: block names the row repair validate-family.py asks for, whose commit is the person's #822 has landed, and its content is in main, which merges clean.
• Predicted squash of bootstrap.py and validate-family.py: a .gitmodules value git dies on as it reads the file fails make bootstrap naming the key, and the family gate names it in every mode, so update = None no longer ends family bootstrap ok (#768) #819 and this branch onto main 33419925 (9aca0f4d, tree ee2ae53c): clean against family.py add: a section astray beside its gitlink, a variant of a row's path under members/, a link or checkout in a move's way, an unmerged entry and a staged or unmerged .gitmodules are named first, so each remedy types clean (#840) #865, shape-doctor.py, family.py and the family root's bootstrap.py and validate-family.py: a holder's members: that is not a list, or holds an entry that is not a mapping, is said in the gate's words, so the doctor's members row waits on the family row instead of calling the family empty, add, bump, remove, make bootstrap and make siblings refuse it by name where each ended in a TypeError, read no members or committed the entry away, and --no-members finds the entry (#864) #870 and family-root README.md and AGENTS-shape.md, and AGENTS.md: the holder's docs say its parent folder is named EXACTLY after its repository, case included, on every disk, and the never to hand-edit the members: block names the row repair validate-family.py asks for, whose commit is the person's #822's head. family-root README.md and AGENTS-shape.md, and AGENTS.md: the holder's docs say its parent folder is named EXACTLY after its repository, case included, on every disk, and the never to hand-edit the members: block names the row repair validate-family.py asks for, whose commit is the person's #822's head leaves the tree unchanged.

Left out

• After line 3's lines, typed as printed in the git mv state, the checkout git mv moved stays at Members/IRRS, untracked. Measured: the gate passes in every mode, and family.py add then refuses it, family-members-dir-variant, with mv -- Members/IRRS <new name>. Neither the STRAY finding nor member-gitlink-removal-staged says what that directory is, or that its commits are in the member's store. The wrong-case move's closing sentences say both. Filed in the family root's validate-family.py and bootstrap.py: with the work tree's .gitmodules gone, three remedies undo each other or refuse; a git mv'd checkout is never named; bootstrap says ok beside git's exit 128 #879 (C4).
• scripts/family.py's guard: see line 4.

Closes #839

🤖 Generated with Claude Code

Summary by Sourcery

Make the family holder’s bootstrap and validation diagnostics accurately explain failures and provide remedies that execute safely across affected repository states.

Bug Fixes:

• Report credential guidance only for fetch failures where Git actually requested credentials, and clarify which members the guidance applies to.
• Distinguish member command failures and refusals from failures in make itself by reporting the command exit code when make provides it.
• Make generated git remediation commands handle staged checkouts, symbolic links, and absent paths correctly, including forcing gitlink removal when required.
• Preserve existing .gitmodules sections by restoring a missing work-tree file only before the first remediation that rewrites it.

Enhancements:

• Improve validation remediation text so printed commands accurately describe their effects and the conditions under which they are safe.

Tests:

• Add comprehensive coverage for credential advice, make exit-code reporting, forced gitlink removal, guarded moves, and .gitmodules restoration ordering.

[brettheap]

(lane openRepoShape-5, 2026-10-18T05:11Z) @codex review

[sourcery-ai]

Reviewer's Guide

Updates the family-root bootstrap and validation scripts so credential advice, Make exit reporting, gitlink removal commands, guarded moves, and .gitmodules remediation text accurately reflect the underlying Git and Make states, with extensive typed-command and integration coverage.

Sequence diagram for member bootstrap exit reporting

sequenceDiagram
    participant Bootstrap as bootstrap.py
    participant Make as make
    participant Member as member command

    Bootstrap->>Make: run_make(make, target, path)
    Make->>Member: run target
    Member-->>Make: exit code
    Make-->>Bootstrap: stderr chunks and make exit code
    Bootstrap->>Bootstrap: parse last MAKE_ERROR_RE line
    alt Error <code> line found
        Bootstrap-->>Bootstrap: report member's command exit code
    else no Error <code> line
        Bootstrap-->>Bootstrap: report make's own failure
    end

Loading

Flow diagram for credential advice after fetch failure

flowchart TD
    A[git submodule update] --> B{git exits non-zero}
    B --> C[unfetchable and module state analysis]
    C --> D{stopper, unmerged, refused, or unreadable?}
    D -->|yes| E[print fetch failure only]
    D -->|no| F{members denied as not credential-fixable?}
    F -->|yes| G[print advice for members the denial does not name]
    F -->|no| H[print credential advice for the members]
    E --> I[report not checked out members]
    G --> I
    H --> I

Loading

Flow diagram for guarded variant move and gitlink removal

flowchart TD
    A[variant path] --> B{symbolic link?}
    B -->|yes| C[mv variant to outside holder]
    B -->|no| D{missing, same directory, or checkout .git entry?}
    D -->|yes| E[do not move]
    D -->|no| C
    C --> F{git rm --cached needs force?}
    E --> F
    F -->|yes| G[print git rm --cached -f]
    F -->|no| H[print git rm --cached]
    G --> I[commit index-only removal]
    H --> I

Loading

Flow diagram for preserving .gitmodules sections during remediation

flowchart TD
    A[inspect HEAD and work-tree .gitmodules] --> B{work-tree file absent?}
    B -->|yes| C[put file back before first remediation]
    B -->|no| D[keep current file]
    C --> E[write first missing section or URL]
    D --> E
    E --> F[clear put-back action]
    F --> G[process later findings without restoring again]

Loading

File-Level Changes

Change	Details	Files
Restrict credential guidance to fetch failures where Git actually requested credentials, while preserving precise denial wording and fetch diagnostics.	Extract fetch failure reporting and credential-advice selection into dedicated helpers. Suppress advice for missing, invalid, unreadable, or otherwise uninitialized .gitmodules/gitlink states. Clarify that denial-adjacent advice applies to members not named by the denial. Add exhaustive tests for credential-requested and non-requested failure states.	templates/family-root/scripts/bootstrap.py tests/test_family_holder_bootstrap_texts.py
Distinguish a member command’s exit status from GNU Make’s exit status by streaming stderr and parsing the final Error <code> diagnostic.	Add make execution that forwards stderr incrementally and retains a bounded diagnostic tail. Report the member command’s code when Make identifies it, and explicitly describe Make’s own or unrecognized failures otherwise. Update spelling expectations and cover findings, refusals, successful commands, and missing targets for bootstrap and validate.	templates/family-root/scripts/bootstrap.py tests/test_bootstrap_spelling.py tests/test_family_holder_bootstrap_texts.py
Make validation remedies accurately handle staged gitlink moves, including when git rm --cached requires -f, and print the corresponding command rationale.	Detect forced removal from index/HEAD state plus an on-disk checkout. Add -f to stray and wrong-case remediation commands only when required, with explanatory text. Test committed and untracked work in moved checkouts, no-worktree cases, and preservation of member commits.	templates/family-root/scripts/validate-family.py tests/test_family_head_lost_gitmodules.py tests/test_family_holder_bootstrap_texts.py
Replace the wrong-case move guard with a POSIX-shell-safe guard that is no-op for absent paths and always moves symbolic links.	Check -L before dereferencing existence/equivalence tests, while retaining protections for the row directory and checkouts. Add the explicit nothing-there clause and document Git Bash/POSIX-shell behavior. Type the emitted command in dash and bash across link, checkout, directory, and absent-path layouts.	templates/family-root/scripts/validate-family.py tests/test_family_head_lost_gitmodules.py tests/test_family_holder_bootstrap_texts.py
Preserve the work-tree .gitmodules contents while emitting multiple remediation findings that modify it.	Restore the file from the source Git reads before the first config-writing finding when it is absent. Track that restoration across findings so later remedies do not overwrite earlier changes or restore after a value-refused finding. Add tests for absent, URL-less, and value-refused combinations and commit the printed remedies.	templates/family-root/scripts/validate-family.py tests/test_family_holder_bootstrap_texts.py

Possibly linked issues

• the family root's bootstrap.py and validate-family.py: the credential advice prints beside "not for want of a credential", a member's refusal and finding both read make bootstrap exited 2, and the wrong-case move's git rm --cached may refuse #839: The PR implements the issue’s requested fixes in both pinned scripts, including credential advice, exit-code reporting, git rm -f, and guarded moves.

---

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-18T05:17:22.577709Z	77f8a6c	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: 77f8a6c894

ℹ️ 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]

Check preserved HEAD sections before restoring .gitmodules

When the work-tree .gitmodules is absent and HEAD contains both a missing/url-less member section and an unrelated section whose path no member names, _check_gitmodules_named sees the empty work tree and never reports the unrelated section. This restoration then copies HEAD wholesale and repairs only the member sections, so following every printed command and committing leaves the unrelated section behind; I reproduced this with --no-members, a deleted .gitmodules, a URL-less members/IRRS, and submodule.weird.path=vendor/IRSS, after which the next run newly reported gitmodules-path-unnamed. Inspect the preserved HEAD sections in the same run or include their remediation, since the repository procedure relies on each finding naming its exact fix.

AGENTS.md reference: AGENTS.md:L235-L240

Useful? React with 👍 / 👎.

[brettheap]

Independent review of #876 (#839) at 77f8a6c8, for lane openRepoShape-5: READY

I found no MUST. The one SHOULD is a fix to the PR body only, so no code changes and W856 has nothing to merge for it. The rest are NITs and new issues to file. I changed no branch, pushed nothing and posted nothing. $Q below is /tmp/claude-1000/-workspace-projects-openRepoShape/50cede29-ab82-4ca3-9f67-d98e1a58c8d2/scratchpad; my logs are in $Q/v118/rv876/. While I worked, main moved from 3bf2d3c9 to acc5a7bf.

Findings, most severe first

SHOULD 1 (PR body only): two claims in line 6 are false in two states I measured. Both states behave the same on 233a334e, so neither is a regression.

• The body says "Now the first finding that writes the file puts it back first" and "No later finding puts it back". The code's own docstring (validate-family.py:2282-2290) is narrower: "the first of these findings". That narrower claim is true.
• P5b. The state: OTHER recorded only at Members/IRSS, with HEAD's section giving that path; MEMBER's url gone; the work tree's .gitmodules deleted.
  • --no-members prints the wrong-case move first. Its lines are git config -f .gitmodules submodule.members/IRSS.path members/IRSS and git add -- .gitmodules (_move_context, :689-704).
  • member-gitmodules-url-absent follows with git checkout -- .gitmodules, which restores the one-key file the move has just staged.
  • Typed as printed and committed, HEAD's file holds only [submodule "members/IRSS"] path = members/IRSS and [submodule "members/IRRS"] url = …. The gate then reports member-gitmodules-absent and member-gitmodules-url-absent in every mode.
  • P5e. In members mode the move is a refusal printed alone. Typed, it leaves HEAD with one key, so every other section is lost.
• P5d. The state: git rm --cached members/IRRS, the file gone, and OTHER's section gone from HEAD.
  • The absent finding puts the file back and writes OTHER's section.
  • Then member-gitlink-removal-staged prints git checkout HEAD -- .gitmodules. _put_back_lines (:2434-2435) reads "section taken" from the empty work tree, and that line wipes OTHER's section.
  • The commit then fails with nothing to commit, and member-gitmodules-absent comes back.
• Fix: narrow the body to "the first of the url-absent and absent findings". File the two code faults as new issues (candidates C1 and C2 below).

NIT 1 (line 3): _forced (validate-family.py:717-738) is not exactly git's rule. I mapped 15 disk layouts × 3 index states × both git rm forms. The verdicts are identical on 2.43 and 2.47.1 ($Q/v118/rv876/p3/w243.txt, w247.txt).

• Matches git on every checkout layout:
  • a .git file at another commit, with an untracked file, or with a modified file;
  • a .git directory, clean or with an untracked file;
  • a checkout moved there by a plain mv;
  • a link to the row's checkout;
  • nothing on disk, or an empty directory.
• -f printed where git needs none (4 rows): a clean checkout at the staged commit. This includes the plain git mv state with nothing in the checkout. It loses no work.
• -f missing where git refuses (14 rows):
  • a staged gitlink over a non-empty directory without .git, a regular file, or a dangling link (git exits 1);
  • a broken .git file with the index matching HEAD (git exits 128, not a git repository).
  • At gate level (P3, no-dot-git), the gate printed git rm --cached -- Members/IRRS with no -f, and typed it exited 1.
• Why NIT and not MUST. By the brief's rule as written this is a MUST. I grade it NIT because:
  • each state needs a hand-made object at the variant; no git command or printed remedy makes one;
  • git's own error says (use -f to force removal);
  • base prints the same line.
• If you hold the rule literally, the fix is a check for anything at the path that is not an empty directory, in place of the .git check.
• In every row where git refused, -f touched no file. In P3b the untracked file stayed, and the checkout's commit was still in .git/modules/members/IRRS.

NIT 2 (line 2): what piping make's stderr changes. Measured on a pty with $Q/v118/rv876/p2/ptyrun.py.

• Prompts and progress: fine, not a MUST. Continue? showed at 0.17 s, before the answer; progress dots arrived 1 s apart.
• The member no longer has a terminal on stderr. Base: "stderr is a terminal"; head: "stderr is NOT a terminal". So git's default progress meters and any isatty(2) check go quiet. Colours that key on stdout are unchanged.
• Line order is lost. A member's stdout and stderr lines no longer come out in order. Base alternated out 1|err 1|… three times out of three. Head once printed all six out lines before any err line.
• A background child now holds the run. A recipe sleep 6 & … takes 6.1 s on head against 0.1 s on base. run_make reads until EOF, so a daemon that inherits stderr blocks the holder's make bootstrap until it exits.
• Windows, read only: a member's Python writes stderr to the pipe in the ANSI code page, and passed_on writes those bytes to a console that expects UTF-8, so non-ASCII text comes out garbled.
• None of this is in the docstring. Document it, or stop reading once make has exited and nothing is waiting in the pipe.

NIT 3 (line 2): three cases where the final line misleads. All under the holder's make, GNU Make 4.3.

• A recursing member. With $(MAKE) inner and the inner command exiting 1, the line reads "the member's command exited 2", which looks like a refusal. No shape template's bootstrap or validate recurses, but an adopted root may keep its own Makefile.
• make -k with more than 4 KiB on stderr after the Error line. The line says make "printed no Error <code> line", which is false: the line is there, just outside the last 4096 bytes.
• A signal under dash. dash prints Killed and exits 137, so the line reads "exited 137". The run_make docstring says a signal leaves make's code alone.

NIT 4 (tests): my mutants. 7 of 13 are caught; 6 survive the new file ($Q/v118/rv876/mut/summary.txt).

• R1, named[0]: survives. No test makes two Error lines. Also, w839's "M6, first Error line" was really named[0] + 1, so it does not test the first line.
• R2, the regex without $: survives, but it is nearly equivalent to the original.
• R7, sink.flush() dropped: survives 253 tests. On the pty that mutant hid Continue? until after the answer, which is the brief's MUST scenario, and no test pins it.
• R8, regex anchored on ^make: : survives. The line-2 test never runs under a parent make, though make bootstrap always prints make[1]:.
• R10 and R11, credential_advice ignoring unreadable or refused: both survive. In all nine no-advice states, stopper alone drops the advice.
  • With every member checked out and a value git refuses (or an unparsable file), the mutants print the advice right beside "NO MEMBER UPDATED, and not for want of a credential". That is line 1's own contradiction. Head is right there (probe5-*.log).
• Caught: R3, R4, R5, R5b, R6, R9 and R12.
• A docstring claim with no assert: test_a_git_mv_staged_with_work_in_the_checkout_is_undone_as_printed says "the checkout's commit is still in the member's store" but never checks it. It is true: I measured it in P3b.

NIT 5 (line 4, the link clause): scripts/family.py in #865 should carry it, not this PR.

• At f7072d08, _closing_sentences (:5121) still prints [ ! -e V ] && [ ! -L V ] || [ V -ef R ] || [ -e V/.git ] || mv.
• In dash and bash that guard keeps both a link to the row and a link to a clone.
• the family root's bootstrap.py and validate-family.py: the credential advice and a member's exit code say what happened, and the gate's git rm --cached, guarded mv and section lines type as printed where they failed (#839) #876's validator text is right. Each PR should carry its own text's half, as the family root's bootstrap.py and validate-family.py: the credential advice prints beside "not for want of a credential", a member's refusal and finding both read make bootstrap exited 2, and the wrong-case move's git rm --cached may refuse #839's Direction says ("in both texts"; the family.py half "can be fixed in family.py add: six states its case-spelling remedies still miss after #828, from a section that names a variant beside a correct gitlink to a Members that is a link to a directory outside the holder #840's own pull request").

The added item: RV877's O1

• Measured at both heads, on both gits. Holder with both members checked out, a bad line branch = "unterminated committed after the first section of HEAD's .gitmodules, and the work tree's copy gone.
  • Git's update exits 128: No url found for submodule path 'members/B'.
  • Bootstrap prints the advice, then family bootstrap ok. This is the same at 77f8a6c8 and 233a334e, on 2.43 and on 2.47.1.
• It is older than the family root's bootstrap.py and validate-family.py: the credential advice and a member's exit code say what happened, and the gate's git rm --cached, guarded mv and section lines type as printed where they failed (#839) #876. the family root's bootstrap.py and validate-family.py: the credential advice and a member's exit code say what happened, and the gate's git rm --cached, guarded mv and section lines type as printed where they failed (#839) #876 only made the advice conditional.
• The causes:
  • unfetchable returns before init_stop when no row is left (bootstrap.py:758-759), so stopper is None.
  • git config --blob refuses the whole blob, while git's own reader takes the first section, so gitmodules_sections comes back empty and not unreadable. unreadable is set only for a work tree file.
  • The ok follows bootstrap.py and validate-family.py: family bootstrap ok with no member checked out, an earlier row's member-uninitialized hiding a case-variant refusal, and member-gitlink-unnamed with no command under --no-members #648's rule that a run with every member checked out passes. the family root's bootstrap.py and validate-family.py: the credential advice prints beside "not for want of a credential", a member's refusal and finding both read make bootstrap exited 2, and the wrong-case move's git rm --cached may refuse #839's "Not here" already lists the blob syntax error as a known limit.
• Ownership: not bootstrap.py and validate-family.py (family root): a .gitmodules in the index or HEAD with one bad line is read as no sections where git reads those before it, so a no-credential run fails wrongly, and the value rules are written twice #856 or bootstrap.py: a .gitmodules blob with a line git cannot parse is read up to that line, as git reads it, and a parity test holds the value rules both family scripts write to one answer (#856) #877. bootstrap.py: a .gitmodules blob with a line git cannot parse is read up to that line, as git reads it, and a parity test holds the value rules both family scripts write to one answer (#856) #877 inherits it.
• Rating: a new issue, not a fix to the family root's bootstrap.py and validate-family.py: the credential advice and a member's exit code say what happened, and the gate's git rm --cached, guarded mv and section lines type as printed where they failed (#839) #876. The fix would be to ask init_stop even when no row is left, and to fail on a stop there.

Checked and true

Line 1: the credential advice. Probe output: $Q/v118/rv876/p1/probe1-243.txt and probe1-247.txt.

• In every state where the advice is now dropped, git's init dies before any clone starts. Exit 128 and no clone attempt, on both gits, in each of:
  • a url-less gitlink after a private member (with or without submodule.active=.), and one before it;
  • a value git refuses;
  • an unparsable file;
  • a stray gitlink;
  • an unmerged .gitmodules.
• A control with no fault made 3 clone attempts, so the probe does see clones.
• With every member checked out and a leg refused for want of a credential, the advice stands. The test pins that.
• The token is scrubbed: fetch_incomplete replaced it with <redacted> in git's text. No other line prints a URL.

Line 2: the make cases. Under the holder's make bootstrap:

• exit 1, exit 2 and exit 3 read "the member's command exited 1/2/3";
• "no rule" gets the third line;
• kill -TERM $$ makes make print Terminated, and the third line follows;
• an ignored error then success is no failure;
• an ignored error then a missing prerequisite gets the third line (the (ignored) line has no ***);
• make -j2 prints a jobserver warning per member, as base does (11 of 11).
• Make's stdout is byte-identical between base and head (1122 bytes).
• No shape template's bootstrap or validate target recurses (templates/assembly-root/Makefile:44-50, family-root/Makefile:74-83).

Line 4: the guarded mv.

• Run on the issue's 7 layouts, in dash and bash, with 3 spellings of the name (plain, a space, $'"&;), and with ../Members-

  ` either absent or an existing directory.

• Exit 0 everywhere, and the behaviour matches the sentence.
• shlex.quote quotes every use of the name.
• When ../Members-<P> is an existing directory, mv moves the variant inside it. Base's target was the same, so this is as validate-family.py and validate-pins.py: a staged gitlink removal says to commit it, member-gitlink-absent puts the gitlink back by hand, the variant mv is guarded, and a pin row with no path: is shape-pin-row (#681) #747 and family.py add: the move back of a member's gitlink points its section of .gitmodules back, sets aside first what the index holds in its way and puts back a member whose removal is staged, its closing mv is guarded and leaves a checkout to delete by hand, and --sparse follows the sparse rules, so no printed line leaves a section naming another spelling or un-registers a member (#761) #828 left it.

Line 3: what is left to commit. "Nothing of it is left to commit" holds in the clean, commit and untracked git mv states: nothing is staged after typing. Where HEAD records another commit at the variant, the finding says "commit that", and after the commit the gate passes in every mode.

Line 6: three findings that each write .gitmodules (P5a). A value git refuses in HEAD, MEMBER without a url, and OTHER's section gone:

• in all three modes, with the file deleted or its deletion staged, the value finding is printed first and puts the file back;
• typed as printed and committed, the gate passes in every mode.

Line 13: the moved checkout left behind. Confirmed in all three git mv states. After typing, the gate passes, git status reads ?? Members/, and family.py add --dry-run refuses with family-members-dir-variant, mv -- Members/IRRS <new name>. It belongs in an issue of its own (C4).

Pinned copies and docs.

• Both scripts are in FAMILY_COPIED_VERBATIM (shape_materialize.py:528-530).
• update-shape.py decides upstream-changed per row from P != T (:721-736), so each holder takes one row per file.
• No test pins either file's bytes. The scripts' imports are unchanged and their mode stays 100755. No finding code is new.
• No doc is touched. README.md:966-969, templates/family-root/README.md, AGENTS-shape.md and docs/cli.md still say only true things.

Fail on base. With the PR's three test files on 233a334e: 27 failed, 7 passed, 14 errors. The same IDs as w839's log, and the 7 passes are the body's 7.

Runs at 77f8a6c8.

• 2.43, the 13 files: 747 passed, rc 0 (h243.log).
• 2.47.1, the new file: 48 passed, rc 0 (h247.log).
• w839's logs:
  • Both versions read 6335 passed and 59 skipped, rc 0, head 77f8a6c8, no tracked changes.
  • The shards are 1596+5, 2203+27 and 2536+27.
  • The skips break down as 27 + 22 + 5 + 5, none in the new file. All of this matches the body.

The restack.

• simrestack.sh printed:
  • #819: simulated update-branch 58d88ca8
  • #876 77f8a6c8 over #819 58d88ca8: main acc5a7bf, predicted squash 2e584b6c (tree aab6f75d)
  • #876: simulated restack 46969fee (tree 0b54842c), clean
• On that tree: 426 passed (restack.log).

Merge checks. merge-tree of 77f8a6c8 is clean against:

against	tree
main acc5a7bf	0b54842c
main 3bf2d3c9	0c9cf07c
#865 f7072d08	73b10b76
#870 c0d8dd16	f87829fb

Sonar-style checks.

• Duplication: no 10-line duplicate in the new file. At 8 lines, only the import block (9 of 600 lines) matches another file.
• Asserts: none joins conditions with and at the top level.
• Cognitive complexity (rv865b's counter, checked by hand):

function	base	head
fetch_members	16	14
_check_gitlinks_registered	19	18
_check_gitlinks_named	21	21 (the body omits it)
_check_members	14	14
new functions	—	3 or under

The functions over 15 keep the first line of their def unchanged, so SonarCloud should count them as old code.

Candidates for new issues

• C1. The wrong-case move's git config -f .gitmodules …path line, where the work tree's file is gone, commits a one-key file (P5e). In --no-members it also defeats the later put-back (P5b).
• C2. member-gitlink-removal-staged prints git checkout HEAD -- .gitmodules when the work tree's file is gone, and that undoes an earlier finding's writes (P5d).
• C3. A committed value git refuses, plus a stray gitlink. The value finding's --unset-all leaves .gitmodules unstaged, and the stray's git rm --cached then exits 128, "please stage your changes to .gitmodules" (P5g, the same on base). -f does not help.
• C4. "validate-family.py: after the git mv state's lines are typed as printed, the checkout git mv moved stays at Members/<P>, untracked and never named, so family.py add refuses family-members-dir-variant. After the update, git -C Members/<P> status reads the row's path, so the checkout's untracked file goes unseen."
• C5. RV877's O1, above.

Could not verify

• Windows, macOS, Git Bash sh and bash 3.2. No busybox is installed here.
• A make in another language: this host has only the C, C.utf8 and POSIX locales and no make.mo.
• SonarCloud's real verdict, including whether S9073 flags the or assert at line 534. The Sonar MCP would not connect.
• CI's git 2.55.0.

Scratch left; nothing of mine is running

• Worktrees under /workspace/projects/openRepoShape-worktrees/_lane5-scratch/:
  • rv876-head, with an untracked tests/test_rv876_probe.py;
  • rv876-mut, with the scripts restored and an untracked probe copy;
  • rv876-base, with the PR's tests copied in and the probe file.
  • Each has untracked or changed files, so I left them, as you asked. rv876-restack was removed.
• Logs and probes: $Q/v118/rv876/.
• Basetemps: $Q/v110/bt/rv876-{h243,h247,probe,base,baseprobe,mut,restack}.
• Git objects: simrestack.sh left unreferenced commits in the shared repo: 58d88ca8, 2e584b6c, 3f01e34d and 46969fee.
• Processes: none of mine. The runs I see belong to c7-c819.

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