Skip to content

openRepoShape --doctor says it writes nothing into the repository but a --placement-plan file you name, keeps its clone for the NEXT lines that name it, shows its synopsis description again, and refuses a second --placement-plan or a second path (#546) - #580

Merged
brettheap merged 10 commits into
mainfrom
l3-546-doctor-writes-wording
Oct 15, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openreposhape-3 (openRepoShape-3)
Claim: #546 (comment)

Built on #551 before it landed (at 8237225), so the branch carries #551's pre-squash commits. It was then merged with main 3102027, which holds #551's squash 5e59e3b (the same tree as #551's final head 01c6d8e) and #552. That merge conflicted only on the handbook's two § Keeping a project's shape current markers (current and doctor): main's handbook was still #551's (blob 383e80e7), so both markers were resolved to this branch's 418f38b5475a2dbabc9a244882f386b1de6b2c63. git diff origin/main HEAD names only this PR's 10 files, and the merged tree (dddf9952) equals main plus git diff 8237225e c93633af applied as a patch. #554 has landed since; it merges cleanly (git merge-tree exits 0).

l3-550 (#550) also moves README § Keeping a project's shape current. Whichever of the two lands second needs that section's third hash in both markers, after a re-read of current and doctor.

What

  • "Writes nothing." The doctor's two refusals, README.md, the handbook, docs/cli.md's purpose line (scripts/render-cli-reference.py's TOOLS) and shape-doctor.py's docstring now say it writes nothing into the repository but the --placement-plan file you name. The handbook adds that a relative file lands in the directory you stand in, inside the repository when you stand there. --help already said so. AGENTS.md already named the exception, and it now follows the claim it qualifies directly.
  • The kept clone. The installed doctor's last line reads "the standard this run used is kept at , for any NEXT command above that names it; …". --help (and so docs/cli.md), README.md, AGENTS.md and the handbook give the same reason. A pins row's NEXT names no file in the clone when the project carries its own validator. And "runs from it" read as "cd into it", where the placement row's relative --placement-plan placement-plan.yaml would land inside the directory the same line calls safe to delete.
  • The synopsis. The doctor's description, "diagnose a REPOSITORY against this standard", is back in the column the other synopsis lines use, on the line under the full form, the way argparse places the help of a long option. The form stays one line, the one the refusal prints. docs/cli.md is regenerated.
  • A second --placement-plan is refused, exit 2, before anything is cloned: "two placement plans, 'a.yaml' and 'b.yaml'. --doctor writes one: pass --placement-plan once." Both used to reach shape-doctor.py, whose argparse kept the last, so a.yaml silently went unwritten. This follows the launcher's convention: two modes and a second positional path were already refusals, never last-wins.
  • Scope addition: a path after a second --doctor is refused. --doctor a --doctor b replaced a with b silently. It is now taken as a second path, which the doctor already refuses ("--doctor takes a path, --json and --placement-plan , and nothing else"). It is included because the issue's own premise is that the command "refuses ... a second path", and this was the one way around that. It is one hunk and one test case.
  • Handbook markers. README § Keeping a project's shape current moved. Both page sections that read it were re-read: current says nothing about the doctor and stands, and doctor's opening paragraph is the twin amended here. Both markers moved to 418f38b. README.md stays at 1422 lines and AGENTS.md at 490, so no cap is raised.

Why

#546, from the v1.1.0 triage of #460's known limits: "writes nothing" was false of a relative --placement-plan typed inside the repository, and the kept clone's "any NEXT command above runs from it" was false of the pins and placement NEXTs. The synopsis and the repeated plan rode along. The second commit is the fix round from the independent review of df8b672 (RV546). The third takes RV546's two optional nits on a4692b2: the test's sentence split also ends at .), ." and .**, and the rewrapped docstring paragraph has no short line.

Tests

  • tests/test_repo_hygiene.py::test_the_doctor_says_what_it_writes_and_why_it_keeps_its_clone covers five passages: README, AGENTS, the handbook, docs/cli.md's --help and docs/cli.md's purpose line. Every "writes nothing" sentence, matched in any case, must carry its --placement-plan exception, or be followed at once by the sentence that names it. Every passage about the installed form must give "the NEXT lines that name it" as the reason the clone is kept, and never "run from".
  • tests/test_openreposhape_command.py: the doctor line's description column, and the two-modes refusal's wording.
  • tests/test_shape_doctor.py:
    • the new kept-line tail;
    • --doctor --placement-plan plan.yaml typed inside the repository (default path .) writes the plan there and changes nothing else;
    • the two-plan refusals, in both spellings, and the second-path refusal, each with no file written and nothing cloned;
    • the --doctor refusal's full new wording.
  • Each fails on the old text or behaviour (21 failed on the old launcher and docs). Mutants killed: docs/cli.md's purpose back to "Writes nothing.", in both the generator and the file (render-cli-reference.py --check passes on it, so only the hygiene test sees it); and AGENTS.md's exception sentence deleted.
  • On merged head 4a4abf2, all with a stub gh first on PATH, GH_TOKEN=bogus and an empty GH_CONFIG_DIR:
  • The full suite is left to CI on three OSes.

Closes #546.

🤖 Generated with Claude Code

Summary by Sourcery

Clarify the doctor's write and clone semantics while making duplicate placement plans and paths explicit command-line errors.

Bug Fixes:

  • Ensure the doctor accurately describes its only repository write, the named --placement-plan file, including relative paths inside the repository.
  • Reject repeated --placement-plan options and repeated doctor paths before cloning instead of silently applying last-wins behavior.
  • Correct the installed doctor's kept-clone guidance so it is retained only for subsequent commands that name it.

Enhancements:

  • Restore the doctor synopsis description in the command help output and keep its generated CLI documentation aligned.

Documentation:

  • Align README, AGENTS.md, handbook, CLI documentation, and doctor messaging with the corrected write and kept-clone behavior.

Tests:

  • Add coverage for help formatting, documentation wording, placement-plan locations, duplicate arguments, and pre-clone refusal behavior.

Brett Heap and others added 9 commits October 14, 2026 15:37
…ion records the hash of every README section it reads, and README.md's line cap is 1422 plus one file per raise in tests/readme-cap.d/, so two README pull requests no longer conflict on the footer's one blob or the cap's one line, and a stale page names exactly which sections to re-read (#519)

Every README pull request had to edit the same two places: the handbook
footer's `README.md blob <hash>`, which test_repo_hygiene.py held equal to
README.md's whole blob, with an "added (#N)" clause beside it; and the
`assert len(lines) <= 1422` line in the same file, with a dated comment
for every raise. Any two open at once conflicted on both. Brett Heap's
ruling 29 (2026-10-14T15:18:19Z), "Hash per section (Recommended)": "Each
handbook section records the hash of the README section it covers; a
failing test names exactly which handbook sections to re-read. The single
README hash and the per-PR "added (#N)" notes in the footer retire. Each
line-cap raise becomes its own small file with its reason."

The handbook. README.md is cut into segments: the preamble, then one per
`## ` heading up to the next, fenced `## ` lines skipped as `_readme_h2s`
always did (it now reads its headings from the same segmenter). A
segment's fingerprint is the git blob sha1 of its text with CRLF read as
LF, computed in Python and never by `git hash-object`, whose path form
applies .gitattributes and whose --stdin form does not. Every page part
that reads a segment carries one comment per segment it reads,
`<!-- reads README.md § <heading or preamble>: blob <40 hex> -->`; the
masthead gains `id="masthead"` so a failure can name it. The test's old
check (4) becomes (a) every marker names a segment that exists, (b) every
segment is read by a marker, (c) every marker's blob is its segment's
fingerprint now, naming the segment, every page section that reads it
and the new value, with the #89 re-read wording, and (d) every heading
README_HEADING_TO_HANDBOOK_ID maps has a marker in its mapped section and
every page section carries one unless HANDBOOK_SECTION_READING_NO_README
(`agents`, which reads AGENTS.md) says otherwise. A mistyped marker, one
outside every page part, a repeat in one section, and two segments'
markers on adjacent lines (git conflicts on adjacent-line edits, which is
what the markers exist to avoid) are refused too. Every check now reports
together, so a renamed heading is told about the table and the markers in
one run. HANDBOOK_README_BLOB is gone; HANDBOOK_RETIRED_README_BLOB
asserts the footer's old fact stays gone.

The mapping, read from both documents: preamble by masthead and confers;
Starting by masthead, start, partway, ci (the worked example's
private-legs paragraph), legs (the naming-check command), doublepin
(editing a pin by hand) and park (openRepoTools' install line); The three
legs by masthead, ci and legs; The double pin by masthead and doublepin;
Keeping a project's shape current by current and doctor; every other
section by the one page section README_HEADING_TO_HANDBOOK_ID names.

The footer's prose is frozen as history at #502, and one sentence now
says each section records the README section it reads and a test checks
those records.

The cap is README_CAP_BASE (1422) plus the raises in tests/readme-cap.d/:
one `<issue or PR number>.txt` per README-growing change, line 1 `+N`,
line 2 an ISO date, then the reason. Every file is validated and a
malformed one fails with its path; the message carries the format, so
the directory needs no README and does not exist until the first raise.
The docstring and dated comments stay as history, with a note that raises
from 2026-10-14 on go in the ledger.

README.md is unchanged: the page's blob was README's (ae817c5), so the
fingerprints recorded are the README as the page already read it.
tests/test_readme_sections.py holds the machinery to fixture text: the
segmenter (fences, preamble, CRLF), the fingerprint against git's own
blob hashes, each failure message, and the ledger.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ker stands alone between blank lines at the top of its section, HANDBOOK_READS names exactly the README sections each part of the page reads, and a carriage return or a `##` heading the cut would miss in README.md, a hash of the whole README in any spelling, or an "added (#N)" clause after the footer's history closes is refused (#519)

The fix round from the independent review of 0f808df on PR #551.

SF1. Four § Starting markers sat inline beside page text quoted from
another README section (ci, legs, doublepin, park), so a branch that
reworded README 606 and its page line conflicted with one that moved the
§ Starting markers. Every marker now stands at the top of its section,
right after the opening tag, one per line with a blank line above and
below, and the test refuses a marker that shares its line or touches a
non-blank line. That rule replaces the adjacent-marker rule.

SF2. Two markers on one line passed the adjacency check; the
alone-on-its-line rule refuses them, with a fixture case.

SF3. A secondary marker could be deleted silently, because (d) asked only
for the mapped heading's marker and one marker per section. HANDBOOK_READS
now lists the README segments each part of the page reads (masthead,
every section, footer; `agents` empty), and the markers must match their
row exactly, none missing and none extra. Every part has a row, every
row names a real part and real segments, every README segment is in a
row, and README_HEADING_TO_HANDBOOK_ID agrees with it. The messages say
the table changes only when the page's structure does, and that a
section added is a row added. HANDBOOK_SECTION_READING_NO_README is gone.

N1. HANDBOOK_RETIRED_README_BLOB matches `README.md\s+blob\s+<7-40 hex>`
in either case, across a line break. The footer's history must close
once with "That list is history, and it ends there.", and no "added
(#N)" clause may follow it. The check is keyed on that sentence, not on
a PR number, so a clause an older PR merges into the history stays
history.
N2. A carriage return anywhere in README.md is refused: README is LF-only
by .gitattributes, and folding CRLF would hide a lone-CR change.
N3. Ledger files are read as utf-8-sig, UTF-16 is refused with the
`Set-Content -Encoding utf8` remedy, and dotfiles and editor backups are
ignored.
N4. A heading two segments share is refused once, and (c) offers no hash
for it.
N5. Outside fences, an `##` indented one to three spaces, `##` followed by
a tab or nothing, and a setext `-` underline under paragraph text are
refused with "write it as `## <name>` at the start of the line".
`##Name` is no heading and passes.
N6. The footer reads "Each section that reads the README records every
README section it reads ...".
N7. The three comments in .github/workflows/tests.yml that described
#91's single README hash now name #519's per-section hashes.
N8. The cap test reads README.md as bytes decoded UTF-8, like its
neighbours.

Every new rule bites: switching each off in a scratch copy (layout,
missing and extra marker, heading map, CR, uncut headings, late clause,
the N4 skip, BOM, ignored names, UTF-16, short retired hash) fails at
least one test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ook marker must sit at the top of its section, the footer's "added (#N)" clauses are frozen as a tuple wherever a new one is put, and a horizontal rule under a list item's or a block quote's paragraph, or under an indented line, is not taken for a heading (#519)

The three nits from the re-review of 2f26ce3 on PR #551.

A. "At the top of its section" was not enforced: a marker moved below a
paragraph, blank lines and all, passed, and a branch deleting that
paragraph then conflicted with one moving the markers. A marker now has
nothing but blank lines and other markers between it and its part's
opening tag (`handbook_parts` gives where that tag ends). There is a new
fixture case for it.

B. A new "added (#N)" clause put before "That list is history, and it
ends there." passed, because the check looked only after the sentence.
HANDBOOK_HISTORY_CLAUSES freezes the footer's clause numbers in order.
Any `added (#N)` in the footer that is not in it, and any reordering, is
refused, and a clause after the closing sentence is still refused by name.
The fixture that let a merged-in clause through is replaced by one showing
a new clause refused, plus one for order. The tuple ends at (#502) here;
the merge of main that follows adds #511's (#471).

C. The setext check refused valid horizontal rules. README_NOT_PARAGRAPH
treats a line indented by a tab or four spaces as no paragraph text, and
readme_uncut_h2s tracks whether the paragraph being read is inside a list
item or a block quote (a lazy line, or one indented under an open list,
continues it), so a `-` run under such a paragraph is a rule. A setext
refusal now adds "or, if you meant a horizontal rule, put a blank line
above it". The review's three cases (list continuation, lazy quote line,
tab- or four-space-indented line) and a second paragraph of a list item
are fixtures that pass, and a paragraph after a closed list still fails.

Each fix bites: switching off the top-of-section rule, the clause tuple,
the container tracking or the tab rule in a scratch copy fails at least
one test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s footer clause joins the frozen history, its README blob line goes, and the handbook's markers for the three README sections #511 changed are moved after a re-read (#519)

#511 landed in v1.0.0 with README and handbook edits made under the old
footer rule, and conflicted with this branch in the footer only.

Footer: "added (#471)" is kept as the last clause of the frozen history,
before "That list is history, and it ends there."; #511's `README.md blob
f3b378d...` sentence is dropped. HANDBOOK_HISTORY_CLAUSES gains 471 as its
last entry, the one change this merge makes to the test.

.github/workflows/tests.yml merged cleanly: #511's install step is byte
for byte main's, and this branch's three comments stand.

The README sections #511 changed (git diff 3121945 5ed3e5d -- README.md),
re-read against the page:
- § Adopting an existing repository (read by `adopt`): the `###` heading
  "... no source line is overwritten", the block appended to a kept root
  `.gitattributes`, and its verification as the source's bytes plus the
  block are all in #511's own page edits. The rest is compression to keep
  the line count (`.gitignore` left out of the example list, "root tree"
  for "root tree after the split", "as `drop`"); the page's longer wording
  stays true. Marker moved.
- § Bootstrap is COPIED (read by `copied`): the appended block as the
  other answer is in #511's page edit; "who just clones" is compression.
  Marker moved.
- § Keeping a project's shape current (read by `current` and `doctor`):
  `apply --add` now names each file it adds in an adopted root's own
  `.gitattributes` block. #511's page edits did not cover this, so
  `current`'s upstream-added item now says it. The doctor subsection is
  untouched by #511. Both markers moved.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oot's own `.gitattributes` only when that file carries #471's line-ending block, so a root adopted before #471, which has the file and no block, is not read as one it writes to (#519)

The wording nit from the re-review of 1839889 on PR #551. The amendment
to `current`'s `upstream-added` item said "when the root carries one",
which reads as "when the root has its own `.gitattributes`". A root
adopted before #471 has that file but no block, and `apply` writes
nothing there. It now says "when that file carries #471's block". The
README is untouched, so no marker moves.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the repository but a `--placement-plan` file you name, and keeps its clone for the NEXT lines that name it rather than ones that "run from it"; the synopsis line has its description again; and a second `--placement-plan`, or a path after a second `--doctor`, is refused instead of silently replacing the first (#546)

What the doctor writes. `shape-doctor.py` writes one file at most, the
`--placement-plan <file>` it is given (`write_placement_plan`, through
`Path(...).expanduser()`, so a relative path counts from the process's
working directory, and no directory is created). The launcher never `cd`s
(`run_doctor`), so that is the directory the command was typed in, inside
the repository when the person stands there. Nothing else: no `.pyc`
(`sys.dont_write_bytecode`, `PYTHONDONTWRITEBYTECODE` for the validators)
and git reads take `--no-optional-locks`. Installed, the launcher also
clones the standard into `$TMPDIR/openRepoShape-doctor.XXXXXXXX/shape`
and keeps it once the doctor has run.

Line 1, "writes nothing". The two-modes refusal (`set_mode`) and the
refusal under `--doctor` now say it writes nothing into the repository
but that file; README.md's doctor paragraph and the handbook's twin say
the same, and the handbook adds where a relative one lands. The `--help`
paragraph already said so and is unchanged. AGENTS.md already named the
exception.

Line 2, the kept clone. The last line now reads "kept at <dir>, for any
NEXT command above that names it". A `pins` row's NEXT, where the project
carries its own validator, names no file in the clone, and "runs from it"
read as "cd into it", where the `placement` row's relative
`--placement-plan placement-plan.yaml` would land inside the directory
the line calls safe to delete. `--help` (so docs/cli.md), README.md,
AGENTS.md and the handbook give the same reason now: the NEXT lines that
name it. The footer's frozen history clause for #500 is left as written.

The synopsis. The doctor's description, "diagnose a REPOSITORY against
this standard", is back in the column the other lines use, on the line
under the full form, as argparse places the help of a long option. The
form stays one line, the one the refusal prints. docs/cli.md regenerated
with scripts/render-cli-reference.py.

Repeated flags. Two modes and two positional paths were already refusals,
never last-wins. Two `--placement-plan`s reached `shape-doctor.py`,
whose argparse kept the last, so `a.yaml` silently went unwritten; now
"two placement plans, 'a.yaml' and 'b.yaml'. --doctor writes one: pass
--placement-plan once.", exit 2, before anything is cloned. A path after
a second `--doctor` replaced the first one silently; it is now taken for
a second path, which the doctor already refuses.

README.md § Keeping a project's shape current moved (418f38b...). Re-read
both page sections that read it: `current` says nothing about the doctor
and stands; `doctor`'s opening paragraph is the twin amended here, and
the rest of it stays true. Both markers moved. README.md stays at 1422
lines and AGENTS.md at 490, so no cap is raised.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…own docstring say it writes nothing but a `--placement-plan` file you name, AGENTS.md puts that exception right after the claim it qualifies, and the hygiene test holds every "writes nothing" sentence, in any case, to its own exception (#546)

The fix round from the independent review (RV546) of df8b672.

1. docs/cli.md:37 still said the doctor "Writes nothing." The text is
   `TOOLS`' purpose for shape-doctor.py in
   scripts/render-cli-reference.py, which now says "Writes nothing but a
   `--placement-plan` file you name."; docs/cli.md is regenerated, that
   one line moves, and `--check` exits 0.
2. shape-doctor.py's docstring said "It never writes into the repository
   it is pointed at". It now says it writes nothing there but a
   `--placement-plan <file>` the caller names. Only the docstring's first
   line reaches `--help`, so docs/cli.md does not move for it.
3. test_the_doctor_says_what_it_writes_and_why_it_keeps_its_clone asked
   only that a paragraph saying "writes nothing" mention
   `--placement-plan` somewhere, case-sensitively, so deleting AGENTS.md's
   exception sentence passed, and a sentence-initial "Writes nothing"
   escaped. It now matches `(?i)\bwrites nothing\b` per sentence, and a
   sentence passes only when it names `--placement-plan` itself or the
   very next sentence names it as the exception to "writes nothing". It
   covers docs/cli.md's purpose line as a fifth passage.
   AGENTS.md's exception sentence sat two sentences after the claim, behind
   the REGISTRY sentence, so the two now swap places. The words are the
   same except the paragraph's last clause, now "the human's to delete
   once those have run", which keeps the file at its 490-line cap.
4. The `standing-in-the-repository` case runs `--doctor --placement-plan
   plan.yaml` from inside the project, so it covers the default path `.`
   as well.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…so ends at `.)`, `."` or `.**`, and shape-doctor.py's "WHAT IT NEVER DOES" paragraph is rewrapped with no word changed (#546)

The two optional nits from RV546's review of a4692b2.

- `writes_nothing_unexcepted` split sentences only at a stop followed by
  whitespace, so a stop inside a bracket, a quotation or bold text did
  not end the sentence. It now splits after `.)`, `."`, `.'`, `.*` and
  two of those. Both mutants are still killed: docs/cli.md's purpose back
  to "Writes nothing.", and AGENTS.md's exception sentence deleted.
- The fix round's rewrap left a short line at shape-doctor.py:24. The
  paragraph is rewrapped at 78 columns, which also evens its two older
  ragged lines; the words are the same, checked by comparing them
  whitespace-folded. Only the docstring's first line reaches `--help`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 1 day and 22 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR updates the doctor’s user-facing contract across source, generated documentation, and handbook references, while hardening the launcher against duplicate plans and paths before any clone or write occurs, with comprehensive regression and hygiene tests.

Sequence diagram for duplicate doctor argument validation

sequenceDiagram
    actor User
    participant Launcher as openRepoShape
    participant Doctor as shape-doctor.py
    participant Clone as TemporaryClone
    participant Plan as PlacementPlan

    User->>Launcher: --doctor [path] [--placement-plan file]
    Launcher->>Launcher: validate duplicate --doctor, path, and --placement-plan
    alt duplicate option or path
        Launcher-->>User: refusal, exit 2
        Note over Launcher,Clone: No clone or file write
    else valid arguments
        Launcher->>Clone: clone standard when required
        Launcher->>Doctor: invoke doctor
        Doctor->>Plan: write named placement plan when requested
        Doctor-->>User: report and kept-clone path
    end
Loading

Flow diagram for doctor output and kept standard clone

flowchart LR
    Start["Run openRepoShape --doctor"] --> Validate["Validate options and path"]
    Validate -->|valid| Source{"Standard checkout available?"}
    Validate -->|duplicate plan or path| Refuse["Refuse with exit 2"]
    Source -->|yes| Doctor["Diagnose repository"]
    Source -->|no| Clone["Clone standard into temporary directory"]
    Clone --> Doctor
    Doctor --> Plan["Optionally write named --placement-plan"]
    Plan --> Output["Print report and clone path"]
    Output --> Next["NEXT commands may name the kept clone"]
Loading

File-Level Changes

Change Details Files
Align documentation and generated CLI text with the doctor's actual write behavior and retained-clone handoff semantics.
  • Document --placement-plan as the sole repository write exception, including relative-path placement.
  • Explain that installed runs retain the standard clone for subsequent commands that name it.
  • Restore the doctor synopsis description and update handbook content markers.
AGENTS.md
README.md
docs/cli.md
docs/handbook.html
scripts/render-cli-reference.py
shape-doctor.py
Add launcher validation so duplicate doctor inputs are rejected before cloning or writing.
  • Reject a second --placement-plan with an explicit exit-2 diagnostic.
  • Reject a path supplied after a second --doctor instead of silently replacing the first path.
  • Preserve refusal behavior for invalid doctor arguments and verify no clone or output file is created.
openRepoShape
tests/test_shape_doctor.py
tests/test_openreposhape_command.py
Expand regression coverage for documentation consistency, help formatting, placement-plan behavior, and refusal boundaries.
  • Test relative plans written in the current repository while confirming no other tree changes.
  • Check all documented doctor passages for the placement-plan exception and correct kept-clone rationale.
  • Cover duplicate plans, duplicate paths, refusal wording, and synopsis column placement.
tests/test_repo_hygiene.py
tests/test_shape_doctor.py
tests/test_openreposhape_command.py

Assessment against linked issues

Issue Objective Addressed Explanation
#546 Correct the claim that the doctor writes nothing into the repository, making clear that a named --placement-plan file can be written there when the path is relative and the command is run from inside the repository. ✅
#546 Clarify that the installed doctor's kept clone is for NEXT commands that name it, rather than implying every NEXT command runs from the clone. ✅
#546 Cover the ride-along items: restore the --doctor synopsis description and refuse a second --placement-plan instead of silently keeping only the last one. ✅

Possibly linked issues


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

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-3, W546) Merged with main 9f809390 at c637fb86. Main brought in #554, #553, #561, #562, #579 and #567.

  • The one conflict: docs/handbook.html, the two § Keeping a project's shape current markers (current and doctor). Both are now 545639ebf5e9ab7837df5c012f216832dc7de56f, the value test_repo_hygiene.py check (c) printed for the merged README. Relative to main, that section differs only in this PR's doctor paragraph, and the handbook's doctor paragraph is its amended twin. README.md and the handbook: the by-hand setup commands dry-run the scaffold with the --visibility and --elected-by setup passes, and apply --add names all five refusals it gives by their codes, each held by a test (#550, #429) #579's current wording for the --add refusals and its § Starting markers are kept as main has them.
  • docs/cli.md auto-merged and was regenerated with scripts/render-cli-reference.py anyway; --check is ok. Against main, it differs only by this PR's three hunks: the doctor's purpose line, the synopsis description, and the kept-clone reason. Lane 4's bump-leg help is main's.
  • shape-doctor.py and tests/test_shape_doctor.py auto-merged. git diff --stat origin/main HEAD lists only this PR's 10 files. Every file but the handbook and docs/cli.md has the same patch-id as before the merge.

Tests on c637fb86, with a stub gh first on PATH, GH_TOKEN=bogus and an empty GH_CONFIG_DIR:

  • test_repo_hygiene 58 passed;
  • test_readme_sections 76;
  • test_agents_md_codes 47;
  • test_openreposhape_command 46;
  • test_cli_reference 22;
  • test_doctor_machine_row 8;
  • test_doctor_next_is_a_command 44;
  • test_shape_doctor.py 382 passed, 3 skipped.

@sonarqubecloud

Copy link
Copy Markdown

@brettheap
brettheap merged commit dd3f23a into main Oct 15, 2026
5 checks passed
@brettheap
brettheap deleted the l3-546-doctor-writes-wording branch October 15, 2026 12:19
brettheap pushed a commit that referenced this pull request Oct 15, 2026
Brings in #580 (#546), dd3f23a. One conflict:
	tests/test_repo_hygiene.py: #564's one-line-install test and #580's
	  DOCTOR_PASSAGES test were added at the same place; both are kept,
	  #564's first.
The launcher, its test, README.md, AGENTS.md, docs/handbook.html and
docs/cli.md merged cleanly, each holding both sides' hunks unchanged.
README.md stays 1449 lines (cap 1449) and AGENTS.md 490 (cap 490). Every
handbook marker of § Starting, § Layout and § Keeping equals its segment's
fingerprint in the merged README after a re-read, so none moves.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 15, 2026
…y-hand test and #581 among them) into #569, so the PR's CI runs on current main

Merges origin/main fb786c1, read from the
shared store without a fetch. Main moved from 7d9bec9 (#569's last merge) by
#556, #557, #558, #559, #560, #562, #567, #568, #579, #580 and #581. #579
(#550) landed tests/test_setup_by_hand_commands.py, which this PR's
`--no-next-steps` makes red; the next commit answers it. The merge itself is
clean, and its tree is the pm569 prediction's #581 step (53b6d0f7)'s, so the
resolution prepared against the predicted main carries over.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 15, 2026
… fix/540-pin-source-unread

This merges origin/main 2bf8fd4 as the shared store held it at
2026-10-15T16:50Z, thirteen landings past the 7d9bec9 this branch last
merged: #555 (shape-doctor's manifest kinds), #565 (scaffold's
`scaffold-remote-exists`), #581 (adopt's `check` of a kept pin path and
an unwritable work directory), #568, #559, #557, #560, #580, #556, #558,
#567, #579 and #562. It is done so that the PR's CI merge ref and this
round's rewording run on current main.

The merge was clean, with no conflict hunk. `adopt-project.py`,
`scaffold-project.py`, `scripts/shape_materialize.py`, `docs/cli.md` and
`tests/test_adopt_root_symlinks.py` took main's changes beside this PR's
by the textual merge, and `scripts/render-cli-reference.py --check`
passes on the result.

Main: 2bf8fd4.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 15, 2026
…mit, so the next landing no longer cancels it, and every landing gets its own Linux result (#592) (#593)

A push to main has no pull request number, so every push run on main
shared the group `tests-refs/heads/main`, and `cancel-in-progress: true`
made each landing cancel the run on the main before it. From #561
(7d9bec9) to #580 (dd3f23a) on 2026-10-15 every push run on main ended
`cancelled`, and nine PRs landed with no Linux run over their
combination.

- The `concurrency:` group's fallback changes from `github.ref` to
  `github.sha`, and the group now names the event:
  `${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.sha }}`.
- A pull-request run is still grouped per PR, and a new push to a PR
  still cancels its older run.
- A `push` run on main is grouped by its own commit, so no later landing
  cancels it, and each landing keeps its own Linux result.
- `github.event_name` keeps a push run and a dispatch apart: a dispatch
  on main never cancels that commit's push run, and a second dispatch on
  the same commit still replaces the first.
- A comment above the block says why and cites #592. The header comment,
  which #597 rewrote for Linux-only CI, is unchanged.
- `tests/test_workflow_concurrency.py` reads `tests.yml` as text and pins
  the group expression, refuses both earlier spellings, pins
  `cancel-in-progress: true`, and fails if a job sets a `concurrency:` of
  its own.

Lane: openreposhape-3 (openRepoShape-3)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 15, 2026
…es not have as the index has it (missing, unstaged, untracked, or behind assume-unchanged or a sparse checkout), so neither commits a change to it nobody staged (#407) (#571)

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

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

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

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

- **`gitmodules-missing`** (`remove`): the work tree has no `.gitmodules`, while the index (at any stage) or HEAD has one. The remediation names the restore git takes in that state:
  - `git checkout -- .gitmodules` (the index's copy);
  - `git checkout HEAD -- .gitmodules`, where the deletion is staged;
  - for an unmerged file, by the stages the index holds: `git checkout --merge -- .gitmodules` where it has both sides' copies; `git checkout --ours` where only this side's is left (the other side deleted it); `git checkout --theirs` where only the other side's is; `git checkout-index -f --stage=1` where both sides deleted it. Each is followed by `git add .gitmodules`.
- **`gitmodules-unstaged`** (`remove` and `add`): the work tree's copy is not the index's. That means one of:
  - git lists it as modified (`git ls-files -m`). Where `git add` would still stage the index's own blob from it (`git hash-object --path=.gitmodules` says so, and it is a regular file), the refusal adds that the difference may be its line endings alone;
  - it is unmerged;
  - its deletion is staged while the file is still on disk;
  - the index marks it assume-unchanged, which `ls-files -m` does not look past, and its content is not the index's blob;
  - it is untracked: neither the index nor HEAD holds one. This includes the first member's `add` in a new holder.

  The remediation offers both ways out: stage the work tree's copy, or put back the index's (or HEAD's) copy. Behind assume-unchanged, staging first takes the mark off (`git update-index --no-assume-unchanged -- .gitmodules`), because `git add` stages nothing while it is on. An untracked file is staged with `git add -f .gitmodules`, since an operator's global excludes may match it (and, if wanted, committed on its own first), or moved out of the holder.
- **`gitmodules-sparse`**: the index marks `.gitmodules` skip-worktree, as a sparse checkout that leaves it out does. git neither compares nor stages that file, and `git checkout -- .gitmodules` matches nothing. The remediation is `git sparse-checkout disable`.
  - `add` refuses it whatever the file's state, since `git submodule add` cannot stage it.
  - `remove` refuses it where the file is missing or is not the index's copy, and commits a clean copy on disk rightly, as it did on bfed143.

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

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

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

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

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

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

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

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

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

Round 2's targeted runs, on cb159b1 (round 3 changes `family.py`'s skip-worktree condition and one remedy, and the new test file):
- `tests/test_family_remove_gitmodules.py tests/test_family_bump_remove_rollback.py tests/test_family_dry_run.py tests/test_family.py tests/test_cli_reference.py tests/test_repo_hygiene.py tests/test_family_member_path.py tests/test_family_tracking_branch.py tests/test_family_printed_quoting.py tests/test_family_member_name.py tests/test_family_bump_to_a_tag.py tests/test_family_init_landing.py tests/test_family_init_values.py tests/test_template_text_placeholders.py`: `706 passed, 19 skipped in 1013.94s (0:16:53)` (the skips are `test_family_init_values.py`'s template-quoting cases, as on main);
- `tests/test_family_add_refusals.py tests/test_validate_family_duplicates.py tests/test_validate_family_member_path_glob.py tests/test_validate_family_unnamed_mounts.py tests/test_validate_family_without_git.py tests/test_family_add_excludes.py tests/test_family_add_member_id.py tests/test_refusal_default_remediation.py tests/test_agents_md_codes.py tests/test_shape_doctor.py`: `632 passed, 3 skipped in 1482.84s (0:24:42)` (the skips are the Windows-only PowerShell line).

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

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

Nothing here needs real GitHub or a cloud tool.

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

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

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

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

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

`git merge-tree --write-tree` of this head (ebe6b51) with each of these is clean:
- origin/main dd3f23a. Since bfed143, main took #551, #552, #554, #553, #561, #562, #579, #567, #558, #556 and #580; none touches `scripts/family.py` or a test file this branch changes. This branch does not merge main;
- `fix/family-bump-to-checks` 8024f9c (#568), `fix/family-members-case-variants` 8a95345 (#574), `fix/link-source-chain-defaults` a707d4b (#576), `fix/family-siblings-tracking-branch` 2156a01 (#578);
- lane 2's `fix/537-dry-run-asks-remotes` 69c58aa (#573), and #575's head 0680799 (lane 2, which changes `validate-family.py`; its unreadable-file remedy still ends "then run this again.", which `test_refusal_default_remediation.py` expects).

- **(a) `family.yaml` hand edits are committed by `add`, `bump` and `remove`.** Each commits `family.yaml` as the work tree has it, so an unstaged hand edit outside `members:` (a comment line) goes into the commit: the review reproduced `+# a hand note nobody staged` in a commit of each verb. It is #407's rule for a different file, and a staged-only rule there would chain into every remedy that says "edit `family.yaml`, then re-run", as S1 did here. It needs its own issue.
- **(b) A HEAD that already lost `.gitmodules` fails after the first write.** With no `.gitmodules` anywhere, this check asks nothing. `remove --dry-run` exits 0 and says no section is removed; the real `remove` runs `deinit` and `git rm`, then dies at `git --literal-pathspecs add -f -- .gitmodules family.yaml` (`pathspec '.gitmodules' did not match any files`) and puts back only part of the unmount (`members/Nova` is left empty). Same on bfed143. Once the old file is put back in the work tree, this PR now refuses it as untracked, and staged as printed `remove` commits it back into HEAD; but nothing tells the person to do that.
- **(c) The gate passes a row with no `.gitmodules` section (#459, the issue's decision 3).** `validate-family.py` should find a `members:` row whose path no section of HEAD's `.gitmodules` gives. A holder already carrying a #407 commit passes its own gate, and only a fresh recursive clone shows it.
- **(d) A sparse checkout whose rules leave `.gitmodules` out, with the file put back on disk by hand.** git then clears the skip-worktree mark, so this check passes, and `remove`'s commit fails at `git add` ("outside of your sparse-checkout definition") and is rolled back. Nothing wrong is committed; it is a late failure, and reading the sparse rules is not this PR's.

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

Review: rv407 found 1 should and 6 nits on b1120c3; fix round 2 (cb159b1) fixed them and answered Codex's `:4476` and `:4519` threads; re-review rv407b found 4 nits, fixed in round 3 (ebe6b51). CI (Brett Heap's ruling 27): H0 ebe6b51 passed tests, tests-macos, tests-windows and SonarCloud; the head 52c7c88 is a clean update-branch merge of main 2989b4c, with tests and SonarCloud green. Landed on Brett Heap's word of 2026-10-15T13:32:47Z, "All five, once green".

Closes #407

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The installed doctor's wording: "writes nothing" despite a relative --placement-plan, and the kept clone's last line

1 participant