Repository navigation
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
Merged
Merged
Conversation
…hild 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) `subprocess.run(..., text=True)` with no `encoding=` decodes a child's output with the LOCALE'S codec: ASCII under a POSIX `C` locale with UTF-8 mode off, the ANSI code page on Windows without PYTHONUTF8=1. Git writes a path as UTF-8, so a byte that codec cannot read raised `UnicodeDecodeError` inside `subprocess.run`, before `CommandFailed` existed. `execute` died with a traceback and exit 1 where `REFUSED pushing the spec leg` and the ruleset hint belong, with both leg remotes already created. Thirty text-mode calls in the shipped tools named no codec; this names one on twenty-eight of them. - `encoding="utf-8", errors="replace"` on 25 calls whose output is shown, parsed or compared: `shape_materialize.run()`, which carries most of the git calls `execute` makes; the commit helpers of bump-leg.py, family.py and update-shape.py; the `gh repo view` and `gh api` probes of scaffold-project.py, family.py and repo_shape.py; and the git calls of adopt-project.py (`_require_filter_repo`), bump-leg.py, scaffold-project.py, update-shape.py and the family root's bootstrap.py and siblings.py. A byte that is not UTF-8 is read as U+FFFD rather than raised. - UTF-8 and STRICT, no `errors=`, on the two `gh api` tree listings that become a digest (scaffold-project.py `pin_digest_from_gh`, repo_shape.py `tree_digest_from_gh`): a replaced byte would be a wrong digest stated as fact. - `errors="replace"` and no `encoding=` on bump-leg.py `run_validator`: the child is a Python process writing in the locale's own codec, so that codec reads it, and `replace` keeps two interpreters that disagree from turning a refusal into a traceback. - tests/test_git_output_decoding.py drives `run()` in a child interpreter started under `LC_ALL=C PYTHONUTF8=0` with a UTF-8 name and a non-UTF-8 one, and reads every shipped tool's `subprocess` calls so that a text-mode call naming no codec fails the suite. Two calls are left to sibling pull requests and listed in the guard's EXEMPT table, so no pair of branches carries a conflicting hunk: `shape_materialize.env_commit` (#196's branch gives it both arguments) and adopt-project.py `cross_leg_references` (#216's branch reads its `git grep` as bytes). Unchanged: in a UTF-8 locale the repro's `plan`, `check` and `execute` output is byte-identical to main's once the run directory is masked, and `execute` still exits 2. The Windows job already sets PYTHONUTF8=1 and skips the C-locale test. Left out: on a git without `commit --trailer`, a non-ASCII `--trailer` travels in the stdin message, which under `LC_ALL=C PYTHONUTF8=0` is now encoded with `replace` (`Jos??`) where main raised UnicodeEncodeError; carrying it as typed needs the commit helpers to encode stdin with `surrogateescape` apart from decoding their output. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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 5 hours by commenting |
Contributor
Author
|
(lane openRepoShape-3, 2026-10-11T19:05Z) @codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideThe PR prevents locale-dependent subprocess decoding from turning Git refusal paths into UnicodeDecodeError tracebacks by applying explicit UTF-8/replacement policies throughout the shipped tools, retaining strict decoding where bytes affect digests, and adding both runtime regression tests and an AST-based guard for future calls. Sequence diagram for locale-independent Git refusal handlingsequenceDiagram
actor User
participant Execute as adopt-project.py execute
participant Materialize as shape_materialize.run
participant Git
User->>Execute: execute --yes
Execute->>Materialize: run(git push)
Materialize->>Git: git push
Git-->>Materialize: UTF-8 stderr with non-ASCII path
Materialize-->>Execute: CommandFailed with decoded output
Execute-->>User: REFUSED pushing the spec leg
Execute-->>User: ruleset hint
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Author
|
(lane openRepoShape-3, 2026-10-11T19:24Z) independent review (codex is out of quota): ready; PR body updated with three composition and follow-up facts, no code change. |
This was referenced Oct 11, 2026
This was referenced Oct 11, 2026
Contributor
Author
|
(lane openRepoShape-2, 2026-10-11T19:57:58Z) Lane openRepoShape-2 reviewed PR #230 at b6f7cfd against main a5b09c3, read-only in its own detached worktrees, with nothing authored and no claim. MERGEABLE WITH NITS. Sequencing: whichever of #230 and Findings (no P1 to P3):
Tests:
Merges: 27 trial merges, against 54aa69a, the 24 open heads that touch these files, and the newer heads of #218 and #227. 25 are clean, and the guard passes on every clean merge except 04792d2. d5fdc22 and b0c959e conflict in adopt-project.py exactly as they do with main alone, and none of the conflicts is in this PR's lines. Full report: |
brettheap
added a commit
that referenced
this pull request
Oct 11, 2026
…rns HEAD to a non-ASCII start branch on Windows too; the branch HEAD is still on is never deleted after a switch back that failed; and the NEXT hint quotes every path and adds them literally, so a name holding `[` stages itself and no ignored neighbour (#202) Fix round 2 for PR #233, answering the four nits of the re-review at 396b946. NIT 1 (Windows only, a regression of fix round 1): `_symbolic_head` now reads HEAD through `_git_answer`, which decoded git's output with the locale's codec. Under the ANSI code page (Windows without PYTHONUTF8=1) a start branch `feature/café` came back as `feature/café`, the switch back named a branch that does not exist, and a name holding a byte that code page leaves undefined (`Ýmir`) raised out of the very answer that may not raise. The two-line change that fixes it, `encoding="utf-8", errors="replace"` on `_git_answer`'s `subprocess.run`, is PR #230's own (#217, fix/decode-git-output-utf8 @ fbdf8db), copied byte for byte and with none of its other hunks, so whichever of the two lands second merges it as a no-op: `git merge-tree --write-tree origin/fix/decode-git-output-utf8` over this head is clean. Pinned twice, neither depending on the platform's code page: the rollback's own undo run with every text-mode call that names no codec forced to cp1252 (both names, on every OS), and `_symbolic_head` in a child started under `LC_ALL=C PYTHONUTF8=0` (POSIX). NIT 2 (wording): `_by_hand`'s docstring said an ordinary root prints what it always printed. That holds on POSIX only: every Windows root holds a backslash, which `shlex` quotes. It now says so, and why that is left alone. The two smaller slips: `_index_by_hand` said `git reset` "would replace with HEAD's" an entry that may BE HEAD's, and `force_stage`'s first line left out adopt's seeded leg, which reaches it through `git_init_commit`. NIT 3: the hint `add -f -- {added}` was unquoted, and `-f` is what made that dangerous: pasted into a shell, an added `docs/guide[1].md` is a pattern the shell expands to an ignored `docs/guide1.md`, which `-f` then forces in. Quoting alone is not enough, because git reads a pathspec as a glob too: `git add -f -- 'docs/guide[1].md'` still stages the neighbour, as the first round's `force_stage` already found. So each added path is quoted with `shlex.join`, the helper `_by_hand` uses, and the add is `--literal-pathspecs`. The commit's own path list in the same line is quoted the same way, so the whole line pastes whole. An ordinary name prints exactly as it did. Tested by taking the line apart as a shell would on every OS, and by pasting it into `sh` from the root beside an ignored neighbour. NIT 4: when the switch back failed, `_undo_branch` still ran `update-ref -d` on the branch HEAD was attached to, which takes no index lock. HEAD then named a ref with no commit and `git status` said "No commits yet" over a repository with a history. The deletion is skipped while HEAD is still on the branch, and the partial-rollback report lists the branch as left, with the switch and then the deletion (`git branch -D` refuses the checked-out branch). Tested end to end through `apply`: a `post-checkout` hook takes `.git/index.lock` once the branch exists, so the staging fails and the switch back with it; once the lock is cleared the printed line makes the tree whole. Each new test was shown to fail against its regression, by mutants in a scratch worktree: `_git_answer` without its codec (4 failed), the hint with its added paths unquoted (2), without `--literal-pathspecs` (3), with the commit list unquoted (2), and the deletion not guarded (1). Not touched, and said so: an intent-to-add entry at an `--add` path (NIT 5 of the review), `git commit -- <paths>` reading each name as a glob in `commit_on_branch`, and the unquoted root in the NEXT lines, all of which predate this pull request. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 11, 2026
…very Windows stdout by default, now escapes a name it cannot encode as well and still writes a byte that is not UTF-8 back as that byte, so a redirected cp1252 `plan` on `日本.md` exits 0 on real Windows too; the root `bootstrap` shim calls the same helper WHAT WAS WRONG WITH 3006303. Its guard changed a stdout whose handler is `strict` and returned for every other one. On Windows a redirected stdout is never `strict`. CPython gives EVERY Windows stdout the `surrogateescape` handler by default: `config_get_stdio_errors()` in `Python/initconfig.c` ends `/* On Windows, always use surrogateescape by default */ return L"surrogateescape";` (3.9 and 3.12/3.13 alike), and `Doc/c-api/init_config.rst` says "Default error handler: On Windows: use "surrogateescape"". A stdout that is not the console gets the locale encoding, the ANSI code page. So the issue's own case, a file, a pipe to `tee`, CI or an agent on a Western-European Windows, is `cp1252:surrogateescape`. That handler covers only U+DC80..U+DCFF, so `日本.md` still raised, and the helper did nothing. The tests stood in for Windows with `cp1252:strict`, which Windows never gives by default (`PYTHONIOENCODING=cp1252` with no handler does, and still works). Reproduced by the review under `PYTHONIOENCODING=cp1252:surrogateescape`: `plan` exited 1 in `_print_entries`, unchanged from main. THE FIX. `escape_what_stdout_cannot_encode()` now changes two handlers and no others, and never the encoding: - `strict` becomes `backslashreplace`, as before. - `surrogateescape` becomes `openreposhape.surrogateescape-then- backslashreplace` (`SURROGATEESCAPE_THEN_BACKSLASHREPLACE`), registered with `codecs.register_error` by the first call that needs it and by no other. It writes a surrogate-escaped byte back as that byte, which is surrogateescape's own rule, and anything else the encoding cannot hold in `codecs.backslashreplace_errors`' spelling. It is the review's prototype, with the run of unencodable characters split into escaped-byte and other stretches, so `caf\udcff` followed by `日本` writes `caf` 0xff then `\u65e5\u672c`. - Every other handler (`replace`, `backslashreplace`, `ignore`, `xmlcharrefreplace`, `namereplace`, `surrogatepass`, one an operator set) is left as it was. Each replacement writes what the handler it replaces would have written and differs only where that one raised. In particular the inherited `utf-8:surrogateescape` of UTF-8 mode and the C locale still writes a name that is not UTF-8 as its own bytes. `日本.md` prints as `\u65e5\u672c.md` on a cp1252 stdout under either handler. The same applies to the `ascii:surrogateescape` of `LC_ALL=C PYTHONUTF8=0` (checked in process). The root `bootstrap` shim (not a shape copy: no copy list or `shape-pin.yaml` names it; projects hold `templates/*/scripts/bootstrap.py`) now imports and calls the helper before it hands over to the canonical bootstrap with `runpy`. Run in a directory named `日本` under cp1252 it exited 1 at `print(f"bootstrap: {name} ({root})")`. TESTS. tests/test_stdout_encoding.py, 29 tests: - Every tool run is now made under `cp1252:surrogateescape` (the Windows default), `cp1252:strict` and `utf-8:strict`. The tool runs are `plan` and `check`, the doctor, `update-shape.py check`, `bump-leg.py --dry-run`, the scaffold's and `family.py init`'s dry runs, and now the `bootstrap` shim. - `plan --out` into a directory whose name holds byte 0xff, under `utf-8:surrogateescape` and `cp1252:surrogateescape`: the raw byte comes out raw, and in the cp1252 run `日本.md` is escaped in the same report. POSIX filesystems only; skipped where a name that is not UTF-8 cannot be made. - In process: a surrogateescape stream in utf-8 and in cp1252 writes the raw byte and escapes the rest, with the encoding kept; a second call changes nothing; the handler is registered once, under a name that starts `openreposhape.`; six other handlers are left alone. - `reconfigure` raising a plain `ValueError`, a plain `OSError`, a closed stream, a stream with no `reconfigure`, and no stdout fail nothing. On 3006303 the same file gives 12 failed, 17 passed. The failures are every cp1252:surrogateescape run, each a `UnicodeEncodeError` (7 tool runs, `plan`, the raw-byte cp1252 case), and the three in-process tests that need the new handler. THE CALL'S PLACE. Moving the doctor's call after `parse_args` changes nothing anyone can observe, so no test tells the two apart. Before the first report line, only argparse writes to stdout, and only for `--help`. Every tool's and subcommand's `--help` is pure ASCII (17 outputs, 7 tools and 10 subcommands, under `ascii:strict`: all exit 0, no byte above 0x7f). The doctor's `prog` is fixed and its help interpolates nothing from the environment. A usage error goes to stderr, which is already `backslashreplace`. CORRECTIONS TO 3006303's MESSAGE (pushed, so not amended): - "`日本.md` prints as `日本.md`" lost its escapes; it is `\u65e5\u672c.md`. - "identical exit, stdout and stderr in every pair" holds for every pair that completes on main. Where main raised, its partial stdout is a byte prefix of the head's. - "ONLY `strict` IS CHANGED", and the Left-out line that called a `<code page>:surrogateescape` stdout an operator's explicit choice, are both superseded above. On Windows that handler is the default. LEFT OUT. A real `bump-leg.py` run, and `update-shape.py apply`, still fail under a code page that cannot hold the project's display name. Their child `validate-manifest.py` prints `manifest ok: <name>` with the inherited stdout and its own default handler. The fix belongs in those two tools' `run_validator`. `bump-leg.py`'s is the call #230 (fix/decode-git-output-utf8) rewrites, with the opposite decoding choice (the locale codec with `errors="replace"`), so it is left to a follow-up that composes with #230. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 11, 2026
This was referenced Oct 11, 2026
This was referenced Oct 11, 2026
brettheap
added a commit
that referenced
this pull request
Oct 11, 2026
…erator's own excludes, and a branch, stage or commit that fails before the commit lands now puts back the copies, the pin, the README line, the index and the branch and names the step, so `check` no longer exits 0 over an uncommitted re-pin (#202) (#233) update-shape.py `apply --add` stages its additions through `shape_materialize.force_stage` (`git --literal-pathspecs add -f --`, on the named paths only), so an addition the operator's own excludes ignore is committed. Any failure up to and including the commit puts back every byte it wrote (the copies, the pin, the README line, the index records, HEAD and the branch) and names the step that failed. - The `Rollback` ledger records the branch `apply` creates and the index entries it forces. A failure after the commit lands rolls nothing back, and `--branch` naming the current branch keeps it. - A partial rollback says what was left and prints a pasteable fix per path (`update-index`, never `git reset`). A failed switch back keeps the branch HEAD is still on. - The NEXT hint stages with `--literal-pathspecs` and quoted paths. - `_git_answer` decodes UTF-8, #230's own one-line change, so a non-ASCII branch survives the rollback on Windows. - `tests/test_update_shape_add_ignored.py` (new, 26 tests). Reviewed adversarially twice (MERGEABLE WITH NITS), with two fix rounds. CI is green on Linux, macOS and Windows. Closes #202 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
brettheap
pushed a commit
that referenced
this pull request
Oct 11, 2026
…m, not from a codec setting, so they keep meaning the same thing once `main` escapes what a strict stdout cannot encode (#253) The first cut of the echo test ran `bump-leg.py` with `PYTHONIOENCODING=cp1252`. #214's `escape_what_stdout_cannot_encode` reconfigures exactly that stream to `backslashreplace` at the top of `main`, so that test would have started to see a bump that succeeds the day #214 landed. The echo test is now in-process with a stdout whose `write` raises `UnicodeEncodeError` for what cp1252 cannot hold, beside a `UnicodeDecodeError` raised from the validator read, so both ways a validator's output can fail the statement after the staging are covered whichever of #214 and #230 is in. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This was referenced Oct 11, 2026
brettheap
added a commit
that referenced
this pull request
Oct 11, 2026
…s body as a pair instead of NUL-joined text, so `"a\x00b": |` no longer reads as the key `a` with the body `b<NUL>text`, and a line that begins with a raw NUL and `BLOCK` no longer reads as a block scalar nobody wrote (#248) (#265) The YAML reader kept a block scalar (`key: |`, `key: >`) as one `_Rec` whose text was `"\x00BLOCK\x00" + key + "\x00" + body`. `_parse_mapping` took it apart with `split("\x00", 3)`, and both `_parse_mapping` and `_parse_sequence` recognised it by that text prefix. Now: - `_Rec` has a `block` slot holding `(key, body)`. It is set only on the record a block scalar's header makes, and that record's `text` is empty. - `_tokenise` builds the record with the pair. `_parse_mapping` and `_parse_sequence` test `rec.block`, not a text prefix. - `_BLOCK_SENTINEL` is gone. Its comment, which named the collision as a known limit, becomes a `_Rec` docstring saying why nothing can collide now. No other function in the file changes, and no writer changes. Since #199 the reader resolves `\x00`, `\u0000` and `\U00000000` in a double-quoted key, which is valid YAML. A key holding a NUL was cut short at the NUL, and the rest of it read into the body. A raw NUL did the same before #199. The marker was also only a text prefix, so an ordinary line beginning with a raw NUL + `BLOCK` + NUL was taken for a block record: | document | main (150c41d) | this branch | |---|---|---| | `"a\x00b": \|` | `{'a': 'b\x00text\n'}` | `{'a\x00b': 'text\n'}` (PyYAML agrees) | | `"a\x00b": text` (control) | `{'a\x00b': 'text'}` | same | | `<NUL>BLOCK<NUL>k<NUL>v: 1` | `{'k': 'v: 1'}` | `{'\x00BLOCK\x00k\x00v': 1}` | | `- <NUL>BLOCK<NUL>x` | bare `ValueError` out of `load_yaml` | `['\x00BLOCK\x00x']` | | `k:\n <NUL>BLOCK<NUL>x<NUL>y` | `{'k': {'x': 'y'}}` | `YamlError`, as `k:\n xyz` is | A structure that is not a string has no character to collide with, so this removes the class and not just `\x00`. A different separator only moves the collision, and refusing a NUL in a block key leaves the prefix misreads in place (a `\x01` separator and a NUL refusal were each tried as mutants, and both are caught by the new tests). **A NUL in a key is read, not refused.** YAML allows it escaped. PyYAML 6.0.3 reads every escaped row the way this branch does. The reader already read the same key over a plain value, and `double_quoted` writes a NUL as `\x00`. A raw NUL, which YAML forbids, is read the way the reader reads a raw NUL in any other key (`a<NUL>b: text` reads as `{'a\x00b': 'text'}` on main). Refusing raw control characters is a reader-wide decision and is left out (see below). Issue row 2 (`"a\^@b": >`) does not reproduce, not even on a02240a: `\^@` is no escape the reader (or YAML) resolves, so the key keeps its backslash on main and here. It is pinned as a control. - `tests/test_yaml_block_scalar_keys.py` (new, 51 tests). On 150c41d: 41 fail and 10 pass, and the 10 that pass are controls. On this branch: 51 pass. - Targeted runs on 9be6287: - the new file plus `test_yaml_flow_quotes.py`, `test_load_yaml_refusals.py`, `test_adopt_plan_escapes.py`, `test_manifest_validator.py`, `test_pin_validator.py`, `test_neutral_pin_validator.py`, `test_repo_hygiene.py`, `test_cli_reference.py` and `test_adopt_plan.py`: 326 passed, 1 skipped (`SHAPE_MEDXEHR_CLONE` unset); - `test_shape_doctor.py`: 181 passed. - Mutation: each fix hunk reverted on its own (`_tokenise`, `_parse_sequence`, `_parse_mapping`, `_Rec`) fails 46, 4, 48 and 51 tests. Of the narrow alternatives, refusing a NUL in a block key fails 41, including every marker-prefixed-line test. A `\x01` separator fails the every-character round-trip test. Dropping the sequence check fails the placement test, and taking the pair reversed fails 46. - Every code point in a written block key, `double_quoted("a" + chr(c) + "b")` under `|` and `>`: all 2,228,224 documents read back, and PyYAML agrees on every one. Main misreads exactly 2 of them, both NUL. - A base-against-head fuzz of 200,000 documents built around block scalars finds 0 differences outside the class. All 23,298 documents with no NUL in any spelling read the same. Over the same corpus, main lets 4,250 bare `ValueError`s out of `parse_yaml`, and this branch none. - Reach: all 14 tracked YAML files read the same on main and here, rendered templates too. Block scalars in `tests.yml`, `path-classification.yaml` and both `validate.yml` templates read unchanged. **No writer in this repository emits the construct:** - no shipped Python writes a block-scalar header; - `adopt-project.py`'s keys are literals, and its one variable key comes from `summary_of`'s fixed names; - `y` never writes `|`, `>` or a raw NUL plain; - the doctor's placement plan uses `y`/`emit`, and `family.py`, `update-shape.py` and `bump-leg.py` write literal keys. So the defect shows only on a file a person edits or another tool writes. Every YAML file every tool reads goes through this reader. `git merge-tree --write-tree` from 9be6287: - origin/main 60d08a1 (with #214 and #230): clean. - `fix/yaml-quote-boundaries` (#249/#250) at 21fc213: clean; the review also passed 363 tests on this branch merged with it and #252 together. - `fix/yaml-flow-mapping-keys` (#252) at 2438190: clean. The merged tree passes this file, `test_yaml_flow_quotes.py` and `test_yaml_flow_mappings.py` (114 passed). - `fix/scaffold-referent-chain-links` (#251) at 114bec9: clean. - **Refusing the raw control characters YAML forbids** (PyYAML: `unacceptable character #x0000`). The reader takes them everywhere today, and `double_quoted`'s docstring says so. A refusal belongs to the whole reader, not to one path. The plain-twin tests here say those rows become refusals together if that changes. A follow-up issue is proposed. - Honouring `+` chomping and the indentation digit is documented as accepted, not honoured, and is unrelated. Reviewed adversarially (APPROVE WITH NITS, 0 blockers; a 2,228,224-code-point oracle and 840,000 fuzzed documents show no misread). CI is green on Linux, macOS and Windows. Closes #248 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 12, 2026
…, #252 via #257, #195 via #226, #210 via #219, #211 via #220, #248 via #265, #212 via #223) into fix/adopt-record-submodule-placement Brings in 06b5204 (#214), 60d08a1 (#217), 2727b08 (#209), 4b2add8 (#252), aaa40eb (#195), d89fffa (#210), eb6df75 (#211), c3f4987 (#248) and 5751460 (#212) on top of 39ac59c. No rebase. 5751460 landed while the merge of c3f4987 was being checked, and it changes adopt-project.py, scripts/shape_materialize.py and tests/test_adopt_submodules.py, as this branch does, so this one merge takes it too. No conflicts. Both sides change adopt-project.py, scripts/repo_shape.py, scripts/shape_materialize.py, shape-doctor.py, tests/test_adopt_submodules.py and tests/test_shape_doctor.py. Every top-level definition only one side changed is that side's text in the merge, byte for byte. The one thing both changed is shape_materialize.py's imports, and both are kept: main's `codecs` and this branch's `path_spelling_fault` and `PATH_*` codes. Checked: - adopt-project.py: main calls `escape_what_stdout_cannot_encode()` first in `main` (#214). It makes a `.gitmodules` git's submodule reader cannot read a finding where the tip holds no submodule and the history does, or where `ignore`, `update` or `branch` has no value (#195). And every path a finding names is now `_spelled` (#212). This branch's `submodule_record`, `_resolution_text` and the record's preamble are unchanged by the merge, and so are `_answered_legs`, `_kept_in_a_leg`, `_commit_the_split`, `_refuse_what_check_finds`, `cmd_execute` and `Plan`. The record and #195's finding read the kept submodules the same way, `_kept_in_a_leg(_answered_legs(...))` over the tip's gitlinks. `execute` refuses #195's finding in `_refuse_what_check_finds`, before any leg exists and before `_commit_the_split` writes the record. So a source #195 refuses gets no record, and so does a source whose tip holds no submodule. - scripts/repo_shape.py: main refuses an explicit key in a flow mapping and reads an empty flow-mapping value as null (#252). A block scalar's record is now a (key, body) pair, and `_BLOCK_SENTINEL` is gone (#248); nothing on this branch names it. The record is a block sequence of double-quoted scalars, and it reads back through the merged reader with 0 mismatches (`submodule_record`, then `parse_yaml`, path and resolution): - 4,525 names, plus 35 more; - 240 aimed at #252 and #248 (`? a`, `{a: }`, `|`, `a: |`, a NUL, the old sentinel). - scripts/shape_materialize.py: main adds `escape_what_stdout_cannot_encode`, `run()` reading UTF-8 (#217), and `first_and_more(paths, spell)` (#212). This branch's `LEG_SEGMENT_REASONS` and `_spelling_reason` are untouched. - shape-doctor.py: main adds the `manifest kinds` finding for a `family.yaml` beside `project.yaml` (#211), and the stdout escaping. This branch's placement row and `recorded_submodules` are untouched. - tests: #212 respells the finding text that the earlier tests in test_adopt_submodules.py assert. None of this branch's tests quotes a finding. - 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 or update-shape.py. Each merged module's names are the base's, plus what either side added, minus what either side removed. No side added a name the other side also added, in the tests either. - AGENTS.md is untouched by main: 472 lines, under the cap of 472. - The branch's diff against main is still #189 alone: 12 files, +1670 -156, file by file the same as against 39ac59c. 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
…, #209, #252, #195, #210, #211, #248, #212, #251 and #206 merge with one hunk, the materializer import list, kept on both sides, and `_entry_findings` still hands `_leg_findings` the tree and ends with the root-symlink findings origin/main moved 39ac59c -> 60588d5: 06b5204 (#214 via #246), 60d08a1 (#217 via #230), 2727b08 (#209 via #218), 4b2add8 (#252 via #257), aaa40eb (#195 via #226), d89fffa (#210 via #219), eb6df75 (#211 via #220), c3f4987 (#248 via #265), 5751460 (#212 via #223), 23e16ae (#251 via #271) and 60588d5 (#206; its subject names no pull request). Conflict, resolved by keeping both sides: - adopt-project.py, the `shape_materialize` import list. This branch adds BOOTSTRAP, SYMLINK_HOPS and template_files; #214 adds escape_what_stdout_cannot_encode. The resolution is the union. Auto-merged: scripts/shape_materialize.py, where #214's escaping, #217's UTF-8 `run()` and #212's `first_and_more(paths, spell=None)` sit apart from this branch's #183 section, and adopt-project.py's other hunks. #195 adds `import os` at the line where this branch added it, and the merge keeps it once. Checked on the merged tree: - `_entry_findings` still calls `_leg_findings(plan, tree)` and ends with `_root_symlink_findings(plan, source, tree)`. `main` still calls `escape_what_stdout_cannot_encode()` first. - 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 (AST). `first_and_more` gives its new `spell` a default, so this branch's call is unchanged. - Both sides are intact by +/- line sets: each side's change against the base reappears against the other side, apart from the import hunk and the shared `import os`. - #212 says this branch's `plan-root-symlink` names its paths raw, and that whichever pull request lands second should spell them. That was 04792d2. Since d11cf9f every path the finding names goes through `_spelled`. One clash the textual merge does not show is fixed in the next commit: #217's tests/test_git_output_decoding.py refuses a shipped text-mode `subprocess` call that names no codec, and this branch's `_checks_out_symlinks` probe names none. 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
…explicit PYTHONIOENCODING, so a real bump or `apply` on a project named `日本` is no longer refused as `bump-leg-validators-red` or `update-validators-red` under a cp1252 stdout (#254) (#281) A real `bump-leg.py` (not `--dry-run`) and `update-shape.py apply` run the PROJECT'S own `scripts/validate-pins.py` and `scripts/validate-manifest.py` as child processes after they rewrite the pin. A child takes its stdout configuration from its environment (`PYTHONIOENCODING`, then the locale), not from the `reconfigure` its parent did in #214, and the validators are digest-pinned shape copies that print the display name with a plain `print`. So on a stdout whose code page cannot hold the name (`PYTHONIOENCODING=cp1252`, or the cp1252 with `surrogateescape` that Windows gives any stdout that is not the console), `validate-manifest.py` raised `UnicodeEncodeError` at `manifest ok: <name>`, exited 1, and the tool refused and rolled back a bump the pins had no quarrel with. The refusal blamed the validators. - `scripts/shape_materialize.py`: two helpers build the child's environment, and each tool passes the one it needs as `env=`. - `captured_child_environment()` sets `PYTHONIOENCODING=utf-8:backslashreplace`. - `inherited_child_environment(stream=None)` sets the stream's OWN canonical encoding with its own handler where that handler never raises (`replace`, `ignore`, `backslashreplace`, `xmlcharrefreplace`, `namereplace`), and `backslashreplace` for every other one. - `scripts/bump-leg.py` `run_validator` CAPTURES the output. It passes `env=captured_child_environment()` and reads the pipe with `encoding="utf-8", errors="replace"`, where #230 had `errors="replace"` and no `encoding=`. - `update-shape.py` `run_validator` lets the child INHERIT stdout. It passes `env=inherited_child_environment()`. - `tests/test_git_output_decoding.py`: the `bump-leg.py run_validator` row leaves `EXEMPT`, because that call now names both `encoding` and `errors`. - `tests/test_validator_child_encoding.py`, 51 new tests. **Captured case: the child writes UTF-8, and the tool reads UTF-8.** #230 gave `run_validator` `errors="replace"` and no `encoding=`, "so that codec is the one that reads it back". That was right only while the child wrote in the locale's codec. Once the child is told UTF-8, the locale's codec misreads it: `'日本'.encode('utf-8')` read as ascii is six U+FFFD, and read as cp1252 is `'\xe6\u2014\xa5\xe6\u0153\xac'`. So `encoding="utf-8"` is explicit. Dropping it fails `test_a_real_bump_reads_the_validators_as_utf_8_whatever_the_locale_is` (mutant `bump-decode-with-locale`). The words then reach the person through the tool's own stdout, which #214 made safe: `manifest ok: \u65e5\u672c` on a cp1252 stdout, `manifest ok: 日本` on UTF-8 and cp932. **An operator's `PYTHONIOENCODING` is replaced for the captured pipe and respected for the inherited one.** The operator's value describes the stream their own words land in. The captured pipe is the tool's, so respecting `cp1252:strict` there would put the failure back one pipe away from the console. The inherited fd IS the operator's stream, so the child is told what that stream is. The tests run under `cp1252:strict` and `cp1252:surrogateescape` (the brief's two), plus `cp1252:replace` (an operator's own lenient handler: `??` before and after), `cp932:strict` (a Japanese Windows) and `utf-8:strict`. **Inherited case: the stdout's own encoding, never UTF-8, and built-in handler names only.** UTF-8 bytes on a cp1252 pipe would be a different mistake. A child cannot use a handler only its parent registered: `PYTHONIOENCODING=cp1252:openreposhape.surrogateescape-then-backslashreplace` still STARTS the child and fails with `LookupError` at the first character it cannot encode. `test_the_handler_a_child_would_not_know_is_never_passed_on` runs that child and asserts both halves. At the point the child is built, `main()` has already turned a `strict` stdout into `backslashreplace` and a `surrogateescape` one into the registered handler, so the helper maps `strict`, `surrogateescape`, `surrogatepass` and the registered name to `backslashreplace`, and keeps a handler that never raises. One difference, and it is small: a byte of a file name that was not UTF-8, read in as U+DC80..U+DCFF, is written by a validator as the text `\udcff` where the tool writes the byte back. A validator prints one only for a path it lists. Fails on 60d08a1: with the new test files copied into a throwaway worktree at 60d08a1, **42 failed, 12 passed** (the 12 are controls that pass either way: the five `--dry-run` bumps, `cp1252:replace` and `utf-8:strict` for both tools, `cp932:strict` for `apply`, and #230's own two C-locale tests). The decisive lines: - bump, `cp1252:strict` and `cp1252:surrogateescape`: exit 2, `REFUSED bump-leg-validators-red: scripts/validate-manifest.py exited 1 after the rewrite`, `UnicodeEncodeError: 'charmap' codec can't encode characters in position 13-14`. - apply, the same two: exit 2, `REFUSED update-validators-red: after the rewrite, scripts/validate-manifest.py refused.` - the guard in `test_git_output_decoding.py` flags `run_validator` once its EXEMPT row is gone. Passes on the head (5757e29): - the new file (51), `test_stdout_encoding.py` (48), `test_bump_leg.py` (23), `test_update_shape.py` (77), `test_update_shape_add.py` (17), `test_git_output_decoding.py` (3), `test_repo_hygiene.py` (50) and `test_cli_reference.py` (13): **281 passed, 1 skipped** (the existing Windows-only default-stdout test); - `test_shape_doctor.py`: **181 passed**; - `test_family.py` and `test_update_shape_add_ignored.py`: **102 passed**; - the head merged with main d89fffa, on the files above plus `test_update_shape_pr_base.py`: **392 passed, 1 skipped**. **One extra finding, fixed by the same two changes.** Under `LC_ALL=C PYTHONUTF8=0` (an ASCII stdout; UTF-8 mode has to be switched off by hand, as it is on by default in a C locale) `validate-pins.py` raises on the `…` (U+2026) it prints, so on 60d08a1 a real bump of a project named `Atlas` was refused as well: exit 2, `REFUSED bump-leg-validators-red: scripts/validate-pins.py exited 1`, `UnicodeEncodeError: 'ascii' codec can't encode character '\u2026'`. On the head the same run exits 0. This is why the C-locale bump test fails on 60d08a1 at the PINS validator. **Emulation caveat.** The `cp932:strict` BUMP also fails on 60d08a1 when the test runs on Linux, but only as an artefact: the child wrote cp932 and a UTF-8 locale cannot read it, where a Japanese Windows' own locale could. It is not the defect and is not cited as one. The Windows-specific runs here are emulations (`PYTHONIOENCODING=cp1252:surrogateescape`); **I did not run anything on Windows**, and these tests first run on Windows in this PR's `tests-windows` job. The two C-locale tests are POSIX-only (`skipif os.name == "nt"`) and skip where the platform reads UTF-8 under `LC_ALL=C PYTHONUTF8=0`. **Mutation probes.** Eleven hunks reverted one at a time in a throwaway worktree, the whole new file run each time: all eleven killed. | hunk reverted | failed | first killers | |---|---|---| | `captured_child_environment` sets nothing | 6 | both `captured_child` unit tests, bump `cp1252:strict`, `cp1252:surrogateescape`, `cp932:strict`, the C-locale bump | | captured value `utf-8` with no handler | 2 | the two `captured_child` unit tests | | `bump-leg.py` drops `env=` | 5 | the three bump cp1252/cp932 runs, the C-locale bump, the AST guard | | `bump-leg.py` drops `encoding="utf-8"` | 1 | the C-locale bump | | `update-shape.py` drops `env=` | 4 | apply `cp1252:strict`, `cp1252:surrogateescape`, the C-locale apply, the AST guard | | inherited handler passed verbatim | 14 | `strict`/`surrogateescape`/`surrogatepass` unit and child cases, apply `cp1252:surrogateescape`, the C-locale apply | | inherited always `backslashreplace` | 11 | `ignore`/`namereplace`/`replace`/`xmlcharrefreplace` cases, apply `cp1252:replace` | | `replace` dropped from the never-raises set | 5 | the `replace` cases, apply `cp1252:replace` | | inherited encoding forced to `utf-8` | 27 | every non-UTF-8 case | | inherited encoding not canonicalised | 3 | `ANSI_X3.4-1968`, `latin-1`, the no-stdout case | | inherited drops the no-stdout guard | 1 | `test_a_stdout_that_is_not_there_leaves_the_environment_alone` | **Byte-identical where nothing was wrong.** A real bump and an `apply`, from 60d08a1 and from the head, on the same project, under `utf-8:strict`, `utf-8:surrogateescape` and no `PYTHONIOENCODING`, named `Atlas` and named `日本`: twelve pairs, stdout, stderr and exit code identical once the run directory and the shas are masked. **Sibling merge-trees** (`git merge-tree --write-tree HEAD <branch>`, local tips of lane 5's branches and every open branch that touches `bump-leg.py`, `update-shape.py` or `shape_materialize.py`): clean against `fix/yaml-quote-boundaries`, `fix/yaml-block-key-nul`, `fix/yaml-flow-mapping-keys`, `fix/scaffold-referent-chain-links`, `fix/adopt-check-unsafe-path`, `fix/adopt-plan-quote-yaml-keywords`, `fix/adopt-record-submodule-placement`, `fix/adopt-template-values-utf8`, `fix/bump-leg-rollback-every-failure`, `fix/bump-leg-tracked-workflows-only`, `fix/leg-path-yaml-scalars`, `fix/update-shape-add-dangling-symlink`, `fix/update-shape-pr-base-branch` and `fix/update-shape-pr-create-failure`. One conflicts: `fix/adopt-root-kept-symlinks` (`adopt-project.py` and `scripts/shape_materialize.py`), and it conflicts the same way with origin/main itself, so it is not this change. - The shape copies a project runs itself. `scripts/bootstrap.py`, `scripts/validate-manifest.py` and the family root's `siblings.py` are digest-pinned and print the display name raw, so `make bootstrap` and `make validate` on a project named `日本` fail under a cp1252 stdout at their own `print`, before any child is run (checked: `bootstrap.py` raises at `bootstrap: <name> (<root>)`). That is a shape change, and it reaches projects through `update-shape.py`. - `shape-doctor.py`'s `run_validator` already sets `PYTHONIOENCODING=utf-8` for the same validators and reads them as UTF-8, and it is unchanged. A display name holding a lone surrogate (`name: "a\ud83db"`) reaches it today. The project's own `validate-manifest.py` accepts that name, then raises printing it, so `make validate` and the doctor fail on a traceback (`INVALID (manifest)`). After this change, a real bump and `apply` print `manifest ok: a\ud83db` and pass. That is pre-existing, and the fix belongs in the validator or the doctor, not in these two callers: #280. An adversarial review of 5757e29 found no blocker. Its one should-fix was a false sentence in this body's "Left out", corrected above and filed as #280. - It ran the inherited case on Python 3.10, 3.12, 3.13, 3.14 and 3.15rc2, including UTF-8 mode and the C locale. In each 28-run cell the head had 0 failures; main had 8, or 12 under `LC_ALL=C PYTHONUTF8=0`. - It compared 30 real runs between main and the head under UTF-8: stdout and stderr were identical. - The head merged with c3f4987 passed: 264 passed, 1 skipped. Three nits are kept as they are: - **The inherited child's stderr.** It follows the parent's STDOUT encoding, because `PYTHONIOENCODING` sets both streams. With a console stdout and a redirected stderr, the child writes UTF-8 to a cp1252 stderr. - **`errors="replace"` in `bump-leg.py`'s `run_validator`.** It is defensive. The child now always writes UTF-8, so a `strict` mutant survives every test. - **The `\udcff` text.** This is the difference stated above. The first head, 5757e29, failed `tests-windows`: 5 failed and 2477 passed, and macOS and Linux passed. All five failures were `test_an_apply_on_a_project_named_in_japanese_lands_under_any_stdout`. Its `assert_the_update_landed` compared the applied copy with the upstream clone's WORKING-TREE file. On Windows that file is CRLF, because the shared test helper `commit_upstream_changes` rewrites it with text-mode `write_text`. Git stores the blob as LF, which is what `apply` writes. The test now compares with the committed blob (`git show <b>:<path>`), as `test_update_shape.py` already does, and no product code changed. - **Emulation on Linux.** The helper was made to write CRLF: the old assertion failed 6 `apply` cases, and the new one passed them. - **Targeted runs.** The seven targeted files gave 274 passed and 1 skipped on the head merged with main b30595c (97c4280). - **A candidate follow-up.** `commit_upstream_changes` leaves the upstream clone's working-tree copy CRLF on Windows for the rest of its module. Writing the appended bytes in binary would remove the trap at its source, but the helper is shared and outside this change. Reviewed adversarially (APPROVE WITH NITS, 0 blockers); its one should-fix was a sentence in this body, corrected and filed as #280. Fix round 1 compares the Windows test with the committed blob. CI is green on Linux, macOS and Windows. Closes #254 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap
added a commit
that referenced
this pull request
Oct 12, 2026
…e-notice It merged with no conflict. Main brings nine commits since 39ac59c: - 06b5204, six tools escape a name their stdout cannot encode (#214 via #246), with tests/test_stdout_encoding.py; - 60d08a1, `shape_materialize.run()` and 26 other git and gh calls read the child's output as UTF-8 (#217 via #230), with tests/test_git_output_decoding.py; - 2727b08, update-shape.py's `gh pr create --base` (#209 via #218), with tests/test_update_shape_pr_base.py; - 4b2add8, scripts/repo_shape.py's flow mappings (#252 via #257), with tests/test_yaml_flow_mappings.py; - aaa40eb, adopt-project.py refuses a `.gitmodules` the history's gitlinks make unreadable (#195 via #226), with tests/test_adopt_history_gitlinks.py; - d89fffa, family.py `bump` to the current pin (#210 via #219); - eb6df75, shape-doctor.py's family.yaml beside a project.yaml (#211 via #220); - c3f4987, scripts/repo_shape.py's block-scalar records (#248 via #265), with tests/test_yaml_block_scalar_keys.py; - 5751460, adopt-project.py spells every path a FINDING or a refusal names (#212 via #223), with tests/test_adopt_finding_spelling.py. Three files changed on both sides: adopt-project.py, tests/test_adopt_ignored_shape_files.py and tests/test_adopt_verify.py. In each, `git diff -U0` of this merge against 5a8b649 equals main's diff against 39ac59c, and this merge against 5751460 equals the branch's diff against 39ac59c, apart from the hunk headers. Neither side's lines are changed. How main's changes meet this branch's failure handling: - Every definition the late failures go through is byte-identical to 5a8b649's: LATE_FAILURES, the notice (`_nothing_rolled_back`), `_failed_loudly`, `_report_late`, the three builders' handlers, `_verify` and `cmd_execute` among them. Three definitions differ, each by main's hunk alone. - `main()` gains `escape_what_stdout_cannot_encode()` as its first statement (06b5204). The branch writes every REFUSED block and notice to stderr, which Python opens with `backslashreplace` whatever the locale. - `run()` names its codec (60d08a1). Output that is not UTF-8 now reaches the branch's handlers as `CommandFailed`, not as `UnicodeDecodeError`. LATE_FAILURES keeps `UnicodeError`, because `git_out` in scripts/repo_shape.py still decodes strictly. - `Source.registered_submodules` reads the history (aaa40eb). Its one caller in `execute` is `_refuse_what_check_finds`, before `_create_leg_remotes`, so the new refusal comes before any leg repository exists and needs no notice. - 5751460 spells the paths of the verification's findings. The branch's footer and notice follow those findings unchanged. No top-level name is bound twice in adopt-project.py, scaffold-project.py, scripts/repo_shape.py or scripts/shape_materialize.py, and a `symtable` check finds no name they read that nothing binds. On the merged tree, ten test files give 439 passed and 1 skipped (Windows' own stdout default): the two shared ones, main's four new ones that read adopt-project.py (finding_spelling, history_gitlinks, stdout_encoding, git_output_decoding), and test_adopt_late_failure, test_adopt_filter_repo_failure, test_adopt_execute_refusals and test_adopt_plan_escapes. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 12, 2026
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>
This was referenced Oct 12, 2026
brettheap
added a commit
that referenced
this pull request
Oct 13, 2026
…-ASCII value into the commit as typed under any locale and on a git without `commit --trailer`, and refuses one whose bytes are not UTF-8 by name before anything is staged, so `Co-Authored-By: José` under `LC_ALL=C PYTHONUTF8=0` is no longer refused as "not a trailer" and a direct caller's is no longer committed as `Jos??` (#322) (#376) Mechanism. Under a POSIX `C` locale with UTF-8 mode off `sys.argv` is decoded as ASCII with `surrogateescape`, so the two bytes of `é` arrive as the lone surrogates `\udcc3\udca9`. Measured on c7c02fa, two things happened to such a trailer. At the command line `trailer_line` refused it as "not a `<Key>: <value>` trailer" (exit 2, any git), because `one_printable_line` rejects category `Cs`. Handed straight to a commit helper, with `git_takes_trailer_option` forced False (git 2.43 here has the option, so an older git was emulated), `commit_trailers` appended it to the stdin message and the helper's `errors="replace"` (#230) wrote each surrogate as `?`: `Co-Authored-By: Jos??`. The issue named the second; the first is what the three tools actually do. - `argv_text` puts the bytes back: `os.fsencode` gives them, and if they decode as UTF-8 the value is the text (`José`), the same string a UTF-8 locale hands over. A value with no escaped byte is returned as it is. - `trailer_line` recovers first and judges the text after, so a trailer is accepted or refused as the characters it is: bytes that spell U+0085, U+2028 or a zero-width joiner are still refused. - A byte that is not UTF-8 (a Latin-1 `\xe9`, a cut multi-byte sequence) is refused at the command line, exit 2, naming the trailer by its bytes (`b'Co-Authored-By: Jos\xe9'`) and saying why, instead of "not a trailer". - `commit_trailers` recovers too and raises `Refusal("trailer-not-utf8")` for a trailer that still holds such a byte, on both paths, so a caller that skips the command line's check cannot commit a `?`. `bump-leg.py`'s `commit_once` and `family.py`'s `_commit_one` call it ahead of their stage, so the refusal leaves nothing staged; `update-shape.py apply` raises it inside its ledger, which rolls the branch and the copies back. - The `--trailer` argument given to a git that takes it is `argv_spelling`: the string `os.fsencode` turns back into the UTF-8 bytes under any locale (the text under a UTF-8 one, nothing under Windows, which passes UTF-16). Without it the recovered `é` is not encodable under a `C` locale and `subprocess` raises. Unchanged: an ASCII trailer takes exactly the paths it took, byte for byte (`argv_text` and `argv_spelling` return it as it is, and `test_no_trailers_leaves_the_commit_argv_byte_identical` and the order and equality tests pass untouched); the commit helpers' `errors="replace"` on the message BODY stays as #230 wrote it, and none of the three call forms changed, so `tests/test_git_output_decoding.py` is untouched. Left out: the body of a message still writes a lone surrogate as `?` (a path a plan names, #230); no `--help` text changed, so `docs/cli.md` is not regenerated. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lane: openreposhape-3 (openRepoShape-3)
Claim: #217 (comment)
What
subprocess.run(..., text=True)with noencoding=decodes what the child wrote with the LOCALE'S codec. That is ASCII under a POSIXClocale with UTF-8 mode off, and the ANSI code page on a Windows withoutPYTHONUTF8=1. Git writes a path as UTF-8. When a byte cannot be decoded,subprocess.runraisesUnicodeDecodeErrorbeforeCommandFailedis built, soexecuteprinted a traceback and exited 1. It should have printedREFUSED pushing the spec legand the ruleset hint. By then both leg remotes already exist. On main, 30 text-mode calls in the shipped tools name no codec. This branch fixes 28 of them:encoding="utf-8", errors="replace"on 25 calls whose output is shown, parsed or compared. These are:shape_materialize.run(), which carries most of the git callsexecutemakes;gh repo viewandgh apiprobes of scaffold-project.py, family.py and repo_shape.py;_require_filter_repo), bump-leg.py, scaffold-project.py and update-shape.py, and of the family root's bootstrap.py and siblings.py.A byte that is not UTF-8 is read as U+FFFD instead of raising.
UTF-8 and STRICT (no
errors=) on the twogh apitree listings that become a digest: scaffold-project.pypin_digest_from_ghand repo_shape.pytree_digest_from_gh. A replaced byte there would produce a wrong digest stated as fact.errors="replace"and noencoding=on bump-leg.pyrun_validator. Its child is a Python process, which writes its pipe in the locale's own codec, so that codec should read it back.replacestops a mismatch between the two interpreters from turning a refusal into a traceback.tests/test_git_output_decoding.pyhas two tests:run()in a child interpreter started underLC_ALL=C PYTHONUTF8=0, with a UTF-8 name and with a non-UTF-8 name.subprocesscalls, so a text-mode call that names no codec fails the suite.Two calls are left to sibling PRs and listed in the guard's
EXEMPTtable, so no pair of branches carries conflicting hunks:shape_materialize.env_commit: adopt-project.pycheckaccepts plan pathsexecuterefuses asunsafe-value(a space or a non-ASCII character), soplan okis followed by a refusal and #166's split instructions can name entriesexecutewill refuse #196'sfix/adopt-check-unsafe-pathgives it both arguments.cross_leg_references: adopt-project.pyplan'scross_leg_referencespasses a spec directory's name togit grep -Fwith no-eand the code entries as glob pathspecs, so a spec directory named-notesor a code entry named:cachesilently 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'sfix/plan-literal-follow-up-grepreads itsgit grepas bytes.Why
The issue's repro, measured on this machine against current main
a5b09c3and against this branch's treece41ef3(headb6f7cfd):specs/001-feature/{spec,plan}.md,src/main.pyandREADME.md.pre-receivehook throughGIT_TEMPLATE_DIR. The hook rejects the push witherror: push rule: the file 'specs/001-feature/Übersicht.md' is blocked, as a hosted push rule would.planandcheckrun in this shell.execute --yes --local-remote-dirruns underenv -u LANG LC_ALL=C PYTHONUTF8=0, wherelocale.getpreferredencoding()isANSI_X3.4-1968.a5b09c3b6f7cfdplan,checkexecuteunderLC_ALL=C PYTHONUTF8=0_extract_leg→shape_materialize.run→subprocess.py _translate_newlines, endingUnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 54: ordinal not in range(128)REFUSED pushing the spec leg. THE EXACT COMMAND WAS:…remote: error: push rule: the file 'specs/001-feature/\xdcbersicht.md' is blocked(stderr'sbackslashreplace) …[remote rejected] HEAD -> main (pre-receive hook declined)If the organisation applies a ruleset …)Northwind-code.git,Northwind-spec.gitNorthwind-code.git,Northwind-spec.git(unchanged:NOTHING has been rolled back)specs/001-feature/Übersicht.mdexecuteoutput identical to the row aboveexecutein a UTF-8 shellREFUSED pushing the spec leg+ hintplanoutput,checkoutput,executeoutput,adoption-plan.yamland exit codes are byte-identical to main's, diffed after masking the run directoryTests
On tree
ce41ef3812965cb08d19fa9ad222d2abd0cd4078, which is headb6f7cfdcfbd6fb0cb03299de99c69889899edb52, based ona5b09c3, with git-filter-repo 2.47.0 first on PATH. The runs were made at328c099, which has the same tree;b6f7cfddiffers from it only in commit metadata:tests/test_git_output_decoding.py tests/test_adopt_e2e.py tests/test_adopt_execute_refusals.py tests/test_adopt_ignored_shape_files.py tests/test_adopt_leg_path_collisions.py tests/test_adopt_leg_paths.py tests/test_adopt_mount_ignored.py tests/test_adopt_plan.py tests/test_adopt_spec_only.py tests/test_adopt_submodules.py tests/test_update_shape.py tests/test_update_shape_add.py tests/test_family.py tests/test_bump_leg.py tests/test_scaffold_e2e.py tests/test_scaffold_ignored_shape_files.py tests/test_scaffold_pin_and_reuse.py tests/test_setup_project_py.py tests/test_shape_doctor.py tests/test_repo_hygiene.py tests/test_cli_reference.py: 1344 passed, 1 skipped in 793.93s. The one skip istests/test_adopt_plan.py:393, the MedxEHR-backed plan test, which needsSHAPE_MEDXEHR_CLONE.tests/test_scaffold_leg_paths.py tests/test_free_plan_hint.py tests/test_reuse_visibility.py tests/test_commit_trailers.py tests/test_park_resume_targets.py tests/test_windows_paths.py: 250 passed in 55.48s.Platform skips:
test_run_names_the_path_git_complained_of_under_a_c_locale(os.name == "nt"). A C locale is a POSIX idea, and the Windows job setsPYTHONUTF8: '1'(.github/workflows/tests.yml), so CI does not run the cp1252 case.LC_ALL=C PYTHONUTF8=0still decodes as UTF-8 skips that test too, saying so.Composes with
Trial merges,
git merge-tree --write-tree <sibling> <this branch>(treece41ef3) with main ata5b09c3, run 2026-10-11T18:51Z. On every clean merge, the guard's own rule was run over the merged tree.a5b09c3)origin/mainby itself. For adopt-project.pycheckaccepts plan pathsexecuterefuses asunsafe-value(a space or a non-ASCII character), soplan okis followed by a refusal and #166's split instructions can name entriesexecutewill refuse #196 and materialize_assembly_root writes the shape files through a root-kept symlink to anywhere on disk, and crashes with a traceback on a link loop, after both legs are pushed #183 the newer local form merges clean. None of the conflicts comes from this change.fix/adopt-root-kept-symlinks@04792d2needs one line, because it adds a text-mode call that names no codec:adopt-project.py:2216,_checks_out_symlinks, the callsubprocess.run(["git", "config", "--type=bool", "--get", "core.symlinks"], cwd=str(probe), capture_output=True, text=True, check=False). The line number is the same on that branch and in the merged tree.tests/test_git_output_decoding.pygives1 failed, 2 passed, withadopt-project.py:2216 (_checks_out_symlinks) asks for text and does not name ['encoding', 'errors'].encoding="utf-8", errors="replace",as a new line between that branch's lines 2217 and 2218. Applied to a scratch copy of the merged tree, the guard passes (1 passed, 2 deselected).checkaccepts plan pathsexecuterefuses asunsafe-value(a space or a non-ASCII character), soplan okis followed by a refusal and #166's split instructions can name entriesexecutewill refuse #196 and adopt-project.pyplan'scross_leg_referencespasses a spec directory's name togit grep -Fwith no-eand the code entries as glob pathspecs, so a spec directory named-notesor a code entry named:cachesilently 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 each fix the call this branch exempts for them, so they compose in either order:env_commitonfix/adopt-check-unsafe-pathnames both arguments.cross_leg_referencesonfix/plan-literal-follow-up-grepno longer asks for text.planraisesUnicodeEncodeErrorin its terminal report, after the plan file is written, for a printable name stdout cannot encode (日本.mdunder cp1252, as on a redirected Windows stdout), soplanexits 1 beforeplan written to#214 (fix/plan-report-stdout-encoding) is the other direction, the tool's own stdout. It shares six files with this branch, merges clean and passes the guard.adopt-project.py executewhose leg push is rejected with a message naming a non-ASCII path endsREFUSED pushing the spec legand the ruleset hint underLC_ALL=C PYTHONUTF8=0, not a UnicodeDecodeError traceback (#217) #230 does not make a successfulexecutesafe underLC_ALL=C PYTHONUTF8=0.shape_materialize.env_commit(exempt here for adopt-project.pycheckaccepts plan pathsexecuterefuses asunsafe-value(a space or a non-ASCII character), soplan okis followed by a refusal and #166's split instructions can name entriesexecutewill refuse #196) still encodes the split commit's message with the locale codec, and every cross-leg follow-upplanwrites holds an em dash. Soexecuteon such a plan still raisesUnicodeEncodeError('ascii' codec can't encode character '—') after both legs are pushed, until adopt-project.pycheckaccepts plan pathsexecuterefuses asunsafe-value(a space or a non-ASCII character), soplan okis followed by a refusal and #166's split instructions can name entriesexecutewill refuse #196 (PR adopt-project.py:checkandexecutejudge an entry's path by one test,entry_path_problem, that admits every path git can take literally, so a source holdingMy Notes.mdno longer checksplan okand is then refused byexecuteasunsafe-value, and a path git cannot take is acheckfinding refused before any leg exists (#196) #231) lands. Land this together with or after adopt-project.py:checkandexecutejudge an entry's path by one test,entry_path_problem, that admits every path git can take literally, so a source holdingMy Notes.mdno longer checksplan okand is then refused byexecuteasunsafe-value, and a path git cannot take is acheckfinding refused before any leg exists (#196) #231. Measured onb6f7cfdby the independent review.Left out
commit --trailer, a--trailertravels in the stdin commit message, which the three commit helpers now encode as UTF-8 withreplacein every locale. UnderLC_ALL=C PYTHONUTF8=0a non-ASCII trailer becomes?(Jos??), and in a UTF-8 locale so does a trailer argument whose bytes are not valid UTF-8; main raisedUnicodeEncodeErrorin both. Carrying it as typed needs the commit helpers to encode stdin withsurrogateescapewhile decoding their output as UTF-8.checkaccepts plan pathsexecuterefuses asunsafe-value(a space or a non-ASCII character), soplan okis followed by a refusal and #166's split instructions can name entriesexecutewill refuse #196 and adopt-project.pyplan'scross_leg_referencespasses a spec directory's name togit grep -Fwith no-eand the code entries as glob pathspecs, so a spec directory named-notesor a code entry named:cachesilently 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 have landed, drop their two entries fromEXEMPTintests/test_git_output_decoding.py, so the guard coversenv_commitandcross_leg_referenceslike every other call.TextModeCallsintests/test_git_output_decoding.py) misses some call forms:from subprocess import run, a module alias (import subprocess as sp),**kwargs, a call naming onlyerrors=(text mode with the locale codec), andsubprocess.getoutput. None exists in shipped code today; hardening it means treatingencoding=/errors=as asking for text, refusing**in a spawner call, and resolving aliases and from-imports.Closes #217
🤖 Generated with Claude Code
Summary by Sourcery
Make command-output handling locale-independent so failed operations with non-ASCII paths remain actionable across environments.
Bug Fixes:
Enhancements:
Tests: