Skip to content

shape_materialize.py, scaffold-project.py and family.py: the folder refusals and family-operation-in-progress name a path as shown_source names it, so an ESC or a line break in a path no longer reaches the terminal (#692) - #731

Merged
brettheap merged 3 commits into
mainfrom
fix/692-raw-folder-paths
Oct 17, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openreposhape-5 (openRepoShape-5)
Claim: #692 (comment)

What changed and why

#622's folder refusals printed a path as it was. family.py init --into <a file named wd-ESC[31mRED> --dry-run gave REFUSED family-folder-cannot-be-made: the family folder .../wd-^[[31mRED/Orbit cannot be made: ... through cat -v. A name with a line break and then WARNING forged-line: pwned printed that line as a line of its own. R612's family-operation-in-progress printed the mark's path across two lines for a holder whose path holds a newline.

Each path is now spelled as shape_materialize.shown_source spells a --pin-source: as it is, and as Python's repr spells it where a character in it is not printable.

  • scripts/shape_materialize.py: both answers of folder_cannot_be_made (a file or a bad stat in the way, and a folder this user cannot write in), the folder in refuse_unmakeable_folder, and the folder in make_folder.
  • scaffold-project.py: scaffold-target-exists.
  • scripts/family.py: family-target-exists, and the mark's path in family-operation-in-progress (where.as_posix()).

A printable path is named byte for byte, so no existing row changes and why.endswith(CANNOT_WRITE_IN) in refuse_unmakeable_folder still holds. No help text changed, so docs/cli.md is untouched. No README, handbook or template sentence says how a refusal names a path (git grep of shown_source, target-exists and folder-cannot-be-made in AGENTS.md, README.md, docs/handbook.html and the templates found none), so no doc changes.

Decisions and their evidence

Tests

tests/test_dry_run_parity.py:

  • SCAFFOLD_CASES gains five rows: ESC and line break in the --work-dir name, ESC in a folder name above it, and ESC and line break in a work dir that holds a file named Atlas (scaffold-target-exists).
  • FAMILY_CASES gains five rows: ESC and line break in --work-dir, ESC in --into (the family folder), and ESC and line break in a work dir that holds a file named Orbit (family-target-exists).
  • Two rows join UNWRITABLE_FOLDER_AT (ESC in the scaffold's and in init's --work-dir, a folder this user cannot write in).
  • Rows named ESC-... and line-break-... are skipped on Windows, whose file names cannot hold them (POSIX_NAMES, case_params). Every expectation spells the path with repr, and not with shown_source.
  • assert_refused now fails any stderr line that starts with WARNING forged-line.
  • Three unit tests: test_each_folder_question_spells_a_name_that_is_not_printable (ESC, line break; folder_cannot_be_made, refuse_unmakeable_folder and make_folder by themselves), test_a_folder_this_user_cannot_write_in_is_spelled_safe_to_show, and test_a_printable_name_is_named_byte_for_byte (a name with a space and an accented letter stays unquoted).

tests/test_family_refuse_up_front.py: test_a_holder_path_with_a_newline_still_shows_its_merge asserted the raw newline in the refusal ("Ink\nRouter/.git/worktrees/" in ran.stderr). It is now test_a_holder_path_with_a_control_character_still_shows_its_merge, over a line break, an ESC and a forged line, and asserts the one-line repr form (refusal() fails unless the stderr is two lines). It is the one existing test this change had to edit.

Results at head ab55c29f (Python 3.12.3, Linux):

  • tests/test_dry_run_parity.py tests/test_dry_run_asks_remotes.py tests/test_family_dry_run.py tests/test_family_init_landing.py tests/test_repo_hygiene.py tests/test_pin_source_unreadable.py tests/test_scaffold_leg_path_collisions.py tests/test_scaffold_reuse_seeded_legs.py: 494 passed.
  • tests/test_family.py tests/test_family_refuse_up_front.py tests/test_family_init_values.py tests/test_family_add_refusals.py: 356 passed, 19 skipped (the skips are test_family_init_values.py:219 and :291, template-quoting premises).
  • Not run: the full suite (the coordinator's gate), and macOS and Windows (CI is Linux-only since ci: run the test suite on Linux only #597). The new hostile-name rows are POSIX only by design.

On the base code: the review overlaid this head's two test files on main 2fac9790 and ran tests/test_dry_run_parity.py and tests/test_family_refuse_up_front.py with -k "dry_run_parity or control_character". The result is 20 failed, 87 passed (76 deselected, 43.5 s). The 20 are exactly the new and edited tests: the 5 scaffold and 5 init rows, the 4 unwritable-folder rows, the 3 unit tests (test_each_folder_question_spells_a_name_that_is_not_printable in two cases, and test_a_folder_this_user_cannot_write_in_is_spelled_safe_to_show), and the 3 cases of the edited test_a_holder_path_with_a_control_character_still_shows_its_merge. The one new test that passes on main's code is test_a_printable_name_is_named_byte_for_byte, by design: a printable name is named byte for byte on both. An earlier draft of this body said 21 failed, 2 passed. That run was on 8062810d with a wider selection, and it held a scaffold row for ESC in the local remote dir's name (ESC-in-the-local-remote-dir-name) that this head's tests no longer have; its second pass was the existing test_the_refusal_quotes_gh_s_first_line_and_escapes_what_is_unprintable, which also passes in the new run, one of the 87.

Reproduction

family.py init and scaffold-project.py --dry-run against a file named wd-ESC[31mRED, a file named wd-\nWARNING forged-line: pwned, and a work dir with a file Atlas or Orbit in it, run with a stub gh and no real gh:

  • On main, through cat -v: ... the family folder /.../wd-^[[31mRED/Orbit cannot be made: /.../wd-^[[31mRED is a file, not a folder, and a stderr line WARNING forged-line: pwned cannot be made: ... of its own.
  • On this head: REFUSED family-folder-cannot-be-made: the family folder '.../wd-\x1b[31mRED/Orbit' cannot be made: '.../wd-\x1b[31mRED' is a file, not a folder, and one line for the forged name: --work-dir '.../wd-\nWARNING forged-line: pwned' cannot be made: '...' is a file, not a folder. scaffold-target-exists and family-target-exists are one line each in the same form.

Mutation probes

Each spelled site reverted alone in a throwaway clone at 19d126cd (the call sites are the same at ab55c29f); the new tests that fail for each:

  • folder_cannot_be_made, the _not_a_folder answer: 8 failed (six CLI rows and the two unit tests).
  • folder_cannot_be_made, the CANNOT_WRITE_IN answer: 5 failed (the four unwritable rows and the unit test).
  • refuse_unmakeable_folder: 12 failed.
  • make_folder: 2 failed (the unit test only).
  • scaffold-target-exists: 2 failed (its two rows).
  • family-target-exists: 2 failed (its two rows).
  • family-operation-in-progress: 3 failed (line break, ESC, forged line).

Merge checks

git merge-tree --write-tree of this head ab55c29f against origin/main 20c8873b and against every ref on origin that could meet it, fetched 2026-10-16: the 41 branches (fix/*, l3-*, wip/*, docs/* and release/v1.0) and the 55 PR heads from #630 up. That is 96 refs. (The same list was run for #730's head 7f2bb36b.)

#634 is on main, and this change does not edit shown_source or its docstring in scripts/shape_materialize.py.

Review

The review approved with nits. Its S1, the stale base-run numbers and merge lines, is taken here, in this body: no code changed, and the head is still ab55c29f. Its N1, renaming the call sites to shown_path, is not taken (see "Decisions and their evidence"). It also re-ran 5 of the 7 mutation probes on ab55c29f and got the same failure counts as above (12, 2, 3, 2 and 2), and ran 144 hostile-name dry runs (16 names in 9 scenarios): on main 69 rows put a control character on stderr and 6 print a forged line of their own; on this head all 144 exit 2 with no traceback, no control character and no forged line.

Left out, and why

  • scripts/family.py init's plan prints folder and landing lines with the path raw (print(f"landing {holder} ..."), print(f"folder {folder} ...")). On a dry run or a refused run the plan is on stdout before the refusal, so WARNING forged-line: pwned/Orbit (--work-dir: no family folder) still appears there. It is the plan, not a refusal, and not one of the issue's lines.
  • Other refusals that name a holder or folder path raw, none of them in the issue's lines: family-holder-detached and family-on-tracking-branch ({family.root}), family-index-locked (lock.as_posix() in the detail), family-landing-in-standard ({holder}, {checkout}, {beside / family / family}), family-holder-already-here ({into}), and family-remote-exists and scaffold-remote-exists ({target}, a local remote path).

Checklist candidates

  1. family.py init's plan prints the holder and folder paths raw. scripts/family.py landing and folder rows (about :1756-1759) print holder and folder as they are, so a forged line or an ESC reaches stdout in a --dry-run and in a run refused after the plan. Direction: spell them with shown_source.
  2. The same raw spelling in the refusals listed above. family-holder-detached, family-on-tracking-branch, family-index-locked, family-landing-in-standard, family-holder-already-here, family-remote-exists and scaffold-remote-exists each print a path the person supplied or git keeps. Direction: the spelling this change gives its sites.

Closes #692

🤖 Generated with Claude Code

Summary by Sourcery

Prevent control characters and line breaks in user-controlled paths from reaching refusal output unsafely.

Bug Fixes:

  • Safely escape non-printable characters in paths shown by folder, scaffold, and family-operation refusals so terminal control sequences and forged lines cannot be injected into output.

Enhancements:

  • Preserve byte-for-byte rendering for printable paths while standardizing safe path presentation across affected refusal messages.

Tests:

  • Add POSIX-focused coverage for ESC, newline, and forged-line path names across scaffold, family, folder-creation, and operation-in-progress refusals.

Brett Heap and others added 2 commits October 16, 2026 14:19
…efusals and `family-operation-in-progress` name a path as `shown_source` names it, so an ESC or a line break in a folder's name no longer reaches the terminal or begins a line of the person's choosing (#692)

#622's folder refusals printed a path as it was. `family.py init --into
<a file named wd-ESC[31mRED> --dry-run` gave `REFUSED
family-folder-cannot-be-made: the family folder .../wd-^[[31mRED/Orbit
cannot be made: ...` through `cat -v`, and a name holding a line break and
then `WARNING forged-line: pwned` printed that line as a line of its own. The
same two tools already spell a `--pin-source` that way
(`shape_materialize.shown_source`): as it is, and as Python's `repr` spells
it where a character in it is not printable.

Each path is spelled so now: both answers of `folder_cannot_be_made`, the
folder in `refuse_unmakeable_folder` and `make_folder`,
`scaffold-target-exists`, `family-target-exists`, and the mark's path in
`family-operation-in-progress`. A printable path is named byte for byte, so
no existing row changes and `why.endswith(CANNOT_WRITE_IN)` still holds.

`test_dry_run_parity.py` gains rows for an ESC and for a line break then
`WARNING forged-line: pwned` in `SCAFFOLD_CASES`, `FAMILY_CASES` and the
unwritable-folder rows, and `assert_refused` fails any forged line;
`test_family_refuse_up_front.py`'s newline-in-the-holder test now expects
the one-line `repr` form and covers an ESC and a forged line too.

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ock each file already has for the folder questions, so the next import another change adds after the block does not meet it (#692)

The first commit imported `shown_source` on a line of its own after each
block. Lane 4's #660, #688 and #693 add an import of their own at the same
place, which `merge-tree` reports as add/add conflicts in
`scripts/family.py` and `scaffold-project.py`. The name goes into the
existing `from shape_materialize import (...)` block instead, in
alphabetical order. No line of behaviour changes.

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

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-5, 2026-10-16T17:23Z) @codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-16T17:27:17.172146Z ab55c29 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@sourcery-ai

sourcery-ai Bot commented Oct 16, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR makes selected scaffold and family refusal paths terminal-safe by applying shown_source, preventing ESC sequences and embedded newlines from altering output, and adds focused POSIX regression tests while preserving existing printable path output.

Sequence diagram for terminal-safe path refusal

sequenceDiagram
    participant CLI
    participant Command
    participant Materializer
    participant Refusal

    CLI->>Command: run scaffold or family operation
    Command->>Materializer: folder_cannot_be_made(path)
    Materializer-->>Command: shown_source(path) plus reason
    Command->>Refusal: raise refusal with shown_source(path)
    Refusal-->>CLI: one-line terminal-safe stderr

    alt existing target
        Command->>Refusal: scaffold-target-exists or family-target-exists
        Refusal-->>CLI: shown_source(target) already exists
    else operation in progress
        Command->>Refusal: family-operation-in-progress
        Refusal-->>CLI: shown_source(where.as_posix())
    end
Loading

Flow diagram for terminal-safe folder refusal paths

flowchart TD
    A[Path supplied to scaffold or family operation] --> B{Path is printable?}
    B -->|Yes| C[shown_source returns path byte for byte]
    B -->|No| D[shown_source uses Python repr escaping]
    C --> E[Refusal message]
    D --> E
    E --> F[Single-line safe stderr output]
Loading

File-Level Changes

Change Details Files
Escape control characters in refusal messages by consistently using shown_source for paths.
  • Updated folder creation and writability refusals to safely render paths while preserving printable output and existing remediation checks.
  • Updated scaffold and family target-exists refusals to use safe path rendering.
  • Updated the in-progress family operation refusal to render the Git worktree path on one terminal-safe line.
scripts/shape_materialize.py
scaffold-project.py
scripts/family.py
Expand regression coverage for hostile path names and preserve byte-for-byte rendering of printable names.
  • Added POSIX-only ESC, newline, and forged-line cases across scaffold, family, target-exists, unwritable-folder, and folder-creation paths.
  • Added assertions preventing forged warning lines and verifying one-line repr-style operation-in-progress output.
  • Retained coverage that printable paths, including spaces and accented characters, remain unchanged.
tests/test_dry_run_parity.py
tests/test_family_refuse_up_front.py

Assessment against linked issues

Issue Objective Addressed Explanation
#692 Spell every path named by the #622 folder refusals safely, including paths in folder feasibility checks, folder-creation failures, scaffold target-exists refusals, and family target-exists refusals. ✅
#692 Spell the holder path in the family-operation-in-progress refusal safely so control characters cannot reach the terminal or create additional lines. ✅

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

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: ab55c29f52

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

This was referenced Oct 16, 2026
brettheap added a commit that referenced this pull request Oct 17, 2026
… verification row's colon, `run_validator`'s "costs no child an import" and the decoy test's `sitecustomize` claim say what main does (#666) (#730)

Four comments and docstrings said something main no longer does. They now say what the code beneath them does. A fix round after review added two more of the same kind (see "Fix round"). Comments and docstrings only: no behaviour, no `--help` text, no pinned copy, so no README, handbook or `docs/cli.md` change.

- **`adopt-project.py`, the module docstring.** It said the verification table reads `code: 0 of N source paths (seeded from template)`. The printed row has no colon (`  {leg:<6} {count:>5} of {N} source paths{note}`). It now says the table's `code` row reads `0 of N source paths (seeded from template)`, the words `AGENTS.md` and `README.md` use since #505 and the docstring of the verification function uses too. `git grep 'code: 0 of'` finds nothing now.
- **`shape-doctor.py`, `submodule_config`.** The last sentence of the docstring gave a reason that is true of the string and not decisive: "It is also what the row PRINTS, and a lone surrogate is a string no UTF-8 stream can write." The table goes through `ascii_text` and `--json` through `json.dumps`, so neither would raise on a lone surrogate. I dropped the sentence, the issue's first direction. The comparison with `origin_url`'s reading of a clone's remote is the reason, and the paragraph already gives it first. **This hunk carries the bytes of lane 3's #700**, which landed as `55c6ba7d`: the two removed lines and the one added line (`    working clone.`) are the same. So the hunk is on main already and drops out of the squash (see "Evidence").
- **`shape-doctor.py`, `run_validator`.** "It costs no child an import" holds for an unedited copy only. A new paragraph says a root whose own copy of a validator gained an `import yaml` read `passes` while `site` was on, and now reads `crashed before reaching a verdict (exit 1): ModuleNotFoundError: No module named 'yaml'`, the crash its own CI gets, because `validate.yml` installs nothing. It calls that row a fair answer and says `-S` does not ask whether a copy was edited. No code change.
- **`tests/test_shape_doctor.py`, the decoy test's two comments** (the block above `DECOY_REPO_SHAPE` and the inline comment before `reached`). `run_validator` starts every child as `[sys.executable, "-S", script, ...]`, so no child imports a `sitecustomize`. The comments now say that, say that replace-not-prepend and the #306 review S1 case are held by `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given`, and say that the decoy test holds the template row's real verdict under an exported `PYTHONPATH`.

The review of the first two commits approved with nits. Two new commits take S1 and N1: `7f2bb36b` makes both edits, and `426877bf` rewrites the last sentence of the N1 comment, which did not parse ("...a future removal of `-S`, which `test_run_validator_starts_every_child_with_no_site` ... is the test that holds"). It now ends "...a future removal of `-S`; `test_run_validator_starts_every_child_with_no_site` (in `tests/test_doctor_validator_site.py`) is the test that holds `-S`." Both are comments and docstrings only.

- **S1, a third stale sentence.** The docstring of `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given` said that for the callers which pass no `pythonpath`, "only a probe like this one, or (b)'s decoy, can see that they have none". That is false under `-S`: a `run_validator` that keeps the variable for those callers passes the decoy test in all three cases and fails the probe test twice (the second row of the table below). It now says only a probe like this one can see that they have none, and that the decoy of (b) could before `-S` (#342). It is the same defect as the issue's, in a place the issue did not name. "Could" holds for the `manifest` and `pins` cases: with `-S` removed, 2 of the 3 decoy cases fail, and the `family` case never caught it.
- **N1, the inline comment overstated the loop.** The comment above `reached` said "What this loop still holds is that no row quotes the decoy or says it exited 86." Under `-S` the loop cannot fire. It now says so, says the loop is kept as a tripwire for a future removal of `-S`, and names `test_run_validator_starts_every_child_with_no_site` in `tests/test_doctor_validator_site.py` as the test that holds `-S`. That test is on main.
- **N2, this body.** The merge checks and the notes on #630 were stale (they cited `de926142` and a conflict that is gone). They are rerun and rewritten below.

**Only comments and docstrings changed.** `git diff 8062810 426877b` has 74 changed lines (44 added, 30 removed) in three files. A script checked each changed line against the file's comment tokens and docstring spans (both sides of the diff): 0 outside. A second check parsed each file at the base and at the head, blanked every docstring, and compared the ASTs: identical in all three files. For the two fix-round commits together, the AST of `tests/test_shape_doctor.py` with docstrings blanked is identical to the one at `a2113cff`, `git diff --check` is clean, and the longest added line is 75 columns (73 in the second commit).

**What the squash holds.** `git merge-tree --write-tree` of this head with origin/main `20c8873b` gives the tree `6ead2ffc`. `git diff 20c8873 6ead2ffc` is 71 changed lines in three files, and `shape-doctor.py` has 8 of them: the new `run_validator` paragraph and nothing else. `git diff` of the two trees has no hunk that mentions `submodule_config`, `surrogate`, `PRINTS` or `working clone`. On main, `submodule_config`'s docstring has no "PRINTS" sentence and ends in `working clone.`. The same two checks on that tree: 0 lines outside comments and docstrings, ASTs identical. On main `2d988200` with the head `426877bf` the squash is the same: 71 changed lines, 8 of them in `shape-doctor.py`, 0 outside comments and docstrings, ASTs identical, and no hunk that mentions `submodule_config`.

**`python3 scripts/render-cli-reference.py --check`**, at `426877bf`: `[ok] docs/cli.md is what the generator produces`. Only the first line of a module docstring is help text, and none of the first lines changed.

**The decoy comments, measured.** I did not take the issue's reading on trust. In a throwaway clone at the base I put each #306 regression into `run_validator` and ran the two tests (`test_an_exported_pythonpath_does_not_reach_a_template_validator`, three cases, and `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given`, six):

| mutant | with `-S` (main) | with `-S` removed |
|---|---|---|
| the standard's `scripts/` put in FRONT of the operator's value | decoy test: 3 of 3 pass. Probe test: 1 of 6 fails | decoy test: 3 of 3 fail |
| the variable kept for the callers that pass none (S1) | decoy test: 3 of 3 pass. Probe test: 2 of 6 fail | decoy test: 2 of 3 fail |
| the operator's directory AHEAD of the standard's | decoy test: 3 of 3 fail. Probe test: 1 of 6 fails | not run |

So on main the decoy test catches neither #306 regression and the probe test catches both, as lane 4's R342 measured on #636's head. The third row is why the comment says the raising `repo_shape.py` "still bites": it kills an operator-first order. The review re-ran these mutants on `a2113cff` and got the same counts.

**The `run_validator` paragraph, measured.** A root whose own `validate-manifest.py` is `import yaml` plus a print, run through `run_validator` from this tree: exit 1 and `ModuleNotFoundError: No module named 'yaml'`. The same call with `-S` taken out of the child's argv: exit 0, `manifest ok`. `yaml` is importable from the machine's site-packages and not without `site`.

With the real `gh` unreachable (a stub first on PATH, a bogus token, an empty `GH_CONFIG_DIR`).

On `a2113cff`, before the fix round:

- `tests/test_cli_reference.py`, `tests/test_repo_hygiene.py`, `tests/test_agents_md_codes.py`, `tests/test_scaffold_leg_path_collisions.py`: 250 passed (57.5 s).
- `tests/test_doctor_gitmodules_not_utf8.py`, `tests/test_doctor_validator_rows.py`, `tests/test_doctor_validator_site.py`, `tests/test_doctor_validator_verdicts.py`, `tests/test_adopt_spec_only.py`, `tests/test_adopt_entry_paths.py`, `tests/test_adopt_submodules.py` (the files that drive `run_validator`, `submodule_config` or the seeded-leg row): 334 passed (28 min 18 s).
- `tests/test_shape_doctor.py`, in two halves run side by side (the first 204 test ids, then the last 204): 204 passed in 11 min 31 s, and 201 passed, 3 skipped in 4 min 58 s. That is all 408 tests. The 3 skips are the PowerShell-line tests, which need `powershell.exe` on a Windows host.
- The review ran all of `tests/test_shape_doctor.py` there: 405 passed, 3 skipped (512 s). On the predicted merge with main `2fac9790` it ran `test_shape_doctor.py`, `test_doctor_validator_site.py`, `test_doctor_validator_rows.py`, `test_doctor_validator_verdicts.py`, `test_doctor_gitmodules_not_utf8.py`, `test_repo_hygiene.py`, `test_cli_reference.py` and `test_agents_md_codes.py`: 632 passed, 3 skipped (760 s).

On `426877bf`, after the fix round, which changes comments and docstrings only, so the runs above stand:

- The two tests the fix round touches, `test_an_exported_pythonpath_does_not_reach_a_template_validator` (3 cases) and `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given` (6 cases), run by node id: 9 passed (11.7 s). The same 9 also passed (16.1 s) at `7f2bb36b`.
- `tests/test_repo_hygiene.py`: 77 passed (1.9 s). It also passed (2.0 s) at `7f2bb36b`.

No test fails on the base, because nothing here changes behaviour. The AST comparison above is the proof in their place.

`git merge-tree --write-tree` of this head against origin/main and against every ref on origin that could meet it. The full run was on 2026-10-16, with the head `7f2bb36b` (the one before the last comment commit) and main `20c8873b`: the 41 branches (`fix/*`, `l3-*`, `wip/*`, `docs/*` and `release/v1.0`) and the 55 PR heads from #630 up. That is 96 refs. I ran #731's head `ab55c29f` through the same list. After `426877bf` I re-ran main, now `2d988200`, and the six heads that edit `tests/test_shape_doctor.py`, the one file that commit touches (#705, #737, #740, `l3-669-members-row`, `l3-680-doctor-signals` and `l3-697-crash-cure-next`), and #630: all clean.

- **origin/main: clean.** At `20c8873b`, with the head `7f2bb36b`, the predicted tree is `6ead2ffc`. At `2d988200` (three `adopt-project.py` commits later, with their tests and a change in `scripts/shape_materialize.py`), with the head `426877bf`, it is `7ee5caf4`, and the squash is the same three files (43 added, 28 removed). #731's head `ab55c29f` is clean with `2d988200` too. This head with #731's head: clean.
- **#630's head `6fd33a93`: clean.** It conflicted with main in `docs/handbook.html` when this body was first drafted, and no longer does, with or without this change.
- **#700 and #702 have landed** (`55c6ba7d` and `20c8873b`), and so has #701, and #699, #717 and #719 for `adopt-project.py`. Their heads still merge clean with this head.
- **Every ref that edits `shape-doctor.py`, `tests/test_shape_doctor.py` or `adopt-project.py` merges clean with this head**, except the two old heads named in the bullet after next. They include the heads of PRs not on main yet (#705, #721, #737 and #742, and lane 3's `l3-680`, `l3-683`, `l3-687`, `l3-691` and `l3-697`, which were not on origin when this body was first drafted). #673 has no branch or PR head on origin.
- **Four refs conflict with origin/main on their own**, and so with this head: `docs/release-notes-v1.0.0` and `release/v1.0`, and the pre-squash heads of the landed #637 and #641. Their conflicts do not come from this change.
- **Two landed pre-squash heads conflict with this head only because they are old.** #632 conflicts with main on its own. #636 merges clean with main, but its old head conflicts with this head in `run_validator`'s docstring, next to the text #636 added; main has that text, and this head merges clean with main.
- **Refs that conflict with main on their own are not made worse.** #682, #745, #747, `fix/648`, `fix/681` and `fix/704` conflict with main in `templates/family-root/scripts/validate-family.py`; #742 and `l3-683` in `shape-doctor.py`. Each merges clean with this head.
- **Every other open head and branch: clean.**

Nothing from the issue. Its four lines are done. The decoy test's `sitecustomize` machinery is dead code under `-S` and I left it, because the issue says comment only. The comment above `reached` now says it is kept as a tripwire.

- **The decoy test's `sitecustomize.py` and exit-86 trap never run.** `DECOY_SITECUSTOMIZE`, `os._exit(86)` and the `(exit 86)` and `DECOY` clauses in `reached` can only fire for a child started without `-S`. They are kept as a tripwire and the comment says so. The other choice is to remove them; `test_run_validator_starts_every_child_with_no_site` already holds `-S`. Small, tests only.

Review: rv666, lane 5's independent review, approved a2113cf with nits; 7f2bb36 and 426877b take its S1 and N1 (above), and codex found no major issue at a2113cf. CI (Brett Heap's ruling 27 and his fast-refresh ruling of 2026-10-15T22:48:35Z): H0 426877b passed tests, tests-git-bash and SonarCloud; the head 32722b3 is a clean update-branch merge of main bf1f099, with tests-git-bash and SonarCloud green; and the local gate passed on its predicted squash tree 55d5d3b over main dcf62ed: the shape_doctor, scaffold_leg_path_collisions, repo_hygiene, agents_md_codes, cli_reference sets, 686 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 #666

Lane: openreposhape-5 (openRepoShape-5)
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 17, 2026 17:07
@sourcery-ai

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

@brettheap
brettheap merged commit d66a905 into main Oct 17, 2026
5 checks passed
@brettheap
brettheap deleted the fix/692-raw-folder-paths branch October 17, 2026 17:08
brettheap pushed a commit that referenced this pull request Oct 17, 2026
main gained #748 (#662, `431c8cf8`), #731 (#692, `d66a9054`), #810
(#674, `d17564d9`), #809 (#688, `42aed788`), #816 (refs #684,
`fa04d2cd`), #813 (#689, `f6d17bc5`) and #723 (#663, `bad05d21`) since
64ec47c. No conflict, and none of them touches README.md, the handbook,
AGENTS.md, docs/ or the README cap's ledger, so every handbook marker
reads as before. README.md is 1474 lines, its cap; 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 17, 2026
#813 and #723, v1.0.0's quoted-header and plain-path-clone limits closed and the counts current to main bad05d2 (#563)

Each entry is the one RV630 corrected in its pre-review of the B2 stage,
put at its anchor, and checked against its squash on main: the
`git merge-tree --write-tree` of main before the squash with the head the
entry was written from equals the squash's tree for all seven.

- #810 (#674, `d17564d9`): a new bullet, "`apply`'s lines, its branch and
  its headers.", after "Headers, branches and messages.". It closes
  v1.0.0's quoted `files:`/`shape:` header (v1.0.0.md:439) and `apply`'s
  half of the paths listed as staged (:433); `bump-leg.py`'s half waits
  for #746.
- #731 (#692, `d66a9054`): appended to "Folders, asked before they are
  made.".
- #813 (#689, `f6d17bc5`): appended to "`--tracking-branch`.".
- #809 (#688, `42aed788`): a new bullet, "What `family.py init` records
  and commits as.", after "Names that are not UTF-8.".
- #816 (part of #684, `fa04d2cd`): a new bullet, "More holder states,
  named.", after "family.py refuses before it writes.". #816 closes no
  issue; #684 keeps its reftable case and is on v1.1.1.
- #748 (#662, `431c8cf8`), then #723 (#663, `bad05d21`): in the CI
  bullet, after #713's sentence. With #554 and #578, #748 closes v1.0.0's
  limit on fixtures that clone a just-pushed remote by plain path (:530).

The counts, in the header comment, the counts paragraph, "What changed"
and the open list: main `bad05d2`; 120 pull requests since `bfed143`;
at 18:02Z, 3 of the `v1.1.0` milestone's 133 issues open and 130 closed.
The open list is #563 (#630), #672 (#746) and #686 (#795); #684 and #695
(#738) are on v1.1.1. #746 and #795 have not landed, so neither has an
entry yet. Nothing since 2000488 touches pyproject.toml,
src/openreposhape/, install.sh or .github/workflows/.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 17, 2026
brettheap pushed a commit that referenced this pull request Oct 17, 2026
…ush-line), landed as fc86720

tests/test_dry_run_parity.py conflicted. The merge bases are 431c8cf
and a5d473e, and neither holds #731's rows, so ort's virtual base lacks
them: #731's `spelt` came back as a conflict, and its POSIX_NAMES,
ESC_NAME and FORGED_NAME block came back unflagged, beside #766's move
of all four to tests/conftest.py. The file is the same at ab55c29,
13f643d, a5139a3 and fc86720, so only #766 changed it, and it is
taken as ours, which is ef6db40's byte for byte.
brettheap pushed a commit that referenced this pull request Oct 17, 2026
scripts/bump-leg.py (two hunks) and tests/test_bump_leg_branch_and_push_line.py
(three, add/add) conflicted, each where #746's text meets #766's
addition beside it. Main holds both files exactly as #746's final head
a5139a3 does (blobs 226e7b0 and 11dc4bd at both), and this branch
already merged a5139a3, so each is taken as ours. fc86720's tree is
a5139a3's, so this merge leaves the tree as it was (f4bb551).
brettheap added a commit that referenced this pull request Oct 18, 2026
…even refusals and bump-leg's root name each path as `shown_path` names it, so an ESC or a line break in a holder, folder, lock, local remote or root no longer reaches the terminal from those plan rows or those refusals' details (#766) (#814)

#692 (#731) spelled the folder refusals with `shown_source`, and #672
(#746) spelled each dirty file's name; the paths beside them still printed
as they were. Measured with a stub `gh`: a `--work-dir` or `--into` holding
a line break and then `WARNING forged-line: pwned` printed that line as a
line of `init`'s plan, on a dry run and on a run refused after it; each
refusal below printed it as a line of its own; and an ESC reached the
terminal raw.

Each of these now names its path as `repo_shape.shown_path` does: as it
is, and as Python's `repr` spells it where a character in it is not
printable. A path `str.isprintable` accepts is named byte for byte, so no
existing test row changed.

- `scripts/family.py`: the plan's `folder` and `landing` rows;
  `family-holder-detached` and `family-on-tracking-branch` (the holder);
  `family-landing-in-standard` (the holder, the checkout, and where
  `--into ..` lands); `family-holder-already-here` (`--into` and the
  holder in it); the detail of `family-index-locked` (the lock); and
  `family-remote-exists` for a local remote.
- `scaffold-project.py`: `scaffold-remote-exists` for a local remote.
- `scripts/bump-leg.py`: `bump-leg-root-dirty`, `bump-leg-root-detached`,
  `bump-leg-on-tracking-branch` and the dry run's `the real run needs a
  branch` line (the root).

A GitHub `<org>/<name>` is named as it is. The commands a remedy prints
for a person to paste (`switch -c`, `add`, `rm -f`, and the `switch -c`
line of `add`/`bump`/`remove --dry-run`) are quoted for a shell as they
were, and still hold such a name raw inside the quotes. The docstrings of
`_checked_branch` and `_checked_index_unlocked` say so; the follow-ups are
on #459 and #458.

The new rows hold a path with an ESC and one with a line break then
`WARNING forged-line: pwned`, POSIX only, skipped where the disk refuses
the name. The rows for `family-holder-already-here` and the bump-leg
branch refusals also hold a printable name, named unquoted, byte for byte.
`tests/conftest.py` keeps the names and helpers they share, #731's among
them. `family-remote-exists` and `scaffold-remote-exists` cannot be reached
from the command line with such a name (`*-unwritable-value` refuses it
first, already spelled), so their rows ask the question in process. 34 new
tests: 30 fail on the code before the change (the printable rows pass by
design), and each of 21 one-line mutants fails at least one.

Landed after #731 and #746, on which it was stacked.

Closes #766
Lane: openreposhape-2 (openRepoShape-2)
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