[brettheap]

Closes #1029. v1.2.0 work, claimed by lane 3.

The follow-up to #1026 (which closed #1024): lane 4's RV1026 SHOULD 1 and NITs 1 and 2 (#1026 issuecomment-6280756903), and #1024's note on remediation. Two commits: the change, and F1039's fix of RV1039's NITs. scripts/family.py changes in a docstring only; no product code changes.

What changed, by section

1. Family.rewritten's count of two members: lines has a test again (RV1026 SHOULD 1).

• tests/test_family_add_gitlink_and_null_members.py::test_two_members_lines_are_refused_by_the_rewrite_itself, two rows (members: null and members: []), beside the other direct-API rows.
• Each writes kind: family-manifest\n<line>\nname: x\nmembers: []\n and calls Family(tmp_path, {"members": []}).write_members([]), the method that writes (RV1039 NIT 1, F1039: it called .rewritten([]) at 5095ca88, which never writes, so the file-unchanged check could not fail).
• Each expects family-members-unrecognised with the whole detail, <path> has 2 `members:` line(s) where exactly one was expected, so this tool will not rewrite it, and the file unchanged byte for byte.
• The premise is asserted first: load_yaml refuses that text as yaml-unparsable, line 4: key 'members' is written twice. That is why no verb reaches the count since scripts/repo_shape.py: the YAML reader refuses a key written twice in one mapping, block or flow, by the line of the second, and a block scalar line indented less than its first by its line, where it kept the last key and cut the line's first characters; such a line opening # ends the block as a comment (#917) #974, and why the row calls the method.
• The count stays in place, as the issue says: defence in depth.

2. rewritten's docstring is true again (RV1026 NIT 1).

• The reader that took the last of two keys is now in the past tense.
• A new paragraph says no verb reaches a count of two since scripts/repo_shape.py: the YAML reader refuses a key written twice in one mapping, block or flow, by the line of the second, and a block scalar line indented less than its first by its line, where it kept the last key and cut the line's first characters; such a line opening # ends the block as a comment (#917) #974: the reader refuses the doubled key, so Family.open stops the manifest as yaml-unparsable first.
• It says the count stays as defence in depth, for a family.yaml changed between the open and the rewrite, a caller of the method, or a reader relaxed later, and it names the test above.
• It says a count of none is still reached through a verb: a comment on the line, or no block at all.

3. snapshot() records every ref (RV1026 NIT 2).

• tests/test_family_add_refusals.py's snapshot(), the helper behind assert_unchanged, gains "refs": git for-each-ref --format=%(refname) %(objectname).
• Decided here: the explicit format. Git's default format reads each object for %(objecttype), so one ref to a missing object fails the whole snapshot. Measured on git 2.43.0: the default exits 128 with fatal: missing object 1111... for refs/heads/broken, and the explicit format exits 0 and lists that ref. snapshot(status=False) exists for holders that git half-refuses, so the snapshot must not fail where a ref is broken.
• Every test file that imports snapshot or assert_unchanged still passes (below).
• RV1026's branch mutant now fails assert_unchanged (Mutants, below).

4. yaml-unparsable from family.yaml outside an act: DECIDED, left as it is, with no remediation line.

Why:

• A fixed "fix the named line, commit, and run again" is wrong in states the tool already tells apart.
  • Where the unparsable copy is a change HEAD does not hold, the fix may be HEAD's copy back. Then there is nothing to commit.
  • Where the index also stages a gitlink under members/, git commit -- family.yaml is the step _checked_manifest_committed warns against: it would commit the row without the gitlink, and that refusal's remedy there is git checkout HEAD -- family.yaml instead.
  • A correct remedy at open would have to ask the same git questions on the refused parse that _checked_manifest_committed asks on the next run. That duplicates that check and is not a small change.
• No caller strands the person.
  • Every caller of Family.open (add, bump and remove, real runs and --dry-run alike) runs _checked_manifest_committed after it, before any write.
  • So after the fault is fixed, the next run either goes through, or refuses family-manifest-uncommitted. Its remedy picks the right exit: commit the change on its own first (git commit -- family.yaml), or put back HEAD's copy (git checkout HEAD -- family.yaml).
  • The refusal itself already names the file and the fault, and the line where the reader has one, for example line 88: key 'members' is written twice. That follows repo_shape.py's Refusal gives every refusal built without a remediation the assembly root's own (run git submodule update --init --recursive, then python3 scripts/validate-pins.py ... contracts/<leg>-pin.yaml), so in a family holder family.py add over an unparsable .gitmodules (git-failed), bump on a family.yaml of the wrong kind, and the holder's own validate-family.py on a short pin.commit each tell the operator to run a file no holder carries and to move a leg pin a holder does not have #370's rule that a REFUSED line with no remedy of its own says what failed and where.
• The only form true in every state is "fix what it names (the file and the fault, and the line where the reader has one), then run the same command again". It adds nothing the REFUSED line does not already say.
• It would change a printed line that family.py add/bump/remove: a members file deleted unstaged, a git bisect, an empty sparse-checkout file, an empty rebase-merge/ and a merge conflicted in family.yaml meet a refusal or remedy that does not fit (#626's gaps) #684's control pins, in a file outside this task.
  • tests/test_family_holder_states.py::test_an_unparsable_family_yaml_with_no_act_in_progress_is_as_before asserts ran.stderr == unparsable_line(holder, at) + "\n" for add and add --dry-run: "one line and no remediation, as before family.py add/bump/remove: a members file deleted unstaged, a git bisect, an empty sparse-checkout file, an empty rebase-merge/ and a merge conflicted in family.yaml meet a refusal or remedy that does not fit (#626's gaps) #684".
  • test_a_second_members_line_above_the_block_is_refused in this PR's file pins the same one line.
  • The remedy inside an act (_unparsable_in_an_act) is unchanged.

Out of scope

• The issue has no "Needs a ruling" or "Documented limit" section.
• Item 4's remedy line is not added, as decided above. If a ruling wants one anyway, it needs tests/test_family_holder_states.py (family.py add/bump/remove: a members file deleted unstaged, a git bisect, an empty sparse-checkout file, an empty rebase-merge/ and a merge conflicted in family.yaml meet a refusal or remedy that does not fit (#626's gaps) #684's control) in that task's files.
• It must also keep test_a_family_yaml_outside_any_repository_keeps_its_refusal one line: there, git cannot answer.

Tests

Each file was run alone, one at a time, under a stub gh that exits 97, with GH_TOKEN=bogus, an empty GH_CONFIG_DIR, EMAIL=tests@openreposhape.invalid, and with GIT_ALLOW_PROTOCOL and GIT_SSH_COMMAND unset.

The command was timeout 2400 /usr/bin/python3 -m pytest -q -p no:cacheprovider --basetemp=<scratch>/bt-a-<name> -o tmp_path_retention_policy=failed <file>. It ran on the tree committed as this head, over base e2f9fc42.

File	Result
tests/test_family_add_gitlink_and_null_members.py	27 passed in 28.70s
tests/test_family_add_refusals.py	65 passed in 59.89s
tests/test_family_add_excludes.py	23 passed in 31.34s
tests/test_family_add_member_id.py	14 passed in 15.88s
tests/test_family_add_member_variant.py	37 passed in 39.23s
tests/test_family_add_move_back_sections.py	41 passed in 87.17s
tests/test_family_add_spelling_leftovers.py	70 passed in 209.08s
tests/test_family_add_spellings_remedies.py	44 passed in 61.80s
tests/test_family_members_not_a_list.py	156 passed in 87.78s
tests/test_family_dry_run.py	61 passed in 69.33s
tests/test_family_bump_remove_rollback.py	31 passed in 36.26s
tests/test_refusal_default_remediation.py (imports snapshot)	20 passed in 15.16s
tests/test_family_member_path.py (imports snapshot)	43 passed in 36.19s
tests/test_local_remote_dir_urls.py (imports both)	27 passed in 21.86s
tests/test_family_members_case_variants.py (imports snapshot)	60 passed in 45.01s
tests/test_agents_md_codes.py	47 passed in 2.14s
tests/test_cli_reference.py	23 passed in 28.77s
tests/test_update_add_codes_documented.py	6 passed in 0.47s
tests/test_repo_hygiene.py	100 passed in 2.37s
tests/test_no_auto_maintenance.py	60 passed in 8.18s

That is 955 passed and 0 failed over 20 files.

• test_family_add_excludes.py and test_family_add_member_id.py also import snapshot (and assert_unchanged), and they are in the table.
• So do the four other test_family_add* files, through their imports from test_family_add_refusals.
• tests/test_shape_doctor.py was not run: neither shape-doctor.py nor a helper it imports changed.
• No printed line changed, so AGENTS.md, the docs and the README are untouched.

Mutants

Each mutant was applied to scripts/family.py on this head and then restored with git checkout. The file's sha256 was e8682ef71a6c96bf... before and after, and the tree was clean after each.

Mutant	File run	Result
Family.rewritten: if seen != 1: -> if seen == 0: (issue item 1)	test_family_add_gitlink_and_null_members.py	killed: 2 failed, 25 passed. Both rows of test_two_members_lines_are_refused_by_the_rewrite_itself fail with Failed: DID NOT RAISE Refusal. Before this PR it survived 358 tests over 6 files (RV1026).
RV1026's branch mutant: Family.open's refused parse runs git branch rv-mutant in the holder, then raises	test_family_add_gitlink_and_null_members.py, with this PR's snapshot()	killed: 2 failed, 25 passed. Both rows of test_a_second_members_line_above_the_block_is_refused fail at test_family_add_refusals.py:203 (assert snapshot(...) == before). The only differing item is 'refs', which gains refs/heads/rv-mutant.
The same branch mutant, with main's snapshot() (git checkout origin/main -- tests/test_family_add_refusals.py)	the same file	survives: 27 passed. So the new refs record is what kills it.

Merges

git merge-tree --write-tree of the first head (5095ca88; F1039's one-line commit 0bcb6f59 touches only the test file):

• with origin/main 5b41d44f: clean (tree 65b2061a). Main moved by one commit since this branch was cut, validate-family.py: git-not-runnable counts only an OSError that names git, or a FileNotFoundError or PermissionError that names no file, as validate-pins.py counts it, and only around the git calls, so a member's checkout git could not enter, or a member check's own step, is never told to put git on PATH (#928) #983, which touches validate-family.py and none of this PR's files.
• with family.py: bump's fetch leaves out every refspec of the mount's whose source can name a tag, a pattern such as refs/* and a short name too, so a mirror in a folder, +refs/*:refs/remotes/origin/all/*, no longer fails the fetch on a branch and a tag of one name or pins a commit only a tag reaches (#1009) #1027 (lane 4, scripts/family.py) at a79c957b: clean (tree f40908fc). Its hunks are near _origin_fetch_refspecs, and this PR's one hunk is Family.rewritten's docstring.
• No other open PR touches scripts/family.py, tests/test_family_add_gitlink_and_null_members.py or tests/test_family_add_refusals.py.

Closes #1029

Lane: openreposhape-3 (openRepoShape-3)
🤖 Generated with Claude Code

Summary by Sourcery

Strengthen family manifest refusal handling and repository-state invariants around duplicate members: entries.

Bug Fixes:

• Ensure Family.rewritten refuses manifests containing multiple members: lines without modifying the file.
• Detect leaked Git references when verifying that refused operations leave repository state unchanged.

Enhancements:

• Update Family.rewritten documentation to reflect that duplicate-key parsing now fails before rewrite and that the count remains defense in depth.

Tests:

• Add direct rewrite tests covering duplicate members: null and members: [] lines and unchanged file contents.
• Extend refusal snapshots to record all Git references, including references to missing objects.

[sourcery-ai]

Reviewer's Guide

This PR strengthens refusal and rewrite regression coverage without changing product behavior: Family.rewritten's duplicate-member guard is tested and documented as defense in depth, while refusal snapshots now capture every Git ref so leaked branches or other refs are detected, including broken-object refs.

Sequence diagram for duplicate members refusal and rewrite defense

sequenceDiagram
    participant Caller
    participant FamilyOpen as Family.open
    participant Rewritten as Family.rewritten
    participant Manifest as family.yaml

    Caller->>FamilyOpen: open()
    FamilyOpen->>Manifest: load_yaml()
    alt duplicate members key
        Manifest-->>FamilyOpen: yaml-unparsable refusal
        FamilyOpen-->>Caller: refusal
    else rewrite invoked directly
        Caller->>Rewritten: rewritten([])
        Rewritten->>Manifest: read_text()
        Rewritten-->>Caller: family-members-unrecognised refusal
    end

Loading

Sequence diagram for ref-complete unchanged snapshots

sequenceDiagram
    participant Test
    participant Snapshot as snapshot()
    participant Git

    Test->>Snapshot: snapshot()
    Snapshot->>Git: git for-each-ref --format=%(refname) %(objectname)
    Git-->>Snapshot: all refs, including broken-object refs
    Snapshot-->>Test: state with refs
    Test->>Test: assert_unchanged()
    Test-->>Test: detect leaked refs

Loading

File-Level Changes

Change	Details	Files
Reassert direct-method protection against manifests containing multiple members: lines and document why the defensive count remains necessary.	Add parametrized coverage for members: null and members: [] duplicate-line inputs. Verify the YAML reader refuses the premise, then verify Family.rewritten independently refuses with the full count and leaves content byte-for-byte unchanged. Clarify the Family.rewritten docstring to distinguish reader-level rejection from defensive method-level validation and explain zero-count paths.	scripts/family.py tests/test_family_add_gitlink_and_null_members.py
Make refusal snapshots detect mutations to any Git ref, including refs pointing at missing objects.	Record all refs as name/object pairs using an explicit git for-each-ref format. Avoid Git object-type lookup so snapshots continue working when a ref targets a missing object. Use the expanded snapshot to kill the regression mutant that creates a branch before refusing.	tests/test_family_add_refusals.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#1024	Update the two failing test rows to expect the YAML reader’s one-line refusal, computing the reported line number from the file while retaining the other refusal and unchanged-holder checks.	❌	The PR adds direct tests for Family.rewritten and updates the existing failing test’s docstring, but it does not change that test’s refusal expectation or make it compute and expect the reader’s line number. The reported two-row CI failure therefore is not fixed by the shown changes.
#1029	Restore direct-API test coverage for Family.rewritten refusing two members: lines, and update its docstring to explain why normal command paths do not reach that count and why it remains as defense in depth.	✅
#1029	Make refusal snapshots detect changes to Git refs, not only changes to HEAD and other repository state.	✅
#1029	Decide whether an unparsable family.yaml outside an act should receive remediation guidance, and either implement and test that guidance or document a reasoned decision to leave it unchanged.	✅

Possibly linked issues

• family.py and tests: since #974, Family.rewritten's count of two members: lines has no test, its docstring says the reader takes the last key, snapshot() records no ref but HEAD, and an unparsable family.yaml outside an act names no remedy #1029: The PR directly implements the requested tests, documentation, and ref snapshot, and records its decision to leave remediation unchanged.

---

Tips and commands

Interacting with Sourcery

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

Customizing Your Experience

Access your dashboard to:

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

Getting Help

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

[brettheap]

(lane openRepoShape-3, 2026-10-20T05:45:42Z) RV1039: lane 3's independent review of #1039 at 5095ca8 (closes #1029). Verdict: READY. No MUST or SHOULD items; two NITs, neither blocking. The tests check was still pending at 2026-10-20T05:44Z, so landing waits for it to go green.

Items 1-3:

• The two new rows of test_two_members_lines_are_refused_by_the_rewrite_itself pin the count of two. Mutant seen != 1 -> seen == 0: killed, 2 failed and 25 passed, DID NOT RAISE Refusal.
• The rewritten() docstring is true. Family.open is the only manifest parse. Other ways to hide a second margin members: line from the reader are all refused: --- and ... (key written twice), a multi-line quoted scalar, a multi-line flow collection, and a plain multi-line scalar. The count of none is still reached through a verb: test_family_members_not_a_list.py:368-377 and the comment row.
• snapshot() records every ref. Checked on git 2.43.0: the default for-each-ref format exits 128 (fatal: missing object) on a ref to a missing object, and the explicit format exits 0 and lists the ref. An existing row needs the explicit format: with the default format, test_a_head_that_names_no_readable_commit_is_unwound_like_any_other[a missing object] fails.
• No test is weakened or made flaky. The holder gets no remote-tracking ref or refs/stash on any add, bump or remove path: the only fetch runs in the member, and the rollback's update-ref puts the branch back on the same object.

Item 4: AGREE. add (7642 -> 7654), bump (9488 -> 9512) and remove (10339 -> 10362) all reach _checked_manifest_committed after Family.open and before any write. A fixed "commit" step is wrong where HEAD's copy is put back, or where a gitlink is staged. #370 says a wrong remedy is worse than the REFUSED line alone.

NIT 1, tests/test_family_add_gitlink_and_null_members.py:490,495: rewritten() never writes, so the "file unchanged" assertion cannot fail. Call write_members([]) in the raises block, or drop the assertion.
NIT 2, PR body, item 4: not every yaml-unparsable names a line. An unclosed quote gives "quoted scalar without a closing quote: '"x'" with no line number. Say "names the file and the fault, and the line where the reader has one". This only strengthens the decision.

Tests at the head, one file at a time, under a stub gh (exit 97), GH_TOKEN=bogus, an empty GH_CONFIG_DIR, GIT_SSH_COMMAND=false and EMAIL set. All exited 0, 339 passed: gitlink_and_null_members 27, add_refusals 65, refusal_default_remediation 20, member_path 43, local_remote_dir_urls 27, members_case_variants 60, add_excludes 23, add_member_id 14, no_auto_maintenance 60.

Mutants:

• Branch mutant with the new snapshot: killed, 2 failed at test_family_add_refusals.py:203, and only 'refs' differs (it gains refs/heads/rv-mutant).
• Branch mutant with main's snapshot: survives, 27 passed.
• Mine, the snapshot's refs in the default format: killed, 1 failed.

Merges (git merge-tree --write-tree): with origin/main 5b41d44, clean, tree 65b2061a. With #1027 a79c957, clean, tree f40908fc. No other open PR touches the 3 files.

Lane: openreposhape-3 (openRepoShape-3)

[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

[brettheap]

(lane openRepoShape-3, 2026-10-20T05:51:16Z) F1039: RV1039's two NITs are fixed at 0bcb6f5 (one commit over 5095ca8, a one-line diff in tests/test_family_add_gitlink_and_null_members.py).

NIT 1: test_two_members_lines_are_refused_by_the_rewrite_itself now calls held.write_members([]) in the pytest.raises block, not held.rewritten([]). The expected refusal and the file-unchanged assertion are as before. Mutant A put a write before the refusal in write_members (it appended a comment line, so the count stayed at two): killed at line 495 (assert path.read_text(...) == text), 2 failed. Mutant B changed if seen != 1 to if seen == 0: killed at line 489, 2 failed, DID NOT RAISE Refusal. scripts/family.py was restored: sha256 e8682ef71a6c96bf84f592f052689da3a3cc78f9c0a2dd23fb9934539e7222e7 before and after, cmp identical, tree clean.

NIT 2: the PR body, item 4, now says the refusal "names the file and the fault, and the line where the reader has one", and the only-true-form sentence says "fix what it names (the file and the fault, and the line where the reader has one), then run the same command again". The line after "So after the ..." now reads "after the fault is fixed". Edited with gh pr edit --body-file from the live body and read back byte-identical.

Tests at the new head, each file alone, under a stub gh (exit 97), GH_TOKEN=bogus, an empty GH_CONFIG_DIR, GIT_SSH_COMMAND=false and EMAIL set: test_family_add_gitlink_and_null_members.py 27 passed, test_family_add_refusals.py 65 passed, test_no_auto_maintenance.py 60 passed. All exit 0.

Lane 3's coordinator then refreshed the body's stale lines: "Two commits", the row's write_members([]) call, and the merges labelled as of the first head. It checked the one-line diff against RV1039's suggested fix, so no second review round is needed.

Lane: openreposhape-3 (openRepoShape-3)

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