[brettheap]

Lane: openreposhape-4 (openRepoShape-4)
Claim: #612 (comment)

What changed and why

add, bump and remove each end in ONE git commit -- <paths>, made after the verb's first writes (add mounts the member, bump checks it out at --to, remove unmounts it) and put back by a rollback if it fails. Three states of the holder made it fail, or land in the wrong place, every time. Each is now asked once, before the first write and under --dry-run alike, so the two runs refuse in the same words. The source lines are on #459 (issuecomment-6192725913). Fix round 1 (R612's review of 54c30a8) is folded in below.

An act git has unfinished (571(e)). _checked_no_operation_in_progress, called by all three verbs beside their other early refusals.

• It looks for the holder's MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD, rebase-merge/, rebase-apply/ and sequencer/, each where git rev-parse --git-path says git keeps it, so a linked worktree is right too. ONE git rev-parse answers every mark; where the lines and the names disagree (a holder whose path holds a newline) each name is asked alone and its whole answer is kept, so a merge in a linked worktree under such a path is found.

• It refuses family-operation-in-progress, exit 2, nothing written. The detail names the act and what the verb's commit would do in it. The remediation gives that act's own exit: git commit or git merge --abort; git cherry-pick, git revert, git rebase or git am, each with --continue or --abort. Then: run the same command again.

• It is asked before the branch refusal and before every refusal that reads the holder, so a rebase is named as a rebase. A rebase is asked before a merge begun inside it (a merge at a rebase's edit stop leaves MERGE_HEAD and rebase-merge/ both; git rebase --abort clears both, git merge --abort would not), and a test holds the order.

• Measured on git 2.43, each stopped on a conflict (a test makes the verb's partial commit with git itself, so each sentence is tied to git):

  act	what git does with git commit -- <paths>	what the refusal says
  merge (also --no-commit, with no conflict)	refuses: fatal: cannot do a partial commit during a merge., after the writes, then the rollback	the same, and that the writes are put back
  cherry-pick	refuses, the same way	the same
  revert	makes the commit on the branch as the verb's own, and git takes it as the END of the revert: REVERT_HEAD goes, git revert --continue and --abort then say no cherry-pick or revert in progress, the revert's changes are left half done in the index and the work tree	exactly that
  rebase	makes the commit on the HEAD the rebase detached, in the middle of its sequence: git rebase --continue replays the rest on top of it into the rebased branch, git rebase --abort drops it. The real run met family-holder-detached first, which committed nothing, and whose git switch -c is wrong advice there; the dry run printed a plan	exactly that
  git am	makes the commit on the branch, in the middle of the patches	the same
  cherry-pick or revert SERIES, only sequencer/ left	makes the commit in the middle of the series: --continue replays the rest on top of it, --abort exits 0 with You seem to have moved HEAD. Not rewinding and keeps it	the same

• A series (fix round 1, F2). git cherry-pick A..B and git revert A..B keep sequencer/ for the steps to come. Once the stopped step is committed by hand no CHERRY_PICK_HEAD or REVERT_HEAD is left, though git status still says the act is in progress. It is named from the first word of sequencer/todo, as git status names it: pick or p, a cherry-pick; revert, a revert; with that act's --continue and --abort. A todo that is missing, empty or no series reads as nothing in progress to git status and --continue fails on it (exit 128), while --abort and --quit exit 0: it is refused as "a cherry-pick or revert series", naming only git cherry-pick --abort (git revert --abort is the same) and git cherry-pick --quit.

• N4 is NOT taken (falls back). Asking about a merge before Family.open parses family.yaml would make git run before the refusal of a hand-edited path:, which test_family_member_path.py::test_the_refusal_is_made_before_any_git_call[remove|bump] holds to no git command at all (it failed 2 of 2 when tried). The comments that said the check "is asked first" now say what is true. A merge conflicted in family.yaml itself still reads yaml-unparsable (the <<<<<<< is read as a YAML merge key), with no remediation: left for Tracking: setup, scaffold, family and the shared helpers, creating projects and families #459. An option for it: ask only when the parse fails.

A path the sparse checkout leaves out (571(d), 574/5). _checked_inside_sparse_rules.

• It asks git sparse-checkout check-rules about the paths the verb stages: .gitmodules, members/<Project> and family.yaml for add and remove; members/<Project> and family.yaml for bump. So bump is covered, at no extra cost. remove also stages the gitlink, through git rm, which fails the same way (measured: deinit ran, then git rm refused).
• 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 gitmodules-sparse reads the skip-worktree MARK, which git clears for a file put back by hand, so the rules are asked instead. gitmodules-sparse keeps precedence where its mark is set, and every existing test of it passes unchanged.
• It refuses family-sparse-excluded, naming the command that brings the path back and git sparse-checkout disable.
• The pattern (fix round 1, F3, N7). The mode is read as a listing, git config -z --list, key core.sparsecheckoutcone (never git config --get: test_family_dry_run.py counts any git config that is not a listing as a write); unset is non-cone.
  • Cone mode: git sparse-checkout add members. members/<Project> is refused there for a gitlink git tracks.
  • Non-cone rules: git sparse-checkout add members/<Project>, no leading slash, anchored by its inner slash. The bare members of round 0 exited 0 and left the path out under rules that name the member (!/members/<P>, !/members/*), so the verb was refused again with the same remedy: a loop.
  • A root file (.gitmodules, family.yaml) can only be out of non-cone rules (in cone mode git keeps every root file inside, measured). It is named UNANCHORED: it wins over !/.gitmodules, !.gitmodules and !/.* (tested) and over !/* (the review), and meets no Git Bash conversion of a leading slash. git prints a warning for it about how far a slashless pattern reaches; nothing else in a holder's root is named so.
• It is asked only where git sparse-checkout list says sparse checkout is on. git sparse-checkout disable turns the setting off and leaves .git/info/sparse-checkout, and check-rules still reads it: a holder that is no longer sparse would be refused for rules git no longer applies. A test holds this.

A FILE named members (574/5). _members_is_a_file, called by _checked_mount_path before its other questions.

• It refuses member-path-unaddable, reusing the code of a mount path git will not take. Its remediation already says members/ must be a plain directory, and a file is the third way it is not, after a link and a submodule.
• It says the file is tracked or untracked, and names the move: git mv with a commit first where git tracks it, mv where it does not. Left staged, the move is not left alone: the add's commit takes the deletion half into itself (A .gitmodules, M family.yaml, D members, A members/<Project>) and leaves A members-file staged alone.
• Git's own dry run does not refuse it, so the member was read and cloned first.

Fix round 1, the rest. F1 (the revert and rebase sentences, above) and F4 (the two S9073 asserts: one went with the test it sat in, one is split). N1: the merge-only test is folded into assert_refused_alike (which now also checks the act's mark is still there) and a control the control test covers is dropped. N3: _members_is_a_file's account of why the move is committed first. N8: one rev-parse call, with a test that counts it.

Tests

New file tests/test_family_refuse_up_front.py, 77 tests, on the harness test_family.py uses. Nothing existing is edited.

• Operation in progress. Each of eight acts and states (merge --no-commit, cherry-pick, revert, rebase, am, a cherry-pick series, a revert series, and an unreadable sequencer/), under each verb, real and --dry-run: exit 2 under the code, nothing on stdout (so no member was read), HEAD, index, family.yaml, work tree and config as they were, the act's mark still there, the two runs' words identical. Each act's exit typed as printed (--continue for the two series and the merge's git commit, --abort for the rest), then the same command makes its one commit. The premise test for every effect sentence. A merge begun inside a rebase. A merge in a linked worktree, and in one whose holder's path holds a newline (POSIX only). One git rev-parse for all the marks.
• Sparse. .gitmodules put back by hand under three rules that leave it out (anchored, unanchored, every dotfile): remove refused before deinit, mount untouched, the printed pattern typed, then remove goes through. The mount path out under cone rules and three non-cone shapes (!/members/, !/members/<P>, !/members/*), for add and for bump and remove, each typing the printed command and then running the verb. Cone mode refusing only the mount path; all three paths out, one command bringing them back; a disabled sparse checkout.
• A file named members, tracked and untracked, real and dry: refused before the member is read, the move made as printed, then add mounts.
• Controls. No act and no sparse checkout, each verb runs as before; sparse checkout on and keeping every path.
• Cannot tell. Rules git cannot read; and a git first on PATH that answers unknown subcommand to check-rules (POSIX only): remove in the 571(d) state is then NOT refused and fails at its commit as before, with its rollback restoring the holder.

Run over the committed merge result (head 207d4e1d, scripts/family.py sha256 442a0f0b…, the test file e1529a4f…): tests/test_family_refuse_up_front.py, tests/test_family.py, the other 17 tests/test_family_*.py and tests/test_family_bump_to_local_only.py (#625), tests/test_commit_trailers.py, tests/test_refusal_default_remediation.py, tests/test_template_text_placeholders.py, tests/test_validate_family_branch_step.py, tests/test_validate_family_unnamed_mounts.py, tests/test_repo_hygiene.py, tests/test_cli_reference.py, tests/test_agents_md_codes.py: 1330 passed, 19 skipped, 0 failed in 13m49s. The skips are all in tests/test_family_init_values.py (a template that quotes its placeholder or id:); none of the new file's tests skipped. And tests/test_shape_doctor.py -k "members or family" (which runs family.py add): 36 passed, 1 skipped (the PowerShell line, Windows only). The full suite was not run.

Fix round 2 (head 04ad53af; scripts/family.py sha256 6d8dc89e…, the test file d708eccc…): tests/test_family_refuse_up_front.py, tests/test_family_bump_to_local_only.py, tests/test_family_member_path.py, tests/test_family_dry_run.py, tests/test_family_remove_gitmodules.py and tests/test_repo_hygiene.py: 289 passed, 0 skipped, 0 failed, 4m02s. Main was not merged this round: 4cf5fb07 touches neither scripts/family.py nor any tests/test_family* file since 8f453d0e.

Mutants, each over the new file (76 tests; 77 for the last row), on the fix's code before its last docstring sentence and before main was merged:

mutant	result
remove the operation check	31 failed. The merge tests fail at the commit, as before: fatal: cannot do a partial commit during a merge.
check only MERGE_HEAD	15 failed: every cherry-pick, rebase, revert and am case and the rebase-and-merge order case; the merge cases pass
skip the sparse check	24 failed
skip the members-is-a-file check	4 failed
look for the marks in .git/<name> instead of git rev-parse --git-path	2 failed: the linked worktree and the one-call count
ask the operation check in add's real run only	8 failed, the dry run's refusal among them
drop the git sparse-checkout list gate	3 failed: the disabled sparse checkout, each verb
drop the sequencer mark (F2)	10 failed: every series case
always name members (F3)	13 failed: every non-cone mount row, and the three-paths case
ask MERGE_HEAD before the rebase marks (N2)	3 failed: the rebase-and-merge case, each verb
one rev-parse per mark (N8)	1 failed: the one-call count
the fallback keeps only the first line of a split answer (V-F1), run on fix round 2's code	1 failed: the newline-path linked worktree

What it cannot tell

• A git with no git sparse-checkout check-rules (an unknown subcommand, exit 129), or one whose check fails on the rules themselves (the file is missing): "cannot tell". Nothing is refused, and the late failure and its rollback stay as they were. The operation check and the members file check need no such git.
• A git that answers check-rules differently from its own git add or git rm. On git 2.43 they agree for gitlinks, in cone and non-cone rules, with and without a trailing slash on the pattern (measured); that is the one git run here.
• Windows and macOS were not run (CI is Linux only now). Reasoned instead: paths go to git on stdin as bytes, files are written as bytes, and the git-shim test is POSIX only. The root-file patterns carry no leading slash for that reason.
• A stale mark. A real stale rebase-merge/ with its files is cleared by git rebase --abort. An EMPTY one is refused as a rebase too (git status agrees), but git rebase --abort FAILS there (could not read '.git/rebase-merge/head-name') and git rebase --quit clears it. The refusal names --abort; recorded for Tracking: setup, scaffold, family and the shared helpers, creating projects and families #459 (R612's E5).
• A merge conflicted in family.yaml reads yaml-unparsable before this check can name it (N4 above); Tracking: setup, scaffold, family and the shared helpers, creating projects and families #459.

Shared files

• scripts/family.py only, plus the new test file. family.py: bump --to refuses a commit only the member's mount holds, reachable from no branch of the member's remote, as bump-leg.py refuses its twin, so a holder is never pinned to a commit nobody else can fetch (#613) #625 (family.py bump --to: pins a commit only the member's mount holds, which bump-leg.py refuses as local-only #613) changed it too and has landed; main is merged in (207d4e1d, a --no-ff merge of main 8f453d0e, clean). Lane 2's scaffold-project.py and family.py: both runs ask whether each folder they make can be made and take only gh's not-found answer as "absent", so --dry-run refuses a file where a folder goes, or a gh repo view that fails otherwise, by name as the real run does, and a refused init leaves no empty family folder (#609) #622 (fix/609-dry-run-parity) is the one open PR that also changes it.
• Hunks: one new block after _branch_said; _members_is_a_file before _checked_mount_path, a docstring bullet and two lines in it; one docstring sentence in _checked_gitmodules_as_staged; the cmd_add docstring and two call lines; three lines in cmd_bump, before its member-uninitialized refusal; two call sites in cmd_remove. No import, parser or --help text changed, so docs/cli.md is as it was.
• git merge-tree --write-tree of this head: with main 4cf5fb07 exit 0, clean; with scaffold-project.py and family.py: both runs ask whether each folder they make can be made and take only gh's not-found answer as "absent", so --dry-run refuses a file where a folder goes, or a gh repo view that fails otherwise, by name as the real run does, and a refused init leaves no empty family folder (#609) #622 227bc3b0 exit 0, clean. (Round 1's merge of main 8f453d0e is 207d4e1d.)
• AGENTS.md is untouched. Its item 3 for families names family-on-tracking-branch; it could name family-operation-in-progress beside it, and that is for whoever edits AGENTS.md, which sits at its cap.

Closes #612

🤖 Generated with Claude Code

Summary by Sourcery

Make family add, bump, and remove refuse unsafe repository states before performing any writes.

New Features:

• Add early validation to prevent family add, bump, and remove operations from running while Git operations are unfinished, required paths are excluded by sparse checkout, or members is a file.

Bug Fixes:

• Prevent partial writes, misplaced commits, and late rollback failures caused by merges, cherry-picks, reverts, rebases, git am, sparse checkout exclusions, and an invalid members path.
• Keep real and dry-run refusals consistent by detecting these invalid repository states before any writes or member reads.

Enhancements:

• Handle operation state correctly across linked worktrees, multi-commit cherry-pick and revert sequences, git am, and sparse checkout cone and non-cone modes.
• Provide actionable refusals that identify the active Git operation or excluded paths and suggest appropriate recovery commands.

Tests:

• Add comprehensive coverage for operation-in-progress detection, sparse checkout behavior, linked worktrees, newline-containing paths, invalid members files, dry-run parity, recovery commands, and fallback behavior when Git cannot evaluate sparse rules.

[sourcery-ai]

Reviewer's Guide

The PR moves three classes of known late or misplaced failures ahead of the first write for add, bump, and remove: active Git operations, sparse-excluded staged paths, and a file occupying members; it adds operation-aware remediation and extensive tests confirming real/dry-run parity, linked-worktree handling, rollback safety, and successful recovery.

Sequence diagram for upfront operation-state refusal

sequenceDiagram
    participant User
    participant Family as family.py
    participant Git
    participant Holder as Family root

    User->>Family: add, bump, or remove
    Family->>Git: rev-parse --git-path operation marks
    Git-->>Family: mark location
    Family->>Holder: Check unfinished operation mark
    alt Operation in progress
        Family-->>User: Refusal with finish and abort commands
        Note over Family,Holder: No member read, write, or rollback
    else No operation in progress
        Family->>Family: Continue existing refusal checks
        Family-->>User: Execute or report dry-run plan
    end

Loading

Flow diagram for upfront family command refusals

flowchart TD
    Start["add, bump, or remove"] --> Operation["_checked_no_operation_in_progress"]
    Operation -->|unfinished merge, cherry-pick, revert, rebase, or git am| RefuseOperation["Refuse family-operation-in-progress"]
    Operation --> Mount["_checked_mount_path"]
    Mount -->|members is a file| RefuseMembers["Refuse member-path-unaddable"]
    Mount --> Sparse["_checked_inside_sparse_rules"]
    Sparse -->|staged path excluded| RefuseSparse["Refuse family-sparse-excluded"]
    Sparse --> Continue["Continue to existing branch, member, and write steps"]
    RefuseOperation --> End["No writes; same result in real and dry-run"]
    RefuseMembers --> End
    RefuseSparse --> End

Loading

File-Level Changes

Change	Details	Files
Add pre-write detection for unfinished Git operations so family mutations cannot commit into or conflict with an active operation.	Detect merge, cherry-pick, revert, rebase, and git am state using Git-resolved paths, including linked worktrees. Emit operation-specific refusal codes, effects, continuation commands, abort commands, and consistent dry-run behavior. Run the check before branch and holder refusals for add, bump, and remove.	scripts/family.py
Validate sparse-checkout coverage for every path each mutation stages before performing writes.	Query sparse-checkout state and check-rules using NUL-delimited paths, while treating unavailable or unreadable rules as cannot-tell. Cover .gitmodules, the member gitlink, and family.yaml according to each verb’s staged paths. Report excluded paths with patterns or disable guidance and preserve existing gitmodules-sparse behavior.	scripts/family.py
Refuse a top-level members file before add reads or mounts the member.	Distinguish tracked and untracked files and provide the appropriate git mv or mv remediation. Reuse member-path-unaddable and integrate the check into existing mount-path validation.	scripts/family.py
Add comprehensive regression coverage for early refusal, remediation, rollback preservation, and compatibility controls.	Test all verbs in real and dry-run modes across Git operations, sparse modes, linked worktrees, and tracked or untracked members files. Verify refusal wording, exit status, untouched holder state, typed remediation, and successful retry. Cover sparse-checkout controls, unavailable check-rules, disabled sparse checkout, and existing behavior without the new conditions.	tests/test_family_refuse_up_front.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#612	Have add, bump, and remove refuse before their first write when a merge, cherry-pick, or rebase is in progress.	✅
#612	Have remove refuse before writing when sparse-checkout rules exclude .gitmodules, including when the file has been put back on disk.	✅
#612	Have add refuse before reading or mounting a member when members is a file or sparse-checkout rules exclude the member path.	✅

---

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.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
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 16 hours and 17 minutes by commenting @sourcery-ai review. Upgrade to get a review now.