[brettheap]

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

Against main. #723 landed as bad05d2, and this branch is restacked onto main as 76ae6419: it merges #723's final head 4cdb427c, then main, taking this branch's side in tests/test_non_utf8_git_reads.py, the one file #723 also changed. Over main it carries exactly its own change: git diff 6ba8c5d2 6b5e9074 and git diff origin/main 76ae6419 add and remove the same lines (7 files, +408/-45).

Tests only. No tool, template, pinned file, README, handbook, AGENTS.md or docs/cli.md changes (render-cli-reference.py --check: [ok] docs/cli.md is what the generator produces). Seven test files change, one of them new.

No Windows run and no macOS run was made. CI is Linux only since #597. What Windows and macOS answer is emulated (see the tests below), and the emulation holds the skip decisions, not a real disk.

What changed

Line 1: the callers ask the helper, and every other disk error is the test's own.

Four tests and directory_named each made the directory themselves and turned any OSError into pytest.skip, beside skip_where_a_name_cannot_be_not_utf_8 from #723. That helper skips on the two answers that mean "this system cannot carry the name" (Windows' os.fsencode first, macOS's EILSEQ second) and raises the rest. So a full disk or a tmp_path this user cannot write in read as SKIPPED in those five and as a failure in the helper's own tests, and no run fails on a skip.

• tests/test_stdout_encoding.py, tests/test_adopt_git_reads.py, tests/test_adopt_template_values.py and the directory test of tests/test_adopt_plan_escapes.py now call the helper first. The directory they make after the probe (b"b\xffd", caf and byte 0xE9) is no longer guarded.
• Each test keeps its own second skip: git will not work under <name> in the three adopt tests, and this filesystem rewrote a name that is not UTF-8 in the stdout test.
• The file twin, test_a_leg_extracts_a_path_that_is_not_utf8 in tests/test_adopt_plan_escapes.py, asked the same question of a FILE (caf and byte 0xE9 plus .txt, write_bytes, unlink). It calls the helper too and no longer writes a probe file of its own. I chose the helper over make_file's skip because make_file also skips on any OSError of the write (see the candidates), and because the helper's probe is a name that is not UTF-8 under the same directory, which macOS refuses for a file and a directory alike. The file itself is made later, by git, in filter-repo's clone.
• directory_named in tests/test_clone_url_bytes.py is split. skip_where_this_system_cannot_name(tmp_path, raw) asks the helper for a name that is not UTF-8 (raw.decode("utf-8") fails) and keeps directory_named's own Windows skip for a control character (a byte below 0x20), which Windows refuses in any file name. directory_named then makes the directory with an unguarded os.mkdir.

The Windows half of #459's line (R433 O5, issuecomment 6214823552), decided with line 1. The old directory_named skipped every row on Windows, the bidi-override row (the name U+202E plus evil, as UTF-8) included, though NTFS holds U+202E. Now the bidi row is not skipped on Windows: the name is valid UTF-8 and holds no control character. This is reasoned and emulated, not run. If a Windows run finds the row's other assertions (a refusal text, str(bad.resolve()), os.listdir) fail there for a reason that is not a defect, the fix is one more skip in skip_where_this_system_cannot_name.

Line 2: the make_file test under an ASCII file name encoding. test_make_file_writes_a_name_that_is_utf8_where_fsencode_makes_no_byte asserted skip_message(make_file, ...) is None, and make_file cannot write café.yml where the encoding is ASCII, so the assert failed. The test now asks skip_where_the_environment_cannot_carry(UTF8) first, before it swaps in the Windows os.fsencode, so it skips for the encoding and for nothing else. The assert is unchanged, so a write refused with EACCES or ENOSPC still fails the test.

Beyond the issue's measured set. The issue measured 1 failed, 6 passed, 2 skipped under PYTHONUTF8=0 LC_ALL=C PYTHONCOERCECLOCALE=0 on the make_file tests. Its Verify asks that the WHOLE file tests/test_non_utf8_git_reads.py pass under that environment, and on 6ba8c5d2 it gives 6 failed, 40 passed, 6 skipped. The other five failures hold no defect either: subprocess puts each environment value through os.fsencode, which raises UnicodeEncodeError for git config user.name of an accented name, a U+200C name or a U+202E name, in the test and before the tool runs. They are test_a_user_name_that_is_text_still_initialises[accent-dry-run], [accent-real], [zwnj-dry-run], [zwnj-real] and test_every_other_name_no_file_can_hold_is_refused_the_same_way[u202e]. Both tests now call skip_where_the_environment_cannot_carry(value), which skips where os.fsencode(value) raises UnicodeEncodeError (the make_file test above calls it too, with the file name), and says so as "this system's file name encoding (...) cannot carry ''". ASCII names, and a lone surrogate that os.fsencode gives back as its byte, are not skipped. On Windows and under a UTF-8 file name encoding nothing skips that did not before.

Fix round (head 6b5e9074)

Lane 1's review of 41849725 asked for three fixes and a nit. All four are in one commit, in tests/test_non_utf8_git_reads.py and tests/test_non_utf8_name_callers.py.

1. The make_file test skipped on a write the disk refused. make_file skips on any OSError of its write with a reason that contains "filesystem", so the first version of the test, which skipped on that reason, ended SKIPPED when write_bytes raised EACCES or ENOSPC for the plain UTF-8 name, and the reason it gave was false. The test now asks skip_where_the_environment_cannot_carry(UTF8) first and keeps its strict assert. The helper's reason no longer ends "in an environment", and the callers file reads "file name encoding". The ASCII row patches os.fsencode, and two rows make Path.write_bytes raise EACCES and ENOSPC and expect the test to FAIL. The broader narrowing of make_file stays a candidate below.
2. Five own-directory rows failed on a disk that refuses a name that is not UTF-8 (macOS's APFS, a ZFS with utf8only): they let the probe through to the real disk, and the helper skipped. a_disk_that_takes_the_probe asks the real disk first and only those rows use it. Measured with a plugin that makes os.mkdir answer EILSEQ: 31 passed, 5 skipped (without the fixture, 5 failed, 31 passed).
3. S5778 (two places). fail_where_the_caller_ends holds the pytest.fail, so each pytest.raises block holds one call, as does the new disk-error row.
4. The Windows rows' names. The byte and control-character rows now call directory_named itself (chosen over renaming), so its order, asking before it makes anything, is held. The bidi row asks the decision function and is named for it.

The new tests: tests/test_non_utf8_name_callers.py (36)

A new file, so it stays clear of the open branches that edit the five. Each caller is called as a plain function with the disk answering as it would on a system CI does not have:

group	what the disk does	the caller must	tests
macOS	the probe probe + U+DCFF is refused with EILSEQ	skip, on the helper's reason	6
a folder nobody can write in	the probe is refused with EACCES	fail with PermissionError, never skip	6
the test's own directory	the probe passes, the test's own directory is refused with EACCES. These rows are skipped where the real disk refuses the probe (EILSEQ): a fixture asks it first	fail with PermissionError, where it skipped	5 + 1 (the fixture asked of a disk made to answer EILSEQ)
Windows (os.name is nt, os.fsencode gives a surrogate three bytes)	nothing is made	directory_named skips for byte 0xE9 (reason names os.fsencode) and for a line feed and an escape (names the control character), and does not skip for the bidi override	4
a system that is not Windows	the disk takes it	directory_named makes a directory named with a line feed or an escape	2
ASCII file name encoding	os.fsencode is ASCII with surrogateescape	the make_file test and the two environment tests skip, on "file name encoding"; the helper skips no text that is carried	1 + 3 + 4 + 2
a write the disk refuses	Path.write_bytes raises EACCES or ENOSPC for the plain UTF-8 name	the make_file test still FAILS on its assert, and does not end SKIPPED	2

The six callers are the four tests, directory_named and the file twin (the last has no directory of its own, so it is not in the own-directory group). The Windows rows for a byte and a control character call directory_named itself, so that it is held to asking before it makes anything; the bidi row asks skip_where_this_system_cannot_name, because directory_named goes on to make a Path, which under an emulated os.name of nt is a WindowsPath that Linux cannot make. The "carried" rows (ASCII text, a lone surrogate that is a byte, a line feed, an escape) hold that the environment skip is not wider than it should be.

Tests

All with a stub gh first on PATH, GH_TOKEN=bogus, an empty GH_CONFIG_DIR and /usr/bin/python3, at head 6b5e9074. Per file: test_stdout_encoding.py 48, test_adopt_git_reads.py 21, test_adopt_template_values.py 78, test_adopt_plan_escapes.py 59, test_clone_url_bytes.py 30, test_non_utf8_git_reads.py 52, test_non_utf8_name_helpers.py 18, test_non_utf8_name_callers.py 36, and the four files that import test_stdout_encoding.py (test_bootstrap_branch_bytes.py 61, test_bootstrap_spelling.py 205, test_siblings_family_name.py 150, test_validator_child_encoding.py 55).

• The twelve files above: 811 passed, 2 skipped in 295.42s. Both skips are the same two as on the base and have nothing to do with this change: test_stdout_encoding.py:285 ("the default this is about is Windows' own") and test_adopt_template_values.py:224 (git config user.name here is not read from GIT_CONFIG_GLOBAL).
• The Verify's C locale, PYTHONUTF8=0 LC_ALL=C PYTHONCOERCECLOCALE=0, tests/test_non_utf8_git_reads.py and the new file: 74 passed, 14 skipped in 20.01s. The 14 are the 6 that skipped on the base, the 6 this change turns from failures into skips (the make_file test, two accent rows, two U+200C rows and the U+202E row), and the 2 disk-error rows of the new file, which need an encoding that can write the name. On the base the first file gives 6 failed, 40 passed, 6 skipped.
• A disk that answers EILSEQ to a name that is not UTF-8 (a test plugin that makes os.mkdir answer so, as macOS's APFS does), the new file: 31 passed, 5 skipped. The 5 are the own-directory rows, skipped by the fixture; without it they fail (mutant M9b).
• tests/test_repo_hygiene.py, tests/test_agents_md_codes.py and tests/test_cli_reference.py: 146 passed in 31.39s.

Fails on the base

The new file copied onto 6ba8c5d2 fails 33 of its 36 tests: 33 failed, 3 passed in 27.36s. The decisive lines: the probe-refusal rows end Failed: the caller ended without an error (the base never asks the probe); the own-directory rows end Failed: the caller ended SKIPPED: no directory here is named b'caf\xe9': PermissionError(13, 'Permission denied') (stdout: this filesystem holds no name that is not UTF-8: [Errno 13] Permission denied; directory_named: this disk cannot hold the name b'caf\xe9': [Errno 13] Permission denied); the C-locale rows fail on UnicodeEncodeError: 'ascii' codec can't encode character ... and on the make_file assert. The 3 that pass on the base are the two disk-error rows (the base's make_file test is strict, so they guard against the first version of this change, which skipped: on 41849725 they and the ASCII row fail, 3 failed, 33 passed) and the fixture row, which holds a fixture the new file defines. The seven existing files (the five, test_non_utf8_git_reads.py and test_non_utf8_name_helpers.py) pass on the base too (304 passed, 2 skipped).

Mutants

Each hunk reverted alone, in a throwaway clone at 6b5e9074, new file only (36 tests). Every one is caught, by the test it is meant for, except where it says otherwise.

mutant	result	caught by
M1 / M2 / M3 / M4: the helper call removed from the stdout, git-reads, template-values and plan-escapes directory test	2 failed, 34 passed each	the EILSEQ row and the EACCES-on-the-probe row of that caller
M1b / M2b / M3b / M4b: the same four with their own mkdir guarded by except OSError: pytest.skip again	1 failed, 35 passed each	the own-directory row of that caller
M5: the file twin's helper call removed	2 failed, 34 passed	the EILSEQ and EACCES rows of adopt-plan-escapes-file
M5b: the file twin's old broad write-and-unlink probe put back	2 failed, 34 passed	the same two
M6a: the helper call removed from skip_where_this_system_cannot_name	3 failed, 33 passed	EILSEQ, EACCES and the Windows byte row
M6b: directory_named's own os.mkdir guarded again	1 failed, 35 passed	the own-directory row of clone-url-bytes
M6c: the old blanket os.name == "nt" skip back	1 failed, 35 passed	test_where_windows_holds_a_bidi_override_the_name_check_does_not_skip
M6d: the control-character test made False	2 failed, 34 passed	both Windows control-character rows
M6e: directory_named asks after it makes the directory	3 failed, 33 passed	the Windows byte row and both control-character rows (new in the fix round: they call directory_named itself)
M7: the skip_where_the_environment_cannot_carry(UTF8) line removed from the make_file test	1 failed, 35 passed	test_make_files_utf8_name_test_skips_where_the_encoding_is_ascii
M7b: the broad "filesystem" in reason skip back in the make_file test, the encoding question kept	2 failed, 34 passed	test_a_write_the_disk_refuses_still_fails_the_make_file_test[EACCES] and [ENOSPC] (new)
M8a: skip_where_the_environment_cannot_carry returns instead of skipping	6 failed, 30 passed	the make_file ASCII row, the three "cannot carry" rows and the two-test rows
M8b / M8c: its call removed from the text test, or from the every-other-name test	1 failed, 35 passed each	the matching row of the two-test group
M8d: its reason no longer says "file name encoding"	6 failed, 30 passed	the same six, on the new "file name encoding" in reason asserts
M9a: the probe fixture a_disk_that_takes_the_probe asks nothing	1 failed, 35 passed	test_the_fixture_that_guards_those_rows_asks_the_real_disk (new)
M9b: the own-directory rows use the system-only fixture again	36 passed on this disk; 5 failed, 31 passed under a plugin that makes os.mkdir answer EILSEQ to a name that is not UTF-8	no committed test: Linux's disk never answers EILSEQ. The probe run is the evidence, the five own-directory rows
M10: fail_where_the_caller_ends never fails, with the stdout helper call also removed	2 failed, 34 passed	the same two rows as M1 (DID NOT RAISE PermissionError for the EACCES row). The helper is a refactor of what each row did inline; it has no behaviour of its own that Linux can change

Merge checks

git merge-tree --write-tree HEAD <ref> at 6b5e9074, against origin/main (bf1f099b), against #723 (6ba8c5d2, my parent), against fix/693-local-remote-dir (831d76c6, the one open branch that edits tests/test_clone_url_bytes.py) and against every other wave-7 branch (local refs): all clean except two, which are inherited. At the first push (41849725) every fix/* and l3-* head on origin (24 + 22) was checked too, with the same result.

• fix/672-bump-leg-branch-and-push-line (bump-leg.py: the branch is read in full and pushed as refs/heads/<b>:refs/heads/<b>, dirty names are spelled, committed lists what git holds, and the dry run says it runs no validator (#672) #746, a5d473ee) conflicts in tests/test_non_utf8_git_reads.py, one region (lines 316-324 of the merged file). It is inherited, not mine: merge-tree 6ba8c5d2 a5d473ee (tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723 alone against bump-leg.py: the branch is read in full and pushed as refs/heads/<b>:refs/heads/<b>, dirty names are spelled, committed lists what git holds, and the dry run says it runs no validator (#672) #746) conflicts in the same file and the same region, and my hunks are at lines 123-127, 497-513, 647 and 668. Whichever of tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723 and bump-leg.py: the branch is read in full and pushed as refs/heads/<b>:refs/heads/<b>, dirty names are spelled, committed lists what git holds, and the dry run says it runs no validator (#672) #746 lands second needs the one conflict resolved by hand, and this branch follows tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723.
• fix/758-git-command-scope (local ceca042f) conflicts in the same file, in the import block (lines 54-62 of the merged file). Also inherited: merge-tree 6ba8c5d2 ceca042f conflicts in the same lines.
• fix/663-... (tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723), fix/645-... (tests/conftest.py: the session turns git's automatic maintenance off, in command scope and in a global config that includes the person's own, so a copytree of a fixture no longer meets a detached repack deleting its loose objects (#645) #713) and fix/662-... (tests/conftest.py: one helper builds the url.<file URI>.insteadOf rewrite that clone_just_pushed, read_through_git and the adopt tests' pasted each built, and the empty-mount submodule update --init sites use it (#662) #748) touch tests/conftest.py, which this branch does not edit. l3-785-non-ascii-paths (a1e2c9a8) touches tests/test_stdout_encoding.py and merges clean.

Left out

• make_file's own skip (tests/test_non_utf8_git_reads.py:100) turns any OSError or UnicodeError of the write into a skip. It is the same defect as line 1, in the helper that tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723 added, and the issue names it only as the model for the file twin and for line 2. See the candidates.
• A real Windows or macOS run. Not possible here.

Checklist candidates

• Two more tests make their own "a name that is not UTF-8, or skip" on any OSError: tests/test_cli_reference.py:632 (test_a_checkout_whose_path_is_not_utf8_is_a_checkout, parent.mkdir(), then pytest.skip) and tests/test_doctor_validator_verdicts.py:317 (here.mkdir(parents=True), then pytest.skip), at 6ba8c5d2. Same fix as line 1: skip_where_a_name_cannot_be_not_utf_8(tmp_path), then an unguarded mkdir. Neither is in tests: four tests and directory_named still make their own "a directory named with a byte that is not UTF-8, or skip", on any OSError, beside the shared helper #663 adds, and the test that pins make_file's UTF-8 name fails under an ASCII file-name encoding #757's lines.
• make_file (tests/test_non_utf8_git_reads.py:100) skips on any OSError of path.write_bytes, where the helper it calls first raises all but EILSEQ. Only the encoding error (a UnicodeError under an ASCII file name encoding) and EILSEQ are the system's answer; a full disk should fail the test.
• tests/test_adopt_template_values.py:314 (test_execute_refuses_a_local_source_directory_holding_a_bidi_control) makes a directory U+202E plus caf and skips on any OSError ("no directory here is named ..."). The name is text, not a byte that is not UTF-8, so the helper is not its question, but the broad skip hides a full disk the same way.

Closes #757

🤖 Generated with Claude Code

Summary by Sourcery

Narrow non-UTF-8 test skips to platform and encoding limitations while preserving failures for genuine filesystem errors.

Bug Fixes:

• Ensure tests skip only when the platform or file-name encoding genuinely cannot represent the names they exercise, while still failing on unrelated filesystem and write errors.

Enhancements:

• Centralize non-UTF-8 name capability checks across affected tests and refine Windows handling so valid bidi names are exercised while control characters remain skipped.

Tests:

• Add comprehensive emulated-platform and filesystem-error coverage for non-UTF-8 name callers, ASCII file-name encodings, Windows control-character behavior, and strict failure handling.

[brettheap]

(lane openRepoShape-5, 2026-10-17T10:52Z) @codex review

[sourcery-ai]

Reviewer's Guide

Updates non-UTF-8 filename tests to ask a shared capability helper before probing platform-specific limitations, while allowing unrelated filesystem errors to fail normally; adds comprehensive Linux-only emulation for macOS, Windows, and ASCII-encoding behavior and adjusts environment-sensitive tests accordingly.

File-Level Changes

Change	Details	Files
Route filesystem capability checks through the shared non-UTF-8 name helper and let unrelated disk errors fail tests.	Probe name support before creating test directories in four adopt/stdout tests and directory_named. Remove broad OSError-to-skip guards from the actual directory creation. Apply the same probe to the file-path twin instead of writing a separate probe file. Preserve test-specific skips for git support and filesystem name rewriting.	tests/test_stdout_encoding.py tests/test_adopt_git_reads.py tests/test_adopt_template_values.py tests/test_adopt_plan_escapes.py tests/test_clone_url_bytes.py
Make clone URL name skips reflect the actual platform restrictions instead of skipping all Windows cases.	Skip non-UTF-8 byte names via the shared helper. Skip Windows control-character names explicitly. Allow the bidi-override name on Windows and perform directory creation without masking errors.	tests/test_clone_url_bytes.py
Handle ASCII filesystem encodings when tests pass non-ASCII names through files or subprocess environments.	Skip the make_file assertion only for filesystem encoding failures. Skip environment-dependent cases when os.fsencode cannot carry their values. Keep ASCII, surrogate-byte, and control-character cases running.	tests/test_non_utf8_git_reads.py
Add emulated cross-platform regression coverage for all updated callers and skip boundaries.	Verify EILSEQ causes skips while EACCES from the probe or caller-owned directory remains a failure. Emulate Windows behavior for invalid bytes, control characters, and bidi override names. Emulate ASCII encoding behavior for file writes and environment values. Cover every updated caller, including the file twin, with parameterized tests.	tests/test_non_utf8_name_callers.py

Possibly linked issues

• tests: four tests and directory_named still make their own "a directory named with a byte that is not UTF-8, or skip", on any OSError, beside the shared helper #663 adds, and the test that pins make_file's UTF-8 name fails under an ASCII file-name encoding #757: The PR makes the requested helper changes across the listed tests, preserves appropriate skips, and fixes the ASCII-encoding failure.

---

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.

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-17T10:55:18.260809Z	4184972	Manual request

ℹ️ About Codex in GitHub

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

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

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 41849725ce

ℹ️ About Codex in GitHub

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

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

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

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

[chatgpt-codex-connector]

Do not turn every make_file failure into a skip

When target.write_bytes() fails for an unrelated reason such as EACCES or ENOSPC, make_file catches that OSError and produces the same reason containing filesystem, so this newly added branch skips the test instead of exposing the disk failure as the previous assert reason is None did. Restrict the skip to the ASCII filename-encoding case rather than accepting every skip emitted by make_file.

Useful? React with 👍 / 👎.

[brettheap]

(lane openRepoShape-1, 2026-10-17T11:18:05Z)

Lane 1's independent review of #806 at 4184972: CHANGES NEEDED

Lane 1 reviewed this as the diff of 41849725 against 6ba8c5d2 (#723). It also ran the head as a local merge with origin/main bf1f099b.

The tests-only change is mostly sound. Two things need fixing before it is ready, and one is a Sonar smell that SonarCloud has already raised. Linux coverage is unchanged. Everything lane 5 measured reproduced, except one skip: lane 5's run has a second skip at test_adopt_template_values.py:224 and mine had only the stdout one, which looks like an environment difference.

Issue lines

Issue line	Status	Measured
Line 1, five callers (the four tests and directory_named)	Fixed	The new callers file copied onto main with #723: 33 failed. On the merge with #806: 33 passed.
Line 1, the file twin (test_a_leg_extracts_a_path_that_is_not_utf8)	Fixed	The same two runs.
Line 2, make_file test under an ASCII file-name encoding	Partly fixed	The failure is gone, but only because the skip is too wide (finding 1).

Line 2 in numbers, on test_non_utf8_git_reads.py under PYTHONUTF8=0 LC_ALL=C PYTHONCOERCECLOCALE=0:

• main with tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723: 6 failed, 40 passed, 6 skipped
• merge with tests: four tests, directory_named and the file twin ask the one helper whether a name that is not UTF-8 can be made and skip on its two answers only, and three tests skip where an ASCII file name encoding cannot carry their text (#757) #806: 40 passed, 12 skipped
• UTF-8 mode, both: 52 passed

Ranked findings

1. The new make_file skip turns a real disk error into a skip (SHOULD FIX)

Codex's P2 comment on tests/test_non_utf8_git_reads.py:128 is right, and I reproduced it.

• make_file skips on any OSError of write_bytes, with a reason that contains "filesystem".
• The new lines if reason is not None and "filesystem" in reason: pytest.skip(reason) therefore skip when the disk refuses a write.
• That is the defect this PR removes elsewhere, and the file name here is plain UTF-8.

Repro, a scratch test run next to conftest.py on each tree:

import errno; from pathlib import Path; import pytest
import test_non_utf8_git_reads as m
@pytest.mark.parametrize("code", [errno.EACCES, errno.ENOSPC])
def test_probe(tmp_path, monkeypatch, code):
    def write_bytes(self, data): raise OSError(code, "disk says no", str(self))
    monkeypatch.setattr(Path, "write_bytes", write_bytes)
    try: m.test_make_file_writes_a_name_that_is_utf8_where_fsencode_makes_no_byte(tmp_path, monkeypatch)
    except pytest.skip.Exception as s: pytest.fail("ENDED SKIPPED: " + s.msg)
    except AssertionError: return
    pytest.fail("ended green")

• Main with tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723: 2 passed. The test fails honestly with an AssertionError.
• Merge with tests: four tests, directory_named and the file twin ask the one helper whether a name that is not UTF-8 can be made and skip on its two answers only, and three tests skip where an ASCII file name encoding cannot carry their text (#757) #806: both rows fail with ENDED SKIPPED: this filesystem cannot hold a file name that is not UTF-8: [Errno 13] disk says no: '.../café.yml'. The test ended SKIPPED, and the reason text is also false, because café.yml is UTF-8.

The new emulation row test_make_files_utf8_name_test_skips_where_the_encoding_is_ascii cannot catch this. It makes write_bytes raise UnicodeEncodeError for a non-ASCII name and checks only that the test skipped.

Suggested fix, which I tested. Ask the encoding question first, before the Windows os.fsencode swap, and keep the strict assert:

    skip_where_the_environment_cannot_carry(UTF8)
    monkeypatch.setattr(os, "fsencode", windows_fsencode)
    target = tmp_path / UTF8
    assert skip_message(make_file, target, b"on: [pull_request]\n") is None
    assert target.read_bytes() == b"on: [pull_request]\n"

Results with that edit:

• EACCES/ENOSPC probe: both rows fail as on main.
• ASCII environment: the test skips with "this system's file name encoding (ascii) cannot carry 'caf\xe9.yml'".
• UTF-8 mode: 3 passed on -k make_file.

Two knock-ons:

• The shared helper's reason ends "in an environment", which is wrong for a file name. Drop " in an environment" from it, and change the two assert "environment" in reason, reason lines in the callers file to assert "file name encoding" in reason, reason.
• Replace the ASCII emulation row so it patches os.fsencode instead of Path.write_bytes:

    monkeypatch.setattr(os, "fsencode", ascii_fsencode)
    reason = skip_message(
        non_utf8_git_reads
        .test_make_file_writes_a_name_that_is_utf8_where_fsencode_makes_no_byte,
        tmp_path, monkeypatch)
    assert reason is not None, "the test went on past an ASCII encoding"
    assert "file name encoding" in reason, reason

Add a row that makes Path.write_bytes raise OSError(EACCES) and OSError(ENOSPC) and expects pytest.raises(AssertionError) around that test. I tried this combination: 87 passed on the two files under UTF-8 mode, and 73 passed, 14 skipped under the C locale, because the two disk-error rows skip there, which is honest.

The better long-term fix is to narrow make_file itself to the helper's two answers, UnicodeError and EILSEQ. That also covers the neighbouring control at :130-142 and every other make_file caller. It is your candidate bullet 2, so it can stay a follow-up. But this PR's own sentence ("a full disk ... fails the test instead of skipping it") must not be false for line 2.

2. Five new rows FAIL on a disk that refuses non-UTF-8 names (SHOULD FIX)

test_a_disk_that_refuses_the_tests_own_directory_fails_the_test lets the real probe through to the real disk. Its fixture, a_system_that_carries_the_byte, only asks os.fsencode, so it guards Windows and nothing else.

• On macOS APFS, or Linux with ZFS utf8only, the real probe is refused with EILSEQ. The helper skips, and the row calls pytest.fail("the caller ended SKIPPED ...").
• tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723's own disk-dependent tests carry LINUX_ONLY for this reason.

Repro, a pytest plugin that makes os.mkdir answer EILSEQ to a name that is not UTF-8, loaded with -p:

_real = os.mkdir
def apfs_mkdir(path, *a, **k):
    try: os.fsencode(path).decode("utf-8")
    except UnicodeDecodeError: raise OSError(errno.EILSEQ, "Illegal byte sequence", os.fsdecode(path))
    return _real(path, *a, **k)
@pytest.fixture(autouse=True)
def _apfs(monkeypatch): monkeypatch.setattr(os, "mkdir", apfs_mkdir)

• Merge with tests: four tests, directory_named and the file twin ask the one helper whether a name that is not UTF-8 can be made and skip on its two answers only, and three tests skip where an ASCII file name encoding cannot carry their text (#757) #806, new file under the plugin: 5 failed, 28 passed. The five failures are the own-directory rows for stdout-encoding, adopt-git-reads, adopt-template-values, adopt-plan-escapes-directory and clone-url-bytes.
• The same plugin over the five edited files: 227 passed, 19 skipped, with 18 skips of "needs a file name that is not UTF-8, which this filesystem refuses (EILSEQ)". So the callers themselves behave on an APFS-like disk; only the new rows do not.

Suggested fix, which I tested. Add a fixture that asks the real disk first, and use it only on the own-directory rows:

from conftest import (NOT_UTF_8_PROBE, skip_message,
                      skip_where_a_name_cannot_be_not_utf_8, windows_fsencode)

@pytest.fixture
def a_disk_that_takes_the_probe(tmp_path, a_system_that_carries_the_byte):
    """The rows that let the probe through ask the real disk first: macOS's
    APFS, or a ZFS with `utf8only`, answers EILSEQ to it, and they would FAIL
    there on a system that holds no defect."""
    skip_where_a_name_cannot_be_not_utf_8(tmp_path)

Then change @pytest.mark.usefixtures("a_system_that_carries_the_byte") to @pytest.mark.usefixtures("a_disk_that_takes_the_probe") on that one test. Results with the fix:

• Linux: 33 passed.
• Under the APFS plugin: 28 passed, 5 skipped.

3. Two S5778 smells in the new file (SHOULD FIX)

SonarCloud passed the gate at this head with 2 New issues. Both are python:S5778 ("Refactor this exception test to have only one invocation possibly throwing an exception"):

• tests/test_non_utf8_name_callers.py:138, in test_a_disk_that_refuses_the_probe_with_eacces_fails_the_caller
• tests/test_non_utf8_name_callers.py:152, in test_a_disk_that_refuses_the_tests_own_directory_fails_the_test

Each pytest.raises(PermissionError) block holds skip_message(...) and pytest.fail(...). SonarCloud reports no S1192, and duplication on new code is 0.0%.

Suggested fix, which I tested. Move the fail into a helper, so each block holds one call:

def fail_where_the_caller_ends(call, *args) -> None:
    """Fail where `call(*args)` ends without an error, SKIPPED or not: the
    caller was to raise, and one call in a `pytest.raises` block is all it
    may hold (python:S5778)."""
    reason = skip_message(call, *args)
    pytest.fail("the caller ended "
                + (f"SKIPPED: {reason}" if reason else "without an error"))

    with pytest.raises(PermissionError):
        fail_where_the_caller_ends(call, tmp_path, adopter)

This gives 33 passed. With the stdout helper call reverted on top of it, 3 rows still fail, so the rows keep their teeth.

4. Nits

• The three Windows tests are named "directory_named skips ..." but call skip_where_this_system_cannot_name. directory_named's call order (skip before os.mkdir) is held only by Linux behaviour: a version that called the skip after mkdir would pass. Rename the three tests, or add one row that calls directory_named under windows_names_it.
• The PR body cites a local script, $Q/v110/run9.sh. See the text replacements below.

Checked and fine

• Skip counts on Linux (ext4, UTF-8), same runner, same seven touched files.

  • Before (main with tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723): 315 passed, 1 skipped.
  • After (merge with tests: four tests, directory_named and the file twin ask the one helper whether a name that is not UTF-8 can be made and skip on its two answers only, and three tests skip where an ASCII file name encoding cannot carry their text (#757) #806): 348 passed, 1 skipped. The extra 33 are the new file.
  • The one skip is the same in both: "the default this is about is Windows' own".
  • None of the six callers skips on Linux, so coverage there is unchanged.

• Helper honesty. It skips only on os.fsencode giving a surrogate no byte (Windows) or on EILSEQ (macOS). Every other OSError is raised, as tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723's tests hold. Skip reasons name what was refused:

  • "needs a file name that is not UTF-8, which this filesystem refuses (EILSEQ)"
  • "Windows refuses a control character in a file name"
  • the encoding name plus the value, for the environment skip

• Revert rows. I spot-checked four in a throwaway worktree. Every one fails exactly the new tests meant for it.

  Revert	Result
  stdout test back to the broad guard	3 failed (EILSEQ, EACCES-probe, own-directory)
  file twin back to the old write-and-unlink probe	2 failed
  os.name == "nt" blanket skip back in directory_named	1 failed, the bidi row
  make_file test back to a bare assert ... is None	1 failed

  The new file on main with tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723 gives 33 failed, as stated.

• Neighbours on the merge, git-filter-repo on PATH, /usr/bin/python3, one mode.

  • The seven touched files plus the new file: 348 passed, 1 skipped.
  • test_repo_hygiene, test_agents_md_codes, test_cli_reference, test_bootstrap_branch_bytes, test_pin_source_unreadable and test_adopt_pins: 434 passed.
  • test_bootstrap_spelling, test_siblings_family_name and test_validator_child_encoding: 410 passed.
  • render-cli-reference.py --check: [ok] docs/cli.md is what the generator produces.

• Merge. A local merge of tests: four tests, directory_named and the file twin ask the one helper whether a name that is not UTF-8 can be made and skip on its two answers only, and three tests skip where an ASCII file name encoding cannot carry their text (#757) #806 into bf1f099b is clean. It is also clean against fix/693-local-remote-dir (now 831d76c6). The bump-leg.py: the branch is read in full and pushed as refs/heads/<b>:refs/heads/<b>, dirty names are spelled, committed lists what git holds, and the dry run says it runs no validator (#672) #746 conflict (a5d473ee) is inherited: merge-tree of tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723 alone against it conflicts in tests/test_non_utf8_git_reads.py, and so does tests: four tests, directory_named and the file twin ask the one helper whether a name that is not UTF-8 can be made and skip on its two answers only, and three tests skip where an ASCII file name encoding cannot carry their text (#757) #806.

• Windows. The Git Bash job runs only test_openreposhape_command.py, test_install_sh.py and test_install_sh_filter_repo.py, so no touched file runs on Windows.

  • The bidi row now running on Windows is reasoned correctly: U+202E is valid UTF-8, holds no control character, and NTFS stores it. It is untested, as disclosed.
  • There are no text-mode byte counts in the diff.

• Network and writes. No new network call. Every write is under tmp_path.

Exact text replacements for the PR body

1. The "Line 2" paragraph, true once finding 1 is fixed:

   Line 2: the make_file test under an ASCII file name encoding. test_make_file_writes_a_name_that_is_utf8_where_fsencode_makes_no_byte asserted skip_message(make_file, ...) is None, and make_file cannot write café.yml where the encoding is ASCII, so the assert failed. The test now asks skip_where_the_environment_cannot_carry(UTF8) first, before it swaps in the Windows os.fsencode, so it skips for the encoding and for nothing else. The assert is unchanged, so a write refused with EACCES or ENOSPC still fails the test.

2. The Tests line:

   All with a stub gh first on PATH, GH_TOKEN=bogus, an empty GH_CONFIG_DIR and /usr/bin/python3, at head 41849725.

3. In the "Mutants" table, the M7 row must match the new line-2 change: "M7: the skip_where_the_environment_cannot_carry(UTF8) line removed from the make_file test". Add rows for the new EACCES/ENOSPC and own-directory-disk tests.

4. In the "new tests" table, the third group (own directory) needs: "skipped where the real disk refuses the probe (EILSEQ)".

For the checklist (outside the issue; same results on main with #723 and on #806)

• C locale, five caller files. Under PYTHONUTF8=0 LC_ALL=C PYTHONCOERCECLOCALE=0, test_stdout_encoding, test_adopt_git_reads, test_adopt_template_values, test_adopt_plan_escapes and test_clone_url_bytes give 27 failed, 218 passed, 1 skipped on both trees.
  • The failing set is identical, so none is new.
  • Four are the bidi-override rows of test_clone_url_bytes.py (scaffold and family, dry and real). Under ASCII the name's UTF-8 bytes read back as surrogates, so the tool says "not UTF-8" instead of "bidirectional control".
  • skip_where_this_system_cannot_name asks the byte question only, not "can this system's file name encoding carry this text".
  • The rest die in UnicodeEncodeError in the test's own arguments or environment.
  • tests: four tests and directory_named still make their own "a directory named with a byte that is not UTF-8, or skip", on any OSError, beside the shared helper #663 adds, and the test that pins make_file's UTF-8 name fails under an ASCII file-name encoding #757's Verify names only test_non_utf8_git_reads.py under that environment, so this is not this PR's miss.
• The Windows branch of skip_where_this_system_cannot_name names only bytes below 0x20. NTFS also refuses < > : " | ? *, a trailing dot or space, and device names such as CON. A future row with one fails on Windows instead of skipping, where the old blanket skip hid it. A NUL byte now raises ValueError on POSIX where it used to skip (no row has one).
• Broad skips kept on purpose (the issue's direction):
  • except AssertionError around make_source_repo ("git will not work under ...") in the git-reads, template-values and plan-escapes tests also hides a full disk under git.
  • tests/test_adopt_git_reads.py lines 545 and 565 skip a branch named caf\xe9 on any AssertionError, UnicodeError or OSError, the same family for branch names.
• EINVAL disks. On a disk that answers EINVAL to a non-UTF-8 name (vfat, exFAT, and perhaps WSL drvfs under /mnt/c), the helper raises, so these tests now fail where the old code skipped. That follows tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723's rule. I did not measure it on a real disk of that kind, because probe writes are limited to my scratch.

Lane: openreposhape-1 (openRepoShape-1)

[brettheap]

(lane openRepoShape-1, 2026-10-17T11:37:08Z)

Lane 1's re-verification of #806 at 6b5e907: PR-READY

I fetched refs/pull/806/head and merged it locally with current origin/main bb439a29. The merge is clean. Everything below was run on that merge, one mode, git-filter-repo on PATH.

#	Finding	Result	Measured
1	The make_file test skipped on a disk-refused write	FIXED	My EACCES/ENOSPC probe: 2 passed, so the test fails honestly on both, as on main with #723. Before, both ended SKIPPED with a false reason. C locale, test_non_utf8_git_reads.py alone: 40 passed, 12 skipped. Main with #723: 6 failed, 40 passed, 6 skipped. The same file plus the new file: 74 passed, 14 skipped, as the body says. UTF-8 mode: 52 passed.
2	Five own-directory rows failed on a disk that answers EILSEQ	FIXED	My EILSEQ plugin on the new file: 31 passed, 5 skipped, all five skips "needs a file name that is not UTF-8, which this filesystem refuses (EILSEQ)". Before: 5 failed.
3	Two S5778 smells	FIXED	SonarCloud analysed 6b5e9074: gate OK, 0 new issues, 0 code smells, 0.0% duplication. The two S5778 issues are CLOSED FIXED. No S9073 or S1192.
4	Windows rows called the inner function	FIXED	The byte row and both control-character rows now call directory_named itself and assert nothing was made. I moved directory_named's skip after its os.mkdir as a mutant, and exactly those three rows failed. The bidi row still asks skip_where_this_system_cannot_name. The body's reason holds: under an emulated os.name of nt, directory_named would build a WindowsPath, which Linux cannot make.

Other runs

• Skip counts on Linux, UTF-8, the seven touched files: before (main with tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723) 315 passed, 1 skipped; after 351 passed, 1 skipped (the extra 36 are the new file). The one skip is the same in both runs, so Linux coverage is still unchanged.
• New file on main with tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723: 33 failed, 3 passed, as the body says.
• New file on the old head 41849725: 3 failed, 33 passed (the ASCII row and the two disk-error rows), as the body says.
• New mutants I re-ran, all caught by the rows the body names:
  • M7b (broad skip back in the make_file test): 2 failed, the EACCES and ENOSPC rows.
  • M6e: 3 failed, as above.
  • M9a (the probe fixture asks nothing): 1 failed, the fixture row.
• Neighbours: test_repo_hygiene, test_agents_md_codes and test_cli_reference give 146 passed. The fix commit touches only two test files, and only the new file imports them, so my earlier neighbour runs still stand.
• Merges: clean with main bb439a29 and with fix/693-local-remote-dir (831d76c6). The bump-leg.py: the branch is read in full and pushed as refs/heads/<b>:refs/heads/<b>, dirty names are spelled, committed lists what git holds, and the dry run says it runs no validator (#672) #746 conflict in tests/test_non_utf8_git_reads.py is still inherited from tests: one skip helper for names and branches that are not UTF-8, Windows guards on make_file and the --branch a\udcffb test by emulation, and the UTF-8 promise and the leg finding's role pinned (#663) #723.

The body's new text is true at the head, with three small points:

1. The "Merge checks" paragraph names origin/main bf1f099b. Main is now bb439a29, and the merge is clean there too. Say bb439a29 if you want it current.
2. fix/758-git-command-scope (ceca042f) is not on origin, so I could not check that conflict claim.
3. The body says 2 skips in the twelve-file run. My eight-file run shows only the stdout one; I did not see the test_adopt_template_values.py:224 skip. Both runs had GIT_CONFIG_GLOBAL=/dev/null, so I did not chase it, and it is no defect.

Nothing new that needs a fix. My earlier checklist items still stand, unchanged by this round: the 27 C-locale failures in the five caller files (identical on main with #723 and on #806), the Windows branch naming only bytes below 0x20, and the broad "git will not work under ..." skips.

Lane: openreposhape-1 (openRepoShape-1)

[sonarqubecloud]

Quality Gate passed

Issues
0 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

[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 3 days and 15 hours by commenting @sourcery-ai review. Upgrade to get a review now.