Skip to content

adopt-project.py: check and execute judge an entry's path by one test, entry_path_problem, that admits every path git can take literally, so a source holding My Notes.md no longer checks plan ok and is then refused by execute as unsafe-value, and a path git cannot take is a check finding refused before any leg exists (#196) - #231

Merged
brettheap merged 8 commits into
mainfrom
fix/adopt-check-unsafe-path
Oct 12, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

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

Fix round 3 (b52e8d1), on main 5751460, merged in as b34b738. #223 has landed, so this branch takes it with main. The seven assertions its spelling of finding paths changed now use that spelling. No code changed in round 3. Round 2 (355ab2d) was verified ready. That round refuses a byte that is not UTF-8 as policy, now that #247 put #199's byte-exact path listing on main, and refuses any other surrogate because the path list cannot be written with it. It also tells a name of carriage returns alone that its line would be literal: alone, which matches every path. The refused set is unchanged since round 1. At merge, use the squash message in the last section; the first pushed subject still says "admits every path git can take literally", which round 1 made untrue.

What

  • One test, entry_path_problem(path), asked by check and by execute. It is _misread(path) or _refused_by_policy(path), so a name with a reason of each kind is told the misreading.
    • check reports what it finds as a new finding, plan-unsafe-path: FINDING plan-unsafe-path: `<path>` has leg: <leg>, and `execute` will not extract it into a leg: <reason>.
    • The finding is listed FIRST in _entry_findings, so a path that is a git option is refused as that, not only as the uncovered file it also leaves. Its remediation is printed once.
    • execute refuses the same finding in check's words, through _refuse_what_check_finds, before any leg repository exists.
    • An entry answered root or drop is not judged, because it never becomes a line of the filter's path list.
  • _misread: what git, git filter-repo or this tool's own write of the path list would not carry as it stands. Each was measured with git-filter-repo 2.47.0, whose get_paths_from_file strips \r and \n from each line's end and looks for ==> first:
    • an empty path: a literal: line alone kept every path in the leg;
    • a line feed anywhere: the rest of the name is read as a line of its own;
    • a carriage return at the END: it is stripped, so Icon\r (the macOS folder-icon file) extracted Icon, another file. A name of carriage returns alone leaves literal: alone, which kept every path;
    • a NUL, which no command line carries;
    • a surrogate outside U+DC80 to U+DCFF, such as a hand-edited \ud800. No source path holds one, because Source.tree reads a byte that is not UTF-8 as one of U+DC80 to U+DCFF. write_paths_listing cannot write one: it raised UnicodeEncodeError;
    • ==>, which filter-repo reads as a rename anywhere on a line.
  • _refused_by_policy: names git and git filter-repo take literally, refused anyway. Each finding says "which execute refuses as policy, though git and git filter-repo would take it literally":
  • Everything else a source tree holds is now extracted: spaces, letters outside ASCII, #, $, (, ;, &, quotes, [, *, ?, a leading :, a leading or trailing space, a backslash, and the spellings glob:, regex: and literal:.
  • _leg_paths now only collects the paths and no longer runs checked_value on them. The values that are options stay under checked_value as unsafe-value: a leg's mount path, the branch, the repository names and the organisation.
  • _extract_leg writes each --paths-from-file line as literal:<path>, through adopt-project.py: the plan writer escapes every character that is not printable from the one table repo_shape's reader resolves, so a source holding the macOS Icon\r no longer writes a plan check refuses, and a non-UTF-8 name no longer empties the plan (#199) #247's byte-exact write_paths_listing. Written bare, #notes.md is a comment and glob:/regex:/literal: are directives.
  • _mount_the_legs runs git --literal-pathspecs rm -r -q -- <path>. Even after --, git reads a path as a pathspec: [ID].tsx also matches D.tsx, and :!keep.md matches every path except keep.md.
  • shape_materialize.env_commit passes every commit message to git commit -F - as UTF-8 (encoding="utf-8", errors="replace").
    • The adoption's split message names every entry path.
    • The change also reaches scaffold-project.py's assembly commit, scripts/family.py's holder commit, and the first commit git_init_commit makes in a scaffolded or seeded leg.
    • errors="replace" also applies when encoding. A root or drop entry holding a byte that is not UTF-8 is listed in the split message with a ? in place of that byte (measured: caf?.md). On main, execute refuses that root entry as unsafe-value instead, because main's _leg_paths judges every entry.
  • The module docstring's check paragraph names the entry-path test (lane 2's N4).

Why

Measured on this machine with git 2.43.0, git-filter-repo 2.47.0 and Python 3.12.3, on main 4835fb4 and the head 355ab2d, at 21:43Z. Each source was built in git's index, then planned. Every unanswered entry was answered root, then check and execute were run with --local-remote-dir.

source holds main check main execute head check head execute
My Notes.md, docs/User Guide.md, Übersicht.md (the issue) 0 plan ok 2 REFUSED unsafe-value: a plan path is 'My Notes.md', no leg 0 plan ok 0 adoption verified; all three in the spec leg
#notes.md 0 plan ok 2 REFUSED unsafe-value, no leg 0 plan ok 0 adoption verified; in the spec leg
[ID].tsx (code) beside D.tsx (root) 0 plan ok 2 REFUSED unsafe-value, no leg 0 plan ok 0 adoption verified; [ID].tsx in the code leg, D.tsx still in the root
-notes.md 0 plan ok 2 REFUSED unsafe-value, no leg 1 FINDING plan-unsafe-path: ... it begins with -, which execute refuses as policy, though git and git filter-repo would take it literally ... 2 REFUSED plan-unsafe-path, no leg
notes\t.md 0 plan ok 2 REFUSED unsafe-value, no leg 1 FINDING plan-unsafe-path: ... it holds the control character \t, which execute refuses as policy ... 2 REFUSED plan-unsafe-path, no leg
a==>b.md 0 plan ok 2 REFUSED unsafe-value, no leg 1 FINDING plan-unsafe-path: ... it holds ==>, which git filter-repo --paths-from-file reads as a rename ... 2 REFUSED plan-unsafe-path, no leg
caf\xe9.md (byte 0xE9) 0 plan ok 2 REFUSED unsafe-value, no leg 1 FINDING plan-unsafe-path: ... it holds \xe9, which is not UTF-8 and which execute refuses as policy, though git and git filter-repo would take it literally ... 2 REFUSED plan-unsafe-path, no leg

The last row, before and after #247. Earlier, on this branch merged with #199's first head (d566e12) and with only the Cs branch disabled, check said plan ok. Then execute exited 1 with a UnicodeEncodeError in _extract_leg, after both leg remotes were created. With #247's write_paths_listing on main, the same source on 53e82ed with the refusal disabled gives check 0 plan ok and execute 0 adoption verified. The spec leg holds the byte 0xE9, and the split commit's message lists the file as caf?.md. So round 2 refuses it as policy, not as a misreading (Decision).

Measurements behind the two kinds:

  • Misread. git filter-repo --paths-from-file was run with one literal:<name> line per name:
    • literal: alone kept every path;
    • literal:\r and literal:\r\r kept every path (round 2);
    • docs\nregex:.* kept every path, with or without literal:;
    • literal:Icon\r kept Icon, a different file;
    • literal:a==>b.md alone kept every path, and beside literal:keep.md it kept only keep.md.
    • Separately, git --literal-pathspecs rm -- "" exits 128, and subprocess raises ValueError on a NUL in argv.
    • On 53e82ed, write_paths_listing raises UnicodeEncodeError on \ud800, \udc7f and \udd00, and writes \udce9 as the byte 0xE9. With the Cs refusal disabled there, a hand-edited entry "\ud800.md" is refused anyway, as plan-empty-entry, before any leg.
  • Policy. Each sample was filtered with one literal:<name> line, and removed in a fresh clone with git --literal-pathspecs rm -r -q -- <name>. The samples were a tab, \x01, ESC, DEL, \x85, \x0b, \x0c, \x1c, U+2028, a\rb.md, -n.md, one space and three spaces. Every one exited 0, and filter-repo kept, and git rm removed, exactly that file and nothing else. This is the independent review's measurement, repeated here. A byte that is not UTF-8 was measured end to end on Linux, as above.
  • The refused set is unchanged. Round 1: the old and new entry_path_problem agree on is None for all of 200,001 random names, drawn from an alphabet of every boundary character. Round 2: 53c529c and 355ab2d refuse the same names, byte for byte, of 5,679,428. Those are every code point alone, after a, before a and between a and b, plus every sequence of 1 to 4 of 33 boundary tokens. Each head refuses 1,181,465 of them.
  • env_commit. A message holding 文書.md and Übersicht.md was committed under LC_ALL=C PYTHONUTF8=0 PYTHONCOERCECLOCALE=0. On main it exits 1 with UnicodeEncodeError: 'ascii' codec can't encode characters. On the head it exits 0, and the commit body is UTF-8 bytes.
  • Unchanged (round 1, on a5b09c3 and 53c529c). Three sources hold no refused name: the issue's, #notes.md and [ID].tsx. For each:
    • the plan file and plan's stderr are byte-identical (cmp);
    • plan's stdout differs only in the --out path it prints;
    • on the same answered plan, check's stdout and stderr are byte-identical.

Decision

The lane's decision, which the issue asked for with evidence: accept what a source tree holds. Refuse what git or git filter-repo would misread, or what this tool cannot write into the path list. Refuse as policy a control character, a byte that is not UTF-8, a leading - and a name that is nothing but whitespace.

Tests

The targeted set was run with git-filter-repo 2.47.0 on PATH, on the fix-round-3 head b52e8d1, which holds main 5751460. Earlier rounds ran it on 355ab2d and 53c529c, and on f9656bd and 5a8df76, which share one tree.

The files are test_adopt_entry_paths, test_adopt_e2e, test_adopt_execute_refusals, test_adopt_ignored_shape_files, test_adopt_leg_path_collisions, test_adopt_leg_paths, test_adopt_plan, test_adopt_spec_only, test_adopt_submodules, test_adopt_mount_ignored, test_repo_hygiene, test_windows_paths, test_shape_doctor and test_cli_reference. Since round 2 they also include #247's test_adopt_plan_escapes and #173's test_adopt_verify, and on b52e8d1 #223's test_adopt_finding_spelling, #234's test_scaffold_leg_path_collisions, #230's test_git_output_decoding and #246's test_stdout_encoding, all of which came with merges of main. test_adopt_mount_ignored is #200's: it tests _mount_the_legs, where this branch's git --literal-pathspecs rm sits beside #200's mount changes. The skips are test_adopt_plan.py::test_plan_on_medxehr_matches_the_rulings, which needs SHAPE_MEDXEHR_CLONE, and on b52e8d1 one case of test_stdout_encoding.py that is about Windows' own default.

b52e8d1: 1472 passed, 2 skipped in 734.97s (0:12:14)   (20 files)
355ab2d: 1180 passed, 1 skipped in 622.75s (0:10:22)   (16 files)
53c529c: 1100 passed, 1 skipped in 940.08s (0:15:40)   (14 files)
f9656bd: 1095 passed, 1 skipped in 433.90s (0:07:13)
5a8df76: 1095 passed, 1 skipped in 928.71s (0:15:28)
mutation head tests that fail
no policy refusal of a byte that is not UTF-8 355ab2d 4
no misreading of any other surrogate 355ab2d 3
a name of carriage returns alone told the old "blank" words 355ab2d 2
a policy refusal given the old line-break reason 53c529c 7
no trailing-carriage-return rule 53c529c 2
no policy half at all 53c529c 20
policy asked before the misreading 53c529c 7

The review measured mutations of the mechanism on f9656bd. Dropping literal: fails 1 test, and dropping --literal-pathspecs fails 1, both in the end-to-end test.

  • What the Windows job skips:
    • test_adopt_entry_paths.py::test_a_source_with_such_names_is_adopted_and_verified needs git filter-repo, which that job does not install. So does all of test_adopt_e2e.py, including the changed test_a_plan_path_that_is_a_git_option_is_refused. The same refusal is asserted without filter-repo in test_adopt_execute_refusals.py, which runs everywhere.
    • test_a_control_character_in_the_source_is_found_and_refused is POSIX-only, because NTFS cannot hold a tab in a file name. The hand-edit cases of the same refusal run everywhere.
    • The end-to-end test adds the names only a POSIX disk holds (?, *, ", :, a leading or trailing space, a backslash) only where os.name != "nt". The macOS job runs them.
    • The non-UTF-8 test builds its name in git's index, so it runs on Windows. If git there will not record the name, the test skips and says so.

Composes with

Refreshed at 23:32Z on 2026-10-11, at the shas git ls-remote --heads origin 'fix/*' printed then.

Main. This branch holds main 5751460, merged as b34b738 with no conflict. Before that, main 4835fb4 was merged as 53e82ed. Since its base a5b09c3, it has taken #173, #224, #244, #247, #234, #233, #235, #246, #230, #218, #257, #226, #219, #220, #265 and #223.

git merge-tree --write-tree <sibling> b52e8d1, with every sibling at its pushed sha, and the same against main 5751460. "Clean with main" means every hunk listed is one this branch adds.

sibling result
fix/adopt-relative-submodule-url (#190, PR #245) @ 1abe078 CONFLICT adopt-project.py, 3 hunks; clean with main. Resolution below.
fix/adopt-template-values-utf8 (#255, PR #270) @ aed9190 CONFLICT adopt-project.py, 2 hunks (_entry_findings' docstring and return); clean with main. Resolution below.
fix/adopt-plan-quote-yaml-keywords (#181) @ a0ff15b CONFLICT adopt-project.py, 2 hunks (_entry_findings' docstring and return); clean with main
fix/adopt-root-kept-symlinks @ 04792d2 CONFLICT: main has 1 hunk in adopt-project.py (the import list) and 1 in scripts/shape_materialize.py; this branch adds 2 (_entry_findings' docstring and return)
fix/yaml-quote-boundaries @ 21fc213 CONFLICT scripts/repo_shape.py, which main has too; this branch adds none
fix/adopt-gitmodules-rejected-values (#215, PR #240) @ 0733360 CLEAN
fix/adopt-clone-without-template (PR #268) @ 65073c1 CLEAN
fix/adopt-record-submodule-placement @ 2f5cda9 CLEAN
fix/adopt-late-failure-notice @ cdfb308 CLEAN
fix/adopt-report-filter-repo-failure (#171, PR #184) @ f416e13 CLEAN
fix/leg-path-yaml-scalars (PR #266) @ aa2be9a CLEAN
fix/plan-literal-follow-up-grep (#216, PR #228) @ f0d4334 CLEAN
fix/validator-child-encoding @ 5757e29 CLEAN

The other pushed fix/* branches touch none of this branch's files.

Left out

Squash message for the merge

The pushed commits' subjects cannot be amended, so this is the final message: subject, body and both trailers, true at b52e8d1.

adopt-project.py: `check` and `execute` judge an entry's path by one test, `entry_path_problem`, so a source holding `My Notes.md` no longer checks `plan ok` and is then refused by `execute` as `unsafe-value`, and a path `git filter-repo` would misread, or one refused as policy, is a `check` finding that says which, refused before any leg exists (#196)

On main at 4835fb4 `check` asked nothing of an entry's path, and
`execute` asked `checked_value`, whose alphabet (`SAFE_ARG_RE`) is meant
for the values that are options. A source holding `My Notes.md`,
`#notes.md` or `[ID].tsx` checked `plan ok`, and `execute` refused the
same plan as `unsafe-value`. So did #166's split instruction for a
`docs/User Guide.md` beside a `docs/theme` submodule.

DECISION: accept what a source tree holds. Refuse what git or git
filter-repo would misread, or what this tool cannot write into the path
list. Refuse as policy a control character, a byte that is not UTF-8, a
leading `-` and a name that is nothing but whitespace. An entry's path
is a fact of the source, not an option: `plan` writes it from the tree,
and every command is argv with `shell=False`. The issue's other choice
was to state the restriction in the question. That leaves a top-level
`My Notes.md` with a rename in the source as its only remedy. The policy
refusals are names git and filter-repo take literally, measured, and
each finding says it is policy. A byte that is not UTF-8 is written
byte-exact since #247, and is held until adopting such a name end to
end is measured on macOS and Windows.

WHAT CHANGED
- `entry_path_problem(path)`, ONE test, is `_misread(path) or
  _refused_by_policy(path)`, so a name with a reason of each kind is
  told the misreading.
  - `check` reports what it finds as the new finding `plan-unsafe-path`:
    "`<path>` has leg: <leg>, and `execute` will not extract it into a
    leg: <reason>".
  - It is listed FIRST in `_entry_findings`, so a path that is a `git`
    option is refused as that and not only as the uncovered file it also
    leaves. Its remediation is said once.
  - `execute` refuses the same finding in `check`'s words, through
    `_refuse_what_check_finds`, before any leg exists.
  - An entry answered `root` or `drop` is not judged: it is never a line
    of the filter's list.
- `_misread` is what git, filter-repo, or this tool's write of the path
  list would not carry as it stands. Each case is measured with
  git-filter-repo 2.47.0, whose `get_paths_from_file` strips `\r` and
  `\n` from each line's end and looks for `==>` first:
  - an empty path: a `literal:` line alone kept every path in the leg;
  - a line feed anywhere: the rest of the name is read as a line of its
    own, so `docs\nregex:.*` kept every path;
  - a carriage return at the END: it is stripped, so `Icon\r`, the macOS
    folder-icon file, extracted `Icon`, another file. A name of carriage
    returns alone leaves `literal:` alone, which kept every path;
  - a NUL, which no command line carries;
  - a surrogate outside U+DC80 to U+DCFF, such as a hand-edited
    `\ud800`: no source path holds one, and `write_paths_listing`
    raised `UnicodeEncodeError` on it;
  - `==>`, which filter-repo reads as a rename anywhere on a line.
- `_refused_by_policy` covers names git and filter-repo take literally.
  Samples of each kind were measured: a tab, `\x01`, ESC, DEL, `\x85`,
  `\x0b`, `\x0c`, `\x1c`, U+2028, a carriage return inside a name,
  `-n.md`, and one and three spaces. Each, as one `literal:` line and as
  a literal `git rm`, extracted and removed exactly that path:
  - a name that is nothing but whitespace;
  - a control character (C0, DEL, C1), or U+2028 or U+2029, a carriage
    return that does not end the name included;
  - a byte that is not UTF-8, which `Source.tree` keeps as one of
    U+DC80 to U+DCFF (`NOT_UTF8_BYTES`). With the refusal disabled, a
    source holding `caf\xe9.md` was extracted and verified on Linux,
    the byte in the leg, and the split message listed `caf?.md`;
  - a leading `-`, the threat model's first rule.
- `_leg_paths` collects the paths without `checked_value`. A leg's
  mount path, the branch, the repository names and the organisation are
  options, and stay under `checked_value` as `unsafe-value`.
- `_extract_leg` writes each `--paths-from-file` line as
  `literal:<path>`, through #247's byte-exact `write_paths_listing`.
  Written bare, `#notes.md` is a comment, and `glob:y.md`, `regex:x.md`
  and `literal:z.md` are directives.
- `_mount_the_legs` runs `git --literal-pathspecs rm -r -q -- <path>`.
  After `--` a path is still a pathspec: `[ID].tsx` also removed
  `D.tsx`, and `:!keep.md` removed every path but `keep.md`.
- `shape_materialize.env_commit` hands `git commit -F -` the message as
  UTF-8 (`encoding="utf-8", errors="replace"`).
  - The split message names every entry path. Under a cp1252 or POSIX
    `C` locale without UTF-8 mode, `文書.md` raised `UnicodeEncodeError`
    after both legs were pushed.
  - The change reaches every message `env_commit` writes:
    scaffold-project.py's assembly commit, scripts/family.py's holder
    commit, and `git_init_commit`'s first commits.
  - `errors="replace"` also applies when encoding. A `root` or `drop`
    entry holding a byte that is not UTF-8 is listed in the split
    message with a `?` in its place (`caf?.md`), where a strict encode
    would have raised.
- The module docstring's `check` paragraph names the entry-path test.

MEASURED on this machine (git 2.43.0, git-filter-repo 2.47.0). Each
source was planned, its unanswered entries answered `root`, then
`check` and `execute` were run with local remotes. Main 4835fb4
(a5b09c3 printed the same, but `plan` raised there on `caf\xe9.md`,
which is #199) -> this change:
- `My Notes.md`, `docs/User Guide.md`, `Übersicht.md`: check 0 `plan ok`,
  execute 2 `unsafe-value` -> check 0, execute 0 `adoption verified`,
  all three in the spec leg.
- `#notes.md`: check 0, execute 2 `unsafe-value` -> execute 0, in the
  spec leg.
- `[ID].tsx` (code) beside `D.tsx` (root): check 0, execute 2
  `unsafe-value` -> execute 0. `[ID].tsx` is in the code leg, and
  `D.tsx` is still in the root.
- `-notes.md`, `notes\t.md`, `caf\xe9.md` (byte 0xE9): check 0
  `plan ok`, execute 2 `unsafe-value` -> check 1 `FINDING
  plan-unsafe-path`, which says the refusal is policy; execute 2
  `REFUSED plan-unsafe-path`; no leg remote created.
- `a==>b.md`: the same exits, with the finding naming the rename
  misreading.
- Before #247, merged with #199's first head (d566e12) and without the
  rule, `caf\xe9.md` checked `plan ok`, and `execute` raised
  `UnicodeEncodeError` in `_extract_leg` after both leg remotes existed.
- The set of refused names is the same as at fix round 1, compared on
  5,679,428 names.

UNCHANGED:
- What `plan` writes and prints.
- What `check` prints for a plan none of whose entry paths it refuses
  (byte-identical to main for three sources).
- Every option value's `unsafe-value` refusal.

TESTS:
- tests/test_adopt_entry_paths.py:
  - the admitted names, and the refused ones as MISREAD and POLICY, each
    asserting its own words. A policy finding names no misreading;
  - `check` and `execute` on each hand-edit class, on a control
    character in the source, and on a byte that is not UTF-8 put
    straight into git's index;
  - `root`/`drop` not judged;
  - the split message under a foreign locale;
  - an end-to-end adoption of every admitted kind of name. It skips
    without git filter-repo, as on the Windows job.
- The assertions in test_adopt_execute_refusals.py, test_adopt_e2e.py and
  test_adopt_submodules.py that expected `unsafe-value` for an entry
  path now expect `plan-unsafe-path`, with every path a finding names
  spelled as #212's change (PR #223) spells it.

LEFT OUT:
- Adopting a name that is not UTF-8 end to end: lifting the policy
  refusal, and measuring macOS and Windows (#247's listing is on main).
- `plan` asking `entry_path_problem` of each entry it writes, and
  offering `root` or `drop` where it refuses.
- `cross_leg_references` handing entry paths to `git grep` (#216).

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

Closes #196

🤖 Generated with Claude Code

Summary by Sourcery

Make entry paths literal-safe throughout adoption while consistently identifying and refusing paths that Git would misread or policy disallows.

New Features:

  • Add unified entry-path validation so plans accept literal paths Git can safely process while reporting unsafe paths during check and execute.
  • Preserve literal entry paths during extraction, removal, and commit-message creation, including paths with spaces, special characters, and non-ASCII text.

Bug Fixes:

  • Prevent valid source paths from being incorrectly rejected as unsafe option values or silently misinterpreted by Git and git-filter-repo.
  • Refuse path names that Git tooling would misread or that the adoption policy disallows before creating any leg repositories.
  • Ensure commit messages are encoded as UTF-8 across locale settings, replacing unencodable characters instead of failing late.

Enhancements:

  • Prioritize path-safety findings and provide consistent remediation guidance shared by check and execute.

Documentation:

  • Update adopt-project documentation to describe entry-path validation during check.

Tests:

  • Add comprehensive coverage for admitted, misread, policy-refused, edited-plan, non-UTF-8, locale, and end-to-end entry paths.
  • Update existing refusal and submodule tests for the new plan-unsafe-path finding behavior.

…test, `entry_path_problem`, that admits every path git can take literally, so a source holding `My Notes.md` no longer checks `plan ok` and is then refused by `execute` as `unsafe-value`, and a path git cannot take is a `check` finding refused before any leg exists (#196)

On main at a5b09c3 `check` asked nothing of an entry's path and `execute`
asked `checked_value`, whose alphabet (`SAFE_ARG_RE`) is meant for the
values that are options. A source holding `My Notes.md`, `#notes.md` or
`[ID].tsx` checked `plan ok` and `execute` refused the same plan as
`unsafe-value`; so did #166's split instruction for a `docs/User Guide.md`
beside a `docs/theme` submodule.

DECISION: accept what a source tree holds and refuse only what git or git
filter-repo cannot take literally. An entry's path is a fact of the
source, not an option: `plan` writes it from the tree, every command is
argv with `shell=False`, and the issue's other choice -- state the
restriction in the question -- leaves a top-level `My Notes.md` with only
a rename in the source as its remedy.

WHAT CHANGED
- `entry_path_problem(path)`, ONE test. `check` reports what it finds as
  the new finding `plan-unsafe-path`, listed FIRST in `_entry_findings`
  (so a path that is a `git` option is refused as that, never as the
  uncovered file it also leaves), with its remediation said once;
  `execute` refuses the same finding in `check`'s words through
  `_refuse_what_check_finds`, before any leg exists. An entry answered
  `root` or `drop` is not judged: it reaches no line of the filter's
  list. It refuses:
  - an empty path, or one that is only whitespace;
  - a control character (C0, DEL, C1) or U+2028/U+2029: a line break
    ends the path's line in the filter's list, and `docs\nregex:.*`
    would keep every path in the leg;
  - a byte that is not UTF-8, which `Source.tree` keeps as a lone
    surrogate (category Cs): `_extract_leg` writes the list as UTF-8
    text, which cannot carry one;
  - `==>`, which `git filter-repo` reads as a rename anywhere on a line;
  - a leading `-` (the threat model's first rule, held although both
    call sites already fence paths off from options).
- `_leg_paths` collects the paths without `checked_value`. A leg's mount
  path, the branch, the repository names and the organisation are
  options and stay under `checked_value` as `unsafe-value`.
- `_extract_leg` writes each `--paths-from-file` line as
  `literal:<path>`: bare, `#notes.md` is a comment and `glob:y.md`,
  `regex:x.md`, `literal:z.md` are directives.
- `_mount_the_legs` runs `git --literal-pathspecs rm -r -q -- <path>`:
  after `--` a path is still a pathspec, so `[ID].tsx` also removed
  `D.tsx`, and `:!keep.md` removed every path but `keep.md`.
- `shape_materialize.env_commit` hands `git commit -F -` the message as
  UTF-8 (`encoding="utf-8", errors="replace"`): the split message names
  every entry path, and under a cp1252 or POSIX `C` locale without UTF-8
  mode `文書.md` raised `UnicodeEncodeError` after both legs were pushed.

MEASURED on this machine (git 2.43.0, git-filter-repo 2.47.0), each
source planned, unanswered entries answered `root`, then check and
execute with local remotes; main a5b09c3 (8df21e6 and 20e13f0 printed
the same) -> this commit:
- `My Notes.md`, `docs/User Guide.md`, `Übersicht.md`: check 0 `plan ok`,
  execute 2 `unsafe-value` -> check 0, execute 0 `adoption verified`,
  all three in the spec leg.
- `#notes.md`: check 0, execute 2 `unsafe-value` -> execute 0, in the
  spec leg.
- `[ID].tsx` (code) beside `D.tsx` (root): check 0, execute 2
  `unsafe-value` -> execute 0, `[ID].tsx` in the code leg and `D.tsx`
  still in the root.
- `-notes.md`, `notes\t.md`, `a==>b.md`: check 0 `plan ok`, execute 2
  `unsafe-value` -> check 1 `FINDING plan-unsafe-path`, execute 2
  `REFUSED plan-unsafe-path`, no leg remote created.
- `caf\xe9.md` (byte 0xE9): `plan` raises `UnicodeEncodeError` on both,
  which is #199's. Merged with #199 (origin d566e12), without the Cs rule:
  check 0 `plan ok`, execute 1 with a `UnicodeEncodeError` traceback in
  `_extract_leg` after both leg remotes existed. With it: check 1
  `plan-unsafe-path`, execute 2 before any remote; answered `drop` or
  `root`, `adoption verified`.

UNCHANGED: what `check` prints for a plan none of whose entry paths it
refuses; every option value's `unsafe-value` refusal.

TESTS: tests/test_adopt_entry_paths.py (the one test's admitted and
refused classes; `check` and `execute` on each hand-edit class, on a
control character in the source, and on a byte that is not UTF-8 put
straight into git's index; `root`/`drop` not judged; the split message
under a foreign locale; an end-to-end adoption of every admitted kind of
name, which skips without git filter-repo, as on the Windows job). The
assertions in test_adopt_execute_refusals.py, test_adopt_e2e.py and
test_adopt_submodules.py that expected `unsafe-value` for an entry path
now expect `plan-unsafe-path`.

LEFT OUT: adopting a name that is not UTF-8 end to end, after this and
#199 land; `plan`'s stdout raising on a name outside cp1252 on Windows
without UTF-8 mode (#214); `run()` decoding git's output with the locale
codec (#217).

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

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

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-3, 2026-10-11T19:06Z) @codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@sourcery-ai

sourcery-ai Bot commented Oct 11, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR separates source entry paths from option-like configuration values, validates paths once through shared check/execute findings, and escapes Git’s filter-list and pathspec interpretations so legitimate names are adopted literally while unsupported paths are refused before any legs are created.

Sequence diagram for literal entry-path validation during adoption

sequenceDiagram
    participant User
    participant Check
    participant Execute
    participant Findings
    participant GitFilter as GitFilterRepo
    participant Git as Git

    User->>Check: check(plan)
    Check->>Findings: _entry_findings(plan, source)
    Findings->>Findings: entry_path_problem(path)
    alt path cannot be carried literally
        Findings-->>Check: FINDING plan-unsafe-path
        Check-->>User: finding and remediation
    else path is admissible
        Findings-->>Check: no path finding
        Check-->>User: plan ok
    end

    User->>Execute: execute(plan)
    Execute->>Findings: _refuse_what_check_finds(plan, source)
    alt plan-unsafe-path exists
        Findings-->>Execute: refuse before leg creation
        Execute-->>User: REFUSED plan-unsafe-path
    else all findings pass
        Execute->>GitFilter: _extract_leg with literal:path lines
        GitFilter-->>Execute: extracted legs
        Execute->>Git: mount with --literal-pathspecs
        Git-->>Execute: mounted assembly
        Execute-->>User: adoption verified
    end
Loading

Flow diagram for literal entry-path handling

flowchart TD
    A[Source tree entry path] --> B{Entry answered root or drop?}
    B -->|Yes| C[Do not judge path]
    B -->|No| D["entry_path_problem(path)"]
    D --> E{Git or filter list cannot carry it literally?}
    E -->|Yes| F[check reports plan-unsafe-path]
    F --> G[execute refuses before legs exist]
    E -->|No| H[_leg_paths collects raw path]
    H --> I[_extract_leg writes literal:path]
    I --> J[git filter-repo reads path literally]
    J --> K[_mount_the_legs uses --literal-pathspecs]
    K --> L[Adoption verified]
Loading

File-Level Changes

Change Details Files
Centralize entry-path validation and make check/execute report and enforce the same pre-extraction finding.
  • Add entry_path_problem for empty, whitespace-only, control/line-separator, non-UTF-8 surrogate, ==>, and leading-dash paths.
  • Add first-class plan-unsafe-path findings with shared remediation, excluding entries answered root or drop.
  • Remove path validation from _leg_paths so execute refuses before creating leg repositories and preserves finding precedence.
adopt-project.py
Make Git path handling literal at both extraction and mount-removal boundaries.
  • Prefix filter-repo path-list lines with literal:.
  • Use git --literal-pathspecs rm when removing mounted paths.
adopt-project.py
Ensure split commit messages are sent to Git as UTF-8 across locale configurations.
  • Configure env_commit subprocess input/output with UTF-8 encoding and replacement errors.
scripts/shape_materialize.py
Expand regression coverage for accepted and refused source-tree paths and end-to-end adoption behavior.
  • Add comprehensive unit, refusal-order, non-UTF-8, foreign-locale commit, and filter-repo end-to-end tests.
  • Update existing execute, submodule, and adoption expectations from unsafe-value to plan-unsafe-path where appropriate.
  • Verify refused paths create no leg repositories and accepted metacharacter/directive-like names are preserved.
tests/test_adopt_entry_paths.py
tests/test_adopt_e2e.py
tests/test_adopt_execute_refusals.py
tests/test_adopt_submodules.py

Assessment against linked issues

Issue Objective Addressed Explanation
#196 Make check and execute apply the same shared safety test to plan entry paths, with check reporting the path-specific finding and execute refusing it before creating any leg repositories. ✅
#196 Prevent valid source paths containing spaces or non-ASCII characters from being rejected merely because they are not in the option-safe alphabet, while refusing paths that Git or the path-list format cannot safely carry. ✅
#196 Ensure accepted paths are passed literally through extraction and assembly operations, including paths that could otherwise be interpreted as filter-repo directives, pathspec patterns, or Git options. ✅
#199 Ensure that adopt-project.py plan produces a usable result for source paths containing line breaks, either by escaping them so the generated plan round-trips through the loader or by refusing the source during planning with a clear path-specific remediation. ❌ The PR detects and refuses line-break paths only during check/execute; it does not modify y() or the plan-writing path, nor does it make plan refuse such sources. Consequently, a source containing Icon\r or \n still causes plan to write an unparsable YAML file before the new finding can run, leaving check unable to load the plan and still producing yaml-unparsable without the entry-specific finding.

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-2, 2026-10-11T19:09:10Z) A note ahead of this lane's read-only review of f9656bd: GitHub links #199 to this PR as an issue it closes. The PR's closing references are #196 and #199, because the body's sentence "The #199 merges resolve #199's one import conflict to #199's side" reads as the keyword resolve #199. Merging as it stands would close lane openRepoShape-5's #199, whose fix is still on fix/adopt-plan-linebreak-names. Rewording that sentence (for example "settle #199's one import conflict") drops the link and leaves Closes #196. No hold is requested.

…ason, so a tab is no longer told that a line break would end its line, and a name git and `git filter-repo` take literally is said to be refused as policy

The independent review of #231 (rv-adopt-b) measured seven kinds of name
that git and git-filter-repo 2.47.0 both take literally, and that
`entry_path_problem` refused with a misreading's reason: a tab, `\x01`,
DEL, `\x85`, U+2028, a leading `-` and a single space. The refused set
stays as it was. Each reason is now true.

- `entry_path_problem` is `_misread(path) or _refused_by_policy(path)`,
  so a name with a reason of each kind is told the misreading.
- `_misread` is what git, filter-repo or this tool's write of the path
  list would not carry as it stands. Each is measured:
  - an empty path: a `literal:` line alone kept every path in the leg;
  - a line feed anywhere: the rest is read as a line of its own;
  - a carriage return at the END: filter-repo strips `\r` and `\n` from
    each line's end, so `Icon\r` extracted `Icon`, another file. A
    carriage return elsewhere is carried, and is now a policy refusal;
  - a NUL: no command line carries one, because `subprocess` raises;
  - a byte that is not UTF-8, and `==>`, as before.
- `_refused_by_policy` covers names both take literally. Each finding
  says "which `execute` refuses as policy, though git and `git
  filter-repo` would take it literally":
  - a name that is nothing but whitespace;
  - a control character (C0, DEL, C1), or U+2028 or U+2029;
  - a leading `-`, the threat model's first rule.
- The finding's frame is "`<path>` has leg: <leg>, and `execute` will
  not extract it into a leg: <reason>". It used to say "cannot be
  extracted".
- `env_commit`'s docstring names every caller whose message now goes to
  git as UTF-8: scaffold-project.py's assembly commit,
  scripts/family.py's holder commit, and `git_init_commit`'s first
  commits. It also says that `errors="replace"` applies when ENCODING:
  with #199 merged, a `root` or `drop` entry holding a byte that is not
  UTF-8 is listed in the split message with a `?` in place of that byte
  (measured: `caf?.md`).

UNCHANGED:
- Which paths are refused: the old and new `entry_path_problem` agree on
  `is None` for all of 200,001 random names, drawn from an alphabet of
  every boundary character.
- Every exit code.
- `plan`.
- `check` and `execute` for a plan with no refused entry.

TESTS: tests/test_adopt_entry_paths.py splits the refused cases into
MISREAD and POLICY, and each list's test asserts that kind's words. The
policy test also asserts that no misreading's words appear ("line of its
own", "strips", ...). Mutations:
- the old line-break reason given for a policy refusal: 7 tests fail;
- no trailing-CR rule: 2 fail;
- no policy half: 20 fail;
- policy asked before the misreading: 7 fail;
- no Cs rule: 4 fail.

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

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-3, 2026-10-11T19:57Z) fix round 1 (independent review, codex is out of quota): kept the refused set (the old and new rule agree on 200,001 random names), and every plan-unsafe-path finding now gives its true reason. _misread covers what git-filter-repo or this tool's UTF-8 listing would misread: an empty path, a line feed, a carriage return at the end (Icon\r extracted Icon), a NUL, a non-UTF-8 byte, and ==>. _refused_by_policy covers names git and filter-repo take literally, and each such finding says "refuses as policy": whitespace-only names, control characters and U+2028/U+2029, and a leading -. A tab is no longer told that a line break would end its line. Also: the env_commit docstring now names scaffold-project.py, scripts/family.py and git_init_commit, and says errors="replace" encodes too. The body now has the root-kept-symlinks 04792d2 row and the squash message to use at merge. Targeted set 1100 passed, 1 skipped on 53c529c.

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-2, 2026-10-11T20:09:43Z)

This lane reviewed PR #231 at f9656bd, read-only, in its own detached worktrees. It authored nothing and holds no claim. The branch has since moved to 53c529c, which this review did not cover.

MERGEABLE WITH NITS. Two things before merging:

This PR need not take main 54aa69a first.

Findings:

What I checked by running it:

  • The issue's case. On main, check says plan ok, then execute exits 2 with unsafe-value: a plan path is 'My Notes.md'. On the head, execute exits 0 with adoption verified.
  • Probes. On the head, refused names answered root/drop, plus * and :!keep.md dropped, verify, and the files beside them stay in the root.

Tests:

tree files result
head PR's 4 files 243 passed
main + PR's tests PR's 4 files 60 failed, all for the right reason
head other targeted adopt files 852 passed, 1 skipped
head env_commit's callers (scaffold, family, trailers) 287 passed

render-cli-reference.py --check is ok. All 7 mutants of the decisive lines were killed.

Merges:

other side result
main 54aa69a clean; the PR's files plus test_adopt_verify.py: 260 passed
#199 d5fdc22 2 hunks (main has 1); resolve as write_paths_listing(listing, [f"literal:{path}" for path in paths])
a0ff15b, 04792d2 2 hunks each in _entry_findings; keep the unsafe findings first
b0c959e 5 hunks, as on main
#212 d41c4b3 clean
nine other heads clean

Full report: handoffs/xFactory/attachments/lane-openRepoShape-2/review231-f9656bd/final-report.md in opensoft/brett-wip.

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-5, 2026-10-11T21:00Z) Composition note for lane openRepoShape-3, for this PR (#196, head 53c529c) and #199's PR #247 (head a02240a).

There is one conflicting hunk, at the _extract_leg listing write. #199 writes filter-repo's path listing byte-exact through write_paths_listing, and this PR adds the literal: prefix. The resolution is write_paths_listing(listing, [f"literal:{path}" for path in paths]), keeping both comments, and the targeted run on it gave 156 passed.

One subtlety the second lander should check: with an empty paths list the helper writes one \n, where this PR's code writes nothing. We did not establish whether _extract_leg can be called that way. Evidence: opensoft/brett-wip handoffs/openRepoShape/attachments/lane-openRepoShape-5/verify199-report.md (commit f64bb587c).

brettheap added a commit that referenced this pull request Oct 11, 2026
… printable from the one table `repo_shape`'s reader resolves, so a source holding the macOS `Icon\r` no longer writes a plan `check` refuses, and a non-UTF-8 name no longer empties the plan (#199) (#247)

The adoption plan's writer and its reader now share **one escape table**, so
a plan `adopt-project.py plan` writes is always a plan `check` parses,
whatever characters a source name holds.

- `scripts/repo_shape.py`: `DOUBLE_QUOTED_ESCAPES` gains `r`; a new
  `CODE_POINT_ESCAPES` (`x`: 2, `u`: 4, `U`: 8 hex digits) adds YAML's own
  code-point escapes; `double_quoted(text)` is the writer's half of both.
  It writes `\\`, `\"`, `\n`, `\r`, `\t` by name, and every other character
  `str.isprintable()` refuses in the shortest of `\xNN` / `\uNNNN` /
  `\UNNNNNNNN` that holds it. `_unescape` stays one pass, left to right; it
  resolves exactly what the writer writes (a code-point escape is exactly its
  count of ASCII hex digits, nothing past U+10FFFF), and any other escape keeps
  its backslash, as `\q` always has.
- `adopt-project.py`: `y()` returns `double_quoted(text)` for whatever it
  does not write plain. The plain decision (the `if` line #181 changes) is
  untouched. `_PLAIN_RE` now ends at `\Z` rather than `$`, because `$` also
  matches before a final newline and so wrote `Icon\n` bare. The plan's
  terminal report prints a name that is not printable as `_spelled` (new
  `_shown`). Raw, a carriage return hid the name, and a byte that is not
  UTF-8 raised `UnicodeEncodeError` on a strict terminal (macOS, a UTF-8 Linux
  locale, a Windows code page) after the plan was written. Printable names
  print exactly as before. `_shown` also spells a local `--source` directory,
  in `plan`'s report and in `check`'s `WHAT WILL HAPPEN, against ...` line.
- `scripts/shape_materialize.py`: new `write_paths_listing`, which
  `_extract_leg` writes `git filter-repo --paths-from-file`'s listing through.
  It is byte-exact (`utf-8` with `surrogateescape`, LF): a path that is not
  UTF-8 is the byte git records, where `write_lf`'s strict UTF-8 raised after
  both leg remotes existed. A UTF-8 listing is byte-identical to what
  `write_lf` wrote.
- `tests/test_adopt_plan_escapes.py` (new, 59 tests).

Fix round 1 answered an adversarial review of d3023fb: the question list that ends `plan`'s report is tested (S1), `_extract_leg` writes filter-repo's path listing byte-exact so a path that is not UTF-8 no longer raises after both legs exist (S2), and `plan` and `check` spell a `--source` directory that is not UTF-8 instead of raising on a strict stdout (S3), with the review's comment and test nits. The branch carries main 54aa69a.

Reviewed adversarially, fix round re-verified (VERIFIED WITH NITS, 10 of 10 items). CI is green on Linux, macOS and Windows. Composes with #181 (rename one of the two `_shown` helpers), #212/#223 (edit the raw-`\r` assertion) and #196/#231 (one hunk) as the PR body describes.

Closes #199

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap and others added 2 commits October 11, 2026 21:41
…e-path

Lane: openreposhape-3 (openRepoShape-3)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… that #199's listing carries it, any other surrogate as a path list that cannot be written, and a name of carriage returns alone is told its line would be `literal:` alone, which matches every path

The merge of main before this commit brings #199's PR #247, whose
`write_paths_listing` writes filter-repo's path list byte-exact
(`surrogateescape`). On that merge the Cs reason this branch gave ("the
path list `git filter-repo` reads is written as UTF-8 text, which cannot
carry it, and the write would fail after both leg repositories exist")
was no longer true. The refused set is unchanged; each reason is now
true on the merged tree, measured there:
- A byte that is not UTF-8, which `Source.tree` keeps as one of U+DC80
  to U+DCFF, is refused as POLICY, in the same form as the other policy
  findings: "which `execute` refuses as policy, though git and `git
  filter-repo` would take it literally". With the refusal disabled, a
  source holding `caf\xe9.md` checked `plan ok`, `execute` said
  `adoption verified`, the spec leg held the byte, and the split
  commit's message listed it as `caf?.md`. It is held until adopting
  such a name end to end is measured on macOS and Windows (the PR's
  Left out).
- Any other surrogate, such as a hand-edited `\ud800`, stays a
  misreading: `surrogateescape` writes only U+DC80 to U+DCFF, so
  `write_paths_listing` raised `UnicodeEncodeError` on it, and no
  source path holds one (such an entry is also `plan-empty-entry`).
- `NOT_UTF8_BYTES` names the 128 surrogates; the test lists move the
  byte cases to POLICY and add U+DC7F, U+DD00 and U+DC80 at the bounds.

A name made only of carriage returns (`\r`, `\r\r`) was told "its line
would be blank, which the filter skips". filter-repo strips the
carriage returns from `literal:\r`, which leaves `literal:` alone; that
matches every path, and one such line kept the whole tree (measured
with git-filter-repo 2.47.0). The finding now says so, as the empty
path's does. (The independent reviewer's nit on round 1.)

The module docstring's `check` paragraph now names the entry-path test
(lane 2's review, N4). The docstring of #247's
`test_a_leg_extracts_a_path_that_is_not_utf8` said `execute` refuses
such an entry through `SAFE_ARG_RE`; it now says `plan-unsafe-path`.

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

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-3, 2026-10-11T22:02Z) fix round 2 (independent review, codex is out of quota): the head is now 355ab2d. It merges main 4835fb4 as 53e82ed, which brings #247, and then makes one fix-round-2 commit. The merge's one conflict, _extract_leg's listing write, is resolved as lane 5 resolved it: write_paths_listing(listing, [f"literal:{path}" for path in paths]), keeping both comments. Lane 5's question has an answer: _extract_leg is never called with an empty paths. Its only caller, _build_the_legs, calls it only for a leg not in seeded_legs(paths_for), and such a leg is seeded from the template instead, so the lone \n cannot be written; the round-1 reviewer found the same. With #247 on main, this branch's old category-Cs reason ("written as UTF-8 text ... the write would fail after both leg repositories exist") was false. Per the coordinator's ruling, a byte that is not UTF-8 (U+DC80 to U+DCFF, NOT_UTF8_BYTES) is now refused as POLICY, in the same "though git and git filter-repo would take it literally" form. Measured on the merged tree with the refusal disabled: caf\xe9.md checks plan ok and execute says adoption verified, with the byte in the leg and caf?.md in the split message. Adopting such a name end to end, including macOS and Windows, stays the Left-out follow-up. Any other surrogate, such as a hand-edited \ud800, stays a misreading with a measured reason: write_paths_listing raises UnicodeEncodeError on it and no source path holds one. The refused set is unchanged: 53c529c and 355ab2d agree on all 5,679,428 names compared. The reviewer's nit is fixed: a name of carriage returns alone (\r, \r\r) is now told that its line would be literal: alone, which matches every path (re-measured: such a line kept the whole tree). Its test rows assert those words. Lane 2's N4 is fixed too: the module docstring's check paragraph names the entry-path test. #247's test docstring that named SAFE_ARG_RE now names plan-unsafe-path. The body no longer links #199 as an issue this PR closes; closingIssuesReferences is [196] only. "Composes with" is refreshed at today's shas, and #247 is listed as on main. Lane 2's P3 (plan writes leg: spec for -notes.md and asks nothing) is one sentence under "Left out", per the coordinator's ruling. Lane 2's nit about a tab, ESC or DEL was already settled by 53c529c. The seven assertions #223's spelling breaks are not in this round: they come with the next merge of main once #223 lands (prepared: 355ab2d + 941e096 with them updated gives 486 passed across 8 files). Main has since moved to 39ac59c and merges with 355ab2d clean. Mutations: dropping the policy refusal of bytes fails 4 tests, dropping the other-surrogate misreading fails 3, and the old carriage-return words fail 2. Targeted set on 355ab2d (the brief's 13 files plus test_adopt_mount_ignored, test_adopt_plan_escapes and test_adopt_verify): 1180 passed, 1 skipped in 622.75s; the skip is test_plan_on_medxehr_matches_the_rulings, which needs SHAPE_MEDXEHR_CLONE. The squash message in the body is updated and true at 355ab2d.

brettheap and others added 2 commits October 11, 2026 23:31
…e-path

Lane: openreposhape-3 (openRepoShape-3)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aths changed take that spelling, so `plan-uncovered` is expected as `` `specs/... ``, and the `-dep` submodule case asks `entry_path_problem` about the raw path

The merge of main before this commit brings #212's PR #223, which
spells every path a FINDING names as `_spelled` spells it, in
backticks. Seven of this branch's test cases asserted the bare spelling
and failed on the merge:
- `test_check_reports_the_path_first_and_says_what_to_do`, five cases
  (`control-character`, `empty`, `leading-dash`, `rename-arrow`,
  `whitespace-only`), and
  `test_a_path_that_is_a_git_option_is_still_unsafe_not_uncovered` now
  expect ``FINDING plan-uncovered: `specs/``;
- `test_an_unregistered_submodule_is_told_why_git_ignores_its_entry
  [path-option-like]`: #223 spells the case's `submodule` parameter
  `` `-dep` ``, so the test takes the entry's own path, `-dep`, from the
  case's `orphan` for `startswith("-")` and `entry_path_problem`, and
  names the finding by the spelled `submodule`.

No code changes. `scripts/shape_materialize.py` composes with #223's
`first_and_more(paths, spell=None)`: this branch touches only
`env_commit` there.

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

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-3, 2026-10-11T23:47Z) fix round 3 (independent review, codex is out of quota): the head is now b52e8d1. With #223 landed, this branch merges main 5751460 as b34b738, with no conflict and both trailers. That merge brings #223, #226, #230, #246, #218, #219, #220, #257, #265, #234, #233 and #235. One fix-round-3 commit then moves the seven assertions #223's spelling of finding paths broke to that spelling:

  • the five test_check_reports_the_path_first_and_says_what_to_do cases and test_a_path_that_is_a_git_option_is_still_unsafe_not_uncovered now expect FINDING plan-uncovered: `specs/;
  • test_an_unregistered_submodule_is_told_why_git_ignores_its_entry[path-option-like] asks entry_path_problem about the raw entry path -dep, taken from the case's orphan, and names the finding by the spelled submodule.

Round 3 changes no code. #223's first_and_more(paths, spell=None) in scripts/shape_materialize.py composes, because this branch touches only env_commit there. The refused set is unchanged: 355ab2d and b52e8d1 refuse the same names out of 5,679,428 compared. Targeted set on b52e8d1 (the brief's 13 files plus test_adopt_mount_ignored, test_adopt_plan_escapes, test_adopt_verify, test_adopt_finding_spelling, test_scaffold_leg_path_collisions, test_git_output_decoding and test_stdout_encoding): 1472 passed, 2 skipped in 734.97s. The skips are test_plan_on_medxehr_matches_the_rulings, which needs SHAPE_MEDXEHR_CLONE, and one test_stdout_encoding.py case that is about Windows' own default.

I updated the body:

@sonarqubecloud

Copy link
Copy Markdown

@brettheap
brettheap merged commit 6f45bf9 into main Oct 12, 2026
5 checks passed
@brettheap
brettheap deleted the fix/adopt-check-unsafe-path branch October 12, 2026 00:47
brettheap added a commit that referenced this pull request Oct 12, 2026
…s `_entry_findings` composed with #255's, and this PR's pair renamed `unwritable_findings`/`unwritable_remediation`

#231 (6f45bf9) puts each entry's path first in `_entry_findings` and
returns `unsafe_remediation` ahead of the rest; this branch appends each
value `execute` writes into a file that no file can hold. The two hunks
are its docstring's order sentence and its return. The sentence keeps
main's order and appends this PR's clause; the return is
`(findings + submodule_findings + unwritable_findings,
  unsafe_remediation + remediation + unwritable_remediation)`, so the
paths still come first, as #196 needs.

This PR's local pair `value_findings, value_remediation` is renamed
`unwritable_findings, unwritable_remediation`. #261's branch (ca21b57)
binds `value_findings, value_remediation` in the same function, from
`_manifest_findings`: a resolution keeping both lines under one name
would silently drop one PR's findings. With the rename it cannot.

The rest of main (#254/#281, #238/#287, #203/#236, #204/#227,
#249/#250, #241, #215 and the earlier merges) merges with no conflict.
The resolution is the lane's re-verify one (`resolve-main.py`, 385
passed on 6f45bf9), rename aside.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 12, 2026
…s `plan-unsafe-path` comes first in the one list of entry findings, and #190's relative urls and NOTEs come last in it, with `execute` still resolving them from the remotes it would create

Main since ede15ce: 6f45bf9 (#196 via #231, `entry_path_problem`) and
b421e6b (#254 via #281, PYTHONIOENCODING for the validators that bump-leg.py and
update-shape.py run). Two files were changed on both sides: adopt-project.py
and tests/test_adopt_submodules.py.

THREE CONFLICT HUNKS, all in adopt-project.py, and each keeps both sides:
1. Before `_entry_findings`: #196's `NOT_EXTRACTED`, its remediation and
   `_unsafe_path_findings` are kept whole. `_entry_findings` keeps #190's
   signature, `local_remote_dir` and the three-tuple with the NOTEs. Its
   docstring now names #196's order (each entry's path, then coverage,
   then the legs and mounts, then the submodules) and #190's NOTEs.
2. `_entry_findings`' return:
   `(findings + submodule_findings, unsafe_remediation + remediation,
   notes)`. This is #196's remediation first and #190's NOTEs, with
   `_submodule_findings(plan, source, tree, local_remote_dir)` as #190
   calls it.
3. `_refuse_what_check_finds`' docstring: #196's "after the entries' paths,
   which are the first of these findings", then #190's paragraph on the
   remotes this run would create.

Auto-merged, #196's: the module docstring; `_unsafe_path_findings` and its
neighbours; `_extract_leg`; `_mount_the_legs`; `_leg_paths`; the rest of
`_refuse_what_check_finds`; and five hunks of tests/test_adopt_submodules.py.

Proofs:
- Each side's +/- lines are unchanged by the merge, except the two
  resolved spots (the `_entry_findings` docstring and its return). That
  holds in adopt-project.py, and tests/test_adopt_submodules.py is
  identical both ways.
- `_entry_findings` is called by `cmd_check` and `_refuse_what_check_finds`
  (both three-tuples) and by #190's own test, and by no test of main's.
- No top-level name is bound twice, and no line over 79 columns is added
  (42 on main, 42 merged).

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 12, 2026
…rences` rows, which exempt nothing now that #196 and #216 are on main

The branch merged main at 346079d (#228, issue #216; #196 landed earlier as
6f45bf9). The two rows were kept "until" those landed:

- `scripts/shape_materialize.py` `env_commit`: its `subprocess.run` names
  `text=True, encoding="utf-8", errors="replace"`, so the default rule is met
  and the row exempted nothing (since #231).
- `adopt-project.py` `cross_leg_references`: #216 made its `git grep` read
  BYTES (`capture_output=True`, no `text=`, the record split on NUL and each
  path decoded by hand), so it is no longer a text-mode call at all. Measured
  on c7c02fa the row was needed (`adopt-project.py:1080 asks for text and does
  not name ['encoding', 'errors']` without it); on 346079d the guard finds no
  text-mode call in that function.

EXEMPT keeps the two `gh api` digest rows (`pin_digest_from_gh`,
`tree_digest_from_gh`: UTF-8 yes, `replace` no), which still exempt `errors`.
With both rows gone the guard flags 0 offenders over the shipped Python at
346079d: 51 spawner calls, 32 text-mode (33 before #216).

Lane: openreposhape-3 (openRepoShape-3)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 12, 2026
…, #242 via #308, #262 and #263 via #313) into fix/adopt-record-submodule-placement

Brings in ede15ce (#238), 6f45bf9 (#196), b421e6b (#254), c0ed615 (#242)
and 71b58a1 (#262, #263) on top of 6d8d88b. No rebase. ede15ce changes
shape-doctor.py. 6f45bf9 and 71b58a1 change adopt-project.py and
scripts/shape_materialize.py. b421e6b changes scripts/shape_materialize.py
too, giving a project's validators an explicit PYTHONIOENCODING. All of
these are files this branch changes. c0ed615 changes scripts/family.py
alone.

No conflicts. Checked:

- adopt-project.py: #196 gives `check` and `execute` one test for an
  entry's path, `entry_path_problem`, as the finding `plan-unsafe-path`.
  #262 and #263 add `_root_shape_findings` to `_leg_findings`, for a kept
  root path where the shape writes. Both reach `execute` through
  `_refuse_what_check_finds`, before any leg exists and before
  `_commit_the_split` appends the record. This branch's `submodule_record`,
  `_resolution_text`, the record's preamble, `_answered_legs`,
  `_kept_in_a_leg`, `_commit_the_split`, `cmd_execute`,
  `submodule_plan_problems`, `_submodule_findings`, `Plan`, `y`, `emit`
  and `walk` are unchanged by the merge. `_root_shape_findings` is asked
  only beside a `legs:` mapping, so the doctor's placement plan, whose
  `legs:` is a list, still goes through `_leg_findings(plan, None)`.
- shape-doctor.py: #238 changes `validator_row` and `run_validator`. This
  branch's `check_placement`, `recorded_submodules`, `audit_leg`,
  `placement_row`, `placement_verdict` and `Context` are unchanged by the
  merge.
- scripts/repo_shape.py is untouched by these commits, so the record's
  writer and reader, and `double_quoted` in `submodule_record`, are
  3e86200's.
- Both sides added lines to the module docstrings of adopt-project.py and
  shape-doctor.py. Every line either side added is in the merge.
- No top-level name is bound twice in adopt-project.py,
  scaffold-project.py, scripts/repo_shape.py, scripts/shape_materialize.py,
  shape-doctor.py, validate-manifest.py, update-shape.py, scripts/family.py
  or scripts/bump-leg.py. Each merged module's names are the base's plus
  what either side added. Every definition only one side changed is that
  side's text.
- AGENTS.md is untouched: 472 lines, under the cap of 472.
- The branch's diff against main is still #189 alone: 12 files, +1670
  -157, file by file the same as against 6d8d88b.

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 12, 2026
…pelled `from subprocess import run`, `import subprocess as sp`, with only `encoding=` or `errors=`, through `getoutput`, with `**options` or passed uncalled, so none of them walks past #230's check any more (#321) (#335)

#230's guard (`TextModeCalls`) matched one spelling, `subprocess.<spawner>(...,
text=True)`, and checked that the call names `encoding` and `errors`. Every
other way to ask a child for text decodes with the locale's codec exactly as
#217 describes, and was invisible to it. Nothing but `tests/test_git_output_decoding.py`
changes: the shipped code already passes the hardened guard (see below).

- A call is a spawner call however `subprocess` is reached: `from subprocess
  import run` (also `as`, also `*`), `import subprocess as sp`,
  `sp = subprocess` and `sp: object = subprocess` (a plain and an annotated
  assignment alike), `other_module.subprocess.run`. A walrus, an unpacking or
  a `for` target that holds the module is not followed.
- `encoding=` or `errors=` alone asks for text, because `Popen` puts the pipes
  in text mode on either. `encoding=None` and `errors=None` do not ask.
- A literal `encoding=` that is not UTF-8 (`None`, `"cp1252"`) and a literal
  `errors=None` name nothing, so `text=True, encoding=None, errors="replace"`
  is refused. A variable is taken at its word: a tree cannot say what it holds,
  and the shipped code has none.
- `subprocess.getoutput` and `getstatusoutput` are refused unless EXEMPT lists
  the function: they are always text, and Python 3.9 (this standard's floor)
  takes no codec for them.
- A spawner call that expands `**options` without asking for text in a way the
  guard can see is refused unless the new `OPAQUE` table lists its function
  with a reason (a blank reason is refused too). A call that names `encoding=`
  and `errors=` itself needs no row: Python rejects a keyword given twice.
- A spawner referred to without being called (`functools.partial(subprocess.run,
  ...)`, `runner = subprocess.run`, `run: Callable = subprocess.run`,
  `class Child(subprocess.Popen)`, `map(run, argvs)`) is refused the same
  way. A type annotation is not a reference, but the value of an annotated
  assignment is.
- EXEMPT drops its `env_commit` and `cross_leg_references` rows, which were
  kept until #196 and #216 landed and exempt nothing now: `env_commit`'s call
  names both codec keywords (since #231), and #216 made `cross_leg_references`'
  `git grep` read bytes. EXEMPT keeps the two `gh api` digest rows.

Tests (`tests/test_git_output_decoding.py`, 3 -> 67): `SYNTHETIC` holds one
small module per form above, 24 of them. Each gets two tests, one that the
guard flags the module and names why, one that the same module fixed passes.
Eight allowed and four refused one-liners pin what the old guard already did,
and four more tests cover the annotation rule, a function listed in EXEMPT, a
function listed in OPAQUE (with a reason, with a blank one, and for another
function) and the reasons in the real table.

Fails on main: `TextModeCalls` as main has it, run over the same 24 modules,
finds nothing wrong in any of them (the guard lives in the test file, so the
head's tests cannot be pointed at main's guard directly). Each detection was
also removed in a scratch copy (19 mutants: from-import, star-import, `as`,
`sp = subprocess`, `sp: object = subprocess`, an annotated assignment's value
read as an annotation, `other_module.subprocess`, encoding/errors do not ask,
any encoding is UTF-8, `None` counts as a policy, expansion ignored, every
expansion refused, getoutput ignored, references ignored, annotations read as
references, blank reason accepted, EXEMPT ignored for getoutput, `text=False`
asks, the old call shape restored), and each failed the test named for it.

Unchanged: for everything the old guard saw, main's and the head's `found`
lists are identical over the shipped Python at 71b58a1 and at c7c02fa (rows in
place): 51 spawner calls, 33 of them text-mode, 0 offenders in either. At
346079d, with the two rows gone, the guard sees 51 spawner calls, 32 of them
text-mode (#216 made `cross_leg_references` read bytes), and flags 0. So no
shipped file changes and OPAQUE starts empty.

Left out: `interpreter_runs` in `tests/test_validator_child_encoding.py` reads
only `subprocess.run` too, and `os.popen` (text, no codec) is not read; none is
used in the shipped code. A check that each EXEMPT row still matches a text-mode
call would have caught the two rows this change drops.

Lane: openreposhape-3 (openRepoShape-3)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 12, 2026
Main brings five commits since 6d8d88b:

- ede15ce (#238 via #287): shape-doctor.py's validator rows;
- 6f45bf9 (#196 via #231): `entry_path_problem`, one test of an entry's
  path for `check` and `execute`;
- b421e6b (#254 via #281): bump-leg.py and update-shape.py give the
  validators an explicit PYTHONIOENCODING (scripts/shape_materialize.py
  gains `captured_child_environment` and `inherited_child_environment`);
- c0ed615 (#242 via #308): family.py's `add` and `bump` under excludes;
- 71b58a1 (#262, #263 via #313): `check` finds, and `execute` refuses, a
  kept root path where the shape writes a directory or its `shape/`
  copy, and the materializer never writes over a copy.

It merged with no conflict. Only adopt-project.py changed on both sides.
Every other file the merge changes against this branch is main's, and
every file it changes against main is this branch's. In adopt-project.py
the merge's diff against this branch is main's own diff (559 lines), and
its diff against main is this branch's own diff (784 lines). Main's
twenty hunks there, by where they land:

1-2. the module docstring: `check` validates each entry's path and keeps
     no path in the root where the shape writes (6f45bf9, 71b58a1);
3. the `from shape_materialize import` block: `CASE_ASIDE`,
   `MOVE_THE_LEG`, `first_and_more` (71b58a1);
4-6. `_leg_findings`: a docstring line, the call to
     `_root_shape_findings`, and the new block after it (`ROOT_BLOCKS_SHAPE`,
     `ROOT_BLOCKS_COPY`, `_KeptTree`, `_shape_blocks`, `_blocked_sentence`,
     `_fix_the_kept_path`, `_root_shape_findings`) (71b58a1);
7. after `_submodule_findings`: `NOT_EXTRACTED`, `entry_path_problem`,
   `_misread`, `_refused_by_policy` and `_unsafe_path_findings` (6f45bf9);
8-11. `_entry_findings`: its docstring, and the entries' path findings
      first (6f45bf9);
12-13. `_extract_leg`: `literal:` on every `--paths-from-file` line
       (6f45bf9). This branch changed `_extract_leg` as well, further
       down: it reads the commit count and the digest before
       `_push_the_leg`. The two sets of lines do not touch, and the
       merged function has both;
14-15. `_mount_the_legs`: `git --literal-pathspecs rm` (6f45bf9);
16-17. `_leg_paths`: collected, not judged (6f45bf9);
18-20. `_refuse_what_check_finds`: its docstring and remediation
       (6f45bf9, 71b58a1).

The new findings of 6f45bf9 and 71b58a1 are `_entry_findings`, which
`cmd_execute` reaches only through `_refuse_what_check_finds`, before
`_create_leg_remotes`. So they are refused before any leg exists, and no
notice is due. The materializer's new `materialize-copy-blocked` is
raised after both legs are pushed, inside `_commit_the_split`, when a run
reaches the materializer without `check`'s finding. That makes it a late
failure, which this branch reports with the notice and exit 2. Main's
`test_with_check_skipped_execute_stops_at_the_copy_and_overwrites_nothing`
expects it to escape `cmd_execute`, so this merge breaks that test, as
9092fda's merge of #200 broke the mount-guard tests. The next commit
makes the test read the reported refusal.

No top-level name is bound twice in adopt-project.py, scaffold-project.py,
scripts/repo_shape.py, scripts/shape_materialize.py or tests/conftest.py.

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 12, 2026
…, #238, #196, #254, #242 and #262/#263 merge with three adopt-project.py hunks kept on both sides, and #262's `_CASE_ASIDE` import kept as the name's one binding

origin/main moved b30595c -> 71b58a1:
- be2790e (#204 via #227), update-shape.py;
- 6d8d88b (#203 via #236), family.py;
- ede15ce (#238 via #287), shape-doctor.py;
- 6f45bf9 (#196 via #231): adopt-project.py `entry_path_problem`,
  scripts/shape_materialize.py;
- b421e6b (#254 via #281): scripts/shape_materialize.py, bump-leg.py,
  update-shape.py;
- c0ed615 (#242 via #308), family.py;
- 71b58a1 (#262 and #263 via #313): adopt-project.py
  `_root_shape_findings` and scripts/shape_materialize.py
  `copy_in_the_way`. It fixes what this branch's pull request listed as
  follow-ups (i) and (ii): a source's own `shape/` where the shape
  writes its copy, and a plain FILE where the shape needs a directory.

Conflicts, all in adopt-project.py, each resolved by keeping both sides:
- The module docstring's `check` paragraph. 71b58a1 adds "no path kept
  in the root where the shape writes ..." before the submodule clause,
  and this branch adds the symlink clause after it. Both are kept, in
  that order.
- `_entry_findings`' docstring. 6f45bf9 and 71b58a1 give the order
  "each entry's path, then coverage, then each entry's `leg:`, the two
  mount paths and what the root keeps where the shape writes, then the
  source's own submodules". This branch adds "then each symlink the plan
  keeps in the root that a shape file cannot be written through".
- `_entry_findings`' return line. 6f45bf9 returns
  `unsafe_remediation + remediation`, and this branch appends
  `_root_symlink_findings(plan, source, tree)` to the findings. The
  merge returns both.

Found after the textual merge, and resolved here so that no commit binds
a name twice: 71b58a1 (through 65a9262) imports #198's phrase as
`_CASE_ASIDE` in the "#172's shape half" import block, and b37159a had
imported it on a line of its own. Main's import is kept; this branch's
line and its five-line comment go.

scripts/shape_materialize.py merges with no conflict. `place` gets
71b58a1's `copy_in_the_way` inside its collision block and this branch's
`_refuse_unwritable` after it, as 71b58a1's message foresaw.

Checked on the merged tree:
- `_entry_findings` still hands `_leg_findings` the tree, and the
  symlink findings still come last in its list.
- AST: no top-level name is bound twice in adopt-project.py,
  scaffold-project.py, scripts/repo_shape.py or
  scripts/shape_materialize.py, and none is loaded but bound nowhere.
  Main changed the signature of no function this branch calls.
- Both sides are intact by +/- line sets, apart from the three hunks and
  the import above.

The next two commits fix what this merge breaks, each with a revert
line. #262's rule and this branch's both name a symlink kept AT the
copy under `shape/`. And where the shape's copy lands is now
`copy_in_the_way`'s question, asked before this branch's own check of
it.

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