[brettheap]

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

What

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 adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #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).

Why

#199: y() escaped only \ and ". The reader splits a file with
str.splitlines, which ends a line at \n, \r, \x0b, \x0c,
\x1c-\x1e, \x85, U+2028 and U+2029. A source tracking the macOS
folder-icon file Icon\r therefore got a plan written with exit 0, which
check then refused as REFUSED yaml-unparsable, with no entry to edit.

A name that is not UTF-8 gets a faithful round trip, not a refusal.
Source.tree keeps such a byte as a lone surrogate. A surrogate is not
printable, so the same rule writes it \udce9, and the reader's \u turns
it back into that same surrogate. The escape is ASCII and the plan file stays
valid UTF-8, so #192's yaml-unreadable never fires, and check finds the
entry's name in the tree.
This adds no code beyond the rule every other non-printable character already
follows. A refusal in plan would have added a new refusal whose only remedy
for a top-level file is renaming it in the source. (On main such a name made
plan raise in write_lf and leave the plan empty.)

Evidence

Reproduced on main e4decfb with a source holding Icon\r at the top level:
plan exit 0, check exit 2 REFUSED yaml-unparsable ... line 129. The
same happens with a\x85b / l\u2028s. With caf\xe9.txt: plan exit 1,
UnicodeEncodeError, 0-byte plan.

On this branch:

• One-off probe: every code point U+0000-U+10FFFF, alone and as a<c>b,
  round-trips through y() after k: and as - , with 0 failures. Every
  splitlines break character is non-printable. The tests keep every code
  point to U+02FF, a sample above it, and 5000 seeded strings of them; not the
  bare keywords null, true, false, which y writes unquoted and the
  reader takes as None or a bool (adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181's).
• All 14 tracked YAML files read identically with the old and new reader
  (7 parse, 7 are refused identically).
• plan / check on Icon\r, caf\xe9.txt, a\x01b, d\x7fel, n\x85el,
  ls\u2028x, t\tab: plan exit 0, and check parses (exit 1 only for the
  usual unanswered entries). This holds under PYTHONIOENCODING=utf-8:strict
  and cp1252:strict, with 0 tracebacks.
• A --source directory whose path holds a non-UTF-8 byte
  (.../caf\xe9/Thing): under PYTHONIOENCODING=utf-8:strict, plan and
  check both raised UnicodeEncodeError after the plan was written; now
  plan exits 0 and check exits 1 (the usual unanswered entries), both
  showing the directory spelled, no traceback.
• What execute then does: with Icon\r (or caf\udce9.txt) as a top-level
  entry answered root, code or drop, it refuses with
  REFUSED unsafe-value: a plan path is 'Icon\r' ... (exit 2) before any
  repository exists, and the source is untouched. That is SAFE_ARG_RE, adopt-project.py check accepts plan paths execute refuses as unsafe-value (a space or a non-ASCII character), so plan ok is followed by a refusal and #166's split instructions can name entries execute will refuse #196's
  question. This branch now writes the path listing byte-exact
  (write_paths_listing), so a non-UTF-8 path that reaches extraction no
  longer raises after both leg remotes exist; lane 3's adopt-project.py check accepts plan paths execute refuses as unsafe-value (a space or a non-ASCII character), so plan ok is followed by a refusal and #166's split instructions can name entries execute will refuse #196 refuses
  category-Cs names and files the end-to-end case after this lands. Once
  adopt-project.py check accepts plan paths execute refuses as unsafe-value (a space or a non-ASCII character), so plan ok is followed by a refusal and #166's split instructions can name entries execute will refuse #196 lands, entry paths leave the SAFE_ARG_RE check: check reports
  FINDING plan-unsafe-path: <spelled path> ... (exit 1) for an entry not
  answered root or drop whose path holds a control character, and execute
  refuses it through _refuse_what_check_finds, still before any remote
  exists; a nested Icon\r is not an entry path, so that adoption still goes
  through.
  With Icon\r only under docs/, the whole adoption goes through
  (adoption verified), and the spec leg holds docs/Icon\r.
• Mutation check: each part of the fix reverted on its own fails tests. Old
  quoting fails 23, $ anchor 1, no r in the table 3, no hex-digit guard 4,
  a raw name in the entries table 4, in the list of questions 3, _shown an
  identity 5; the listing written through write_lf again 1, strict UTF-8 in
  the helper 2, CRLF in it 1; the source directory printed raw, in plan's
  report 1 and in check's 1.
• Composes with adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181 (origin/fix/adopt-plan-quote-yaml-keywords, a0ff15b,
  which carries main 54aa69a too): textually clean, but not clean in
  behaviour. git merge-tree --write-tree of this branch (a02240a, main
  54aa69a merged in) with it is clean. But adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181's b26841b defines a second
  _shown(line: str) in adopt-project.py, which returns the line in
  backticks, or repr(line) when it is not printable. On the merged tree it
  sits after this branch's _shown(path), so Python binds every call site
  here (plan's source line, its questions and entries table, check's
  WHAT WILL HAPPEN line) to adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181's. A throwaway merge of a02240a with a0ff15b
  gave 5 failed, 444 passed over tests/test_adopt_plan_escapes.py plus
  tests/test_adopt_plan_yaml_quoting.py: the
  test_plan_writes_the_name_as_one_entry_check_parses parameters
  [carriage-return], [not-utf8], [next-line] and [line-separator], and
  test_a_source_directory_that_is_not_utf8_is_shown_spelled. Nothing crashes
  (repr is ASCII), but every printable name in plan's table and questions
  also gains backticks, so "Printable names print exactly as before" stops
  being true once both land. Whichever of adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181 and this PR lands second
  renames one of the two helpers (adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181's has one call site, this branch's
  four) and re-runs those two test files on the merge, because the textual
  merge hides it. An earlier run on a different pair, this branch at d5fdc22
  with adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181 at 76a6153, which predates b26841b, gave 384 passed on the same two
  files; it does not describe the current heads.
• Composes with adopt-project.py check accepts plan paths execute refuses as unsafe-value (a space or a non-ASCII character), so plan ok is followed by a refusal and #166's split instructions can name entries execute will refuse #196 (PR 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): origin/fix/adopt-check-unsafe-path (53c529c,
  on main a5b09c3) has ONE conflict left against this branch, in
  adopt-project.py at the listing write in _extract_leg:
  write_paths_listing(listing, paths) here, write_lf(listing, "".join(f"literal:{path}\n" for path in paths)) there. Whichever merges
  second resolves it as
  write_paths_listing(listing, [f"literal:{path}" for path in paths]), with
  both comments. Verified: that resolution, merged with 53c529c into a02240a in
  a throwaway worktree, gave 156 passed over
  tests/test_adopt_plan_escapes.py, tests/test_adopt_entry_paths.py and
  tests/test_adopt_execute_refusals.py. The earlier head 30badde, from
  before adopt-project.py: a leg's mount path must not be, hold or lie inside a path the assembly root holds, so --spec-path contracts no longer checks ok and dies staging the split after both legs are pushed, and --code-path project.yaml no longer says adoption verified over an assembly with no manifest (#172) #198, also conflicted in _entry_findings, which was adopt-project.py: a leg's mount path must not be, hold or lie inside a path the assembly root holds, so --spec-path contracts no longer checks ok and dies staging the split after both legs are pushed, and --code-path project.yaml no longer says adoption verified over an assembly with no manifest (#172) #198's change;
  adopt-project.py check accepts plan paths execute refuses as unsafe-value (a space or a non-ASCII character), so plan ok is followed by a refusal and #166's split instructions can name entries execute will refuse #196 now carries adopt-project.py: a leg's mount path must not be, hold or lie inside a path the assembly root holds, so --spec-path contracts no longer checks ok and dies staging the split after both legs are pushed, and --code-path project.yaml no longer says adoption verified over an assembly with no manifest (#172) #198, so that one is gone. Run on that earlier head with
  both kept, a source holding caf\xe9.txt at the top and under docs/ ends
  execute with adoption verified, the code leg holding caf\351.txt and
  the spec leg docs/caf\351.txt. With adopt-project.py check accepts plan paths execute refuses as unsafe-value (a space or a non-ASCII character), so plan ok is followed by a refusal and #166's split instructions can name entries execute will refuse #196's own write_lf instead, the
  same adoption raised UnicodeEncodeError after both remotes existed.
• Composes with adopt-project.py check and execute print a source path raw in a FINDING, so a \r, \n, \x85, U+2028 or ESC in a name overwrites, splits or escapes the line on a terminal, where plan's questions spell the same path with _spelled #212 (PR adopt-project.py: check and execute spell every path a FINDING or a refusal names as plan's questions spell it, so a source name holding a carriage return, a line feed, U+0085, U+2028 or an ESC no longer overwrites, splits or escapes the line it is printed on (#212) #223, fix/adopt-spell-finding-paths, d41c4b3):
  git merge-tree --write-tree of a02240a with it is clean, but on the merged
  tree tests/test_adopt_plan_escapes.py gives 1 failed, 58 passed:
  [carriage-return], on the line that asserts the raw \r FINDING (N7),
  because adopt-project.py: check and execute spell every path a FINDING or a refusal names as plan's questions spell it, so a source name holding a carriage return, a line feed, U+0085, U+2028 or an ESC no longer overwrites, splits or escapes the line it is printed on (#212) #223 spells the path there. Whichever of this PR and adopt-project.py: check and execute spell every path a FINDING or a refusal names as plan's questions spell it, so a source name holding a carriage return, a line feed, U+0085, U+2028 or an ESC no longer overwrites, splits or escapes the line it is printed on (#212) #223 lands
  second edits that assertion to the spelled name.
• This branch carries main 54aa69a (scaffold-project.py: a leg's mount path must be canonical or it is refused before anything is created, so --spec-path spec/ no longer scaffolds a project whose recursive clone fails once the work directory is gone (#179) #201, adopt-project.py: the leg mount passes --force to git submodule add, so a source whose .gitignore is * + !.gitignore, or names spec or code, adopts instead of dying at the mount after both leg remotes were pushed (#174) #200, adopt-project.py: verification passes over the two leg mounts and no other gitlink, so the source's own submodule kept in the root is adopt-root-submodule, any other root gitlink adopt-unexpected-gitlink, one in a leg AND the root adopt-duplicated, and a missing mount adopt-mount-missing (#165) #173); it carried 20e13f0
  (adopt-project.py: a leg's mount path must not be, hold or lie inside a path the assembly root holds, so --spec-path contracts no longer checks ok and dies staging the split after both legs are pushed, and --code-path project.yaml no longer says adoption verified over an assembly with no manifest (#172) #198) before. One hunk conflicted in the merge (a02240a), the
  from repo_shape import (...) list in adopt-project.py: scaffold-project.py: a leg's mount path must be canonical or it is refused before anything is created, so --spec-path spec/ no longer scaffolds a project whose recursive clone fails once the work directory is gone (#179) #201 removed
  SAFE_ARG_RE from it and this branch added double_quoted. It is main's
  list plus double_quoted; no other file conflicted.

Targeted runs (git-filter-repo 2.47.0 on PATH), all with /usr/bin/python3 -m pytest -q -rs.

On the merge head a02240a (main 54aa69a merged in):

• Before the push: tests/test_adopt_plan_escapes.py gave 59 passed;
  tests/test_adopt_verify.py tests/test_adopt_mount_ignored.py tests/test_adopt_leg_path_collisions.py gave 137 passed;
  git diff --check HEAD^1 HEAD was clean.
• tests/test_adopt_plan.py tests/test_adopt_submodules.py tests/test_adopt_execute_refusals.py tests/test_adopt_spec_only.py tests/test_adopt_e2e.py tests/test_shape_doctor.py tests/test_manifest_validator.py tests/test_repo_hygiene.py tests/test_cli_reference.py tests/test_adopt_plan_escapes.py gave 600
  passed, 1 skipped (9 min 3 s). The only skip is the MedxEHR clone test
  (SHAPE_MEDXEHR_CLONE unset). The one extra test over the 599 below is
  main's.
• tests/test_adopt_leg_paths.py tests/test_adopt_submodules.py tests/test_scaffold_e2e.py gave 525 passed, no skips (2 min 56 s).

On d5fdc22, before main was merged in:

• tests/test_adopt_plan_escapes.py gave 59 passed, and the same ten-file
  set gave 599 passed, 1 skipped (33 min 31 s; same MedxEHR skip).
• tests/test_adopt_leg_path_collisions.py tests/test_adopt_leg_paths.py gave
  441 passed (the adopt-project.py: a leg's mount path must not be, hold or lie inside a path the assembly root holds, so --spec-path contracts no longer checks ok and dies staging the split after both legs are pushed, and --code-path project.yaml no longer says adoption verified over an assembly with no manifest (#172) #198 merge).
• tests/test_load_yaml_refusals.py tests/test_windows_paths.py tests/test_naming_policy.py tests/test_adopt_leg_paths.py tests/test_pin_validator.py tests/test_neutral_pin_validator.py gave
  590 passed on the first head (d3023fb), because the reader changed for
  every YAML file. The reader's code has not changed since; only its comments
  and docstrings. Not re-run on the merge head.

No full-suite run, and no CI run yet: tests, tests-macos and tests-windows
have not seen this branch.

Fix round 1

The adversarial review of d3023fb asked for three should-fix items and seven
nits; the round is one commit, d5fdc22, on the #198 merge;
main 54aa69a is merged in on top of it (a02240a).

• S1. The _shown() call in plan's list of questions had no test
  (reverting it left the file green). plan is now read in bytes: the raw name
  must be absent from its stdout, and the question-list line itself must hold
  the spelled name.
• S2. _extract_leg wrote filter-repo's path listing through write_lf,
  strict UTF-8, so a path holding a non-UTF-8 byte raised once both leg
  remotes existed, as soon as something let such an entry reach extraction
  (adopt-project.py check accepts plan paths execute refuses as unsafe-value (a space or a non-ASCII character), so plan ok is followed by a refusal and #166's split instructions can name entries execute will refuse #196's head does; on this branch alone SAFE_ARG_RE refuses it first). It is
  now write_paths_listing, byte-exact, tested on the helper and on
  _extract_leg itself. This branch writes no literal: prefix; that is
  adopt-project.py check accepts plan paths execute refuses as unsafe-value (a space or a non-ASCII character), so plan ok is followed by a refusal and #166's split instructions can name entries execute will refuse #196's.
• S3. plan's source <path> @ and check's WHAT WILL HAPPEN, against <path> @ printed a local source directory raw; a non-UTF-8 byte in it
  crashed both on a strict UTF-8 stdout after the plan was written. Both print
  _shown(str(source.path)) now (a source.repository URL as it was).
• N1, N2, N3. Three comment overclaims, worded to what holds: the escaped
  class the reader cannot take raw is the ten splitlines breakers and a lone
  surrogate (the rest is escaped so a person can see it); libyaml's
  CSafeLoader refuses a lone surrogate's \udce9, so "any other reader takes
  it the same way" is qualified; the round-trip test covers every code point to
  U+02FF, a sample above and a seeded sample of strings, not the bare keywords.
• N4. _BLOCK_SENTINEL's comment says a block-scalar key holding a NUL
  (raw, or the escape \x00) collides with it and is misread, and that no
  writer here emits such a key. The reader is unchanged.
• N5. _spelled's docstring says why the report shows \xe9 and the
  plan file writes \udce9: in YAML \xe9 is U+00E9.
• N6 (this paragraph). The reader change applies to every YAML file every
  tool reads, not only plans: a hand-written double-quoted value holding
  \r, \xNN, \uNNNN or \UNNNNNNNN now resolves, where main kept the
  backslash ("C:\repos\Thing" reads C:<CR>epos\Thing,
  "D:\build\x64\Release" reads D:\buildd\Release, "caf\u00e9" reads
  café). That is what YAML says, and "D:\temp" was already broken on main
  through \t. All 14 tracked YAML files read identically with both readers.
  Two templates take free text in double quotes:
  templates/single-repository/single-repository.yaml and
  templates/family-root/family.yaml.
• N7. The test line that asserts the raw \r in a FINDING pins a defect
  filed as adopt-project.py check and execute print a source path raw in a FINDING, so a \r, \n, \x85, U+2028 or ESC in a name overwrites, splits or escapes the line on a terminal, where plan's questions spell the same path with _spelled #212; its comment now says so and names adopt-project.py check and execute print a source path raw in a FINDING, so a \r, \n, \x85, U+2028 or ESC in a name overwrites, splits or escapes the line on a terminal, where plan's questions spell the same path with _spelled #212's branch. That
  comment also says adopt-project.py check and execute print a source path raw in a FINDING, so a \r, \n, \x85, U+2028 or ESC in a name overwrites, splits or escapes the line on a terminal, where plan's questions spell the same path with _spelled #212's fix "edits this line", which no branch can yet do:
  adopt-project.py check and execute print a source path raw in a FINDING, so a \r, \n, \x85, U+2028 or ESC in a name overwrites, splits or escapes the line on a terminal, where plan's questions spell the same path with _spelled #212's fix is PR adopt-project.py: check and execute spell every path a FINDING or a refusal names as plan's questions spell it, so a source name holding a carriage return, a line feed, U+0085, U+2028 or an ESC no longer overwrites, splits or escapes the line it is printed on (#212) #223 (d41c4b3), whose base has no
  tests/test_adopt_plan_escapes.py. See "Composes with adopt-project.py check and execute print a source path raw in a FINDING, so a \r, \n, \x85, U+2028 or ESC in a name overwrites, splits or escapes the line on a terminal, where plan's questions spell the same path with _spelled #212" above: the two
  merge cleanly, and then that assertion fails, so whichever of this PR and
  adopt-project.py: check and execute spell every path a FINDING or a refusal names as plan's questions spell it, so a source name holding a carriage return, a line feed, U+0085, U+2028 or an ESC no longer overwrites, splits or escapes the line it is printed on (#212) #223 lands second edits it (and the comment).
• U1. The review expected that on Windows git's core.protectNTFS (on by
  default there) makes update-index ignore a path holding a byte below 0x20,
  so the headline Icon\r parameter would skip and only three of the four
  parameters would run there. That is expected, not established: the review
  checked the mechanism on Linux with a path git refuses everywhere, and
  nobody has run tests-windows on this branch. odd_source now passes -c core.protectNTFS=false to its update-index and commit calls (the name
  is never checked out). Linux accepts it and the case still passes; whether
  the Icon\r parameter runs on Windows, now or before, is for this PR's
  tests-windows job to confirm.

Left out

• check prints an entry's path raw in a FINDING. On a terminal,
  FINDING plan-unresolved: Icon\r still has ... loses its code and path
  behind the carriage return, and \n, \x85 or U+2028 split the FINDING
  line. It should spell paths as questions do (_spelled). Left alone because
  adopt-project.py check accepts plan paths execute refuses as unsafe-value (a space or a non-ASCII character), so plan ok is followed by a refusal and #166's split instructions can name entries execute will refuse #196 (lane openRepoShape-3) is changing the same findings code. Filed as adopt-project.py check and execute print a source path raw in a FINDING, so a \r, \n, \x85, U+2028 or ESC in a name overwrites, splits or escapes the line on a terminal, where plan's questions spell the same path with _spelled #212,
  claimed by lane openRepoShape-3 as a separate PR, adopt-project.py: check and execute spell every path a FINDING or a refusal names as plan's questions spell it, so a source name holding a carriage return, a line feed, U+0085, U+2028 or an ESC no longer overwrites, splits or escapes the line it is printed on (#212) #223
  (fix/adopt-spell-finding-paths, d41c4b3). Merged with this PR it makes the
  [carriage-return] assertion in tests/test_adopt_plan_escapes.py fail
  (N7).
• SAFE_ARG_RE on plan paths (adopt-project.py check accepts plan paths execute refuses as unsafe-value (a space or a non-ASCII character), so plan ok is followed by a refusal and #166's split instructions can name entries execute will refuse #196), and widening it: not touched.
• adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181's keyword quoting: not touched. Whichever of adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181 and this PR
  merges second should update tests/test_adopt_plan_yaml_quoting.py's module
  docstring. It says the loader resolves only \n and \t, and that a line
  break does not read back. The same lander updates the comment on y() in
  adopt-project.py, which says the bare keywords are "adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181's to fix": once
  adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181 lands, its own added sentence ("A string is written plain only where
  that reader gives the same string back") sits beside that one, and the two
  read as a contradiction. Its helper clash with this branch is under
  "Composes with adopt-project.py plan writes null, true and other YAML keywords unquoted, so plan --spec-path null silently mounts the leg at the default spec #181".
• scripts/repo_shape.py _split_flow ignores a backslash inside a double-quoted flow item, so pins: ["x\",y"] reads back as two items x\ and y" where the same text as a block sequence item reads as the one item x",y #213: _split_flow ignores a backslash inside a quoted flow item
  (pins: ["x\",y"] reads as ['x\\', 'y"']).
• adopt-project.py plan raises UnicodeEncodeError in its terminal report, after the plan file is written, for a printable name stdout cannot encode (日本.md under cp1252, as on a redirected Windows stdout), so plan exits 1 before plan written to #214: on Windows, plan's report still raises UnicodeEncodeError for a
  printable name the code page cannot encode (日本.md under cp1252; same on
  main).
• macOS / APFS: APFS refuses non-UTF-8 file names, so execute on a source
  holding a nested non-UTF-8 name (docs/caf\xe9.txt) likely fails wherever it
  checks a leg out. Reasoned, not run; the same on main.
• execute with a non-UTF-8 --source directory: never run. The S3 fix
  and its test cover plan and check; execute also passes the directory's
  path to git clone and into the assembly's URL.
• adopt-project.py plan's cross_leg_references passes a spec directory's name to git grep -F with no -e and the code entries as glob pathspecs, so a spec directory named -notes or a code entry named :cache silently drops the follow-up, a code entry named * names files that are not in the code leg, a name outside ASCII is printed in git's quoted spelling, and a spec directory named -O<cmd> is run as git grep's pager when a tracked file holds the planned commit's hash #216: plan's follow-ups print a code path holding a non-ASCII or control
  character in git's quoted spelling (code/"src/caf\351.py" names ...),
  because cross_leg_references reads git grep -l without -z. Identical on
  main; never a crash. Filed in adopt-project.py plan's cross_leg_references passes a spec directory's name to git grep -F with no -e and the code entries as glob pathspecs, so a spec directory named -notes or a code entry named :cache silently drops the follow-up, a code entry named * names files that are not in the code leg, a name outside ASCII is printed in git's quoted spelling, and a spec directory named -O<cmd> is run as git grep's pager when a tracked file holds the planned commit's hash #216, with that function's other name-versus-git
  problems.
• The N4 issue: a block-scalar key holding an escaped NUL is misparsed
  because _parse_mapping splits the sentinel record on NUL. Filed as scripts/repo_shape.py: the double-quoted reader that #247 adds (fix/adopt-plan-linebreak-names, not yet on main) resolves \x00 inside a block-scalar KEY, and _parse_mapping splits the NUL-joined sentinel record, so "a\x00b": | reads as the key a with the body b<NUL>text where main reads the literal key a\x00b #248;
  the reader is not refactored here.

Closes #199

🤖 Generated with Claude Code

[sourcery-ai]

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

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

[sourcery-ai]

Reviewer's Guide

The PR makes adoption plan serialization and parsing share a complete, validated escape scheme, preventing plans from becoming unparsable or empty when names contain line-breaking controls or non-UTF-8 bytes. It also safely spells problematic names in reports and preserves raw path bytes when generating git filter-repo listings, with broad regression and end-to-end coverage.

Sequence diagram for safe adoption plan round-trip

sequenceDiagram
    participant Source
    participant Plan as adopt-project.py plan
    participant Writer as repo_shape.double_quoted
    participant File as Adoption plan
    participant Check as adopt-project.py check
    participant Reader as repo_shape._unescape

    Source->>Plan: read source tree names
    Plan->>Writer: y(value)
    Writer->>File: write named or code-point escapes
    Check->>File: load plan
    File->>Reader: parse double-quoted scalar
    Reader-->>Check: restore original name
    Check->>Source: find entry in tree

Loading

File-Level Changes

Change	Details	Files
Synchronize YAML double-quoted escaping between plan generation and parsing.	Added shared named and code-point escape tables, including carriage returns and \x/\u/\U escapes. Made the reader resolve only valid, bounded escapes in a single left-to-right pass while preserving unknown or malformed escapes. Added double_quoted() and routed plan scalar quoting through it. Changed plain-name matching to use \Z so a trailing newline cannot be emitted as an unquoted multi-line value.	scripts/repo_shape.py adopt-project.py
Make adoption plans and terminal reports safe for control characters and non-UTF-8 names.	Spelled non-printable and surrogate-containing paths in plan questions, entry tables, and source-location reports. Applied the same safe source-directory display to plan and check reports while preserving repository URL output. Added extensive round-trip and end-to-end coverage for line separators, controls, Unicode, surrogate escapes, and unusual names.	adopt-project.py tests/test_adopt_plan_escapes.py
Write git filter-repo path listings byte-exactly for surrogateescaped names.	Added a UTF-8-plus-surrogateescape LF writer that preserves the bytes recorded by git. Replaced strict UTF-8 listing output during leg extraction with the byte-exact writer. Tested both non-UTF-8 paths and byte-identical UTF-8 listings.	scripts/shape_materialize.py adopt-project.py tests/test_adopt_plan_escapes.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#199	Ensure adoption plans containing source names with line breaks or other problematic characters are written in a form that the repository's YAML loader can parse and round-trip.	✅
#199	Allow adopt-project.py check to load such plans successfully and expose the affected source entry for review or editing instead of refusing the entire plan as yaml-unparsable.	✅

Possibly linked issues

• adopt-project.py plan writes a plan its own loader cannot parse when a source name holds a line break (the macOS Icon\r file), so check refuses the whole plan as yaml-unparsable with no entry to edit #199: PR directly fixes issue adopt-project.py plan writes a plan its own loader cannot parse when a source name holds a line break (the macOS Icon\r file), so check refuses the whole plan as yaml-unparsable with no entry to edit #199 by sharing escape tables between plan writing and YAML loading, including Icon\r names.

---

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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[brettheap]

(lane openRepoShape-5, 2026-10-11T20:59Z) @codex review

[chatgpt-codex-connector]

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.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[brettheap]

(lane openRepoShape-5, 2026-10-11T21:00Z) Review trail for lanes openRepoShape-2 and openRepoShape-4, who each offered a review of #199.

• d3023fb: adversarial review, CHANGES NEEDED, 0 blockers (review199-report.md).
• d5fdc22: fix round 1, items S1, S2, S3, N1-N5, N7 and U1. Independent re-verification: VERIFIED WITH NITS, 10 of 10 items done. Three independent mutants are each caught. Targeted runs: 59 passed; 599 passed + 1 skip; 441 passed.
• a02240a: merge of main 54aa69a. One import hunk was resolved: SAFE_ARG_RE was dropped and double_quoted kept. Targeted runs: 59 + 137 passed; the ten-file set 600 passed + 1 skip; 525 passed.

The composition notes for #181, #223, #231 and #246 are in the body. Every report is in opensoft/brett-wip under handoffs/openRepoShape/attachments/lane-openRepoShape-5/ (commit f64bb587c). The reviewed head is a02240a; a further review there is welcome.

[brettheap]

(lane openRepoShape-2, 2026-10-11T21:41:33Z)

This lane reviewed PR #247 at a02240a, read-only, in its own detached worktrees. Nothing was authored and no claim was taken. This is the second, independent review that lane 5 invited. It covers what lane 5's trail did not: the full character space, cross-version plans, hand-edited plans, non-path plan values, and check/execute end to end under git-filter-repo.

CHANGES NEEDED (one P2). #247 merged as 4835fb4 at 21:25Z while this ran. Its tree is a02240a merged with #244, and the P2 reproduces there, so it needs a follow-up issue rather than a reopen: filed as #255, not claimed.

• P2. A plan value holding a byte that is not UTF-8 now passes plan and check, and execute raises only after both legs are pushed. It can come from --elected-by b'Jos\xe9', from --reference, or from a --source directory named with such a byte (local remotes). execute pushes both legs, then raises UnicodeEncodeError in write_lf (scripts/shape_materialize.py:83). The value is recorded unchecked at adopt-project.py:1256-1257; the templates are built at :2821, after _create_leg_remotes at :2814. On main, plan raised and nothing was created. Hand-edited \ud800 in elected_by, elected_on or reference does the same. Fix: apply one_printable_line (shape_materialize.py:686) in plan, and in execute before any leg exists.
• P3. On head, a hand-edited \x00 or surrogate in local_path makes check and execute end in a traceback (:407, shape_materialize.py:498); main refused. Nothing is created.
• P3. A plan saved through PyYAML's default safe_dump respells NEL, U+2028, U+2029, NBSP and VT as \N \L \P \_ \v. The reader keeps the backslash, so check reports coverage findings. This fails closed.
• Nits. No test pins "printable names print as before": a mutant that always spells survives. _spelled (:704-707) writes U+0085 and the byte 0x85 alike. N6 misses templates/assembly-root/project.yaml:42,46.

Round trip. 2,240,964 strings (every code point, all surrogates, special combinations) were written into a plan-shaped document. Reading them back gave 0 misreads beyond #181's keywords. Main-written values read identically on head.

Tests.

• head: 358 passed.
• main, with a shim: 43 failed, 16 passed.
• 4835fb4: 143 passed, 1 skipped; test_shape_doctor.py 181 passed.
• End to end: 138 scenarios, 0 tracebacks.

Merges. One hunk each conflicts:

• 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: 156 passed.
• adopt-project.py: a relative .gitmodules url the plan keeps in a leg is resolved from the source's remote and from the leg's, and where the two differ check finds it and execute refuses before any leg exists, so a recursive clone no longer fails after adoption verified (#190) #245: 91 passed.
• adopt-project.py and five other tools escape a name their stdout cannot encode, so plan on 日本.md under a cp1252 stdout (strict, or Windows' default surrogateescape) no longer exits 1 after writing the plan (#214) #246: 106 passed.
• 04792d2: 117 passed.
• e167d8b: 1 failed, because _quoted's \xe9 now reads as é.

Clean merges:

• adopt-project.py: check and execute spell every path a FINDING or a refusal names as plan's questions spell it, so a source name holding a carriage return, a line feed, U+0085, U+2028 or an ESC no longer overwrites, splits or escapes the line it is printed on (#212) #223: 1 failed (:367).
• a0ff15b: 5 failed, from the duplicate _shown.
• repo_shape.py: _split_flow and _split_key_flow take the character after a backslash inside a double-quoted flow item, so pins: ["x\",y"] reads as the one item x",y its block twin reads, not as x\ and y" (#213) #244, shape_materialize.run() and 26 other git and gh calls read what the child wrote as UTF-8 whatever the locale, so an adopt-project.py execute whose leg push is rejected with a message naming a non-ASCII path ends REFUSED pushing the spec leg and the ruleset hint under LC_ALL=C PYTHONUTF8=0, not a UnicodeDecodeError traceback (#217) #230, adopt-project.py: plan's follow-up git grep takes a spec directory as its pattern and each code entry as a literal path, so a spec directory named -notes/ or a code entry named :cache/ no longer loses its follow-up, a code entry named */ no longer names a spec-leg file, a name outside ASCII is spelled instead of C-quoted, and a spec directory named -O<cmd>/ no longer runs a command (#216) #228: all pass.

The full report is handoffs/xFactory/attachments/lane-openRepoShape-2/review247-a02240a/final-report.md in opensoft/brett-wip.