Skip to content

shape-doctor.py: the the way in rows say a checkout's own contracts/repository-naming.yaml that does not load, or raises, is that checkout's, an n/a environment row, where a tab line was FINDING the way in refused and a pattern that does not compile ended the doctor in a traceback; and the EXIT CODES list says what INVALID and manifest-kinds name (#707) - #778

Merged
brettheap merged 4 commits into
mainfrom
l3-707-way-in-policy
Oct 18, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

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

Closes #707.

Lands after the v1.1.0 tag.

What was wrong

A follow-up to #678 (PR #701). In a directory that is not a shape root, a fault in this checkout's own contracts/repository-naming.yaml reached the the way in rows as an exception, because way_in_project called NamingPolicy.load with no handler and run_checks catches only Refusal and OSError.

Re-measured on main e313701 (a copy of the standard whose policy has one tab-indented line, or a pattern that does not compile, asked about a git repository with no commit and a plain directory, with a gh that exits 97):

  • tab line, repository: FINDING the way in refused: <checkout>/contracts/repository-naming.yaml: line 509: tab used for indentation and FINDING the other way in refused: ..., exit 2, no detail.environment;
  • pattern: "(^open(?:, plain directory: the doctor dies in a traceback, re.error: missing ), unterminated subpattern at position 0, exit 1, the code of a finding.

Every claim in the issue held; the line numbers it cites have moved with main.

What changed (shape-doctor.py, line numbers at head)

Two choices to look at:

Tested

tests/test_doctor_validator_verdicts.py, #701's file:

All seven fail on main e313701 (the traceback, FINDING ... refused:) and pass here. Edits are made in bytes and the pattern takes CRLF, so they hold on Windows. The full suite is CI's.

/usr/bin/python3 -m pytest -q -p no:cacheprovider --basetemp=<scratch>/bt -o tmp_path_retention_policy=failed \
  tests/test_doctor_validator_verdicts.py tests/test_doctor_validator_rows.py tests/test_doctor_next_is_a_command.py \
  tests/test_shape_advisory.py tests/test_cli_reference.py tests/test_repo_hygiene.py
288 passed in 130.07s
... tests/test_shape_doctor.py
436 passed, 3 skipped, 1 xfailed in 301.27s

scripts/render-cli-reference.py --check: docs/cli.md is what the generator produces. No NEXT changed, and test_doctor_next_is_a_command.py passes.

Docs

None changed: README, AGENTS.md and the handbook do not say what the way in rows do where the checkout's policy cannot load, and README is at its line cap. docs/release-notes/ is not touched.

Pinned copies

None. shape-doctor.py is not a pinned copy and scripts/repo_shape.py is untouched, so no project sees an upstream-changed row from this.

Not settled here

Nothing found open. Other lane-3 PRs edit shape-doctor.py (#771, #772, #773, W668's .gitmodules readers); this PR stays in the naming helpers (:2229-2360), the the way in rows (:9095-9290) and the EXIT CODES list.

Round 2 (RV-a's S1 and N1, #778 (comment))

Head fe7b30f.

  • S1, scripts/shape_advisory.py:465 _naming_policy: it caught only (Refusal, OSError, UnicodeDecodeError), so a pattern that does not compile raised re.error out of shape_advisory.read, and leg_clone read a leg clone as "not a leg": the clone lost its leg row and the Triad row, and was offered the other way in and the scaffold, though the leg's own-AGENTS.md route reads no policy. It answers None for any fault now (except Exception, the advisory never fails a report). scripts/shape_advisory.py is not a pinned copy, so no project sees an upstream-changed row.
  • S1 test, tests/test_doctor_validator_verdicts.py:622 test_an_unusable_naming_policy_keeps_a_leg_clones_rows: a leg clone that says so in its own AGENTS.md (the advisory tests' builder), over the three unusable policies. NOT A SHAPE ROOT, exit 2, no traceback; the way in keeps its leg row (no NEXT, no environment mark), triad-advisory keeps "this is a clone of the spec leg of a Triad", and there is no way-in-scaffold. The pattern case fails without the shape_advisory.py change.
  • N1, shape-doctor.py:2340 naming_policy_words, :2262 naming_policy_refusal_of, :9199 way_in_project, :9230 way_in_policy_row: a classify that raises on a policy that DID load is told as policy-unusable, and the row says "the standard's contracts/repository-naming.yaml cannot be used: ..." where it said "does not load". The loader's faults keep "does not load". The classify case of test_an_unusable_naming_policy_is_an_environment_row_in_the_way_in_rows (:582) asserts the verb and the code, for a directory and a repository.
  • Left as it is, on RV-a's word: N2, the exception's module-qualified name, which reads re.PatternError on Python 3.13.
/usr/bin/python3 -m pytest -q -p no:cacheprovider --basetemp=<scratch>/bt -o tmp_path_retention_policy=failed \
  tests/test_doctor_validator_verdicts.py tests/test_shape_advisory.py tests/test_repo_hygiene.py tests/test_cli_reference.py
185 passed in 56.83s

tests/test_shape_doctor.py was not re-run this round (436 passed, 3 skipped, 1 xfailed at f98e610); scripts/render-cli-reference.py --check is ok.

🤖 Generated with Claude Code

Summary by Sourcery

Handle unusable naming policies as non-fatal environment conditions while preserving accurate way-in behavior and exit-code documentation.

Bug Fixes:

  • Prevent unusable repository-naming policies from crashing or misreporting the way in checks, including policies that refuse to load or raise while being evaluated.
  • Keep leg-clone advisory and Triad rows intact when the naming policy is unusable.

Enhancements:

  • Represent naming-policy faults consistently across naming and way-in rows, with environment metadata and clear ownership wording.
  • Clarify the documented INVALID and manifest-kinds exit-code criteria.

Tests:

  • Add coverage for unloadable, uncompileable, and evaluation-failing naming policies across directories, repositories, and leg clones, including text reports.

…s/repository-naming.yaml` that does not load, or raises, is that checkout's: an `n/a` environment row with no NEXT, where a tab line was `FINDING the way in refused` and a pattern that does not compile ended the doctor in a traceback, exit 1; the EXIT CODES list says INVALID names its red gates, or every FINDING row when none is, and every `manifest-kinds` cause (#707)

`way_in_project` called `NamingPolicy.load` with no handler and `run_checks`
catches only a `Refusal` and an `OSError`. It now asks `own_naming_policy`,
the one question the doctor puts to that file (#678's guard, which
`own_naming_policy_refusal` is a view of), and gives back the project or the
fault as a `Refusal`, a `policy-raised` one for what is not. `classify`
raising on a policy that loaded is the same fault. `check_the_way_in` and
`check_the_other_way_in` return `way_in_policy_row` for it, in the words of
the `naming` row (`naming_policy_words`), and the verdict stays NOT A SHAPE
ROOT, exit 2. A leg clone, which is offered no command, keeps its row.

The EXIT CODES list takes RV-a's two nits on #701: INVALID names `merge`
alone, else its red gates, or, when none is red, every row that is a
FINDING; and `manifest-kinds` names all of its causes.

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

sourcery-ai Bot commented Oct 17, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR hardens shape-doctor against failures in the standard checkout's repository-naming policy when evaluating way-in rows, converting them into consistent non-finding environment output while preserving the NOT A SHAPE ROOT verdict, and adds comprehensive regression tests plus clearer exit-code documentation.

Sequence diagram for safe way-in policy failure handling

sequenceDiagram
    participant Doctor as shape-doctor
    participant Policy as own_naming_policy
    participant Loader as NamingPolicy.load
    participant Project as project_token_for
    participant Row as way_in_policy_row
    participant Verdict as verdict_for

    Doctor->>Policy: own_naming_policy(shape)
    Policy->>Loader: NamingPolicy.load(NAMING_COPY)
    alt policy refuses or raises
        Loader-->>Policy: fault
        Policy-->>Doctor: (None, fault)
        Doctor->>Row: way_in_policy_row(refusal)
        Row-->>Doctor: n/a environment row
    else policy loads
        Loader-->>Policy: policy
        Policy-->>Doctor: (policy, None)
        Doctor->>Project: project_token_for(identity, policy)
        alt classify raises
            Project-->>Doctor: fault
            Doctor->>Row: way_in_policy_row(refusal)
            Row-->>Doctor: n/a environment row
        else project derived
            Project-->>Doctor: project
        end
    end
    Doctor->>Verdict: verdict_for(rows)
    Verdict-->>Doctor: NOT A SHAPE ROOT
Loading

Flow diagram for way-in rows with an unusable naming policy

flowchart TD
    A[Check the way in] --> B[own_naming_policy]
    B --> C{Policy loads and answers?}
    C -->|Yes| D[Offer way-in project command]
    C -->|No| E[naming_policy_refusal_of]
    E --> F[way_in_policy_row]
    F --> G[n/a environment row]
    G --> H[No suggested_project and no NEXT]
    H --> I[NOT A SHAPE ROOT, exit 2]
Loading

File-Level Changes

Change Details Files
Make naming-policy failures safe and consistently represent them in way-in checks.
  • Centralize loading of the checkout's own naming policy and convert loader or classification exceptions into row-level refusals.
  • Render plain-directory and repository way-in failures as n/a environment rows with no suggested command, while preserving NOT A SHAPE ROOT and exit 2.
  • Share refusal wording and detail structure with the naming row, including policy-raised exception information.
shape-doctor.py
Clarify documented INVALID and manifest-kinds exit-code semantics.
  • Define INVALID precedence and enumerate the rows that can determine it.
  • List all manifest-kinds finding causes.
shape-doctor.py
Expand regression coverage for unusable naming policies and non-root way-in behavior.
  • Test tab-indentation, invalid-regex, and classification exceptions for plain directories and repositories.
  • Verify JSON and text output has no traceback, no NEXT command, correct environment/refusal details, and unchanged verdict.
  • Refactor policy-fixture construction to support multiple failure modes.
tests/test_doctor_validator_verdicts.py

Assessment against linked issues

Issue Objective Addressed Explanation
#707 When the checkout’s own naming policy cannot load or raises while determining a suggested project name, report the fault in the “the way in” rows as an environment issue about the checkout, rather than as a finding about the target directory or an uncaught traceback. ✅
#707 Preserve the NOT A SHAPE ROOT verdict and exit code 2 for non-root directories, while ensuring affected “the way in” rows offer no unusable command and identify the checkout’s policy file and fault. ✅
#707 Correct the test documentation that previously claimed the doctor must not die when the policy pattern raises, despite the unhandled non-root case. ✅

Possibly linked issues


Tips and commands

Interacting with Sourcery

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

Customizing Your Experience

Access your dashboard to:

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

Getting Help

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-3, 2026-10-17T01:44:53Z) Independent review of #778 by RV-a (opus), on head f98e610b merged with main bf1f099b: READY. No MUST. One SHOULD worth taking in this PR, a one-line fix. Two NITs. It lands only after the v1.1.0 tag.

Cases, each run over a plain directory, a git repository, a leg clone (project/spec) and an assembly root (main → head):

policy state main head
tab line FINDING the way in refused NOT A SHAPE ROOT; way-in rows n/a, environment, yaml-unparsable, no NEXT
bad pattern traceback re.error, exit 1 exit 2; n/a, policy-raised
classify raises KeyError traceback, exit 1 exit 2; policy-raised … KeyError: 'id'
policy file is a directory FINDING n/a, yaml-unreadable … Is a directory
unreadable (chmod 0) FINDING n/a, yaml-unreadable … Permission denied
empty file FINDING n/a, policy-unreadable … not a mapping
classify returns 42 traceback TypeError n/a, policy-raised … TypeError
fine policy rows identical to main everywhere

#701's and #410's rows do not move.

n/a with no NEXT is right.

EXIT CODES is true to the code.

  • NIT-A: merge_verdict, then invalid_verdict's six gates, then the catch-all.
  • NIT-B: matches check_manifest_kinds' five causes (:3401-3639).

S1: a leg clone loses its leg row when the policy raises at load.

  • Where: scripts/shape_advisory.py:468. _naming_policy catches only (Refusal, OSError, UnicodeDecodeError), so re.error escapes shape_advisory.read, and leg_clone (shape-doctor.py:9080) says "not a leg".

  • Measured on project/spec with the pattern edit:

    • the leg row is replaced by the environment row;
    • the other way in appears for a leg, which :9269 exists to prevent;
    • triad-advisory vanishes.
  • Impact: there is no traceback, but the body's "a leg clone keeps its row" does not hold for that case.

  • Fix:

    • except Exception: in _naming_policy, since the advisory already promises never to fail a report;
    • a leg-clone case in test_an_unusable_naming_policy_is_an_environment_row_in_the_way_in_rows.

    RV-a verified it on a scratch commit: the leg row and triad-advisory come back, and there is no way-in-scaffold. shape_advisory.py is in no copy list.

NITs:

  • N1 shape-doctor.py:9213-9219 with naming_policy_words :2337: when classify raises on a policy that DID load, the row still says "does not load". Pass a verb such as "cannot be used" for that path.
  • N2 naming_policy_refusal_of :2269-2272 prints type(fault).__module__ and __qualname__. On Python 3.13 and later, re.error is an alias of re.PatternError, so the row would read re.PatternError. This is not measured, since only 3.12 is here. The tests assert only the message. Cosmetic.

Mutants (-k "naming or unusable or unloadable or way_in"; the head passes 14):

mutant killed by
way_in_project catches only Refusal 3
the row is a FINDING 6
drop the classify catch 2
own_naming_policy_refusal returns any fault 4, including #410's crash-row test

Tests:

Merges: clean with bf1f099b and with each lane-3 v1.1.1 PR, alone and all chained (1ad7e145). Windows, Sonar, docs (README:1170 and handbook:1587 stay true) and pinned copies are clear.

Lane: openreposhape-3 (openRepoShape-3)

…for any fault in the policy, so a pattern that does not compile no longer takes a leg clone's own-`AGENTS.md` route with it, and a policy that loaded and raised when asked is a row that "cannot be used" where it said "does not load" (#707)

Round 2 of #778, RV-a's S1 and N1:

- `shape_advisory._naming_policy` caught a refusal, an `OSError` and a
  `UnicodeDecodeError`, so a `re.error` escaped `read()` and `leg_clone`
  read a leg clone as "not a leg": it lost its leg row and the Triad row,
  and was offered the scaffold. The advisory never fails a report.
- `way_in_project` tells a `classify` that raises as `policy-unusable`, and
  `way_in_policy_row` words it "cannot be used"; the loader's faults stay
  "does not load".

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-3, 2026-10-17T04:13:14Z) RV-a's re-verification of #778 round 2 (fe7b30f6): READY.

Lane: openreposhape-3 (openRepoShape-3)

This was referenced Oct 17, 2026
brettheap added a commit that referenced this pull request Oct 18, 2026
…ile` skips on EILSEQ or a `UnicodeError` only and the U+202E directory has no guard, so a full disk or a read-only `tmp_path` fails the test, not SKIPPED (#834) (#845)

Stacked on #806 (`fix/757-non-utf8-test-dirs`, `76ae6419`); retarget to main once it lands. Tests only: nothing a user runs changes, and no pinned file changes.

One new commit on `9ba530a9`, one test file, one line moved. Main is not merged and neither is #806.

**SHOULD 1 (SonarCloud python:S5778, MAJOR): done, the reviewer's way.** The `pytest.raises(pytest.fail.Exception)` block in `test_the_sweep_names_the_first_errno_that_fails_and_stops_there` held two calls that could raise, `Disk(first)` and `raised_for_every_errno(...)`. `disk = Disk(first)` is now built before the block, and the block holds `raised_for_every_errno(disk, hides_the_error, "a path")` alone. The assertions after the block are unchanged and read the same exception: `failed.value.msg` is still the `Failed` that `raised_for_every_errno` raises, and `asked` is still the list the caller was asked. The file's other `pytest.raises` block (in `the_error_of`) already held one call.

### Runs

Through `run9.sh`, Python 3.12.3. Logs are in the scratchpad's `v118/w834/`.

- The two files, `tests/test_non_utf8_oserror_sites.py` and `tests/test_non_utf8_name_callers.py`: 55 passed (`t-two.log`), as the reviewer measured.
- The seven changed or importing files (`tests/test_non_utf8_git_reads.py`, `tests/test_adopt_template_values.py`, `tests/test_non_utf8_name_helpers.py`, `tests/test_non_utf8_name_callers.py`, `tests/test_non_utf8_oserror_sites.py`, `tests/test_adopt_git_reads.py`, `tests/test_family_init_asks_every_value.py`): 311 passed, 1 skipped (`t-B.log`), the same as round 2. The skip is the one in the PR body, from the run script's blank `user.name`; it does not happen in lane 4's runs.
- `tests/test_repo_hygiene.py`, `tests/test_agents_md_codes.py`, `tests/test_cli_reference.py`: 147 passed (`t-C.log`).

### Mutants

The four label and fail-fast mutants of round 2 are all still caught, each by `test_the_sweep_names_the_first_errno_that_fails_and_stops_there` alone, with the new line in place (`tmutants-summary.log`). A fifth, from lane 4's table, is caught too.

| Mutant | Tests that fail |
|---|---|
| the sweep does not pass the label | 1 |
| `the_error_of` drops the label on the way | 1 |
| `fail_where_the_caller_ends` ignores the label | 1 |
| the sweep swallows each failure and goes on | 1 |
| the label goes after the message (lane 4's) | 1 |

The first one reads `- errno 1 (EPERM): the caller ended SKIPPED: hidden` against `+ the caller ended SKIPPED: hidden`.

### Merge checks

`git merge-tree --write-tree` of `8f6a38e3` against main `9f235da2`, #806 (`76ae6419`) and `9ba530a9`: clean. #778 is as before: the one `from conftest import` line of `tests/test_doctor_validator_verdicts.py`, which no round of this PR touches after `b18b243e`.

One new commit on `efb17273`, two test files. Main is not merged and neither is #806.

**SHOULD 1 (SonarCloud python:S8714 on `raised_for_every_errno`): done, the reviewer's way.** The `except pytest.fail.Exception` caught a failure only to fail again with the errno's name in front. The label is now passed in:

- `fail_where_the_caller_ends` (`tests/test_non_utf8_name_callers.py`) takes a keyword-only `said`, put in front of its message, with `""` as the default.
- `the_error_of` threads `said` through, and the loop passes `said=f"errno {code} ({errno.errorcode[code]}): "`. The loop has no `try`.
- Nothing else changes. A failing errno still reads `errno 1 (EPERM): the caller ended SKIPPED: ...`, and the sweep still stops at the first errno that fails, since the failure ends the loop. The three other callers of `fail_where_the_caller_ends`, all in `tests/test_non_utf8_name_callers.py`, pass no `said` and are not touched: the only hunk in that file is the function's own.

A failing errno still names itself, in a run's output. This is the mutant that re-raises only ENOSPC, EROFS and EACCES in `make_file`, on this commit:

```
E       Failed: errno 1 (EPERM): the caller ended SKIPPED: this filesystem refuses a file name that is not UTF-8 (EILSEQ): [Errno 1] Operation not permitted: ...
```

**One new test** holds the label and the fail-fast, `test_the_sweep_names_the_first_errno_that_fails_and_stops_there`: a caller that skips where it should raise must fail with exactly `errno <first> (<NAME>): the caller ended SKIPPED: hidden`, and be asked once. It passes on `efb17273` as well, by design: that commit already gave the same message, and this round keeps it. So it cannot fail there. It fails on the mutants below that drop the label or the fail-fast.

**NIT 2 stays, and is #854.** Under `PYTHONUTF8=0 LC_ALL=C PYTHONCOERCECLOCALE=0`, `test_execute_refuses_a_local_source_directory_holding_a_bidi_control` fails with a `UnicodeEncodeError` from `parent.mkdir()`, on `efb17273` and on main alike. It is filed as #854. This round does not change it, and the new test of the sweep does not touch that directory.

### Runs

Through `run9.sh`, Python 3.12.3. Logs are in the scratchpad's `v117/w834/`.

- The seven files that import the changed ones or are changed (`tests/test_non_utf8_git_reads.py`, `tests/test_adopt_template_values.py`, `tests/test_non_utf8_name_helpers.py`, `tests/test_non_utf8_name_callers.py`, `tests/test_non_utf8_oserror_sites.py`, `tests/test_adopt_git_reads.py`, `tests/test_family_init_asks_every_value.py`): 311 passed, 1 skipped (`s-B.log`). The skip is the one in the PR body, on the base too. The count is the 310 of the last round plus the one new test.
- `tests/test_repo_hygiene.py`, `tests/test_agents_md_codes.py`, `tests/test_cli_reference.py`: 147 passed (`s-C.log`).
- The four `non_utf8` files together under `PYTHONUTF8=0 LC_ALL=C PYTHONCOERCECLOCALE=0`: 110 passed, 15 skipped, all the ASCII file name encoding (`s-D-clocale.log`).

### Mutants

Ten, each applied to `9ba530a9`'s test files and caught (`smutants-summary.log`):

| Mutant | Tests that fail |
|---|---|
| `make_file` skips on any `OSError` again | 4 |
| `make_file`: denylist of ENOSPC, EROFS, EACCES | 2 |
| `test_cli_reference.py`: a `try` that re-raises 13, 28 and 30 | 1 |
| `test_doctor_validator_verdicts.py`: the same | 1 |
| U+202E `mkdir`: the same | 1 |
| `conftest.py` helper: the probe's guard is a denylist | 3 |
| the sweep does not pass the label | 1 |
| `the_error_of` drops the label on the way | 1 |
| `fail_where_the_caller_ends` ignores the label | 1 |
| the sweep swallows each failure and goes on to the next errno | 1 |

The first six are the earlier mutants that the sweep kills. The last four are new, and a failing run of the first label mutant reads `- errno 1 (EPERM): the caller ended SKIPPED: hidden` against `+ the caller ended SKIPPED: hidden`.

### Merge checks

`git merge-tree --write-tree` of `9ba530a9` against main `190133d5`, #806 (`76ae6419`) and `efb17273`: clean. Against #778 (`fe7b30f6`): the one `from conftest import` line of `tests/test_doctor_validator_verdicts.py`, as before, and this round does not touch that file.

One new commit on `b18b243e`, three test files. Main is not merged and neither is #806. Each point of the review, and what was done:

**SHOULD 1: `REFUSALS` held three errnos, so a guard that re-raised those three and skipped the rest passed. Done.** The emulated disk (`a_disk_that_refuses`) now answers every errno that `errno.errorcode` holds but EILSEQ (129 here) and each caller must raise it, as Python makes it (`PermissionError` for EACCES). The errnos are drawn from `errno.errorcode` and not listed, so one nobody thought of is asked too. The sweep stops at the first errno that fails and names it, so a caller that goes on past a refused name runs its test once and not 129 times. The three-row families are gone, since the sweeps cover them: the new file has 18 tests where it had 32. Both of the reviewer's surviving mutants now fail, with the same mutants and the same run:

- `make_file` with `exc.errno in (EACCES, ENOSPC, EROFS)` in place of `!= EILSEQ`: 2 tests fail (was 0).
- `test_cli_reference.py` with a `try` that re-raises 13, 28 and 30 and skips the rest: 1 test fails (was 0).
- Three more of the same kind also fail: the doctor test's `mkdir`, the U+202E `mkdir`, and the helper in `conftest.py` (a denylist there fails 3 tests, so the sweeps also hold #663's helper to it).

**SHOULD 2: the U+202E docstring claimed more than `refused_then_moved` did. Made true, not narrowed.** The reviewer measured the gap with a `git` that fails `commit` as a full disk does. I measured it the same way, on `b18b243e` and on this commit:

- `b18b243e`: `1 skipped`, "git will not work under '\u202ecaf' here: error: unable to write file .git/objects/xx: No space left on device".
- `efb17273`: `1 failed`, `AssertionError: error: unable to write file ...`, raised at `tests/conftest.py:883`.

Why true and not narrowed: the second skip exists for a name that is not UTF-8, which git may not work under (#757 kept it for that). U+202E is valid text, and git works under it wherever it works at all (reasoned for NTFS and APFS, not run). So the skip there hides a disk that filled after the `mkdir`, which is this issue's whole point, and the fix is one keyword. `refused_then_moved` takes `skip_where_git_fails` (default true, so the byte-name test is as #757 left it), and the U+202E test passes false. Three new tests hold it:

- `test_a_failure_of_git_under_the_bidi_directory_fails_the_test`: a `make_source_repo` that raises the `AssertionError` must reach the test as a failure. It fails on `b18b243e`.
- `test_a_failure_of_git_under_the_byte_directory_still_skips_the_test`: the control, and the known limit. The byte-name test still skips there, and a full disk after its `mkdir` still reads as SKIPPED.
- `test_a_disk_that_refuses_the_bidi_directory_fails_the_test` is now a sweep over every errno.

The docstrings of both tests and of `refused_then_moved` say which skip stays and why.

**NIT 3: done.** `make_file` gives the two skips their own reasons: "this system's file name encoding cannot spell ..." for a `UnicodeError`, and "this filesystem refuses a file name that is not UTF-8 (EILSEQ)" for EILSEQ. The controls assert each one, and that the encoding reason does not say "filesystem". Both fail on `b18b243e`, and a mutant that restores either old wording fails.

**NIT 4: stays.** The emulated disk is still written in four places. Moving `a_disk_that_refuses`, `fail_where_the_caller_ends` and the two fixtures into `tests/conftest.py` adds hunks to a file that #814 and #818 also edit, and Sonar reports 0.0% duplication on new code. It is a follow-up, once those land.

**NIT 5: done.** The own-directory test is now `test_the_cli_reference_and_doctor_tests_fail_where_the_disk_refuses_their_own_directory`, which names its sites. A `-k` selection no longer picks up #757's test of the old name.

**NIT 6: done.** The module docstring keeps `skip_where_a_name_cannot_be_not_utf_8` on one line.

### Tests

The new file passes 18 of 18 on this commit. On `b18b243e`, with only the new file put in, 3 fail and 15 pass. The 3 are the two NIT 3 reasons and the U+202E git failure. The other 15 are mutant-killers for SHOULD 1, and they pass on code that is correct.

Runs on Linux (Python 3.12.3) through `run9.sh`:

- `tests/test_cli_reference.py`, `tests/test_doctor_validator_verdicts.py`, `tests/test_bootstrap_leg_moves.py`: 79 passed.
- The changed files and the files that import them, `tests/test_non_utf8_git_reads.py`, `tests/test_adopt_template_values.py`, `tests/test_non_utf8_name_helpers.py`, `tests/test_non_utf8_name_callers.py`, `tests/test_non_utf8_oserror_sites.py`, `tests/test_adopt_git_reads.py` and `tests/test_family_init_asks_every_value.py`: 310 passed, 1 skipped. The skip is the one named in the PR body (`test_plan_names_the_elector_it_took_from_git_config`, on the base too).
- `tests/test_repo_hygiene.py`, `tests/test_agents_md_codes.py`, `tests/test_cli_reference.py`: 147 passed.
- The four `non_utf8` files together: 124 passed. Under `PYTHONUTF8=0 LC_ALL=C PYTHONCOERCECLOCALE=0`: 109 passed, 15 skipped, and every skip is the ASCII file name encoding.

**Mutants.** 21 in all, one or more per hunk, each applied to the fixed tree and caught. The first eleven are the earlier table, rerun on the new tests (each fails 1 to 4 tests). The ten new ones:

| Mutant | Tests that fail |
|---|---|
| `make_file`: denylist of ENOSPC, EROFS, EACCES (reviewer's) | 2 |
| `test_cli_reference.py`: a `try` that re-raises 13, 28, 30 (reviewer's) | 1 |
| `test_doctor_validator_verdicts.py`: the same | 1 |
| U+202E `mkdir`: the same | 1 |
| `conftest.py` helper: the probe's guard is a denylist | 3 |
| `make_file`: the `UnicodeError` reason says "filesystem" again | 1 |
| `make_file`: the EILSEQ reason does not name EILSEQ | 1 |
| `refused_then_moved`: skips on git's failure whatever the caller asks | 1 |
| U+202E test: does not ask for git's failure to raise | 1 |
| byte-name test: asks for git's failure to raise | 1 |

### Merge checks

`git merge-tree --write-tree` of `efb17273` against main `6f64eea2`, #806 (`76ae6419`) and `b18b243e`: clean. Against the 57 open `fix/*` and `l3-*` branches: 50 clean. The seven that conflict are six of the seven that conflicted at `b18b243e` (`fix/712-validate-pins-contracts` moved and is clean now) plus `fix/835-config-count-appends`, which conflicts in `tests/test_adopt_git_reads.py` as #818 does against #806. Every one conflicts the same against #806 or main, with one exception, below. The exception is #778, which conflicts with this branch only, on the one `from conftest import` line of `tests/test_doctor_validator_verdicts.py`, as at `b18b243e`: this round does not touch that file. This round changes no conflict region against #814 (one region in `tests/test_non_utf8_git_reads.py`, before and after).

### Not changed, and a candidate

- **A candidate, not filed:** under `PYTHONUTF8=0 LC_ALL=C PYTHONCOERCECLOCALE=0`, `test_execute_refuses_a_local_source_directory_holding_a_bidi_control` fails on `b18b243e` and on #806 (`UnicodeEncodeError` from `parent.mkdir()`: the ASCII encoding cannot spell U+202E). #757 skips three tests of `tests/test_non_utf8_git_reads.py` that way with `skip_where_the_environment_cannot_carry`. The new git-failure test asks it for U+202E and skips there, so this round adds no failure to a C locale. The test of the directory itself is not changed, because the review did not raise it.
- The two checklist candidates in the PR body stand, narrowed: `refused_then_moved` no longer belongs to the first one for U+202E, only for the byte-name test.

Five tests make a directory or a file named with a byte that is not UTF-8, or with U+202E. Each turned ANY `OSError` into `pytest.skip`, beside the one helper #663 added, `skip_where_a_name_cannot_be_not_utf_8`. So a full disk, a read-only `tmp_path` or a folder nobody can write in read as SKIPPED, and no run fails on a skip. CI never has one of those, so nothing failed. #806 moved five other tests onto the helper and left these.

Each line of #834, with the direction chosen:

1. **`tests/test_cli_reference.py`, `test_a_checkout_whose_path_is_not_utf8_is_a_checkout`.** It asks the helper on `tmp_path`, then makes its directory with no `try`.
2. **`tests/test_doctor_validator_verdicts.py`, `test_a_name_that_is_not_utf8_is_never_an_exception_quoted_as_a_finding`.** Same: the helper first, then `mkdir(parents=True)` unguarded. The docstring said "a disk that refuses one (APFS) skips"; it now says which skip and that every other error fails.
3. **`tests/test_bootstrap_leg_moves.py`, `test_a_branch_whose_name_is_not_utf8_is_read_back_on_every_run`.** The helper replaces the `write_bytes` probe that skipped on `OSError` and `ValueError`. The Windows `skipif` stays.
4. **`make_file` in `tests/test_non_utf8_git_reads.py`.** It keeps the `UnicodeError` skip (an ASCII file name encoding, #757's second line) and skips an `OSError` only where `errno` is `EILSEQ` (macOS). ENOSPC, EROFS and EACCES are raised, as the helper does for its `mkdir`. Every caller of `make_file` inherits the failure and not the skip. #757's `test_a_write_the_disk_refuses_still_fails_the_make_file_test` (in `tests/test_non_utf8_name_callers.py`) expected the `AssertionError` of the test's own "not a skip" assert; it now expects the `OSError` itself, with its errno.
5. **`tests/test_adopt_template_values.py`, the U+202E directory.** The `try` is dropped, so `mkdir` raises. The issue offered the other way, narrowing to EILSEQ and naming the system that refuses U+202E. None is known: U+202E is valid text. A `mkdir` of a name that starts with U+202E succeeds on this machine's overlay filesystem (measured), and NTFS and APFS are reasoned to hold it (read, not run on those two). So a narrowing would guard a refusal nobody has seen.

`tests/test_non_utf8_oserror_sites.py` is new: 32 tests. It calls each site as a plain function, with the disk answering as it would on a system CI does not run (CI is Linux only, #597):

- EILSEQ on the probe (macOS) skips the three tests that ask the helper, on the helper's reason.
- EACCES, ENOSPC and EROFS on the probe, on the test's own directory, on `make_file`'s write and on the U+202E directory raise. The test asserts the error and its errno, and fails if the call skipped or ended without one.
- An `os.fsencode` that gives a surrogate three bytes (Windows, emulated with `windows_fsencode`) skips the three helper callers before the disk is touched, and nothing is made under `tmp_path`.
- `make_file` still skips on EILSEQ and on a `UnicodeError` (two controls).

The disk is emulated by `os.mkdir` and `Path.write_bytes`, the two calls the base and the fix use to make a name, so the same row fails on the base for the reason it should. It reuses the two fixtures, `PROBE` and `fail_where_the_caller_ends` of #806's `tests/test_non_utf8_name_callers.py`. No Windows or macOS run was made.

Runs, on Linux (Python 3.12.3), under `run9.sh`:

- `tests/test_non_utf8_oserror_sites.py`, `tests/test_non_utf8_name_callers.py`, `tests/test_non_utf8_name_helpers.py`: 86 passed.
- `tests/test_cli_reference.py`, `tests/test_doctor_validator_verdicts.py`, `tests/test_bootstrap_leg_moves.py`: 79 passed.
- Those two sets with `tests/test_non_utf8_git_reads.py`, `tests/test_adopt_template_values.py`, `tests/test_adopt_git_reads.py` and `tests/test_family_init_asks_every_value.py` (the files that import a changed module): 324 passed, 1 skipped. The skip is `test_plan_names_the_elector_it_took_from_git_config`, which skips on the base too: the run script's blank `user.name` overrides its `GIT_CONFIG_GLOBAL`.
- `tests/test_repo_hygiene.py`, `tests/test_agents_md_codes.py`, `tests/test_cli_reference.py`: 147 passed. `render-cli-reference.py --check` passes.
- `tests/test_non_utf8_git_reads.py` under `PYTHONUTF8=0 LC_ALL=C PYTHONCOERCECLOCALE=0`: 40 passed, 12 skipped; every skip is the ASCII file name encoding, as before. Normal locale: 52 passed.

**Fails on the base.** On `76ae6419` with only the two test files put in, 32 of the 68 tests of `tests/test_non_utf8_oserror_sites.py` and `tests/test_non_utf8_name_callers.py` fail: 30 of the new 32 and the 2 changed ones. The other 36 pass: the 2 new controls, which hold what both keep, and the 34 that are unchanged.

**Mutants.** One per hunk, each applied to the fixed tree and caught:

| Mutant | Tests that fail |
|---|---|
| `test_cli_reference.py`: `except OSError` put back around the `mkdir` | 3 |
| `test_cli_reference.py`: helper call removed | 5 |
| `test_doctor_validator_verdicts.py`: `except OSError` put back | 3 |
| `test_doctor_validator_verdicts.py`: helper call removed | 5 |
| `test_bootstrap_leg_moves.py`: the old `write_bytes` probe back | 5 |
| `test_bootstrap_leg_moves.py`: helper call removed | 5 |
| `make_file`: skips on any `OSError` again | 8 |
| `make_file`: EILSEQ no longer a skip | 1 |
| `make_file`: `UnicodeError` no longer a skip | 1 |
| `test_adopt_template_values.py`: `except OSError` put back around the U+202E `mkdir` | 3 |
| `test_non_utf8_name_callers.py`: the old `AssertionError` expectation back | 2 |

`git merge-tree --write-tree` of this head against origin/main (`fc867207`, which has #746), against `bad05d21`, against #806 and against all 52 open `fix/*` and `l3-*` branches: clean, except seven branches.

- **#778 (`l3-707-way-in-policy`) conflicts in one place, and only with this commit.** Both PRs add a name to the `from conftest import ...` line of `tests/test_doctor_validator_verdicts.py` (#778 adds `git`, this one adds the helper). Keep both names. #778 is clean against main and against #806.
- **#818 (`fix/758-git-command-scope`)** conflicts in `tests/test_adopt_git_reads.py`, which this commit does not touch. It conflicts the same against #806 alone, and is clean against main.
- **#814 (`fix/766-spelled-paths`)** conflicts in `tests/test_non_utf8_git_reads.py` and `tests/test_dry_run_parity.py`. It conflicts the same against main and against #806, with one region in each file with and without this commit.
- **`fix/712-validate-pins-contracts`, `fix/798-adopt-legs-asked-first`, `l3-677-pins-hand-step-wip` and `l3-678-naming-checkout-wip`** conflict in files this commit does not touch, and conflict the same against main and #806.

The scan of `tests/` for an `except` that names `OSError` and is followed by `pytest.skip` lists 31 other places. As #834 says, most ask something else (a symlink, a hard link, `locale -a`, a NEL or a line feed in a folder name) and are not changed here. Two kinds are the same defect as this issue's and are not lines of it. Nothing is filed; they are for the checklist.

- **A skip on an `AssertionError` of a git call, kept by #757 as a second skip.** `refused_then_moved` (`tests/test_adopt_template_values.py`), `test_a_utf8_clone_of_a_source_whose_path_is_not_utf8_adopts` (`tests/test_adopt_git_reads.py`) and `test_a_source_directory_that_is_not_utf8_is_shown_spelled` (`tests/test_adopt_plan_escapes.py`) skip with "git will not work under ..." when `make_source_repo` raises `AssertionError`. A disk that fills after the directory is made fails that call the same way, and the test reads as SKIPPED. For the U+202E test of line 5 this means the `mkdir` now fails on a full disk, and a disk that fills a moment later can still skip it.
- **A skip on any `AssertionError`, `UnicodeError` or `OSError` where the branch name is not UTF-8.** `with_default_branch` and `moved_to_a_branch_that_is_not_utf8` (`tests/test_adopt_git_reads.py`) make the branch with `git`, and skip if it fails for any reason.

Review: lane 4's RV845, READY on b18b243 (issuecomment-6240020694), on efb1727 (issuecomment-6240528662) and on 9ba530a (issuecomment-6241540556); 8f6a38e applies RV845's S5778 fix exactly as RV845 probed it. `fc3cb5e9` restacks it onto #806's squash `d6071c7d` with three merges and the same `-U0` lines. CI (Brett Heap's ruling 27): H0 8f6a38e passed tests, tests-git-bash and SonarCloud; the head fc3cb5e merges #806's final head, then #806's squash d6071c7 (the branch's own side for #806's files), then main, with tests, tests-git-bash and SonarCloud green; and the local gate passed on its predicted squash tree b572ef4 over main 576e170: the adopt_template_values, bootstrap_leg_moves, cli_reference, doctor_validator_verdicts, family_bootstrap_gate_order, family_unnamed_gitlink, non_utf8_git_reads, non_utf8_name_callers, non_utf8_oserror_sites, pin_validator, validate_family_unnamed_mounts, repo_hygiene, agents_md_codes, cli_reference sets, 524 passed, 1 skipped. Landed on Brett Heap's word of 2026-10-17T21:47:28Z, in lane 5's session (posted on #473 as issuecomment-6241758069): "merge lane 5's v1.1.1 PRs once reviewed and green".

Closes #834

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One conflict, tests/test_doctor_validator_verdicts.py, one hunk: the
`from conftest import` line near the top. Main's side (#845, squash
0f69339) added `skip_where_a_name_cannot_be_not_utf_8` to it; this
branch's side (#778, closes #707) added `git`. The resolution imports
both names, `WINDOWS_SKIP, clone_just_pushed, git, run_script,
skip_where_a_name_cannot_be_not_utf_8`, in main's wrapped form.

Every other hunk of the file merged by itself, and no other file
conflicted: #845's change to the non-UTF-8 name test and #778's #707 rows and
tests both stay as each side wrote them.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-3, 2026-10-18T11:34:57Z) #778 has a merge round with main, by lane 3's W778m (sonnet): one merge commit, now at 8259ea12. Lane 5's #845 (squash 0f69339) changed tests/test_doctor_validator_verdicts.py too, so fe7b30f6 conflicted with main.

Lane: openreposhape-3 (openRepoShape-3)

brettheap pushed a commit that referenced this pull request Oct 18, 2026
…ith #886, and #778 from its reviewed head (#883)

Round 6 of #883, a refresh. Each line was checked against the code:

- #886 (#884), from its squash e7039b7: a holder's `make bootstrap`
  reads a member's make on stderr a line at a time and names the
  innermost command's code, so a member whose make runs another no longer
  reads `exited 2` for a finding, and `make -k` keeps its `Error` line. A
  line in "What changed" and words in "What 1.1.1 fixes". The #884 limit
  is narrowed to what `run_make`'s and `MakeErrors`' docstrings keep: the
  pipe's three effects and the Windows decoding, the nested-make cases
  the line still misnames, the signal under dash, and make before 4.0
  not run.
- #778 (#707), from its reviewed head 8259ea1, not landed at the fetch
  just before this commit: outside a shape root, a naming policy that
  does not load, or cannot be used, in the doctor's own checkout makes
  the `the way in` rows `n/a` environment rows, where a tab line was a
  FINDING and a bad pattern a traceback, and a leg clone keeps its rows.
  A line in "What changed" and one in "What 1.1.1 fixes". "Before
  tagging" says the line comes from that head.
- "The list runs to" names no commit now: the commit v1.1.1 is tagged on.

The open list is #879 alone: #884 is closed by #886, and #707 by #778
once it lands. The shape-copy range runs to e7039b7; #886 changes the
holder's bootstrap.py, on the lists, and #778's head no pinned copy. No
other file changes.

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

Copy link
Copy Markdown

@brettheap
brettheap marked this pull request as ready for review October 18, 2026 13:19
@sourcery-ai

sourcery-ai Bot commented Oct 18, 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 3 days and 5 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap merged commit 2d372bb into main Oct 18, 2026
5 checks passed
@brettheap
brettheap deleted the l3-707-way-in-policy branch October 18, 2026 13:19
brettheap pushed a commit that referenced this pull request Oct 18, 2026
…g keeps its rows, #802's readers what v1.1.0 already skipped, and the draft lines name #883 and #879's move (#883 follow-up)

Lane 1's independent reviewer r893 found #893 NOT READY at b795ef7, with
one MUST and two NITs. Each was checked against v1.1.0 (190133d) and the
release commit 5017830 before the text changed:

- MUST, #778's leg-clone sentence. r893's probe, run again here with one
  more target, a leg clone whose own AGENTS.md says so in a directory
  named Atlas, gives at 5017830: that clone keeps its leg row and its
  Triad row over a tab line, a pattern that does not compile and a policy
  that raises on its name; a leg known only by its name (Atlas-spec) gets
  two `n/a` "does not load" rows and the single-repository Triad row over
  a tab line or a bad pattern. At v1.1.0 the AGENTS.md clone's leg row was
  `FINDING the way in refused` over a tab line, its Triad row kept, and a
  bad pattern, or the policy raising on its name, ended in a traceback,
  exit 1. The sentence now says that.
- NIT, #802's readers. At v1.1.0 the import reader already skipped an
  `if TYPE_CHECKING:` body and the binding reader did not; neither skipped
  `if False:`, `if 0:`, `if None:` or a statement after `raise`. At
  5017830 both skip all of them. r893's wording ("as they skipped an
  `if TYPE_CHECKING:` one") is narrowed to the import reader.
- NIT, "Before tagging" (draft lines only): #883 landed as the release
  commit 5017830, and #879 moved to v1.2.0 at 2026-10-18T14:10:11Z, after
  2d372bb landed at 13:19:52Z.

No other file changes.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 18, 2026
… the release commit, #778's leg-clone sentence and #802's readers sentence, each checked against runs at v1.1.0 and main (#883 follow-up) (#893)

The v1.1.1 notes landed as the release commit `5017830` (#883), and RV883's round 5 held two SHOULDs and five NITs for after it (#883 issuecomment-6250558821). This applies them to `docs/release-notes/v1.1.1.md`, with two more sentences lane 1's review found:

1. **#778's leg-clone sentence** says what runs at `v1.1.0` and `5017830` show. A leg clone whose own `AGENTS.md` says so keeps its leg row and its Triad row over each fault, where v1.1.0 gave `FINDING the way in refused` over a tab line and a traceback, exit 1, over a pattern that does not compile or a policy that raised on its name. A leg known only by its name gets a repository's two `n/a` rows.
2. **"Is", not "stays", exit 2,** in the pattern case, which was exit 1 in v1.1.0.
3. **#802's clause is dropped:** v1.1.0's `_blocks_inside` already read a `try`'s handlers, `else` and `finally` outside its guard.
4. **"What 1.1.1 fixes"** names "or cannot be used", and `the other way in`.
5. **#815's leftover tracebacks** were measured on Python 3.12; 3.13 is read, not run.
6. **#871 lets `_` through for any owner,** an organisation's name included.
7. **#869 asks the disk** only about a name that is not UTF-8.
8. **#802's readers:** at v1.1.0 only the import reader skipped an `if TYPE_CHECKING:` body; the binding reader skips one now.
9. **A known limit names #940:** under the `C` or POSIX locale, the bash `openRepoShape` and `setup.sh`'s guards refuse a URL, path or ref with a letter past ASCII as "a control character", a regression from v1.1.0 in the launcher through #872 (#866); a UTF-8 locale takes such a URL or path.

The draft-time lines in the sections marked "remove at tag time" say main `2d372bb`, where #778 landed file for file as its reviewed head `8259ea1`, and that #879 moved to v1.2.0. The release body never carries those sections.

Review: lane 1's independent reviewer found `b795ef74` NOT READY, on the leg-clone sentence, and `ca49f125` READY, with each changed sentence checked against runs at `v1.1.0` and `5017830`. Lane 1's second reviewer found `6cdb5551`, which adds the #940 limit, READY, with each clause measured against the launcher at `64464d31` and at v1.1.0.

It lands by fast refresh (Brett Heap, 2026-10-15T22:48:35Z). Its reviewed head `6cdb5551` is green on Linux and SonarCloud. The branch merged main once and was never rebased; the last merge is update-branch's, clean, giving f60654e, which is green on SonarCloud and Git Bash. Lane 1's gate passed on its tree: the 5 test files that read the release notes or the workflows (`test_repo_hygiene.py`, `test_release_workflow.py`, `test_workflow_concurrency.py`, `test_agents_md_codes.py` and `test_cli_reference.py`), 221 passed and 0 skipped.

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

Labels

None yet

Projects

None yet

1 participant