Skip to content

validate-repository-naming.py, validate-manifest.py and validate-pins.py: a project's own neutral_product_pins: or a leg's naming.referent_chain: that is not a list is validate-manifest.py's finding for that field in every gate that reads it, so 5 no longer ends make validate in a TypeError and a scalar openXdox is no longer read as its characters (#443) - #631

Merged
brettheap merged 4 commits into
mainfrom
fix/443-validators-non-list
Oct 16, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

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

What changed and why

#443: four readers of a project's own declarations iterated whatever was there, each as for x in (value or []):

reader field
scripts/validate-repository-naming.py _pins_from_project neutral_product_pins:
scripts/validate-repository-naming.py _chain_from_leg a leg's naming.referent_chain:
templates/assembly-root/scripts/validate-manifest.py _declared_pins neutral_product_pins:
templates/assembly-root/scripts/validate-pins.py PinSources, via _check_neutral_product_pins neutral_product_pins:

What that did, re-measured on main 8f453d0 with the issue's own script:

Now each reader takes a value that is not a list as declaring nothing: no pins, no chain. That is how validate-manifest.py's _recorded_chain already reads a chain that is not a list. A string is still a chain's one link, under that function's string rule. Each gate that reads the field then says validate-manifest.py's own sentence for it, code and words:

FINDING manifest-neutral-product-pins: neutral_product_pins is 5, expected a list of neutral product names
FINDING naming-referent-chain: leg 'assembly': referent_chain is 5, expected a list of neutral-product names
  • The naming gate. _pins_field_findings is said before any leg's finding, in _targets_from_project. _chain_field_findings is said first in _leg_target, naming the leg by its declared role: as validate-manifest.py does. _chain_from_leg is now _recorded_chain rule for rule. The module docstring's exit-1 line names the case.
  • validate-manifest.py. _declared_pins returns the empty set for a non-list. _findings is untouched, and its existing finding is now reached.
  • validate-pins.py. _check_neutral_product_pins asks before PinSources reads the value, records the finding, and checks no pin.

Every case in the issue, after the change (the issue's script, at this branch's bytes):

value naming gate validate-manifest.py validate-pins.py make validate
pins 5, true, 1.5, openXdox, {a: b}, 0, false, '' exit 1, the pins sentence exit 1, the pins sentence exit 1, the pins sentence exit 2, stopped at the naming gate on the sentence
chain 5, true, 1.5, {a: b}, 0, false exit 1, the chain sentence exit 1, the chain sentence (as before) exit 0 (it reads no chain) exit 2, stopped at the naming gate on the sentence
pins [], absent, null; chain [], null, openXdox, '', [openXdox] exit 0 exit 0 exit 0 exit 0
pins [openXdox] exit 0 exit 0 exit 2, neutral-pin-missing for 'openXdox' (as before) as before

There is no traceback in any row. The doctor gives INVALID (naming, manifest, pins), exit 1, for 5 or openXdox in the pins, and INVALID (naming, manifest), exit 1, for 5 in a chain. Each of those rows quotes the sentence, and none is a crash.

Decisions the issue left to the author:

  1. A finding, exit 1, not a refusal (the issue offers either). A field that is not a list is a fault in the manifest a person can fix. validate-manifest.py already reports it with exit 1, and validate-pins.py's nearest kin, neutral-pin-entry-malformed, is a finding too.
  2. One sentence. The two existing sentences are reused, so every gate says the same thing and the doctor quotes one line in every row.
  3. Guards in each validator, not the issue's option 4 (a helper in scripts/repo_shape.py). This is the smallest change that makes every case right. A helper would put a fourth pinned file in the change and make three validators import a new name from it, so a project would have to take all four copies together, and scripts/repo_shape.py: the chain readers identify a link, a declared pin and a --link-source key by casefold() (twelve calls, :1207 to :2428), where #275's fix (PR #409, 6494714) makes a product's identity pin_product_key (lower(), the pin file's own fold), so openß and openss are two products to the pin validator and one link to the chain: a holder's tree that declares openss verifies a chain that says openß, a project that pins openss verifies a chain that begins at openß, a --link-source openss= answers for the link openß, link_pins_from_trees keeps one entry for the pair, and a chain that names both is refused as a cycle #444's branch is changing that file's chain readers now. A test holds the naming gate's two sentences equal to validate-manifest.py's, so the copies cannot drift apart unnoticed.
  4. 0, false and '' are findings in every gate.

Entries inside a list that are not names ([5], [[a]]) are #443's "not examined", and are untouched.

#459's checklist, R557's two lines (issuecomment-6175509630).

  • The first is this issue: "each script should report a non-list field as a FINDING". It is covered here.
  • The second, the naming gate's UnicodeEncodeError on a strict stdout, is a different class (stdout encoding) and is not touched.

Pinned shape copies. All three files are digest-pinned copies in every scaffolded root's contracts/shape-pin.yaml. Each has one copy in this repository, and no test records a digest or blob of any of them. Earlier changes to each edited the file's one copy and no recorded digest: #576 and #575 for validate-pins.py, #390 for validate-manifest.py, and #453 and #576 for the naming gate. So nothing else moves here. scripts/repo_shape.py is not touched. A project takes the change through update-shape.py apply, as three upstream-changed rows.

Tests

New file tests/test_non_list_project_fields.py, 71 cases (68 at 9733547, and 3 added by fix round 1). It uses the harness of tests/test_manifest_validator.py: its edit, the scaffolded assembly leg's ASSEMBLY_NAMING block (both imported) and conftest's project. The doctor cases use tests/test_shape_doctor.py's session standard and scaffolded_here.

  • The pure readers (19). The issue's values are run through _pins_from_project, _declared_pins, _chain_from_leg and _recorded_chain. Nothing is read as characters (openXdox) or keys ({a: b}). The naming gate's two sentences equal validate-manifest.py's, word for word. The controls (null, [], [openXdox], a chain's one-link string and '') read as before.
  • Each gate as the project runs it, from its root (36).
    • The pins: 5, true, 1.5, openXdox, {a: b}, 0, false and '', in each of the three gates.
    • A chain: the first six of those (not the strings), in the naming and manifest gates.
    • Each case asserts exit 1, the sentence exactly once, no Traceback and no neutral-pin-missing.
  • The controls in every gate (8). Pins absent, null and [], and a chain that is [], null, openXdox, '' or [openXdox]. Each keeps the exit main gave (measured at 8f453d0) and says neither sentence. [openXdox] keeps its neutral-pin-missing refusal.
  • The spec leg (2), fix round 1. 5 on the SPEC leg's naming.referent_chain:: both gates say FINDING naming-referent-chain: leg 'spec': referent_chain is 5, expected a list of neutral-product names exactly once, exit 1. Every other chain case records the chain on the assembly leg, so nothing held that the sentence names the leg the chain is on.
  • The order (1), fix round 1. The scaffolded Atlas is rewritten into codexDox, as tests/test_naming_policy.py's test_cli_finds_a_recorded_chain_that_does_not_hold does, with neutral_product_pins: openXdox (a scalar, so no pin) and referent_chain: [openXdox, openDox]. The naming gate exits 1, says the pins sentence once, and says it BEFORE the declared chain openXdox → openDox is broken. That line says openXdox is not in the project's neutral_product_pins:, which reads as false to someone who wrote it, unless the sentence before it says the field is not a list.
  • make validate (2). It stops at the naming gate, exit 2, on the sentence, and never starts the manifest gate. Skipped on Windows and where there is no make.
  • The doctor (3). 5 in the pins, openXdox in the pins, and 5 in a chain. Each gives the verdict above, exit 1. Every row that reads the field is a FINDING quoting the sentence, with no crash and no TypeError; the other rows are ok.

Fix round 1's run, on the COMMITTED merge a98abe2c (this head: fix commit 766d14a5 and main 4cf5fb07, clean): 651 passed, 0 skipped, in 88.13 s (Python 3.12.3, git 2.43.0). The 10 files, with their collected counts:

  • test_non_list_project_fields 71, test_manifest_validator 102, test_naming_policy 242, test_pin_validator 19;
  • test_validate_pins_lockstep_lines 15, test_validate_pins_unreadable_workflow 10, test_validate_pins_without_git 58;
  • test_repo_hygiene 65, test_cli_reference 22, test_agents_md_codes 47.

The bytes:

d463681b117115fe49a2fc32697d212a8dd0bf37166981154fdf92aa5d24220b  scripts/validate-repository-naming.py
bb9488c2a2e87428bab71fc6084dc52c0d848fde21a9a82a17a0ab63602acf77  templates/assembly-root/scripts/validate-manifest.py   (merged with #617; this branch's own bytes are 3a7a4be9..., unchanged)
0d7133d2705b50ae69a138e6b495fad8926b6e1c9a47c9913b39f3961f9d4e1c  templates/assembly-root/scripts/validate-pins.py
884c9479f049401948541dc3961cb8ae8eb83b532c14db53770876c2721d46fc  tests/test_non_list_project_fields.py

The wave writer's run, at 9733547 (before fix round 1): 1424 passed, 0 skipped, in 530.57 s. It ran over the committed bytes:

d463681b117115fe49a2fc32697d212a8dd0bf37166981154fdf92aa5d24220b  scripts/validate-repository-naming.py
3a7a4be98bc30dd9a43ae7f7d447656e2f7470ec6ffc94bec208662cff3d8910  templates/assembly-root/scripts/validate-manifest.py
0d7133d2705b50ae69a138e6b495fad8926b6e1c9a47c9913b39f3961f9d4e1c  templates/assembly-root/scripts/validate-pins.py
74861c3223194e1197f1a3f94a414bb6493c63eec69da345fab4bec48f4d51aa  tests/test_non_list_project_fields.py   (68 cases)

The 23 files were:

  • the new file;
  • test_naming_policy, test_manifest_validator, test_neutral_pin_validator, test_pin_validator, test_pin_source_env_collisions, test_pin_source_unreadable and test_link_source_chain_defaults;
  • test_scaffold_referent_chain_links, test_scaffold_pin_and_reuse, test_doctor_validator_rows and test_doctor_validator_verdicts;
  • test_validate_pins_lockstep_lines, test_validate_pins_unreadable_workflow, test_validate_pins_without_git, test_validator_child_encoding, test_shape_pin_files_not_a_list and test_refusal_default_remediation;
  • test_bootstrap_spelling and test_scaffold_e2e;
  • test_repo_hygiene, test_cli_reference and test_agents_md_codes.

The set is every file that runs the three validators directly, or reads either field through them, plus the doctor's validator-row files. About 48 more files run these validators only as one step of a scaffold, adopt, update-shape, bump or family flow. Those use the scaffold's neutral_product_pins: [] and no chain, and the list branch of each reader is the same expression as before. They were not run, and neither was test_shape_doctor.py, which writes these fields only as lists.

The three validators are pyflakes-clean, and all four files parse with ast.parse(feature_version=(3, 9)). --help is unchanged (only the module docstring's body moved), so docs/cli.md is too, and test_cli_reference passed.

Mutants. M1 to M4 revert one reader to for x in (value or []). M5 and M6 are R443's two that survived at 9733547, and fix round 1 kills them. Each was run against the new file alone, and each was killed:

mutant result
M1, the naming gate's _pins_from_project killed: 10 failed (5, true, 1.5 at the gate; the five non-list readers in process, the scalar string among them; make validate; the doctor)
M2, the list rule of the naming gate's _chain_from_leg killed: 9 failed
M3, validate-manifest.py's _declared_pins killed: 9 failed
M4, validate-pins.py's guard killed: 10 failed (all eight pins values at that gate, and both doctor pins cases)
M5, the naming gate's chain sentence names every leg 'assembly' (scripts/validate-repository-naming.py, _chain_field_findings) killed, fix round 1: 1 failed, test_a_chain_on_the_spec_leg_is_said_with_the_spec_leg_named[naming]. It says leg 'assembly' where the test wants leg 'spec'. 70 passed. The [manifest] case passes, as that gate is not mutated.
M6, both naming-gate sentences said LAST, not first (_leg_target and _targets_from_project) killed, fix round 1: 1 failed, test_the_pins_sentence_is_said_before_the_chain_line_it_explains. The broken-chain line is at index 18 and the pins sentence at 203. 70 passed.

After each mutant the file was restored, and its sha256 read d463681b…, the committed bytes. Taken apart, M6's two halves answer differently: the pins sentence said last (_targets_from_project only) is killed by that same test, and the chain sentence said last (_leg_target only) SURVIVES, 71 passed, because no case here holds a chain that is not a list beside another finding on the same leg. R443's N2 asked for the pins order only; the chain half is left as a test gap.

What it cannot tell

  • A project that has not applied this. The doctor's naming row runs the standard's copy, so it says the sentence at once. Its manifest and pins rows run the project's own copies. Over a project scaffolded from main they still say the copy "crashed before reaching a verdict (exit 1): TypeError: 'int' object is not iterable".
    • The verdict is still INVALID (naming, manifest, pins), exit 1, and no longer CANNOT ANSWER, exit 3. The crash that marked this checkout as unable to ask was the naming gate's.
    • In the same run, the doctor's shape currency row names the cure: 8 unchanged / 3 upstream-changed / 0 locally-modified / 0 both / 0 upstream-added (pinned 8f453d0e7db4, this standard 97335473665d): scripts/validate-pins.py, scripts/validate-manifest.py, scripts/validate-repository-naming.py. Its NEXT is update-shape.py check.
  • Python versions. Only 3.12.3 was run. The three changed files add no syntax newer than what they already use.
  • macOS and Windows. They were not run, and CI is Linux only. The new tests write project.yaml through the same edit helper tests/test_manifest_validator.py uses on every platform. The make validate case is skipped on Windows and where there is no make.

Shared files

Heads were read with git ls-remote at 10:27:27Z on 2026-10-16. This head a98abe2c (fix commit 766d14a5, merged with main 4cf5fb07) was checked with git merge-tree --write-tree against each:

other side head result
main 4cf5fb07 (#618 on #617) clean, tree fa09715a: main is now an ancestor, so this is the head's own tree. The fix commit alone, merged with main, gives the same tree.
lane 4's #633, fix/434-validate-pins-workflows-dir ac508957 clean, tree 49f8755f
lane 4's #638, fix/444-chain-fold 64e6621a clean, tree 723da260
lane 4's #644, fix/455-bootstrap-branch-bytes 733f8530 clean, tree 8fd04ef0
lane 2's #634, fix/610-pinned-text 6dad64bc clean, tree 8f3df105

Closes #443

🤖 Generated with Claude Code

Summary by Sourcery

Handle malformed project pin and referent-chain declarations consistently across validation gates without tracebacks or accidental scalar iteration.

Bug Fixes:

  • Report non-list project pin and referent-chain declarations as validation findings instead of crashing or iterating scalar values and mapping keys.
  • Keep naming, manifest, and pin validators consistent with the same field-type diagnostics and preserve correct handling of absent, null, list, and string chain values.

Enhancements:

  • Ensure naming findings are emitted before dependent leg findings and identify the affected leg by its declared role.

Tests:

  • Add comprehensive coverage for non-list declarations across validator readers, standalone gates, make validate, and shape-doctor results, including regression and ordering cases.

….py: a project's own `neutral_product_pins:` or a leg's `naming.referent_chain:` that is not a list is `validate-manifest.py`'s finding for that field in every gate that reads it, so `5` no longer ends `make validate` in a `TypeError` and a scalar `openXdox` is no longer read as its characters (#443)

Four readers of a project's own declarations iterated whatever was there,
each as `for x in (value or [])`: the naming gate's `_pins_from_project`
and `_chain_from_leg`, `validate-manifest.py`'s `_declared_pins`, and
`validate-pins.py`'s `PinSources`. So `neutral_product_pins: 5` (or `true`,
or `1.5`) ended all three gates in `TypeError: 'int' object is not
iterable`, exit 1. `validate-manifest.py` died one line before its own
`manifest-neutral-product-pins` finding for that value could print. A leg's
`naming.referent_chain: 5` ended the naming gate the same way, though
`validate-manifest.py` already had the finding for it. `make validate` runs
the naming gate first, so its traceback was the first thing a person read.
Since #557 the doctor answered that crash `CANNOT ANSWER`, exit 3, which
outranked the manifest's own finding. A scalar `openXdox` was read as the
pins `X`, `d`, `e`, `n`, `o`, `p`, `x`: the naming gate classified and
exited 0, and `validate-pins.py` refused `neutral-pin-missing` for a pin on
`'o'`. A mapping was read as its keys in either field.

What changed. Each reader now takes a value that is not a list as declaring
nothing, as `validate-manifest.py`'s `_recorded_chain` already read a chain
that is not one: no pins, and no chain. A string is still a chain's one
link, under that function's own string rule. Each gate that reads the field
then says `validate-manifest.py`'s own sentence for it, with the same code:

    FINDING manifest-neutral-product-pins: neutral_product_pins is 5, expected a list of neutral product names
    FINDING naming-referent-chain: leg 'assembly': referent_chain is 5, expected a list of neutral-product names

In detail:

- The naming gate gets `_pins_field_findings`, said before any leg's finding
  in `_targets_from_project`, and `_chain_field_findings`, said first in
  `_leg_target`. That function names the leg by its declared `role:`, as
  `validate-manifest.py` names it. `_chain_from_leg` is now
  `_recorded_chain` rule for rule.
- `validate-manifest.py`'s `_declared_pins` returns the empty set for a
  value that is not a list. Its `_findings` is untouched, so its existing
  finding is now reached.
- `validate-pins.py`'s `_check_neutral_product_pins` asks before
  `PinSources` reads the value, records the finding, and checks no pin.

A finding is exit 1 in each gate, and `make validate` now stops at its first
command on the sentence. The doctor's three rows quote it, and its verdict
is `INVALID (naming, manifest, pins)` for the pins, or `INVALID (naming,
manifest)` for a chain, exit 1.

Decisions the issue left to the author:

- A finding, not a refusal. The issue offers either exit. A non-list field
  is a fault in the manifest a person can fix, the same kind of fault
  `validate-manifest.py` already reports with exit 1 and
  `validate-pins.py`'s `neutral-pin-entry-malformed` reports with exit 1.
- One sentence. The two existing sentences are reused, code and words, so
  every gate that reads the field says the same thing and the doctor quotes
  the same line in each row.
- Guards in each validator, not a new helper in `scripts/repo_shape.py`
  (the issue's option 4). That is the smallest change that makes every case
  right. A shared helper would put a fourth pinned file in the change and
  make three validators import a new name from it, so a project would have
  to take all four copies together, and #444's branch is changing that
  file's chain readers now. The words stay validate-manifest.py's own, and a
  test holds the naming gate's two sentences equal to that file's, so a
  change to one breaks the test.
- `0`, `false` and `''` are findings in every gate. `validate-manifest.py`
  already said they are not a list. The naming gate and `validate-pins.py`
  read them as an empty list (`or []`) and exited 0. Now all three agree:
  absent or null declares nothing, and anything else that is not a list is
  the finding. #338's fix ruled the same for a link's manifest. For a chain,
  `''` is a string, so it stays an empty record with no finding, as
  `_naming_chain_field_finding` has it.

Entries inside a list that are not names (`[5]`, `[[a]]`) were not examined
by #443 and are untouched.

Who this reaches. All three files are pinned copies in every scaffolded
root's `contracts/shape-pin.yaml`. Each has one copy in this repository and
no test records its digest or blob, so nothing else moves with them. A
project takes them through `update-shape.py apply`, as three
`upstream-changed` rows. The doctor's `naming` row runs the standard's copy,
so it says the sentence at once. Its `manifest` and `pins` rows run the
project's own copies, so over a project that has not applied this they
still say the copy "crashed before reaching a verdict" with the
`TypeError`. Even so, the verdict is `INVALID`, exit 1, and no longer
`CANNOT ANSWER`, because the crash that marked this checkout as unable to
ask was the naming gate's.

Tests. New file tests/test_non_list_project_fields.py, 68 cases, on
tests/test_manifest_validator.py's harness (`edit` and the scaffolded
assembly leg's `naming:` block, which it imports) and
tests/test_shape_doctor.py's session standard for the doctor. The cases:

- The pure readers over every value the issue measured. Nothing is read as
  characters or keys, and the naming gate's sentences equal
  `validate-manifest.py`'s.
- Each gate the project runs, from its root, over `5`, `true`, `1.5`,
  `openXdox`, `{a: b}`, `0`, `false` and `''` in the pins, and over the
  first six in a chain: exit 1, the sentence once, no traceback and no
  `neutral-pin-missing`.
- The controls measured on main: absent, null, `[openXdox]` in the pins;
  `[]`, null, `openXdox`, `''` and `[openXdox]` in a chain. Each keeps its
  exit in every gate and says neither sentence.
- `make validate` stops at the naming gate on the sentence.
- The doctor's rows and verdict for `5` in each field, and for `openXdox` in
  the pins.

Closes #443

Lane: openreposhape-4 (openRepoShape-4)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 15, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR hardens the naming, manifest, and pin validators against non-list project declarations by adding consistent findings and safe parsing semantics, then verifies the behavior end to end—including make validate and the shape doctor—with extensive regression tests.

Sequence diagram for consistent validator findings

sequenceDiagram
    participant Project as Project manifest
    participant Naming as Naming validator
    participant Manifest as validate-manifest.py
    participant Pins as validate-pins.py
    Project->>Naming: Read neutral_product_pins
    Naming->>Naming: _pins_field_findings
    Naming-->>Project: FINDING manifest-neutral-product-pins
    Project->>Manifest: Read neutral_product_pins
    Manifest->>Manifest: _declared_pins
    Manifest-->>Project: FINDING manifest-neutral-product-pins
    Project->>Pins: Read neutral_product_pins
    Pins->>Pins: _check_neutral_product_pins
    Pins-->>Project: FINDING manifest-neutral-product-pins
    Project->>Naming: Read leg naming.referent_chain
    Naming->>Naming: _chain_field_findings
    Naming-->>Project: FINDING naming-referent-chain
Loading

Flow diagram for non-list project field validation

flowchart TD
    A[Project manifest] --> B{neutral_product_pins is a list?}
    B -->|Yes| C[Read declared pins]
    B -->|No, absent, or null| D[Declare no pins]
    B -->|No, other value| E[Record manifest-neutral-product-pins finding]
    E --> D
    A --> F{leg naming.referent_chain}
    F -->|List| G[Read chain]
    F -->|String| H[Read one chain link]
    F -->|Other non-list| I[Record naming-referent-chain finding]
    I --> J[Read no chain]
Loading

File-Level Changes

Change Details Files
Make all validators treat non-list project pin and referent-chain declarations as explicit validation findings instead of iterating scalars or mappings.
  • Guard neutral_product_pins readers with list checks and preserve absent/null as no declaration.
  • Align naming-gate chain parsing with manifest parsing: strings remain one-link chains, while other non-lists record no chain.
  • Emit manifest-neutral-product-pins and naming-referent-chain findings before classification or pin-source processing, using shared wording and role names.
  • Update naming-gate documentation to include the new finding condition.
scripts/validate-repository-naming.py
templates/assembly-root/scripts/validate-manifest.py
templates/assembly-root/scripts/validate-pins.py
Add comprehensive regression coverage for reader behavior, validator output, make validate ordering, and shape-doctor verdicts.
  • Test all scalar, boolean, numeric, string, mapping, null, empty-list, and valid-list cases across pure readers and validator entry points.
  • Verify exact finding text, exit codes, absence of tracebacks, and preservation of existing valid and neutral-pin-missing behavior.
  • Cover make validate's naming-first failure and the doctor's per-validator findings and INVALID verdicts.
  • Add mutation-oriented coverage ensuring each former unsafe iteration is detected.
tests/test_non_list_project_fields.py

Assessment against linked issues

Issue Objective Addressed Explanation
#443 Make validate-manifest.py handle non-list neutral_product_pins without crashing, so its existing manifest-neutral-product-pins finding is reached. ✅
#443 Make the naming gate safely handle non-list neutral_product_pins and referent_chain values, report the corresponding manifest-style finding, and avoid interpreting strings as pin characters or mappings as chain links. ✅
#443 Make validate-pins.py detect non-list neutral_product_pins before iterating them and report a field-specific finding instead of crashing or treating a scalar as individual pins. ✅

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

This was referenced Oct 16, 2026
Brett Heap and others added 2 commits October 16, 2026 10:26
… is not a list on the spec leg is held to name `leg 'spec'` in both gates, and the naming gate's pins sentence is held to come before the broken-chain line it explains (#443)

R443's review of 9733547 measured two mutants the new file let survive, and
neither was a wrong answer at that head.

N1. Every chain case recorded the chain on the assembly leg, so a naming gate
that named every leg `'assembly'` (scripts/validate-repository-naming.py,
`_chain_field_findings`) passed all of them. `set_chain` and `chain_said` now
take the leg's `naming:` block and role, and a new case records `5` on the
spec leg: both gates say `FINDING naming-referent-chain: leg 'spec':
referent_chain is 5, expected a list of neutral-product names` exactly once,
exit 1. Reverting the role to the literal `'assembly'` fails the naming-gate
case.

N2. The comments, the docstrings and the PR body all say the pins sentence is
said FIRST, and nothing tested it. A new case rewrites the scaffolded Atlas
into `codexDox` as tests/test_naming_policy.py's
`test_cli_finds_a_recorded_chain_that_does_not_hold` does, with
`neutral_product_pins: openXdox` (a scalar, so no pin is declared) and
`referent_chain: [openXdox, openDox]`. The broken-chain line says openXdox
"is not in this project's `neutral_product_pins:`", which reads as false to
someone who wrote that line, unless the sentence before it says the field is
not a list. The case asserts exit 1 and that the pins sentence's index in
stderr is below the broken-chain line's. Saying the naming gate's two
sentences last fails it.

Only the test file changes; no validator byte moves.

Lane: openreposhape-4 (openRepoShape-4)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Main moved to 4cf5fb0 since 8f453d0: #617 changed validate-manifest.py
(the answered_paths records and the moved-submodule rows) and #618 and others
changed adopt-project.py. The merge is clean, with no resolution, and the
tests ran on the committed merge.

Lane: openreposhape-4 (openRepoShape-4)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@brettheap
brettheap marked this pull request as ready for review October 16, 2026 11:21
@sourcery-ai

sourcery-ai Bot commented Oct 16, 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 10 hours and 30 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap merged commit 5b29ee1 into main Oct 16, 2026
5 checks passed
@brettheap
brettheap deleted the fix/443-validators-non-list branch October 16, 2026 11:22
brettheap added a commit that referenced this pull request Oct 16, 2026
…h, the `switch -c` remedy says to give it another name, or go back only if it holds work not merged yet, and the docs say `family/add-<Project>` (#607) (#637)

### Fix round 1 (rv607)

Review rv607 of 6212c31 said CHANGES NEEDED: 0 blockers, 2 should, 7 nits. This round is one new commit, 66b1043, on top of 6212c31. No amend, and main is not merged in. Each point:

- **S1, the sentence.** It sent a person back to the old branch first. That is right only where the branch holds work not merged yet. `BRANCH_LEFT` now puts another name first and going back second (the text is under "L2-46" below). It is word for word the same in `scripts/family.py`, in the pinned `validate-family.py` and in the pinned test constant, and the comments that described the old order are rewritten. Two new tests type the printed line in the states the review probed: `main` has moved since the earlier act, and a commit was made on a detached HEAD.
- **S2, a guard for every `_switch_line` remedy.** A new test reads the validator's source with `ast` and fails any remedy that names `_switch_line` whose prose does not end with `BRANCH_LEFT`. On a merge of this head with #634 it fails at `_lockstep`, which is the point: whichever of #637 and #634 lands second adds the sentence there. #634's lines are not on this branch and are not touched.
- **N1.** Under "Left out".
- **N2.** The commit body of 6212c31 said neither tool can print the one line that works by asking first. That holds for the validator only. `family.py` could ask; it says the validator's words so the two tools keep one rule. Decision 1 says so, and the new commit's body corrects the old one.
- **N3.** The merge table is refreshed: current origin/main, #620, #622, #626, #630 to #636, #638, #639 (lane 5's #611) and #634 (S2).
- **N4.** Decision 1 no longer says the typed line works "in every shell". It says what a person types is "the printed line with one word changed (the branch name, or `-c` taken out)", so it keeps the printed quoting, and that the tests type both changes as printed.
- **N5.** Decision 3 now says the case of the branch name "does not matter to `add`", and adds the note about case-insensitive disks.
- **N6.** The `init` NEXT STEPS test read the lines with a regex whose `--family-root \S+` breaks on a root with a space in it (`C:\Users\Jane Doe`, or any `tmp_path` with a space). It now reads the words with `host_words`, as the NEXT STEPS test beside it does. Run with a space in the base temp directory, the old test fails with `AttributeError: 'NoneType' object has no attribute 'group'` and the new one passes.
- **N7.** "Three existing tests take the new text" was wrong. It is two test functions plus `GITLINK_REMEDY`, a constant four cases share. Fixed under "Tests". The one new test that passes on BASE is disclosed there too.

### Fix round 2 (rv607b)

The re-verify rv607b of 66b1043 said APPROVE WITH NITS: 0 blockers, 0 should, 5 nits. This round is one new commit, a277695, on top of 66b1043. No amend, and main is not merged in. It changes comments and docstrings only, so no behaviour moves and no mutation row changes.

- **Nit 1.** Two comments called another name "right in every state". It is not: where the old branch holds work not merged yet, another name leaves that work behind. The comment above `BRANCH_LEFT` in `scripts/family.py` and the one in the pinned `validate-family.py` now say a new branch starts where the holder stands now, "which works in every state". The three `BRANCH_LEFT` texts are unchanged and still equal.
- **Nit 3.** The guard's docstring said it finds each `_switch_line` call "however it is passed on". A switch line moved into a variable would defeat that. It now says "wherever the call stands inside the string expression that is passed on".
- **Nit 4.** The N4 line above now quotes decision 1 as written.
- **Nit 5, the docstring half.** The docstring of `test_another_name_works_where_main_has_moved_since` said going back to the old branch conflicts, but the test does not assert that. It now says the test shows the other name works, and that the conflict on the old branch is shown by rv607's probe and decision 2's table. No assertion is added.
- **Nit 2, left as written.** The guard's control counts stay: each case fails loudly with a clear message.

Checks on a277695: the diff to 66b1043 adds ten lines and removes seven, in three files, all comments and docstrings, and the `ast` of each file is equal once docstrings are blanked. The four files `test_family_switch_branch_exists.py`, `test_validate_family_branch_step.py`, `test_validate_family_unnamed_mounts.py` and `test_family_tracking_branch.py` give `96 passed in 51.34s`, and `test_repo_hygiene.py` gives `65 passed in 1.39s`.

### L2-46, the fix

`family.py add`, `bump` and `remove` refuse the holder's tracking branch with `git -C <holder> switch -c family/<act>-<member>` (#487). The holder's `validate-family.py` names the same line before each `add` and `remove` it prints (#539, landed as 54041cc). If an earlier act left that branch, the line fails with `fatal: a branch named 'family/add-atlas' already exists` (exit 128), and neither tool said what to do next.

Both tools now add one sentence, in the same words (`BRANCH_LEFT` in each file):

> If `git switch -c` says that branch already exists, an earlier act left it: give the branch another name, which starts it where the holder stands, or, only if that branch holds work not merged yet, the same command with `switch` in place of `switch -c` goes back to it and keeps every commit on it.

- `scripts/family.py`: the sentence comes on its own line, right under the `switch -c` line, in the `family-on-tracking-branch` and `family-holder-detached` refusals (`_checked_branch`), and in the dry run's `branch` lines (`_branch_said`).
- `templates/family-root/scripts/validate-family.py`: the sentence ends the remedy text of `member-gitlink-absent`, `member-duplicate-id` and `member-gitlink-unnamed`, the three remedies that print `_switch_line`. The command lines under them are unchanged.

### L2-45, rides along

AGENTS.md, the holder's README, its AGENTS-shape.md and `init`'s NEXT STEPS spelled the branch `family/add-<project>`, right beside `--member <org>/<Project>`. All four now say `family/add-<Project>`, the one placeholder the validator already uses for both (`PROJECT_PLACEHOLDER`, #539).

**1. What the remedy prints when the branch exists: one sentence that names the way out, not a second command line.**

- *One line that works in both states* (`git switch X 2>/dev/null || git switch -c X`): rejected. On Windows, `family.py` quotes its lines for PowerShell (`shell_line` uses `os.name`; README.md:155-158 says so). Windows PowerShell 5.1 has no `||`; `&&` and `||` arrived in PowerShell 7, and `family.py`'s own `_print_next_steps` docstring says 5.1 rejects `&&`. PowerShell also has no `/dev/null`. And `2>/dev/null` would hide the real reason a `switch` fails, such as a changed file it would overwrite.
- *The tool checks and prints the one line that works*: rejected for the validator. Its report is read in CI: the holder's `validate.yml` runs `python3 scripts/validate-family.py` (and `--no-members`) in a runner's checkout, which holds none of the person's branches. `family.py` could check, but then the two tools would follow two rules, and one rule in the same words was the aim. So `family.py` says the validator's words; it is not that it cannot ask.
- *Two printed lines, one per state*: rejected. In the validator, each indented line is a step to run in order, and going back to the branch replaces step one; it is not another step. In `family.py`, a new `git switch` line would also be quoted for PowerShell on Windows. README.md:155-158 lists exactly which `family.py` lines are quoted that way (`NEXT`, `switch -c`, `add`, `git rm -r --cached`). That sentence would become false, and fixing it would mean editing README § "Starting a project in a new organisation". Seven handbook parts record that section's hash (`grep -c` gives 7).
- *Chosen*: a sentence that names both ways out. It prints no new command, so nothing new needs quoting, and the text is the same in both tools. What a person types is the printed line with one word changed (the branch name, or `-c` taken out), so it keeps the printed quoting. The tests type both changes as printed (tests below); none of them runs a shell, and Windows and macOS are not checked (see "Left out").

**2. Another name first, going back second (S1).** The reviewer's probes, and mine, in real local holders (a holder `Orbit`, members `Atlas` and `Beacon`, local bare remotes, the `gh` stub first on PATH):

| State of the old branch `family/add-atlas` | `git switch family/add-atlas` (going back) | `git switch -c family/add-atlas-2` (another name) |
|---|---|---|
| No commit on it, `main` has not moved (rv607) | works | works |
| Holds a commit of its own, not merged yet (mine: `probe-atlas.sh`, `probe-b.sh`) | works; `add` then commits on top of it, and the earlier commit is the parent | works, but the earlier commit stays only on the old branch (`git branch --contains` names `family/add-atlas` alone) |
| `main` has moved: a later member (`Beacon`) was squash-merged (mine: `probe-d.sh`; test `test_another_name_works_where_main_has_moved_since`) | `add` exits 0, but `git merge-tree --write-tree main family/add-atlas` exits 1 (`CONFLICT (add/add)` in `.gitmodules`, `CONFLICT (content)` in `family.yaml`), and an untracked `members/Beacon` is left (`warning: unable to rmdir 'members/Beacon'`) | `add` exits 0, `git merge-tree --write-tree main family/add-atlas-2` exits 0, and the validator says `2 member(s)` |
| Merged, then the member removed and that merged too; the old branch is still local (rv607's probe, and mine: `probe-c.sh`) | `add` says `member-already-present`, whose remedy points at `bump` and `remove`: a dead end | `add` exits 0, once the object store `remove` leaves behind is deleted, as its NOTE says (either choice needs that) |
| Detached HEAD with a commit made on it (mine: `probe-detached.sh`; test `test_another_name_keeps_a_commit_made_on_a_detached_head`) | `Warning: you are leaving 1 commit behind, not connected to any of your branches`, and `git branch --contains` names no branch | the commit stays: `git branch --contains` names the new branch |

So going back is right only where the branch holds work not merged yet, and another name is right in every state but that one. The sentence says so, in that order.

**3. The person's earlier work on that branch is never discarded.** `git switch <branch>` keeps the branch's commits. It refuses to switch when that would overwrite a changed file. The act then commits on top of what the branch holds; the tests check that the earlier commit is the parent of the act's commit. "Give the branch another name" leaves the old branch alone. No printed line names `switch -C` (which resets a branch), `branch -D`, `reset` or `--discard-changes`; a test checks that too.

**4. The placeholder: `<Project>` everywhere.**

- The validator already said `family/add-<Project>` beside `--member <org>/<Project>` (#539's fix round, pinned by `tests/test_validate_family_unnamed_mounts.py`). It is the only spelling that is one placeholder for both values.
- The other option, a sentence saying the branch word is the name in lowercase, does not fit: AGENTS.md is at its cap, 492 of 492 lines (`tests/test_repo_hygiene.py:633`). This change keeps it at 492.
- The case of the branch name does not matter to `add`. `_checked_branch` takes any branch but the tracking one, so `family/add-IRQS`, the doc line filled in as written, is a branch `add` commits on (test). Where the tools know the name, they still print it lowercased (`_suggested_branch`). On a case-insensitive disk (macOS, Windows) `family/add-Atlas` and `family/add-atlas` can be one loose ref there. That is untested: CI is Linux only.

**5. Pinned copies.** This changes two pinned holder files: `templates/family-root/scripts/validate-family.py` and `templates/family-root/AGENTS-shape.md` (`scripts/shape_materialize.py` lists the pinned set). Each holder takes them as two `upstream-changed` rows on its next `update-shape.py` run. The holder's README is rendered once and is not pinned. No test pins those files' bytes; the tests that compare them read the template at run time. Fix round 1 changes only `validate-family.py` of the two.

New file `tests/test_family_switch_branch_exists.py`, 25 tests (21 before the fix round). Four are new in the fix round:

- `test_another_name_works_where_main_has_moved_since` (S1, state (d)): `family/add-irqs` is left at the `main` of its day; a later member (`IRXS`, scaffolded in the test) is added on a branch and squash-merged onto `main`, so `main` has one new commit; the refusal's `switch -c`, typed as printed, fails with `already exists` (exit 128); the same line with another name works; `add` exits 0, `git merge-tree --write-tree main <branch>` exits 0, and the validator passes.
- `test_another_name_keeps_a_commit_made_on_a_detached_head` (S1): the refusal is `family-holder-detached`; after the printed line with another name, `git branch --contains <the commit>` names the new branch and no other.
- `test_every_remedy_that_names_a_switch_line_ends_with_the_sentence` (S2, the guard) and `test_the_guard_sees_a_remedy_left_without_the_sentence` (its control: the source with the sentence taken out of any one remedy fails it, and so does a fourth remedy built by a helper as `_lockstep` builds its, until the sentence ends it).

The other 21:

- `test_the_refusal_typed_as_printed_puts_the_act_on_its_branch` [add, bump, remove] x [no-branch, branch-left]: the refusal, then its line typed as printed. With no branch, the `switch -c` makes one. With a branch an earlier act left, holding a commit of its own, the `switch -c` fails with `fatal: a branch named '<branch>' already exists` (exit 128) and changes nothing, and the same command with `switch` in place of `switch -c` goes back to that branch. The act then commits on top of the earlier commit, and NEXT pushes that branch.
- `test_a_dry_run_on_the_tracking_branch_says_it_too` [add, bump, remove].
- `test_a_missing_gitlinks_remedy_typed_as_printed`, `test_a_duplicate_ids_remedy_typed_as_printed` (with the `remove` run as printed) and `test_an_unnamed_gitlinks_remedy_typed_as_printed`, each in both states.
- `test_the_two_tools_say_the_same_words`.
- `test_each_doc_spells_the_branch_as_its_member` [AGENTS.md, AGENTS-shape.md, holder README], `test_init_next_steps_spell_the_branch_as_its_member` (read with `host_words` now, N6), and `test_the_branch_filled_in_as_written_is_one_add_commits_on`.

Two existing test functions take the new text: `test_init_next_steps_make_the_branch_before_the_add` (`<Project>`) and `test_a_missing_gitlink_remedy_makes_the_branch_and_names_the_holder` (the remedy line). `GITLINK_REMEDY` in `test_validate_family_unnamed_mounts.py`, a constant four cases share (`test_a_member_mounted_by_hand_is_two_findings` x 3 and `test_a_removed_row_is_named_by_its_gitlink_and_its_section`), ends with the sentence. The sentence is pinned once, as text, in `tests/test_validate_family_branch_step.py`.

Before and after, with the four changed test files copied in (`run9.sh`, the `gh` stub first on PATH):

- On BASE 396c0ff: `30 failed, 66 passed` in the four files. 24 of the 25 new-module tests fail. The one that passes, `test_the_branch_filled_in_as_written_is_one_add_commits_on`, is the evidence for decision 4: it shows behaviour this PR keeps.
- On 6212c31: `23 failed, 56 passed` in the three files that pin the sentence. Both new S1 tests fail there, where they assert the new wording. The two guard tests pass there: they check where the sentence stands, not its words.
- On the head 66b1043: the new module alone, `25 passed`.

### Targeted runs on the head 66b1043, BASE 396c0ff

- Set A, 29 files: every `tests/test_family*.py` and `tests/test_validate_family*.py`, plus `test_bump_leg.py`, `test_bump_leg_dry_run_tracking.py`, `test_bump_leg_dry_run_truth.py`, `test_bump_leg_paste_lines.py` and `test_update_shape_branch_any_directory.py`. Result: `1016 passed, 20 skipped in 511.01s`.
- From set B: `test_repo_hygiene.py`, `test_agents_md_codes.py`, `test_readme_sections.py` and `test_cli_reference.py`. Result: `210 passed in 15.73s`. The fix round changes no template other than `validate-family.py`'s text, so the rest of set B (the 27 files of the first round) is not re-run.
- Every skip is a platform skip that was there before this change (a case-ignoring disk, a Windows default, powershell.exe), or a template-quoting skip in `test_family_init_values.py`.

A real holder `Orbit`, made with `family.py init`, and a member `Atlas`, scaffolded with `scaffold-project.py --local-remote-dir`.

- **On BASE 396c0ff** (first round): `add` refuses with `switch -c family/add-atlas`. Typed as printed, that gives `fatal: a branch named 'family/add-atlas' already exists`, exit 128, and the remedy says nothing more. The validator's `member-gitlink-absent` remedy prints the same line, with the same result.
- **On the head, an unmerged act** (`probe-atlas.sh`, a branch holding a commit of its own): the refusal prints the new sentence. The same command with `switch` for `switch -c` gives `Switched to branch 'family/add-atlas'`, exit 0, at the earlier commit. `add` then exits 0, its NEXT pushes `family/add-atlas`, and its commit's parent is the earlier commit. The validator carries the same sentence.
- **On the head, `main` moved** (`probe-d.sh`): the printed line fails (128). With another name: `add` exits 0, `git merge-tree --write-tree main family/add-atlas-2` exits 0, `family ok: Orbit (orbit), 2 member(s)`. Going back instead: `add` exits 0, and `git merge-tree --write-tree main family/add-atlas` exits 1 with the two conflicts in decision 2.
- **On the head, a detached HEAD with a commit** (`probe-detached.sh`): the printed line fails (128). With another name, `git branch --contains` names `family/add-atlas-2`. Going back with `switch`: git warns it is leaving 1 commit behind, and no branch contains it.

No row needs real GitHub.

Each change below was made alone in a scratch clone of the head, then the new module was run (and, for the validator changes, the two other files that pin the sentence), and the file was put back. Every change that alters behaviour makes at least one new test fail:

| Change | Result |
|---|---|
| `family.py` `BRANCH_LEFT` back to the first round's words | 12 failed: the six refusal cases, the three dry-run cases, both new S1 tests, `test_the_two_tools_say_the_same_words` |
| `family.py` `BRANCH_LEFT` one clause dropped (`only if`) | 12 failed, the same tests |
| `_checked_branch` remedy with no sentence | 8 failed: the six refusal cases and both new S1 tests |
| sentence printed above the `switch -c` line, not under it | 8 failed: the same eight |
| `_branch_said` (dry run) with no sentence | 3 failed: `test_a_dry_run_on_the_tracking_branch_says_it_too` [add, bump, remove] |
| `_switch_command` prints `switch -C` | 11 failed, both new S1 tests among them: the printed line no longer fails `already exists` |
| validator `BRANCH_LEFT` back to the first round's words | 12 failed in the three files, `test_the_two_tools_say_the_same_words` among them |
| validator `member-gitlink-absent` with no sentence | 5 failed: its two typed-as-printed cases, the remedy test, the guard and its control |
| validator `member-duplicate-id` with no sentence | 4 failed: its two typed-as-printed cases, the guard and its control |
| validator `member-gitlink-unnamed` with no sentence | 8 failed: its two typed-as-printed cases, the four `GITLINK_REMEDY` cases, the guard and its control |
| **S2:** a fourth remedy added that names `_switch_line` and does not end with the sentence (built by a helper, as #634's `_lockstep` does) | 2 failed: the guard and its control, and nothing else in the three files |
| **S2:** the same fourth remedy ending with `{BRANCH_LEFT}` | 79 passed |
| `init` NEXT STEPS branch back to `<project>`; its `--member` lowercased | 1 failed each: `test_init_next_steps_spell_the_branch_as_its_member` |
| AGENTS.md, AGENTS-shape.md, holder README: branch back to `<project>` (and AGENTS.md's `--member`) | 1 failed each: `test_each_doc_spells_the_branch_as_its_member` for that doc |

The other hunks change only docstrings and comments (`_checked_branch`, `_branch_said`, `_print_next_steps`, `_switch_line`, and the comments above the two constants). They change no behaviour, so no test can catch reverting them.

| Ref | Result |
|---|---|
| origin/main f6376d7 (#614 landed; main has moved since BASE through #625, #583, #596) | clean, tree eb91b600. A committed merge of the head with it, e26dca8c, ran set A and #625's `test_family_bump_to_local_only.py`: `1024 passed, 20 skipped in 500.52s` |
| #620 a2b4f89 | clean (it conflicted in `adopt-project.py` in the first round; it no longer does) |
| #622 227bc3b (lane 2, #609, `scripts/family.py`) | clean |
| #626 207d4e1 (lane 4, `scripts/family.py`) | clean |
| #630 82f9403, #631 9733547, #632 86c6b9c, #633 f7f5de5, #635 2c0b157, #636 4aade9f | clean |
| #638 64e6621 | clean |
| #634 40a85e1 (lane 2, #610: `_lockstep` prints a fourth `switch -c`) | clean, and S2 fires on it. A committed merge, 4d1a7161: the new module, `test_validate_family_branch_step.py` and #634's `test_pinned_copy_text.py` give `2 failed, 86 passed`. The two failures are the guard, `assert [124] == []` (line 124 of the merged file is `_lockstep`'s return), and its control. With one line added to `_lockstep`, `f"moving one alone is what this refuses. {BRANCH_LEFT}"` in place of the plain string, the same merge gives `306 passed` over the nine files that read the validator or the lockstep rule (`test_validate_family*.py`, `test_family.py`, `test_family_tracking_branch.py`, `test_pinned_copy_text.py` and the new module). So the second PR to land needs that one line and nothing else. |
| #639 6f938ed (lane 5, #611) | conflicts in the same one sentence, in the same two places: the end of the `member-gitlink-unnamed` remedy text in `validate-family.py` (`_check_gitlinks_named`), and the `GITLINK_REMEDY` constant in `tests/test_validate_family_unnamed_mounts.py`. #611 adds "and commit that removal, since `family.py add` refuses ..." and this PR appends `BRANCH_LEFT`. Resolved as in the first round, keeping #611's clause and appending ` {BRANCH_LEFT}` in the validator and `+ BRANCH_LEFT` in the test constant, in a committed merge (92602308): `scripts/family.py` merged cleanly, and the six test files that either PR changes or pins gave `157 passed`. Neither branch was resolved. |

- No compound `||` line and no state check: see decision 1.
- No README.md edit: no README sentence becomes false, because no new kind of line is printed.
- The docs do not mention the exists case. AGENTS.md is at its cap, and the tools print the sentence where the failure happens.
- The `switch -c` in `init`'s NEXT STEPS gets no sentence: a holder `init` just made has no branch an earlier act could have left.
- **A branch checked out in another worktree (N1).** Where `family/add-atlas` is checked out in another worktree of the holder, `git switch family/add-atlas` gives `fatal: 'family/add-atlas' is already used by worktree at '<path>'` (exit 128), so the going-back half of the sentence does not work there. Git names the worktree, and another name works (`probe-n1.sh`: `switch -c family/add-atlas-2` exits 0). The sentence is not made longer for it.
- I could not run Windows PowerShell or macOS here. The tests read `family.py`'s lines with `host_words`, which parses PowerShell's quoting on Windows. They make the earlier branch with `commit-tree` and `update-ref`, move `main` with the same two, write no file with `write_text` and compare nothing CRLF-sensitive. `test_another_name_works_where_main_has_moved_since` runs `git merge-tree --write-tree`, which needs git 2.38 or newer. No CI job checks Windows or macOS.

- **bump-leg.py: the `git switch -c bump/<role>-<sha12>` remedy fails `already exists` when an earlier bump left that branch.** `_switch_command` (scripts/bump-leg.py:1124) prints the line for the two refusals and the dry run's closing hint. Like #607, it says nothing about a branch that is already there.
- **setup-project.py: the family `git -C <holder> switch -c family/add-<project>` line has the same gap.** The NEXT lines a scaffold into a family folder ends with (setup-project.py:2358) print it with no word on an existing branch. It matters only when an earlier attempt left that branch.

- L2-46 ([#459 comment](#459 (comment)))
- L2-45 ([#459 comment](#459 (comment)))

Review: rv607 found 0 blockers, 2 should-fix and 7 nits on 6212c31 (CHANGES NEEDED), fixed in 66b1043; the re-verify rv607b found 5 nits on 66b1043 (APPROVE WITH NITS), of which nits 1, 3 and 4 and the docstring half of nit 5 are fixed in a277695, and nit 2 is left. CI (Brett Heap's ruling 27, with his fast-refresh ruling of 2026-10-15T22:48:35Z): H0 a277695 passed tests (Linux), tests-git-bash and SonarCloud; the head 62263d2 is a clean update-branch merge of main a524542 (over e041867, a clean merge of 5b29ee1), with SonarCloud, tests-git-bash and package green on it, and the local gate passed on the head itself: the family and validator set, 1016 passed (20 skipped), and the hygiene, AGENTS.md-code, README-section and CLI-reference sets, 222 passed. Landed on Brett Heap's word of 2026-10-16T11:21:49Z, "Merge #637 when its turn comes".

Closes #607

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 16, 2026
main gained #563's own #590 (fe237ec) and #599 (32c22b0), whose trees
this branch already held through #599's 750366b (#599's final head,
eaa0cda, is 750366b plus a merge of main), and #626, #633, #642, #641,
#643, #644, #632, #636, #631, #638, #635, #640, #623, #637 and #622. No
conflict: the merged tree is 9be0d0d, the tree `git merge-tree
--write-tree` predicted. AGENTS.md, docs/cli.md and
tests/test_repo_hygiene.py merged as text; README.md and
docs/handbook.html are unchanged by the merge. README.md is 1461 lines,
its cap (1449 + 12, tests/readme-cap.d/563.txt), and AGENTS.md 494, its
cap.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 16, 2026
… tab shows the one-line install for Git Bash, held to the other copies by a test, and the notes are current to main d4a7fab with the fifteen pull requests since 4cf5fb0, the environments, the tag ruleset and the rehearsal (#563, #461)

The handbook (#461's line "The handbook's step 3 has no Git Bash path",
which lane 3 told lane 5 on #473 that #630 settles): step 3's install
block was `data-platform="linux mac wsl"`, so a reader on the Windows tab
never saw the one line the Windows panel says Git Bash runs. The Windows
tab now has a lead, "In Git Bash, the bash that Git for Windows installs,
the one-line install works as on Linux:", the Linux tab's block copied
byte for byte as `data-lang="sh"`, and the panel's caveat: the preflight
it ends with cannot take a typed `yes` in Git Bash. README.md is
unchanged: no test asked for it, and it is at its cap.

tests/test_repo_hygiene.py: the test above it finds one copy of the one
line per file; `test_every_handbook_copy_of_the_one_line_install_is_the_same`
holds every handbook copy (the masthead's and step 3's two) to
ONE_LINE_INSTALL, so a copy edited on one tab alone fails. A drifted
Windows copy fails it; the page as committed passes.

The release notes:
- "What changed since v1.0.1" gains one entry per pull request that
  landed after 4cf5fb0 and is not #563's own, from `git log
  --first-parent 4cf5fb0..origin/main` at d4a7fab: under update-shape.py
  and bump-leg.py, #644, #633 and #631; under scaffold, family and the
  materializer, #622, #643, #640, #641, #623, #638, #635, #626, #637 and
  #642; under shape-doctor.py, #636 and #632. Each from its squash
  commit and the issues GitHub says it closes.
- The list's placeholder says it is current to main d4a7fab and stays a
  placeholder for what lands after it; the counts are refreshed inside
  their placeholder (66 pull requests, 75 of 97 milestone issues closed).
- #590 and #599 are cited beside #563, and "Before tagging" says they
  landed (fe237ec, 32c22b0) and lists the 22 open milestone issues.
- "How this release is cut": Brett Heap's word at 2026-10-16T12:06:40Z,
  the environments testpypi (12:07:31Z) and pypi (12:07:39Z), the tag
  ruleset 25237399 (12:08:14Z), and the rehearsal, run 38898735926
  (12:10:53Z-12:12:00Z, success, both uploads skipped), each read back
  from the API. The trusted publishers' date stays a placeholder.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 16, 2026
… it what to do where an earlier act left that branch, so `already exists` is no dead end (#607's sentence, in the two tools it left) (#657)

#607 (#637) gave `family.py` and the holder's `validate-family.py` one sentence for a `switch -c` that fails `fatal: a branch named '<branch>' already exists` (exit 128):

> If `git switch -c` says that branch already exists, an earlier act left it: give the branch another name, which starts it where the holder stands, or, only if that branch holds work not merged yet, the same command with `switch` in place of `switch -c` goes back to it and keeps every commit on it.

Two tools still printed a `switch -c` and said nothing more. Both now say the same sentence under that line, each as its own constant, `BRANCH_LEFT`, as #637 does:

- **`scripts/bump-leg.py`**, with "the project" where the others say "the holder". It follows the command in all three places `_switch_command` is named:
  - the `bump-leg-on-tracking-branch` remedy: after `and re-run.`;
  - the `bump-leg-root-detached` remedy: on the line under it;
  - the dry run's closing hint (#477): the new last line, under the hint.
- **`setup-project.py`**: the same sentence, word for word (the holder), as one more comment line under the family block's `switch -c` line. The family block's two comment lines stay, and the line comes after them.

No command line changes, and none is quoted differently. The sentence is words, not a second command, so no PowerShell quoting is added (#532) and no README sentence about quoting becomes false.

Pinned files: none. This changes no file under `templates/`.

1. **Where the sentence goes in bump-leg's tracking-branch remedy: after `and re-run.`, not between the command and `and re-run.`** The remedy reads `Run:`, the command on its own line, `and re-run.` With the sentence between, `and re-run.` would hang under a full sentence. After it, the text before the sentence is byte for byte what it was, so the command line and `and re-run.` keep their places. The sentence is still under the `switch -c` line.
2. **setup's sentence is one comment line, unbroken (about 300 columns).** The brief asked for one comment line, word for word. `family.py`'s dry run prints it unbroken too. The block's other comments are hand-wrapped at 78 columns. A wrapped block would be a small change if the coordinator prefers it; the test would then join the comment lines.
3. **Every printed command line is unchanged**, and the tests prove it: bump-leg's line is read back by `shlex.split` and compared to the argv it always meant (`switch_argv`); setup's by `host_words`, on a folder with a space and one with a space beside an apostrophe. The block still has exactly two command lines.
4. **One rule, four copies.** #637's equality test, now `test_the_tools_say_the_same_words`, also asserts that `setup-project.py`'s constant equals `family.py`'s and the validator's, and that bump-leg's equals it with "the project" for "the holder". It was renamed from `test_the_two_tools_say_the_same_words`, because "two" is no longer true. The project's wording is pinned as text once, as `BRANCH_LEFT_PROJECT` in `tests/test_bump_leg_paste_lines.py`, beside the lines it follows; the holder's is #637's pinned copy in `tests/test_validate_family_branch_step.py`. `test_the_two_pinned_texts_differ_in_the_one_word_only` ties the two pinned copies together.
5. **Existing tests that pinned the old last lines take the sentence.** Seven tests in three files (listed under Tests). Each read the last line of an output, or the exact end of a remedy; none changes what it checks.
6. **The new module imports #637's typing helpers** (`leave_branch`, `refused_as_left`, `type_other_name`, `typed`, `without_c`, `head`, `on_branch`) instead of copying them, so the duplication gate sees one body.

New module `tests/test_branch_left_bump_leg_setup.py`, 19 tests:

- bump-leg, each of three places (`tracking`, `detached`, `dry-run`): the command is the one it always was and what stands under it is the sentence (3); the printed line makes the branch where none is left (3); where an earlier bump left `bump/spec-<id>` holding a commit of its own, the printed line fails `already exists` (exit 128) and changes nothing, then **another name** works (the new branch starts where the root stands, the bump goes through on it, its commit sits on the commit the root stood on, and `HEAD:spec` is the new commit) (3), and **going back** with `switch` for `switch -c` works (back on the branch at its own commit, the bump goes through there on top of it) (3).
- setup, the family block: the line, the two comments, the sentence, the `add` line, two command lines (2, one per folder name); a real `setup-project.py` run prints the sentence once, under the right comment (1); in a real local holder (the `family` fixture's, on its tracking branch), with a real member: the printed line makes the branch where none is left, and where an earlier try left `family/add-<project>` with a commit of its own, **another name** works and `family.py add` goes through on it, and **going back** works and `add` goes through on top of the earlier commit (3).
- `test_the_two_pinned_texts_differ_in_the_one_word_only` (1).

Edited tests: seven cases in six test functions of the three bump-leg files, `test_bump_leg_dry_run_tracking.py` (4: the dry run's report, the difference from a dry run on a branch, the hint pasted for a root with a space, the real refusal byte for byte), `test_bump_leg_paste_lines.py` (1: the exact remedies) and `test_bump_leg_dry_run_truth.py` (2: the hint's branch name, `[local-remote-dir]` and `[origin]`); and, beside them, the extended `test_the_tools_say_the_same_words` in `test_family_switch_branch_exists.py`.

Runs, on be1bff9 (BASE a524542 plus main 95780dd), through `run9.sh`, `gh` stub first on PATH:

- New module with #637's module: `44 passed in 61.90s`.
- Set 1, 21 files: every `tests/test_bump_leg*.py` (7), the new module, `test_bootstrap_after_bump`, `test_stdout_encoding`, `test_non_utf8_git_reads`, `test_agents_md_codes`, `test_commit_trailers`, `test_update_shape_paste_lines`, `test_validate_pins_lockstep_lines`, `test_pin_rewrite_layout`, `test_validator_child_encoding`, `test_validate_pins_unreadable_workflows_dir`, `test_template_text_placeholders`, `test_repo_hygiene`, `test_cli_reference`: `1040 passed, 2 skipped in 459.92s`. Both skips were there before: a case-ignoring disk (`test_bump_leg.py:693`) and a Windows default (`test_stdout_encoding.py:286`).
- Set 2, 10 files: every file `git grep -l 'switch -c' tests/` names outside set 1 (`test_dry_run_asks_remotes`, `test_family_printed_quoting`, `test_family_refuse_up_front`, `test_family_switch_branch_exists`, `test_family_tracking_branch`, `test_validate_family_branch_step`, `test_validate_family_unnamed_mounts`), plus `test_setup_by_hand_commands`, `test_setup_project_py`, `test_setup_sh`: `592 passed in 506.74s`.
- Before #637 landed, on 6ac545f: set 1 `1042 passed, 2 skipped`, set 2 (9 files) `441 passed`.

**Fails on BASE.** The same five test files from the head, run on main 95780dd (no source change): `26 failed, 80 passed in 168.33s`. 18 of the 19 new tests fail, `test_the_tools_say_the_same_words` fails (no `BRANCH_LEFT` in either new place), and the seven edited tests fail. The one new test that passes on BASE is `test_the_two_pinned_texts_differ_in_the_one_word_only`: it reads text only. On the first commit alone, against a524542, the old module copy gave `20 failed, 1 passed`.

A real project (`scaffold-project.py` into local remotes, a recursive clone, the spec leg advanced on a branch of its remote) and a real holder (`family.py init`, then a real `setup-project.py Sample --yes --family Orbit` run). An earlier act left the branch with a commit of its own.

- **On BASE a524542:** `bump-leg.py` on the tracking branch ends `Run:` / the `switch -c` line / `and re-run.` and nothing more. The line typed as printed gives `fatal: a branch named 'bump/spec-712e4ce5d766' already exists`, exit 128. The detached refusal and the dry run's hint end on the command the same way. setup's block is the `switch -c`, `# a branch first: add refuses the` and `#   holder's tracking branch`, then the `add`. Typed, `fatal: a branch named 'family/add-sample' already exists`, exit 128.
- **On the head:** each of the four prints the sentence under the line, and the same line gives the same 128. Then:
  - another name (`<line>-2`): exit 0; `bump-leg.py` goes through on it, on top of the commit the root stood on; `family.py add` goes through on it;
  - going back (`switch` for `switch -c`): exit 0, at the earlier commit; `bump-leg.py` and `family.py add` go through, their commit on top of it.
- Both choices already worked on BASE: the sentence tells a person so. No row needs real GitHub.
- **A real earlier bump, not a made-up branch.** I ran a real bump on `bump/spec-<id>`, then went back to `main`. There the root shows ` M spec` (the leg is still at the bump's commit), and the next `bump-leg.py` is refused `bump-leg-root-dirty` before it reaches the `switch -c` line. After `git submodule update` puts the leg back, the line appears, fails `already exists`, and the other name works. See the checklist candidates.

Each hunk was reverted or broken alone in a scratch clone of the head, and the new module plus `test_the_tools_say_the_same_words` was run. Every mutation makes new tests fail:

| Mutation | Result |
|---|---|
| bump-leg `BRANCH_LEFT` says "the holder" | 13 failed: the equality test, and the three places in each of four tests |
| bump-leg `BRANCH_LEFT` drops `only if` | 13 failed, the same |
| `bump-leg-root-detached` remedy without the sentence | 4 failed (the `detached` case of each of four tests) |
| `bump-leg-on-tracking-branch` remedy without the sentence | 4 failed (the `tracking` case of each) |
| the same remedy with the sentence above `and re-run.` | 4 failed, the same |
| dry run's hint without the sentence | 4 failed (the `dry-run` case of each) |
| `_switch_command` prints `switch -C` | 9 failed (the "another name" and "going back" cases, and the three "command is the one it was" cases); the control, where no branch is left, still passes, as `-C` makes a new branch too |
| setup `BRANCH_LEFT` says "the project" | 7 failed (the equality test and six) |
| setup block without the sentence | 6 failed |
| setup sentence above the block's two comments | 6 failed |
| setup `BRANCH_LEFT` drops `only if` | 7 failed |

The other hunks change only docstrings and comments (`_switch_command`, `_branch_to_bump_from`, `family_next_command`, the comment above the dry run's hint). They change no behaviour.

| Ref | Result |
|---|---|
| origin/main d4a7fab (one commit past my merge of 95780dd; touches `scripts/family.py`, `scaffold-project.py`, `docs/cli.md`) | clean |
| #637 (landed as 95780dd) | merged into this branch once, clean |
| #623 (lane 2, `setup-project.py` steps (3) and (4); landed as abb956e) | merged in with #637, clean: its hunks are far from `family_next_command` |
| #639 08b6245 (lane 5, #611) | clean |
| #641 e87fcb0 (touches `bump-leg.py` and `family.py`) | conflicts in `scripts/family.py`, and the same conflict arises against main alone: not from this branch. `bump-leg.py` merges clean. |
| #615 6ad2303 (touches `bump-leg.py`) | clean |
| #631, #636 (lane 4) | clean |
| #642, #643, #644 | clean |
| lane-5 siblings fix/645 e2110f8, fix/646 2216e1c, fix/648 852c5c4, fix/649 d91b314 | clean |
| #561 (lane 4's `fix/bump-leg-tag-dry-run`) | its work is on main already (7d9bec9, `asked = commit`); its stale pull head conflicts with main alone, so it is not an open PR to check |

- No README, AGENTS.md or handbook edit. I checked every sentence about these lines: AGENTS.md:337-351 and 374 (the dry run "prints the `git switch -c`"), README.md:157 and 1011, `docs/cli.md` (generated; no `--help` text changes) and the handbook. None becomes false.
- Windows and macOS are not checked here, and no CI job checks them since #597. The tests type argvs, run no shell, compare no file that `write_text` wrote, and read setup's lines with `host_words`.
- A branch checked out in another worktree makes `git switch <branch>` fail (#637's "Left out", N1); the sentence is not made longer for it.
- I did not tick or mark the lines on #458 and #459.
- **Review rv647 (APPROVE WITH NITS, 0 blockers, 0 should-fix, 5 nits), and what is left of it:**
  - Commit 6ac545f's message says "Four existing tests … take the sentence". It is six test functions (seven cases) in three files, as the Tests section says. The squash message is this body.
  - `scripts/bump-leg.py`'s comment near :1164 still says "the three tools follow one rule"; there are four copies of the sentence now, and `setup-project.py`'s comment names two. Left for the `bump-leg.py` issue filed from #458's lines, which lands after this and edits that file.
  - No AST guard holds a fourth `_switch_command` caller to the sentence, as #637's `test_every_remedy_that_names_a_switch_line_ends_with_the_sentence` holds the validator's.
  - Setup's sentence prints as one comment line of about 317 columns in a block wrapped at 78. It is unbroken as `family.py`'s `_branch_said` prints it, which is what lets the byte-for-byte equality tests hold (decision 1).

- **bump-leg.py: after a real bump on a branch and a switch back to an older branch, the root is dirty (` M spec`) and `bump-leg-root-dirty`'s remedy, "commit or stash them first", is wrong advice.** The leg is left at the bump's commit and `git switch` does not move it (README says the leg is left detached at the new commit). `git submodule update` (or `make bootstrap`) puts it back and clears the refusal. The remedy could say so where the one dirty path is a leg's mount. It also means the `switch -c` sentence is reached only after that step in this flow.
- **The same flow's "go back" choice meets that refusal too** (rv647): `git switch` to the branch that holds the earlier bump leaves the leg behind, so the re-run is refused `bump-leg-root-dirty`; after `git submodule update` it says `nothing to do`, and only the push is left.
- **update-shape.py:5279 prints `NEXT git -C <root> checkout -b shape/update-<12> && git … commit`** (rv647, read only): it fails `already exists` the same way, and the `&&` chain then commits nothing. It is `checkout -b`, outside #647.
- **family.py, after a real earlier `add` and a switch back:** another name works for the switch, but `family.py add` is then refused `member-path-taken`, because git left the populated `members/<Project>` checkout behind (rv647; identical on the base). It bears on #637's "another name works".

Review: rv647 found 0 blockers, 0 should-fix and 5 nits on be1bff9 (APPROVE WITH NITS); nits 1 and 2 are answered in this body, nit 3 (the stale "three tools" comment in `scripts/bump-leg.py`) rides with #672, and nits 4 and 5 are noted above. CI (Brett Heap's ruling 27, with his fast-refresh ruling of 2026-10-15T22:48:35Z): H0 be1bff9 passed tests (Linux), tests-git-bash and SonarCloud; the head ac8a810 is a clean update-branch merge of main 8062810, with SonarCloud, tests-git-bash and package green on it, and the local gate passed on the head itself: the bump-leg set, 1040 passed (2 skipped), and the `switch -c` and setup set, 592 passed. Landed on Brett Heap's word of 2026-10-16T13:07:34Z, "merge #656 and #657 when green".

Closes #647

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 16, 2026
…-letter `$OPENREPOSHAPE_REPO` is a path, a Windows-spelled `$OPENREPOSHAPE_BIN_DIR` is respelled or refused, and `tests-git-bash` runs #598's installer cases (#649) (#656)

#596's Git Bash follow-ups, the five lines of #649. Two commits on main a524542: **d91b3144**, the fix, and **e85573ac**, fix round 1 from rv649's review. Together they change `openRepoShape`, `.github/workflows/tests.yml` and `tests/test_openreposhape_command.py`, and nothing else. `git diff --numstat`, d91b314: `openRepoShape` +42/-4, `tests.yml` +6/-1, the test file +182/-7. e85573a: +9/-6 (comments only), +3/-2 (a comment only), +56/-27. README, the handbook and `docs/cli.md` are unchanged: every sentence about these blocks stays true (below).

### Fix round 1 (rv649)

rv649 found the launcher's code correct and asked for changes: 0 blockers, 1 should, 6 nits. e85573a answers each point. It changes no line of the launcher's code and no command of the workflow: three comments and the tests.
- **SHOULD-1, the `D:/…` arm had no test.** Both BIN_DIR tests now run over `D:\tools\bin` and `D:/tools/bin`, and `fake_cygpath` takes the typed value. On Windows they use `str(...)` and `.as_posix()` of a path under `tmp_path`. With the arm dropped the suite stayed green (91 passed); now 5 cases fail.
- **NIT-1, the refusal's drive-answer arm had no test.** A third refusal case: a `cygpath` that answers `D:/tools/bin` with exit 0. With that arm dropped the suite stayed green; now 2 cases fail.
- **NIT-2, a `python` that is Python 2, or older than 3.9, reads as present.** Named as a cost in decision 1 and in the comment above `machine_report`. No code change: `--preflight` still refuses those machines.
- **NIT-3, the refusal's reason was false in Git Bash with `cygpath` off `PATH`.** Git Bash does read `D:\…` as a drive. The comment in `install_commands` and decision 3 now give the true reason.
- **NIT-4, the job's opening comment named two files.** It names the three.
- **NIT-5, on Windows the refusal test's value was a real `D:` drive path.** It is now a path under `tmp_path`, and the assert names that value, so a refusal that regressed would place the command where the test looks.
- **NIT-6, four claims.** Fixed in this body: "rows 1-3 are checked through the tests" (now says what runs), the clone URL for each slash (`C:\src\…` gave `https://github.com/C:\src\openRepoShape.git`), the Store stub clause (dropped, see decision 1) and the report's diffstat totals (the exact `--numstat` counts are at the top). d91b314's own commit message keeps its `....` typo and its one-URL sentence in history; the squash message is this body.
- **Composition and merge table.** Refreshed against origin/main 879d804 (below). #630 no longer conflicts.
- **Checklist candidate 3**, from rv649, is added below.

### The fix

**1. The `machine:` row for Python** (`machine_report`).
- Before: the block asked `command -v python3` and nothing else. Where Python came from python.org, Git Bash has `python` and no `python3`, so the row said `python3 MISSING` and printed the line pointing at `--preflight`, which then passed with `python`.
- Now: it asks for `python3`, then `python`, in `find_python`'s order. When only `python` answers, the row says `python3          present, as python`, and nothing points at `--preflight` for it. With neither, it says `MISSING`, as before. With `python3`, it says `present`, byte for byte as before.

**2. A drive-letter `$OPENREPOSHAPE_REPO`** (`shape_clone_url`).
- Before: `C:/src/openRepoShape` and `C:\src\openRepoShape` were read as an owner/name, so setup.sh's clone and `--doctor`'s were handed `https://github.com/C:/src/openRepoShape.git` for the first and `https://github.com/C:\src\openRepoShape.git` for the second.
- Now: a letter, a colon and then `/` or `\` is a path, and goes through as it is, as `/…` and `.…` do. No owner/name holds a colon. The package's own `clone_url` (`src/openreposhape/standard.py`, `DRIVE_RE`) already read a drive letter this way.

**3. A Windows-spelled `$OPENREPOSHAPE_BIN_DIR`** (`install_commands`).
- Before: `--install` named the directory as typed. With `D:\tools\bin` it printed `installed at D:\tools\bin/openRepoShape` and `export PATH="D:\tools\bin:$PATH"`, which the drive's colon splits in two. It also never found the directory on a `$PATH` that holds `/d/tools/bin`, so it printed that line every time.
- Now: a drive-letter value, in either slash, is spelled again with `cygpath -u`, which Git Bash has, before anything is checked or placed. Every line then names `/d/tools/bin`, and the PATH note appears only when that directory is not on `$PATH`.
- Where no `cygpath` answers (none on PATH, it fails, or it answers a drive spelling again), the value is refused before anything is placed: "--install would place this command in 'D:\tools\bin', which is spelled for Windows, and no cygpath here spells it the way bash does, so the directory could not go on $PATH: the drive's colon splits it. Nothing was placed. Set $OPENREPOSHAPE_BIN_DIR to the directory as this shell spells it, then run this again."

**4. `tests-git-bash` runs `tests/test_install_sh_filter_repo.py`.** That file holds #598's uv install, the two readinesses and the pasted next steps, which ran on Linux only, while README says Git Bash runs step 3's one line "tested in CI". It is the third path on the job's pytest line, with a comment saying the installer's suite is two files. The job's other lines, the header's "the launcher's and the installer's suites, and nothing else" and #630's release-notes sentence of the same meaning stay true.

**5. The stale docstring.** `test_a_ref_the_doctor_would_check_out_is_validated` said the refusal "has to arrive before any clone does". The command clones first, then checks the ref, and refuses before `git checkout` is handed it. The docstring now says that. The order is unchanged.

- **Line 1: `command -v` for both names, and no version check.**
  - The block promises to execute nothing but `gh auth status`, and `test_the_machine_report_runs_no_installer` holds it to that: every fake on its PATH logs a run. Running `find_python`'s check would break that promise.
  - What answers is therefore never asked its version, and that has a cost. The Microsoft Store's `python3` stub reads as present, as macOS's Command Line Tools stubs for `python3` and `git` already do (setup.sh's comment at :41-47). That is not new: the base asked `command -v python3` too, and the Store installs its `python.exe` and `python3.exe` aliases together, so `python3` answers first.
  - What is new is a `python` that is Python 2, or older than 3.9, where there is no `python3`. On the base such a machine said `python3 MISSING` and printed the `--preflight` pointer. Now it says `present, as python` and prints no pointer. It is rare (CentOS 7, macOS 10.14 or older, Python 2.7 from python.org), and `--preflight` still refuses it with "no Python 3.9 or newer on this machine (tried python3, python)". The comment above `machine_report` says so.
  - The row keeps its name `python3`, and the state carries the name that answered, in the form `gh`'s row already uses (`present, logged in`). So README's sentence ("`git`, `gh` and whether it is logged in, `python3`, and `git-filter-repo` … each `present` or `MISSING`") and the handbook's stay true, and README is not edited. Labelling the row `python` would have made both false in Git Bash.
- **Line 3: respell, and refuse only where respelling is not possible.** Respelling keeps a Git Bash user's value working: `cp`, `mkdir` and the record took the Windows spelling already, and only the printed lines were wrong. Where no `cygpath` answers, nothing in the command can spell the value as bash's `$PATH` needs it: in Git Bash with `cygpath` off `PATH` the drive is real but its colon still splits `$PATH`, and on Linux `D:\tools\bin` is a relative directory with a colon in its name. No `$PATH` line could work there, so it refuses. A `cygpath` that answers a drive spelling again is refused alike.
- **The tests' `install_with_machine` now names its directory with `bash_path`.** Its `$PATH` holds only the programs `--install` runs, and no `cygpath`. On Windows the old `str(bin_dir)` is a drive path, which would now be refused. Off Windows, `bash_path` changes nothing. Other tests that pass `str(...)` keep the full PATH, so on Windows they now go through Git Bash's own `cygpath`.
- **`run_cmd` takes a `cwd`.** On the base, and under a mutation, the new BIN_DIR tests' `D:\tools\bin` is a relative directory. With `cwd=tmp_path` it lands in the test's own directory, never in the checkout.
- **Both BIN_DIR tests run over both slashes** (`BIN_DIR_SPELLINGS`, `typed_bin_dir`). Off Windows the typed value is the fixed `D:\tools\bin` or `D:/tools/bin`. On Windows it is a path under `tmp_path`, so neither test can place a command on the machine's own `D:` drive.

New, in `tests/test_openreposhape_command.py`, 27 cases:
- `test_the_machine_report_finds_a_python_by_either_name`: `python` alone, both, neither (3).
- `test_a_windows_spelled_bin_dir_is_spelled_the_way_bash_spells_it`: not on `$PATH`, on `$PATH`, each in both slashes (4). On Windows it is a real drive path (`str(bin_dir)` or its `as_posix()`) and Git Bash's own `cygpath`; elsewhere a fake `cygpath` answers as that one would.
- `test_a_windows_spelled_bin_dir_is_refused_where_no_cygpath_spells_it`: no `cygpath`, a `cygpath` that fails, a `cygpath` that answers `D:/tools/bin`, each in both slashes (6), on the machine report's own `$PATH`.
- `test_a_clone_is_handed_a_path_or_url_as_it_is`: owner/name, URL, scp, POSIX path, `C:/…`, `C:\…` and `d:\…`, each through setup.sh (a setup.sh that prints `$OPENREPOSHAPE_REPO`) and through `--doctor` (a `git` that prints the URL it is handed and fails) (14). Nothing is cloned or fetched.

**On the base a524542** (this head's test file over the base's command): **17 failed, 80 passed** in 62.32s. The failures are exactly the new cases that need the fix: `python-alone`, the four respelling cases, the six refusal cases and the six drive-letter cases. The other 10 new cases pass there too, as they should.

**On the head e85573a** (Linux, `dirty=0`):
- `tests/test_openreposhape_command.py`, `tests/test_install_sh.py`, `tests/test_install_sh_filter_repo.py`, `tests/test_openreposhape_package.py`: **220 passed** in 175.18s. The package suite imports 13 of the launcher's cases and swaps `run_cmd`, so it is in the set.
- `tests/test_repo_hygiene.py`, `tests/test_workflow_concurrency.py`, `tests/test_release_workflow.py`: **138 passed** in 14.46s.
- The head merged with main 879d804 (a probe commit, b6cfafe3): the launcher's, the two installer files, the package suite and `tests/test_repo_hygiene.py`, **297 passed** in 188.95s.
- **The three files `tests-git-bash` runs, with Windows' `write_text` emulated**: a plugin made every `Path.write_text` in the test process write CRLF, as it does on `windows-latest`. **162 passed** in 121.06s (97 + 47 + 18). A probe test shows the plugin takes effect (it passes with the plugin and fails without it).
- **actionlint 1.7.12**, with shellcheck on PATH, on `tests.yml` alone and on the four files the CI job names: exit 0.

d91b314's own `tests-git-bash` run was green on `windows-latest`. It ran the first round's 21 new cases and `tests/test_install_sh_filter_repo.py` there, so Git Bash's own `cygpath` answered the backslash respelling on a real drive path, and the five existing tests that pass `str(...)` and now go through it passed. What was checked, and what was not:
- **Checked, from #596's Windows runs (113 passed on 5f001de, and its probes):** a fake that Python writes with `#!/bin/sh` runs under Git Bash with no mode bits, and `command -v` finds it. A script piped in text mode reaches Git Bash's bash as CRLF, and bash drops the CR. A `$OPENREPOSHAPE_BIN_DIR` set from Python reaches bash unconverted: #596's probe 4 printed `D:\a\_temp\…\bin/openRepoShape`, which is why the respelling now runs in those tests.
- **Checked on Linux:** LF bytes (the CRLF emulation above). No assert in the test file compares a path bash printed with a Python path.
- **Not checked, until e85573a's own run:** on Windows, the slash spelling (`as_posix()`) through Git Bash's real `cygpath`, the refusal's values under `tmp_path` on the machine report's `$PATH`, and the new drive-answer case. If one fails there, its log shows the typed value and the directory.

One script ran each row against the base's command and the head's, offline, with a `gh` stub first on PATH. Row 3's slash lines ran again for this round.

| Row | Base a524542 | Head e85573a |
|---|---|---|
| 1. PATH with `git`, a logged-in `gh`, `python`, `git-filter-repo`, no `python3` | `python3          MISSING`, then `run: openRepoShape --preflight …`; the log holds only `gh auth status` | `python3          present, as python`, no `run:` line; the log holds only `gh auth status` |
| 2. `OPENREPOSHAPE_REPO=C:/src/openRepoShape` | setup.sh clones `https://github.com/C:/src/openRepoShape.git`; the doctor's `git clone` too | `C:/src/openRepoShape`, both |
| 2. `OPENREPOSHAPE_REPO=C:\src\openRepoShape` | `https://github.com/C:\src\openRepoShape.git`, both | `C:\src\openRepoShape`, both |
| 2. `OPENREPOSHAPE_REPO=acme/openRepoShape` | `https://github.com/acme/openRepoShape.git` | the same |
| 3. `OPENREPOSHAPE_BIN_DIR='D:\tools\bin'`, no `cygpath` | exit 0, `installed at D:\tools\bin/openRepoShape`, `export PATH="D:\tools\bin:$PATH"` | exit 2, the refusal above, nothing placed |
| 3. `OPENREPOSHAPE_BIN_DIR='D:/tools/bin'`, no `cygpath` | exit 0, `installed at D:/tools/bin/openRepoShape`, `export PATH="D:/tools/bin:$PATH"` (on Linux, a relative directory named `D:` in the working directory) | exit 2, the same refusal with `'D:/tools/bin'`, nothing placed |
| 3. the same, a fake `cygpath -u` answering `<dir>/d/tools/bin` | the same as with none: `cygpath` is never asked | exit 0, `installed at <dir>/d/tools/bin/openRepoShape`, `export PATH="<dir>/d/tools/bin:$PATH"` |
| 4. `tests-git-bash`'s pytest line | two files | three files |
| 5. the docstring | "the refusal has to arrive before any clone does" | "clones this checkout first, then checks the ref" |

Rows 1-3 need Windows to be real: there, `python` is a real python.org install, and `cygpath` and the drive letter are Git Bash's own. On Linux they are fakes. The first Windows run runs rows 1-3's tests under Git Bash: row 3 with Git Bash's own `cygpath` on a real drive path, rows 1 and 2 with a fake `python` and a fake `git`, so a real python.org install and git's own reading of `C:/…` stay unchecked.

Each row reverts one hunk of the launcher, or one part of the BIN_DIR hunk, in a throwaway clone, then runs the 27 new cases.

| Hunk reverted | Result | Failing cases |
|---|---|---|
| none (the head) | 27 passed | — |
| `shape_clone_url`'s drive-letter arm | 6 failed | the six drive-letter cases, setup.sh and doctor |
| `machine_report`'s `python` arm | 1 failed | `python-alone` |
| the BIN_DIR block, whole | 10 failed | the four respelling cases and the six refusal cases |
| the `cygpath -u` call (always refuse) | 4 failed | the four respelling cases |
| the refusal (keep the value as typed) | 6 failed | the six refusal cases |
| `\|\| spelled=""` after the `cygpath` call | 2 failed | `cygpath-fails`, both slashes: `set -e` exits with `cygpath`'s status, 1, not the refusal's 2 |
| **the `D:/…` arm of the drive case (SHOULD-1)** | 5 failed | the two slash respelling cases and the three slash refusal cases (was green) |
| the `D:\…` arm of the drive case | 5 failed | the same five, backslash |
| **the refusal's drive-answer arm, `[A-Za-z]:*` (NIT-1)** | 2 failed | `cygpath-answers-a-drive`, both slashes (was green) |
| the refusal's empty-answer arm, `""` | 4 failed | `no-cygpath` and `cygpath-fails`, both slashes |
| `--` dropped from `cygpath -u -- "$dir"` | 4 failed | the four respelling cases: the fake answers only `-u -- <value>` |
| the respelling after the placement, not before | 4 failed | the four respelling cases |

Not probed by a test: the `tests.yml` line (CI configuration, checked by actionlint and by this PR's own Windows run), the docstring, and `install_with_machine`'s `bash_path` (identity off Windows; on Windows it is what keeps four machine-report tests from the new refusal).

| Against | Result |
|---|---|
| origin/main 879d804 (d4a7fab, then 879d804, since the last table's 95780dd) | **clean**. Of this PR's files, the three suites', README and the handbook, only `tests/conftest.py` has moved since the base a524542 (new helpers for setup-project's tests), and nothing since the last table's 95780dd. The head merged with main (a probe commit, b6cfafe3) passes the suites below. |
| lane 3's #630 61b3907 | **clean**. It conflicted at ca5c49f, at the `tests-git-bash` pytest line, because its base was older than main's #590; its head now holds main d4a7fab, and it does not change `tests.yml`. It changes `AGENTS.md`, README, `adopt-project.py`, the handbook, `docs/cli.md`, the release notes and four test or render files, none of which this PR touches. |
| #620 9f2b67c, #621 10dda17, #622 55666f3, #623 fc08a28, #627 83ca2db, #628 d64252a, #631 21bdac8, #634 b7472df, #636 30a52c0, #637 62263d2, #638 b393e1f, #639 08b6245, #642 1a42df4, #643 1fe3764, #644 733f853 | **clean** (#623 changes `tests/conftest.py`; #627 changes README and the handbook) |
| #641 e87fcb0 | conflicts in `scripts/family.py`, and so does #641 against main alone: its own conflict, not an overlap with this PR. |
| lane 5's `fix/645-fixture-copy-maintenance-race` accc7d2, `fix/646-add-member-variant-remedy` 79754b9, `fix/647-branch-left-bump-leg-setup` be1bff9 (#657), `fix/648-bootstrap-and-gate-order` 852c5c4 | **clean**, all four |

- **The Store stub check.** See the decision on line 1. `--preflight` is where a stub is told apart.
- **`install.sh`'s own lines.** It names a Windows-spelled `$OPENREPOSHAPE_BIN_DIR` as typed in its `preflight:` and `done:` lines, though the command it installs now prints `/d/…`. #649 names `--install`'s PATH note, and `install.sh` prints no PATH line. Checklist candidate 1.
- **`--help`.** Its `$OPENREPOSHAPE_REPO` and `$OPENREPOSHAPE_BIN_DIR` lines stay true, so `docs/cli.md` is not regenerated.
- **`D:`, `D:tools` and UNC values.** Checklist candidate 3.

1. **`install.sh` echoes a Windows-spelled `$OPENREPOSHAPE_BIN_DIR` as typed.** It builds `target="$bin_dir/openRepoShape"` from the raw value, so in Git Bash with `D:\tools\bin` its `preflight:` and `done:` lines say `D:\tools\bin/openRepoShape`, while the command's own lines say `/d/tools/bin/openRepoShape`. Both spellings work in Git Bash, so nothing breaks. On a machine with no `cygpath`, the command's `--install` now refuses the value, and `install.sh` stops with "the <version> openRepoShape's --install refused (exit 2); its own refusal above says why", which is right but could refuse earlier, before the fetch.
2. **The PATH note is not quoted for every directory.** `--install` prints `export PATH="$dir:$PATH"` with the directory between double quotes as it is. A directory holding `"`, `$`, a backquote or `\` prints a line that does not do what it says, and one holding `:` cannot be on `$PATH` at all. A drive letter was the one case a person in Git Bash meets by default; the rest need a deliberately odd directory name.
3. **Two kinds of value slip past both new arms** (rv649). `$OPENREPOSHAPE_BIN_DIR` set to `D:` or `D:tools` matches neither respelling arm, so it is kept as typed and prints `export PATH="D:tools:$PATH"`, as on the base. A UNC `\\server\share\…`, as `$OPENREPOSHAPE_BIN_DIR` or `$OPENREPOSHAPE_REPO`, is never respelled (the PATH note's `\\` collapses) or becomes a github.com URL. The package's `clone_url` passes a UNC path through on Windows (`os.path.isabs`).

Review: rv649 found 0 blockers, 1 should-fix and 6 nits on d91b314 (CHANGES NEEDED); the fix round e85573a answers all seven, so both mutations that survived now fail a test, and lane 5's coordinator checked it. CI (Brett Heap's ruling 27, with his fast-refresh ruling of 2026-10-15T22:48:35Z): H0 e85573a passed tests (Linux), tests-git-bash (windows-latest) and SonarCloud; the head 022bd50 is a clean update-branch merge of main 8062810, with SonarCloud, tests-git-bash and package green on it, and the local gate passed on the head itself: the launcher, installer and package set, 220 passed, and the hygiene, workflow and README-section set, 214 passed. Landed on Brett Heap's word of 2026-10-16T13:07:34Z, "merge #656 and #657 when green".

Closes #649

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 17, 2026
… in command scope and in a global config that includes the person's own, so a `copytree` of a fixture no longer meets a detached repack deleting its loose objects (#645) (#713)

**What changed.** One session fixture in `tests/conftest.py`, `no_auto_maintenance` (autouse), and one new test file, `tests/test_no_auto_maintenance.py`. No shipped file and no other test file changes. Four commits: the fix and its tests (e2110f8), a comment (accc7d2), and the fix round below (8cfc8eb, and 979bb39, which only shares a test helper).

The fixture sets `gc.auto=0` and `maintenance.auto=false` for every git the session starts, in two scopes:

- **Command scope** (`GIT_CONFIG_COUNT`), appended after the entries the environment already holds (the Windows job holds two). Every git the suite starts, every git a tool under test starts, and every git a `git submodule` starts inherits it. It outranks every config file.
- **A global config file** (`GIT_CONFIG_GLOBAL`, in the basetemp). It includes the global config git would read without it (the file `GIT_CONFIG_GLOBAL` already named, or `$XDG_CONFIG_HOME/git/config` and then `~/.gitconfig`) and sets the two keys after it. This one is for the `receive-pack` a push starts in a local remote: git starts it with `GIT_CONFIG_COUNT` and `GIT_CONFIG_PARAMETERS` removed, so command scope never reaches it. Where `HOME` is unset the `~/` paths are left out, and an empty `GIT_CONFIG_GLOBAL` includes nothing (fix round, below).

A comment beside #440's note in `clone_just_pushed` says why, names both sites, and says what is left.

**Why.** `shutil.copytree` reads a repository's `objects/` one file at a time. The suite copies a repository a fixture has just committed in, fetched into or pushed to in 139 places. Each `git commit`, `merge` and `fetch`, and the `receive-pack` in a remote after a push, runs `git maintenance run --auto` as it finishes. From git's source:

- git 2.46: the auto run stays in the foreground.
- git 2.47: it is started with `--detach`, takes `objects/maintenance.lock` and then detaches, so the command returns while it runs.
- git 2.52 and 2.53: the default strategy is still `gc`, which does nothing below 6700 loose objects.
- git 2.54: the default strategy is `geometric`. Its repack runs when packs are out of geometric order, or once its estimate, taken from `objects/17` alone, is above 256 loose objects, so two loose objects there are enough (`maintenance.geometric-repack.auto`, 100, rounded up). It deletes the loose objects it packed.

So on a current git, a copy taken just after a commit can find a loose object gone. It happened twice on the Linux runner:

- `holder` (`tests/test_family.py:129`), in `test_family_member_name.py` (lane 1's report on #473);
- `the_standard_moved` (`tests/test_doctor_pin_restore_merges.py:99`), a copy of the session's `standard` with its `.git`, in `test_copies_neither_side_edited_are_taken_whole_with_the_pin` on #623's run 38895011713: `shutil.Error ... standard5/openRepoShape/.git/objects/1e ... [Errno 2]` (lane 2's #473 comment 6215422231). The same function copies `moved` at :102 just after a commit in it.

Both are covered by the same session setting: the commits before each copy run under `os.environ` (`commit_all` passes `{**os.environ, ...}`).

A listing of `.git/objects` taken before and after such a repack also differs: lane 3's `objects changed` in `tests/test_family_bump_remove_rollback.py` (#736's Linux run 38981191418), 37 loose objects packed into two. The same setting covers it (i645 on #645).

**Decisions and evidence.**

- *Option A (the suite's environment), not B or C.*
  - B (config in each copied repository) would have to reach repositories the tools under test create themselves, such as the bare remotes `scaffold-project.py --local-remote-dir` makes and pushes into before a test sees them. A config written after that first push is too late.
  - C (a copy that survives a repack) cannot be made safe. A copy can list `objects/pack/` before the new pack lands, and then a loose-object directory after it was emptied. That copy succeeds and is missing objects.
  - A covers all 139 places from one place.
- *Two scopes, not one.* Probe on git 2.43, pushing into a local bare remote with `receive.denyDeletes=true` given each way:
  - with `GIT_CONFIG_COUNT`, the branch was deleted;
  - with `-c`, deleted;
  - through a `file://` URL with `GIT_CONFIG_COUNT`, deleted;
  - with `GIT_CONFIG_GLOBAL`, `remote: error: denying ref deletion` (refused).

  So command scope alone misses the remotes, and ten copies are of bare remotes. A global file alone would be dropped wherever a test gives git a global file of its own (14 test files set `GIT_CONFIG_GLOBAL`, five more through `excluding()`). No test file replaces both scopes and also copies a repository. rv645, the reviewer, repeated the `receive.denyDeletes` probe and got the same answers, and instrumented the git calls of the batches it finished before the checkpoint: the only receive-packs that ran without both settings were in `test_family_add_excludes.py` and `test_adopt_clone_template.py`, neither of which copies a remote.
- *Which verbs start auto maintenance* (git 2.43 trace2): `commit`, `fetch`, `pull` and `submodule update --remote` (its fetch in the submodule) start `git maintenance run --auto`. A push's `receive-pack` starts `git gc --auto --quiet`. `clone`, `submodule add` and `submodule update --init` start none.
- *Behaviour no test checks is unchanged.* The person's own global config is still read: it is included, and it comes first. A test of that is part of the new file. The tests that read or set a global config (`test_git_helper_identity.py`, `test_adopt_git_reads.py`, `test_tracking_branch_ref.py` and others) set their own, which replaces the session's.

**Fix round (8cfc8eb), on rv645's review** (no MUST; two SHOULD, five NIT).

- **SHOULD 1: with `HOME` unset every git was fatal.** The file's include of `~/.config/git/config` and `~/.gitconfig` made git exit 128, `error: could not expand include path '~/.config/git/config'`, in every command the session started. The estate runs with `HOME` set, so nothing failed here; an `env -i` or hermetic runner would have lost the whole suite.
  - `session_global_config` now writes a `~/` path only where `home_is_known()` (`HOME` is set, or `os.name == "nt"`, where Git for Windows finds a home itself), and writes no `[include]` header when nothing is included. An `XDG_CONFIG_HOME` that is set is still included, because it needs no `HOME`.
  - The two `test_a_persons_own_global_config...` cases skip where `HOME` was unset. They skip on the environment and not on the file's bytes, so a fixture that dropped its includes still fails them (probe M5 below).
  - A new test, `test_where_home_is_unset_the_file_is_one_git_can_read` (with and without `XDG_CONFIG_HOME`), reads the file back through `git config --global --includes`. It is skipped on Windows. It and the empty-value test share one helper, `probe_read_from_the_session_file`, so the new tests do not repeat a body for SonarCloud's duplication gate.
  - Measured under `env -u HOME -u XDG_CONFIG_HOME`, `run9.sh`, git 2.43.0:

    | file | before (accc7d2) | after (8cfc8eb) |
    |---|---|---|
    | `tests/test_clone_just_pushed.py` | 1 passed, 6 errors | 7 passed |
    | `tests/test_no_auto_maintenance.py` | 3 failed, 2 passed, 2 errors (7 tests) | 9 passed, 2 skipped (11 tests) |
    | `tests/test_family_member_name.py` | not run | 13 passed |

    The 2 skips are the two `test_a_persons_own_global_config...` cases.
  - Where `HOME` is set and the file is written as before, nothing changes: across 27 combinations of `HOME` (set, unset, empty), `XDG_CONFIG_HOME` (set, unset, empty) and `GIT_CONFIG_GLOBAL` (unset, a file, empty), the file's bytes are the same as accc7d2's in the 12 where `GIT_CONFIG_GLOBAL` names a file or `HOME` is set with `GIT_CONFIG_GLOBAL` unset, and differ in the 15 where `HOME` is not set or `GIT_CONFIG_GLOBAL` is empty.
- **SHOULD 2: nothing guarded the fixture's scope.** At function scope all seven tests passed (rv645's probe), and the session fixtures `scaffolded` and `family` would have been built before the setting existed. A new test, `test_the_settings_are_in_place_before_a_session_fixture_builds_a_repo`, takes a session fixture as its FIRST argument, which reads `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_COUNT` as that fixture sees them, and compares them with what the test sees and with the fixture's file. Pytest builds the higher scope first, and then an autouse fixture, so the fixture is only in place early enough if it is session scope and autouse.
- **NIT 1: the receive-pack test failed under a person's global `core.hooksPath`**, which hid the remote's own `post-receive` hook (`FileNotFoundError` on `read-by-receive-pack`). The test now sets `core.hooksPath` to the remote's own `hooks` directory in the remote's local config, which outranks a global one. Measured with `GIT_CONFIG_GLOBAL` naming a file that holds `core.hooksPath`: before, `1 failed, 4 passed, 2 skipped`; after, `9 passed, 2 skipped` (11 tests).
- **NIT 2: an empty `GIT_CONFIG_GLOBAL=` was read as unset.** Git 2.43 reads it as "no global config" (checked here: with `HOME` holding a `.gitconfig` probe, `GIT_CONFIG_GLOBAL=` does not read it). The fixture now includes nothing for an empty value: `if named is not None: included = [...] if named else []`. A new test, `test_an_empty_GIT_CONFIG_GLOBAL_is_no_global_config_and_includes_nothing`, fails on the old `if named:`.
- **NITs 3 to 5: no code change.**
  - NIT 3: `test_family_dry_run.py`'s own `NO_MAINTENANCE` is redundant now. It lives in another test file that this PR leaves alone, and it is harmless.
  - NIT 4: a `git config --global` read without `--includes` sees only the session's two keys. The only reader is `setup-project.py:1105`, behind `os.name == "nt"`, and no test reads it. `_credential_helper_configured` uses `--get-all` with no scope, so it still sees the person's config. The conftest comment already says so.
  - NIT 5: the version facts in the conftest comment ("2.47 detaches", "2.54 default geometric") are from git's source tags and are not run here: only git 2.43.0 is installed. The comment is unchanged.

**Tests** (`run9.sh`: gh stub first on PATH, identity blanked; git 2.43.0, Python 3.12, pytest 9.1.1).

- On the fix round (8cfc8eb; 979bb39 differs from it in the new test file alone, and the batches that read that file were run on it):
  - `tests/test_no_auto_maintenance.py`: 11 passed (7 from the first commit, 4 new: the `HOME` test twice, the empty value, the scope).
  - `tests/test_no_auto_maintenance.py`, `tests/test_clone_just_pushed.py`, `tests/test_repo_hygiene.py` and `tests/test_git_helper_identity.py` together, on 979bb39: 105 passed. With `tests/test_family_dry_run.py` added, on 8cfc8eb: 166 passed.
  - `tests/test_adopt_git_reads.py`, `tests/test_tracking_branch_ref.py`, `tests/test_adopt_late_failure.py`, `tests/test_adopt_github_exits.py` and `tests/test_family_add_excludes.py` (the tests that read or set a global config or a hooks path): 314 passed.
- On the first two commits (e2110f8 for the first four, accc7d2 for the rest; the second commit changes comment lines only). The fix round changes nothing where `HOME` is set and `GIT_CONFIG_GLOBAL` is not empty (the byte comparison above), so these were not run again:
  - Family, `tests/test_family*.py` (20 files): 823 passed, 19 skipped.
  - Adopt, `tests/test_adopt*.py` (46 files): 3463 passed, 2 skipped.
  - Doctor, `tests/test_shape_doctor.py` and `tests/test_doctor*.py` (12 files): 741 passed, 3 skipped.
  - Scaffold, `tests/test_scaffold*.py`: 634 passed.
  - Update-shape and bump-leg, `tests/test_update_shape*.py` and `tests/test_bump_leg*.py` (24 files): 857 passed, 4 skipped.
  - The rest of the files that copy a repository (`test_bootstrap_after_bump.py`, `test_non_list_project_fields.py`, `test_pin_source_env_collisions.py`, `test_pin_source_unreadable.py`, `test_shape_pin_files_not_a_list.py`, `test_validator_child_encoding.py`, `test_siblings_family_name.py`, `test_bootstrap_spelling.py`, `test_non_utf8_git_reads.py`), with `test_repo_hygiene.py`, `test_git_helper_identity.py`, `test_clone_just_pushed.py` and the new file: 924 passed.
  - On a merge of accc7d2 with main c6122e8: `test_no_auto_maintenance.py`, `test_git_helper_identity.py`, `test_repo_hygiene.py`, `test_clone_just_pushed.py` and `test_family_member_name.py`, 114 passed.

**Reproductions.**

- *The mechanism, on git 2.43.* A repository of 300 files plus two loose objects named `17…`, committed under `gc.auto=1`, then copied at once with `shutil.copytree`. This is what git 2.54 does to such a repository by default. With the base's settings, 10 of 10 copies failed, then 9 of 10 (`[Errno 2]` on a loose object directory or file). With the fix's two entries appended after it, 0 of 10 failed.
- *The emulation of the race* (rv645's recipe, run on 8cfc8eb: a global `gc.auto=1` that includes the person's `~/.gitconfig`, plus a `core.hooksPath` pre-commit hook that leaves two `17…` objects and drops `gc.log`, so git 2.43 repacks in the background after each commit; `GIT_TRACE2_EVENT` over the whole run). Base figures are rv645's, on a524542; head figures are the fix round's own, on 8cfc8eb:

  | file | git | auto-maintenance children | repack runs | result |
  |---|---|---|---|---|
  | `test_doctor_pin_restore_merges.py` | base | 78 `maintenance run --auto`, 77 `gc --auto` | 65 | 1 failed, 14 passed, 1 error |
  | | head | 4 `gc --auto` (from `receive-pack`, inert under `gc.auto=0`) | 0 | 16 passed |
  | `test_family_member_name.py` | base | 19 `maintenance run --auto`, 29 `gc --auto` | 15 | 13 passed |
  | | head | 14 `gc --auto` (inert) | 0 | 13 passed |

  On the base, rv645 reproduced the symptom itself: `shutil.Error ... moved/openRepoShape/.git/objects/pack/tmp_rev_...`, at `the_standard_moved`'s copy of `moved` (`tests/test_doctor_pin_restore_merges.py:102`). On head, 0 maintenance runs and 0 repacks in both files.
- *The first writer's runs on the issue's file* (`test_family_member_name.py`, base then head, 19 maintenance and 15 repacks against 0 and 0) agree.

**Mutation probes.** Each reverts one hunk of the fix in a throwaway clone; the new file then fails. First round (against the first seven tests):

| probe | fails |
|---|---|
| fixture sets nothing | 6 of 7 (all but the append test) |
| no command scope | `test_a_commit_a_fetch_and_a_merge_start_no_maintenance[a-tests-own-global]` |
| no global file | the receive-pack test, both person's-config cases |
| count overwritten, not appended | the append test |
| global file without the includes | both person's-config cases |
| the two keys before the includes | both person's-config cases |
| no `maintenance.auto` | 5 tests |
| no `gc.auto` | the receive-pack test, the append test |

Fix round (against the eleven tests):

| probe | result | what fails |
|---|---|---|
| S1, `no_auto_maintenance` at function scope | 1 failed, 10 passed | the scope test, alone (the first seven passed in rv645's run of the same mutant) |
| S2, not autouse | 4 failed, 7 passed | the two commit/fetch/merge cases, the tool test, the receive-pack test |
| H1, `home_is_known()` always true | 2 failed | `test_where_home_is_unset_the_file_is_one_git_can_read`, both variants |
| H2, the XDG include needs `HOME` | 1 failed | the same test, the `xdg` variant |
| E1, an empty value is read as unset (`if named:`) | 1 failed | the empty-value test |
| M5, the file includes nothing (the first round's probe, again) | 3 failed, 8 passed | both `test_a_persons_own_global_config...` cases, and the `xdg` variant of the `HOME` test |
| K1, the receive-pack test without its own `core.hooksPath`, under a global `core.hooksPath` | 1 failed, 8 passed, 2 skipped | the receive-pack test |

S2 is caught by tests that ask for the setting by itself; the scope test does not catch it, because earlier tests in the file build the fixture on request. K1 only fails where a global `core.hooksPath` is set, as NIT 1 says.

On the base itself the new file does not import (`cannot import name 'NO_AUTO_MAINTENANCE'`).

**Merge checks** (`git merge-tree --write-tree` on 979bb39): clean against origin/main 2fac979 and against lane 5's `fix/662-insteadof-helper` (4149cbb; #662 is stacked on accc7d2 and also edits `tests/conftest.py`: the fix round's hunks are in `session_global_config`, its helper `home_is_known`, and the comment above them). Earlier, on e2110f8 and accc7d2: clean against origin/main d4a7fab and c6122e8, #621, #622, #630, #631, #634, #635, #636, #637, #638 and #639, and the sibling branches for #646, #647, #648 and #649.

**What it leaves.**

- A git whose environment drops both scopes (a `GIT_CONFIG_COUNT` overwritten, not appended to, together with a test's own `GIT_CONFIG_GLOBAL`, or an environment built from nothing), and a push made under a test's own global file. No repository the suite copies is made or pushed to that way today.
- A git older than 2.32 ignores `GIT_CONFIG_GLOBAL`. Its `receive-pack` runs `gc --auto`, which does not detach below 6700 loose objects.
- `clone_just_pushed` keeps reading through git. A test with its own global file still needs it.
- `test_family_dry_run.py`'s own `NO_MAINTENANCE` is now redundant, and is left alone (NIT 3).
- Not run: a real Windows job (none runs the full suite since #597, and the `HOME` test is skipped on Windows), and a git newer than 2.43.

**Copies found** (`git grep -n 'shutil.copytree(' -- tests`, 152 calls):

- 127 copy a repository the suite built (non-bare, many holding `.git/modules`): `conftest.py` 1, `test_family.py` 18, `test_shape_doctor.py` 32, `test_doctor_legs_members_next.py` 8, the `test_update_shape*` files 30, and 38 more in 28 files. Four of them copy a subtree that holds no store: `standard / name` twice, and a member's work tree twice.
- 10 copy a bare remote, or a folder of them: `test_bump_leg_paste_lines.py:95`, `test_doctor_unfinished_merge.py:166`, `test_family_add_member_id.py:239`, `test_family_add_refusals.py:487`, `:788` and `:1983`, `test_family_dry_run.py:1446`, `test_family_tracking_branch.py:279`, `test_update_shape_landing.py:156` and `test_update_shape_next_quoting.py:116`.
- 2 copy a `.git` directory: `test_adopt_clone_template.py:190` and `test_scaffold_mount_ignored.py:381`.
- 13 hold no repository: a template, or this checkout with `.git` ignored.
- `test_adopt_plan.py:449` copies a clone named by `SHAPE_MEDXEHR_CLONE`. It is outside the suite and skipped without the variable.

No shipped script copies a repository.

**Checklist candidates** (rv645's; the coordinator places them):

- The import-time git probe at `tests/test_family_refuse_up_front.py:75-96` runs in a throwaway directory before any session setting. It is harmless today.
- Tests that overwrite `GIT_CONFIG_COUNT` (14 files) lose command scope and rely on the global file alone. That holds today, and nothing guards it.

Review: rv645, lane 5's independent review, found no MUST, two SHOULDs and five NITs on accc7d2; the fix rounds above take them, and its re-verification approved 979bb39. CI (Brett Heap's ruling 27): H0 979bb39 passed tests, tests-git-bash and SonarCloud; the head a9f36ce is a clean update-branch merge of main bf1f099, with tests, tests-git-bash and SonarCloud green; and the local gate passed on its predicted squash tree 760892f over main 2000488: every test file that #706, #703 and lane 5's six round-1 PRs change, run under #713's conftest, with the two copytree-race files and the hygiene trio, 1332 passed, 3 skipped, 1 xfailed. Landed on Brett Heap's word of 2026-10-17T10:59:39Z, relayed by lane 3 on #473: "merge lane 5's approved v1.1.0 drafts".

Closes #645

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