Skip to content

adopt-project.py: a refused command's REFUSED block spells each character a terminal acts on in the command, its directory and what it printed, as _first_said spells gh's, so --local-remote-dir no longer reaches the terminal raw there; a NOTE before NEXT: says when the source's path in its clone command holds a character a terminal does not show as itself; and rows pin six sites #1177's 148 left to chance (#1195) - #1225

Draft
brettheap wants to merge 1 commit into
fix/1152-plan-and-remote-paths-spelledfrom
fix/1195-plan-and-remote-paths-leftovers
Draft

adopt-project.py: a refused command's REFUSED block spells each character a terminal acts on in the command, its directory and what it printed, as _first_said spells gh's, so --local-remote-dir no longer reaches the terminal raw there; a NOTE before NEXT: says when the source's path in its clone command holds a character a terminal does not show as itself; and rows pin six sites #1177's 148 left to chance (#1195)#1225
brettheap wants to merge 1 commit into
fix/1152-plan-and-remote-paths-spelledfrom
fix/1195-plan-and-remote-paths-leftovers

Conversation

@brettheap

@brettheap brettheap commented Oct 23, 2026 •
edited by sourcery-ai Bot
Loading

Copy link
Copy Markdown
Contributor

Closes #1195

This PR is stacked on #1177: its base is fix/1152-plan-and-remote-paths-spelled, at 5208e874e. It lands after #1177 is on main, and it stays a draft until then.

What

#1195 follows RV1177's review of #1177 (for #1152). It names two raw routes, N3 and N4, and six mutants, K1, K2, K4, K5, K6 and K7, that all 148 rows of #1177's tests/test_adopt_plan_and_remote_paths_spelled.py let through.

Three files change: adopt-project.py, that test file, and tests/test_adopt_github_exits.py.

N3: a refused command no longer names --local-remote-dir raw

CommandFailed.loudly belongs to scripts/shape_materialize.py, and every tool shares it. It printed the command, its (in <cwd>) line and its output as they were. So when a pre-receive hook refused a leg push under --local-remote-dir <D>/re<U+202E>mote, the raw U+202E was printed three times: in the command, and in git's own To and error: lines.

The change is in adopt-project.py only. shape_materialize.py is unchanged.

  • _unacted(text, kept="") writes each character that str.isprintable refuses, other than those in kept, as _spelled_character spells it, with no backticks. That is what _first_said does to the line gh said.
  • _loudly(exc, step) builds the same REFUSED block from the command's arguments, its directory and its output, each passed through _unacted.
    • The output keeps its line breaks (kept="\n"), so it still reads one line per line.
    • This settles the point the issue left open: loudly prints the whole output, while _first_said keeps one line.
  • The leg push (_push_the_leg), _failed_loudly and main's catch-all now print a refused command through _loudly, not exc.loudly.
  • A printable character prints as it is, so a command with plain paths prints the same block as before.

I measured it with a real execute --allow-empty-leg spec and a global pre-receive hook that refuses every push:

  • U+202E. On 5208e874e, U+202E in --local-remote-dir reaches stderr raw 3 times. On this head it reaches stderr 0 times. The command, the (in ...) line and git's To line each name it as \u202e, six ASCII characters.
  • ESC. ESC behaves the same way. git itself writes ESC as ? in its error: line.
  • Byte 0xff. It was already printed as \udcff before this change, because stderr is backslashreplace. git's own lines hold U+FFFD for it, the character run decodes that byte to.

N4: a NOTE says when NEXT:'s clone command holds a character a terminal does not show

#1195 asks for a ruling: refuse such a source, as the README's clone line does, or add a note under NEXT:. The code settles it in favour of the note:

I measured the old behaviour on 5208e874e, with a full execute --local-remote-dir of a source under <D>/so<c>urce/Thing. U+200B, U+200F, U+200C and U+00A0 each pass, and the split line spells them. NEXT: prints each one raw, once, inside shlex.quote's single quotes.

On this head, the clone command is unchanged. When the url is not str.isprintable, this line prints just before the NEXT: block:

NOTE the path in the clone command below holds a character a terminal does not show as itself, so it can read as another path: it is `<D>/so\u200burce/Thing`, as the `split` line names it.

A url whose every character is printable gets no NOTE. A source name that ends in a space shows between the quotes, so it gets none either.

Where the NOTE goes differs from the issue's wording; please weigh it.

The new rows

The new file goes from 148 to 188 rows, 40 of them new. tests/test_adopt_github_exits.py keeps its 23 rows, with one assertion tightened.

  • K1, K2. test_a_printable_plan_is_named_as_typed_unless_900_spells_it runs check on the uncovered plan, in the plan's own directory, four ways. run_adopt now takes cwd= and over=.

    --plan given Named as
    absolute, ending in a space `<D>/plan.yaml `
    `plan.yaml`, relative, between backticks `\x60plan.yaml\x60`
    plan.yaml, relative plan.yaml
    ./plan.yaml plan.yaml

  • K4, K5. Two new ROWS entries. Each runs under check and execute, with the plan at each of the five path forms.

    • plan-wrong-kind-a-placement-plan expects kind is 'placement-plan', expected 'adoption-plan'.
    • org-too-long, with an org: of 40 o, expects `org:` is oooooooooooo...oooooooooooo, 40 characters.
    • The issue read both expected lines from the code. Both rows pass on 5208e874e, which confirms them.

  • K6. printable-U+00E9 is added to the remote-dir test. re<U+00E9>mote prints as typed, with no backticks.

  • K7. In tests/test_adopt_github_exits.py, the GitHub-mode split line must now match in full: split <12 hex> on adopt/three-repo-shape -> https://github.com/testorg/Northwind.git.

  • N4. SPLIT_SOURCES gains right-to-left-mark (U+200F). Every split row checks either that the NOTE comes before NEXT:, or that there is no NOTE.

  • N3. Three tests:

    • test_a_refused_command_names_the_remote_and_work_dirs_spelled is the probe above as a test. The character goes in both --local-remote-dir and --work-dir, for U+202E, ESC, byte 0xff and a plain control. The line feed is left out, because git's own output names the path and the output's line breaks are kept.
    • test_a_late_refused_command_is_said_spelled checks _failed_loudly, in process, for all five CHARACTERS.
    • test_a_refused_command_no_step_said_is_said_spelled checks main's own block, in process, with cmd_check replaced, for all five.

Proof

Before and after

Code The new file (188 rows) and tests/test_adopt_github_exits.py (23 rows)
#1177's head 5208e874e 12 failed, 199 passed
This head 73c9f57e8 211 passed
  • What fails on 5208e874e. The 12 failures are all N3 and N4 rows:
    • 2 split rows (U+200B and U+200F);
    • 2 refused-push rows (U+202E and ESC);
    • 4 _failed_loudly rows;
    • 4 main rows.
  • The K rows pass on 5208e874e, because its code is right there. Each one fails on its mutant (below).
  • Two N3 controls pass on both heads: byte 0xff, because stderr already wrote \udcff, and the plain control.

Mutants

All 20 were killed. Each was applied to adopt-project.py on this head and restored with git checkout. After every restore, the file's sha256 was 163c692aff5b076410bde27740439070f2b9a4c62b4a7dc9510199cb0a7f890a.

  • For K1 to K6, the whole new file was run (188 rows). For K7, tests/test_adopt_github_exits.py was run too (211 rows).
  • Under every K mutant, only new rows failed, so the old 148 stayed green.
  • M1 to M14 ran the N3 or N4 rows.
Mutant What it does Rows failed
K1 check's last count line names --plan by isprintable() alone 2 of 188
K2 that line names plan.path.absolute() 3 of 188
K4 the PLACEMENT plan's plan-wrong-kind names the plan raw 8 of 188
K5 adopt-org-too-long names the plan raw 8 of 188
K6 the extracted leg line spells every url outside ASCII 1 of 188
K7 the split line names the source's own path 2 of 211
M1 _loudly leaves the arguments raw 10 of 14
M2 _loudly leaves the working directory raw 10 of 14
M3 _loudly leaves the output raw 8 of 14
M4 the output's line breaks are spelled too 14 of 14
M5 the arguments are wrapped in backticks 14 of 14
M6 the leg push goes back to exc.loudly 2 of 14
M7 _failed_loudly goes back to exc.loudly 4 of 14
M8 main goes back to exc.loudly 4 of 14
M9 _unacted keeps every whitespace character 2 of 14
M10 no NOTE 2 of 4
M11 a NOTE wherever the split line spells the path 1 of 4
M12 the NOTE names the path raw 2 of 4
M13 the clone command is spelled, not quoted 4 of 4
M14 the NOTE prints after the block, not before it 2 of 4

Gate

The gate ran on the scratch merge 37c65a609 of origin/main 961640c68 and this head 73c9f57e8, in the landing gate's environment:

Shards Tests Result
g0 to g5: the 52 files listed below 3,250 3,246 passed, 4 skipped
d0 to d7: tests/test_shape_doctor.py 448 445 passed, 3 skipped
  • The shard lists hold each collected id exactly once. Every shard exited 0.
  • The 7 skips are environment skips. Three tests/test_adopt_root_symlinks.py rows need git 2.45 for the reftable backend. One tests/test_stdout_encoding.py row and the three doctor rows run only on Windows.
  • g1 ran twice. A host reboot killed its first run part way. It was re-run whole: 542 passed.
The 52 files in g0 to g5

tests/test_adopt_attributes_eol_block.py, tests/test_adopt_c_locale_bytes.py, tests/test_adopt_clone_template.py, tests/test_adopt_contracts_dir.py, tests/test_adopt_e2e.py, tests/test_adopt_entry_paths.py, tests/test_adopt_execute_refusals.py, tests/test_adopt_filter_repo_failure.py, tests/test_adopt_git_reads.py, tests/test_adopt_git_rm_bytes.py, tests/test_adopt_github_exits.py, tests/test_adopt_late_failure.py, tests/test_adopt_leg_keys.py, tests/test_adopt_mount_ignored.py, tests/test_adopt_pins.py, tests/test_adopt_plan_and_remote_paths_spelled.py, tests/test_adopt_plan_escapes.py, tests/test_adopt_reencoded_files.py, tests/test_adopt_relative_submodule_url.py, tests/test_adopt_root_manifest.py, tests/test_adopt_root_shape_paths.py, tests/test_adopt_root_symlinks.py, tests/test_adopt_spec_only.py, tests/test_adopt_spelled_names.py, tests/test_adopt_split_branch.py, tests/test_adopt_submodule_url_answers.py, tests/test_adopt_submodules.py, tests/test_adopt_temp_work_roots.py, tests/test_adopt_template_values.py, tests/test_adopt_tracking_branch.py, tests/test_adopt_unsearchable_source.py, tests/test_adopt_values_as_typed.py, tests/test_adopt_verify.py, tests/test_doctor_next_is_a_command.py, tests/test_dry_run_asks_remotes.py, tests/test_family_add_refusals.py, tests/test_git_output_decoding.py, tests/test_github_limits_init_check_org.py, tests/test_github_name_limits.py, tests/test_non_utf8_name_callers.py, tests/test_path_guard_hygiene.py, tests/test_refusal_default_remediation.py, tests/test_repo_hygiene.py, tests/test_ruleset_hint_tracking_branch.py, tests/test_scaffold_leg_path_collisions.py, tests/test_scaffold_manifest_values.py, tests/test_scaffold_reuse_seeded_legs.py, tests/test_stdout_encoding.py, tests/test_template_values_single_pass.py, tests/test_update_shape.py, tests/test_update_shape_add.py, tests/test_update_shape_merged_away.py

main has moved since the gate's base, and it touched adopt-project.py.

Merge-trees (2026-10-23T20:14Z)

git merge-tree --write-tree <X> 73c9f57e8 exits 0 (clean) for every <X> below:

No other open PR touches either test file. #1121, which adds lines after NEXT:, has not landed, and it merges cleanly.

Each of the 24 was also merged against a scratch merge of origin/main and this head:

Not in this PR

These are outside #1195, which covers adopt-project.py's --local-remote-dir:

🤖 Generated with Claude Code
Lane: openreposhape-4 (openRepoShape-4)

Summary by Sourcery

Harden adoption diagnostics and next-step guidance against invisible and terminal-sensitive path characters while extending coverage for previously untested output routes.

New Features:

  • Warn users when generated clone commands contain characters that terminals may not display faithfully.
  • Spell non-printable characters in refused command diagnostics while preserving output line structure.

Bug Fixes:

  • Prevent raw terminal-control and invisible characters from appearing in refused command paths, working directories, and command output.
  • Ensure GitHub-mode split diagnostics identify the pushed remote rather than the source path.

Enhancements:

  • Expand adoption path and refusal coverage for printable, invisible, control, and non-UTF-8 characters.
  • Strengthen plan naming and validation assertions across relative paths and additional refusal cases.

Tests:

  • Add regression and mutation-focused coverage for escaped refused-command blocks, clone-command notes, plan naming, remote-directory rendering, and GitHub split output.

Brett Heap @claude
adopt-project.py: a refused command's REFUSED block spells each chara…
73c9f57 
…cter a terminal acts on in the command, its directory and what it printed, as `_first_said` spells gh's, so `--local-remote-dir` no longer reaches the terminal raw there; a NOTE before `NEXT:` says when the source's path in its clone command holds a character a terminal does not show as itself; and rows pin six sites #1177's 148 left to chance (#1195)

Follow-up of #1177 (for #1152), from RV1177's review: its N3, N4 and
the surviving mutants K1, K2, K4, K5, K6 and K7. Stacked on #1177's
branch, and lands after it.

- N3: `_loudly` builds `CommandFailed.loudly`'s block from the command's
  arguments, its directory and its output, each `_unacted`: a character
  `str.isprintable` refuses is spelled as `_spelled_character` spells it,
  with no backticks, and the output keeps its line breaks. The leg push,
  `_failed_loudly` and `main` say a refused command through it.
- N4: a source path that `_refuse_an_unwritable_clone_url` lets through
  (#255) but `str.isprintable` refuses, as U+200B and U+200F, gets a NOTE
  just before `NEXT:` naming it `_spelled`. The clone command stays as
  `sh` reads it. Refusing such a source would undo #255's ruling that a
  `Cf` character other than the nine bidi controls is text a file holds.
  The NOTE is printed before the block, so it touches none of the lines
  that #1121 and #1207 add after it.
- Rows: a `--plan` ending in a space, a relative one between backticks,
  a relative one and `./plan.yaml` (K1, K2); a PLACEMENT plan's
  `plan-wrong-kind` and `adopt-org-too-long` with the plan's path spelled
  (K4, K5); a printable `--local-remote-dir` outside ASCII (K6); the
  GitHub-mode `split` line matched in full (K7); and N3's and N4's rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@brettheap brettheap added this to the v1.2.1 milestone Oct 23, 2026
@sourcery-ai

sourcery-ai Bot commented Oct 23, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR hardens adopt-project diagnostics against terminal-confusing characters by spelling unsafe content in REFUSED blocks, warns before pasteable clone commands containing visually hidden characters, and adds broad regression and mutant-killing coverage for these behaviors and related path-rendering cases.

Sequence diagram for sanitized adopt-project command refusals

sequenceDiagram
    participant AdoptProject
    participant GitOrGh
    participant Terminal
    AdoptProject->>GitOrGh: run command
    GitOrGh-->>AdoptProject: CommandFailed(args_list, cwd, output)
    AdoptProject->>AdoptProject: _loudly(exc, step)
    AdoptProject->>AdoptProject: _unacted(args_list, cwd, output)
    AdoptProject->>AdoptProject: _spelled_character(unsafe_characters)
    AdoptProject->>Terminal: print REFUSED block with sanitized command, directory, and output
Loading

Sequence diagram for hidden-character clone command warning

sequenceDiagram
    participant AdoptProject
    participant Terminal
    AdoptProject->>AdoptProject: _next_steps(...)
    alt clone URL is not printable
        AdoptProject->>AdoptProject: _spelled(url)
        AdoptProject->>Terminal: print NOTE before NEXT:
    else clone URL is printable
        AdoptProject->>Terminal: omit NOTE
    end
    AdoptProject->>Terminal: print pasteable NEXT: clone command
Loading

Flow diagram for safe rendering of adopt-project output

flowchart TD
    A[Command or clone path contains characters] --> B{str.isprintable}
    B -->|Printable| C[Render character as typed]
    B -->|Not printable in refused output| D[_unacted]
    D --> E[_spelled_character]
    E --> F[Sanitized REFUSED block]
    B -->|Not printable in NEXT clone path| G[Print NOTE with _spelled path]
    G --> H[Keep quoted clone command unchanged]
Loading

File-Level Changes

Change Details Files
Sanitize refused-command diagnostics so terminal-sensitive characters are spelled consistently across command arguments, working directories, and command output.
  • Added _unacted and _loudly helpers that preserve printable characters and line breaks while spelling non-printable characters.
  • Routed leg-push, late-failure, and top-level command-failure reporting through the sanitized formatter.
  • Kept plain printable-path output compatible with the previous REFUSED block.
 adopt-project.py
Warn users when a pasteable NEXT clone command contains a character that terminals may not display faithfully.
  • Added a pre-NEXT NOTE for non-printable clone-source paths without changing the quoted command.
  • Pinned NOTE presence, content, and placement for zero-width and directional-mark paths while excluding visibly trailing spaces.
 adopt-project.py
tests/test_adopt_plan_and_remote_paths_spelled.py
Expand regression coverage for path rendering, refusal diagnostics, and GitHub split output.
  • Added coverage for printable plan names, placement-plan and long-organization refusals, non-ASCII printable remote paths, and exact GitHub split lines.
  • Added end-to-end and in-process tests for sanitized refused commands across control, bidi, escape, and non-UTF-8 characters.
  • Extended split-source fixtures to verify the new NOTE behavior.
 tests/test_adopt_plan_and_remote_paths_spelled.py
tests/test_adopt_github_exits.py

Possibly linked issues

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sonarqubecloud

sonarqubecloud Bot commented Oct 23, 2026

Copy link
Copy Markdown

Quality Gate Passed 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

@brettheap

brettheap commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

RV1225: independent review of #1225 at 73c9f57e: READY

Independent adversarial review for lane 4. No MUST and no SHOULD. There are four NITs, below. Non-ASCII characters are written as escapes here.

Merge with current main

N3: every REFUSED block a refused command prints

grep loudly adopt-project.py on the merge: the three sites that printed exc.loudly(...) (_push_the_leg, _failed_loudly, which _report_late, the pull request and the topic use, and main's catch-all) all print _loudly(...) now. The step each one names is a fixed string; none holds a path.

Probes, a real execute --allow-empty-leg spec --local-remote-dir with a global pre-receive hook that refuses every push, the character in --local-remote-dir, in --work-dir, and in both (12 runs, all exit 2, no traceback):

Character Raw in stdout / stderr The block
U+202E 0 / 0 in all three the command, (in ...) and git's To and error: lines name it \u202e
ESC 0 / 0 in all three \x1b in the command, (in ...) and To; git writes ? in its own error: line
byte 0xff 0 / 0 in all three \udcff in the command and (in ...); git's lines hold U+FFFD (run decodes with replace)
LF the command and (in ...) say \n git's To and error: lines are split at the path's line feed (N1 below)
  • Every byte of information. _loudly drops nothing: the arguments, the directory, the exit code and the output all reach the block, and CommandFailed.__init__'s strip() cannot eat a spelled character. The one loss is run's errors="replace" decode of git's output, which predates this PR. The spelling is not reversible (N3 below).
  • Sites still raw, outside loudly. _init_refused (the git init argv), adopt-remote-unreadable and source-unclonable print a command's words or output raw; the PR body lists all three. adopt-mount-unaddable (git will not add the leg mount path {path}: {exc.output}) does too, and the body does not list it (N4 below).

N4: the NOTE before NEXT:

A full execute --local-remote-dir of a source under <D>/<parent>/Thing (exit 0 each):

Parent holds NOTE Raw character in stdout
U+200B one line, just before NEXT:, naming `<D>/so\u200burce/Thing` once, inside the quoted clone command
U+200F the same, `<D>/so\u200furce/Thing` once, inside the quoted clone command
U+00E9 (printable) none as typed, everywhere
U+3164 (printable, default-ignorable) none in split and NEXT: (the body's #1165 note)
nothing none -

Mutants

Each on adopt-project.py in its own worktree at 73c9f57e, run against the whole new file (188 rows; with tests/test_adopt_github_exits.py for K7, 211), restored with git checkout, sha256 163c692a... after every restore. K1 to K7 are RV1177's exact strings (rv1177/mutants.py). "Old rows" are the 148 ids collected at 5208e874e; a K mutant may fail only rows outside them.

Mutant What it does Result
K1 check's last count line names --plan by isprintable() alone killed: 2 of 188, both new (ends-in-a-space, relative-between-backticks)
K2 that line names plan.path.absolute() killed: 3 of 188, all new (the three relative forms)
K4 the PLACEMENT plan's plan-wrong-kind names the plan raw killed: 8 of 188, all new (plan-wrong-kind-a-placement-plan, check and execute)
K5 adopt-org-too-long names the plan raw killed: 8 of 188, all new (org-too-long, check and execute)
K6 the extracted leg line spells every url outside ASCII killed: 1 of 188, new (printable-U+00E9)
K7 split names source.path, not urls['assembly'] killed: 2 of 211, gh-pr-create-refused and the-topic-will-not-set of tests/test_adopt_github_exits.py's row whose split assertion this PR tightens
R2 _loudly spells the directory only when it is a str (a Path stays raw) killed: 2 (the real refused push, U+202E and ESC)
R4 _loudly leaves the output raw killed: 8
R5 _loudly keeps a line feed raw in the arguments killed: 2 (the two in-process line-feed rows)
R6 the leg push prints exc.loudly again killed: 2
R7 _failed_loudly returns exc.loudly again killed: 4
R8 main prints exc.loudly again killed: 4
R9 _loudly drops the (in <cwd>) line killed: 14
R11 the NOTE prints after the NEXT: block (the writer's M14, re-made) killed: 2 (zero-width-space, right-to-left-mark)
R12 the NOTE goes to stderr killed: 2
R14 no NOTE killed: 2
R13 the NOTE wherever _shown spells the url (the writer's M11) killed: 1 (name-ends-in-a-space)
R1 _unacted spells every character outside ASCII, printable or not survives: 188 passed (N2)
R3 _unacted keeps a tab raw in what the command printed survives: 188 passed (N2)
R10 the NOTE fires for any url outside ASCII, so a caf\u00e9 source gets one survives: 188 passed (N2)

Gate, on 43af5516c (targeted files only, sharded, -p no:cacheprovider, retention=failed)

Shards Ids Result
g0 to g5: 59 files, the 54 tests/test_adopt*.py files that print a REFUSED block, NEXT: or loudly (both changed files among them), test_repo_hygiene.py, test_path_guard_hygiene.py, test_stdout_encoding.py, test_non_utf8_name_callers.py and test_doctor_next_is_a_command.py 4553 4545 passed, 8 skipped; g0 758+1, g1 758+1, g2 758+1, g3 757+2, g4 758+1, g5 756+2
d0 to d7: tests/test_shape_doctor.py, from a bash script with mapfile, 25 ids holding spaces, no timeout under 600 s 449 446 passed, 3 skipped; 57, 56, 56, 55+1, 55+1, 55+1, 56, 56
  • Every shard exited 0, and each set of shard lists held every collected id exactly once.
  • The 11 skips are environment skips: three reftable rows (this bench has git 2.43), two test_adopt_eol_block_through_links.py rows, SHAPE_MEDXEHR_CLONE unset, a case-insensitive-disk row, test_stdout_encoding.py's Windows-default row, and the three Windows-only doctor rows.

Merge-trees

git merge-tree --write-tree of 73c9f57e against #1177's head 5208e874e: exit 0. Against origin/main 50ce8ab15: exit 0. Against all 24 other open PRs that touch adopt-project.py, run from this head, from the scratch merge and from origin/main alone (heads as fetched at 20:32Z):

Findings

  • N1 (NIT), a line feed in a path git echoes still splits the output block.
  • N2 (NIT), three new survivors of the K6 class. Each leaves behaviour this head gets right, measured above, but no row holds it:
    • R1. _unacted spelling every character outside ASCII passes all 188 rows: no row puts a printable non-ASCII character in a refused command, so the body's "a printable character prints as it is" is unpinned. Fix: add pytest.param("\u00e9", "\u00e9", id="printable-U+00E9") to the parameters of test_a_late_refused_command_is_said_spelled and test_a_refused_command_no_step_said_is_said_spelled (the_block_of then expects it as typed).
    • R3. _unacted keeping a tab raw in the output passes all 188; _first_said spells a tab. Fix: give a_refused_push's output a tab (for example " ! [remote rejected]\t(pre-receive hook declined)"), which the_block_of expects as \t.
    • R10. The NOTE keyed on url.isascii() passes all 188, so a caf\u00e9 source would be told its path "holds a character a terminal does not show as itself". Fix: add a printable-U+00E9 source to SPLIT_SOURCES (noted False), with named as it is for it, as where_a_leg_is does for spelled=None.
  • N3 (NIT), the spelling is not reversible. _unacted leaves a backslash as it is, so a path that holds the six characters \u202e and one that holds U+202E print the same line, under a header that says THE EXACT COMMAND WAS:. A Windows path such as C:\x1b\... reads like a spelled ESC. _first_said shares this, and nothing is dropped. Fix: one sentence in _loudly's docstring saying a spelled character and a typed backslash sequence read alike; no code change.
  • N4 (NIT), one raw route the body's "Not in this PR" leaves out. adopt-mount-unaddable prints git will not add the leg mount path {path}: {exc.output}, git's output as it is, beside the _init_refused, adopt-remote-unreadable and source-unclonable routes the body lists. I read this and did not probe whether a value that reaches it can hold a control character. Fix: add it to that list, and to the follow-up with validate-repository-naming.py, validate-manifest.py, validate-pins.py, repo_shape.py, shape_materialize.py and adopt-project.py: the helpers that spell a name for a line or for the plan test str.isprintable() or use repr, so a combining mark, a variation selector or a Hangul filler in a name prints raw, and ma U+034F in reads as main #1165.

No MUST and no SHOULD. The NOTE's place before NEXT: (the issue's N4) is the better one, for the reasons above; I would not move it.

Lane: openreposhape-4 (openRepoShape-4)

This was referenced Oct 23, 2026
Open
Open
Draft
Merged
This was referenced Nov 8, 2026
Draft
Merged
Draft
Draft
Draft
brettheap added a commit that referenced this pull request Nov 8, 2026
@brettheap @claude
adopt-project.py: execute unsets the leg clone's `remote.origin.url…
8e6f532 
…` before `git filter-repo`, so under a Latin-1 locale a source under a directory named with the byte 0xE9 is extracted, where filter-repo died printing that url as U+FFFD, and every branch of the source is still rewritten as before (#1247) (#1251)

**Head `abeac66d` is a fix round for RV1251's review of `825923c7`** (#1251 (comment)): one SHOULD and two NITs, set out under "Review round 1" below. The sections after that describe this head.

`_extract_leg` (`adopt-project.py`) now unsets the leg clone's `remote.origin.url`, in the clone's own config file (`--file <work>/.git/config`), right after its `checkout -B` and before `git filter-repo` runs. filter-repo then prints no url, so a Latin-1 stdout has no U+FFFD to choke on, and it still turns every remote-tracking branch into a branch first, so the legs are the ones main makes.

## The defect, reproduced on main

- **Where:** main `b1f45fa7`, git 2.43.0, Python 3.12.3, git-filter-repo 2.47.0 (`--version` prints `a40bce548d2c`).
- **Launcher:** pip's entry point, `#!.../frvenv/bin/python3`, WITHOUT `-E`. So any `PYTHON*` variable would reach filter-repo, and the reproduction sets none: no `PYTHONIOENCODING`, no `PYTHONUTF8`.
- **Locale:** Latin-1, built as #1214 and #1106 build it (`localedef -c`, `LOCPATH`, `LC_ALL=en_US.ISO-8859-1`).
- **Source:** `make_source_repo` under a directory `caf` plus U+00E9, made as the byte 0xE9. The probe runs `plan`, `check` and `execute --local-remote-dir` as the existing row does, but without its `PYTHONIOENCODING=utf-8:strict`.
- **Result:** `plan` exit 0, `check` exit 0 (`plan ok`), and `execute` exit 2:

```text
creating the leg repositories
  bare  .../remotes/Northwind-spec.git
  bare  .../remotes/Northwind-code.git
REFUSED extracting the spec leg. THE EXACT COMMAND WAS:
    git filter-repo --paths-from-file .../work/spec-paths.txt --force
    (in .../work/Northwind-spec)
    exit 1
--- output ---
Traceback (most recent call last):
  File ".../frvenv/bin/git-filter-repo", line 8, in <module>
    sys.exit(main())
  File ".../git_filter_repo.py", line 4973, in main
    filter.run()
  File ".../git_filter_repo.py", line 4878, in run
    self._migrate_origin_to_heads()
  File ".../git_filter_repo.py", line 4462, in _migrate_origin_to_heads
    print(m)
UnicodeEncodeError: 'latin-1' codec can't encode character '\ufffd' in position 237: ordinal not in range(256)
--- end output ---
```

  Both bare leg repositories were left behind. So the defect does not need a `-E` launcher: a person in a Latin-1 locale sets no `PYTHONIOENCODING`, and pip's launcher fails as well.

**The existing row, `test_adopt_template_values.py::test_a_utf8_source_directory_still_adopts_with_local_remotes`, under Latin-1 on main:**
- With pip's launcher it PASSES (1 passed). Its `PYTHONIOENCODING=utf-8:strict` reaches filter-repo through `execute`'s environment.
- With a copy of that launcher whose only change is `-E` on the shebang (a model of the pipx launcher #1247 quotes), it FAILS with the traceback above. That matches W1196's log.
- A check that `-E` drops the variable: with `PYTHONIOENCODING=bogus-codec`, pip's launcher dies at start-up, while the `-E` copy prints its version.
- On this head the row passes under Latin-1 with both launchers.

## The fix, and the two candidates measured first

**Candidate 1 in #1247, `git remote remove origin` before filter-repo, changes what the legs hold.** It is rejected.
- `git remote remove` deletes `refs/remotes/origin/*` with the remote. filter-repo then no longer makes the source's other branches into branches, and never rewrites them.
- **Measured** with `execute` on main and on candidate 1, over two ASCII sources with fixed dates. For each leg's bare repository the probe compared: every ref, every commit with its tree, parents and full message, and every reachable object.
  - **The standard source** (`make_source_repo`, one branch): identical.
  - **A source with a `release` branch** whose commit R changes `specs/001-feature/tasks.md`, no tag reaching it, and on `main` a `git cherry-pick -x R`: the spec leg's tip differs, `0316169e` on main against `82176493` on candidate 1, with the same tree `9ef5b676`. On main its message reads `(cherry picked from commit 407d03a0...)`, R as rewritten. Under candidate 1 it reads `(cherry picked from commit 30e9dd30...)`, R's id in the source.
- **It also changes behaviour that `check` describes.** As a mutant, it fails the existing `test_adopt_split_branch.py::test_the_extraction_stops_at_a_source_branch_in_its_way` (both rows, `DID NOT RAISE CommandFailed`). With it, a source branch in git's way no longer stops the extraction.

**Candidate 2, `LC_ALL=C.UTF-8` for the child, depends on the host.**
- **Measured here:** under `python3 -E`, a pipe's encoding is `utf-8` under `LC_ALL=C.UTF-8`. It is also `utf-8` under `C`, `POSIX` and a locale name that does not exist, because CPython falls back to C and turns on its UTF-8 mode.
- **But** it hands the child a locale by name and leans on that interpreter behaviour. It changes the locale of every git command filter-repo starts. And on Windows, `LC_ALL` does not set a pipe's encoding (read, not run).
- No `PYTHON*` variable is a cure, because a `-E` launcher drops it.

**What this PR does: unset the url only (`git config --file <work>/.git/config --unset remote.origin.url`).** It depends on no locale. It names the clone's own config file, so `GIT_CONFIG` in the environment cannot redirect it to a file of the person's (RV1251's SHOULD-1, below). It goes on from a clone that has no url to unset, and from no other failure (NIT-1, below).
- filter-repo's `_migrate_origin_to_heads` (2.47.0) turns `refs/remotes/origin/*` into `refs/heads/*` first. Then it returns when `remote.origin.url` is not in the config, before the NOTICE that printed the url and before its own `git remote rm origin`.
- **The legs are byte-identical to main's** in both probes above, the cherry-pick source included (`diff` of the dumps: empty).
- **What stays in each work clone** (measured on this head, a plain `execute`): the url-less `[remote "origin"]` with its fetch refspec `+refs/heads/*:refs/remotes/origin/*`, and `branch.main.remote = origin` and `branch.main.merge = refs/heads/main`. Nothing reads any of it. `_push_the_leg` pushes to the leg's url, and `_mount_the_legs` mounts the clone by its path. RV1251 measured that the mounted submodules hash the same as main's.
- The change is one `run` in `_extract_leg` (now with a `try`), with a comment. No pinned or template copy of `adopt-project.py` exists, and no document describes this step.

## Review round 1 (RV1251 at `825923c7`)

**SHOULD-1, fixed: the unset named no config file.** `GIT_CONFIG` in the environment (a legacy variable git still reads) names the file a bare `git config` writes.
- **Reproduced** with `GIT_CONFIG` naming a file that holds `[remote "origin"] url = ...` and the standard source, in `execute`:
  - main: exit 0, `adoption verified`, the file unchanged.
  - `825923c7`: exit 2, `REFUSED extracting the spec leg`, both bare legs made and nothing pushed, and the file empty. The first leg's unset took the only key out of it, and filter-repo, reading that empty file as the config, died with a `ValueError` in `get_config_settings`. (RV1251's file ended in an unset exit 5 instead.)
  - this head: exit 0, `adoption verified`, the file byte for byte as it was.
- **Fix:** `run(["git", GIT_CONFIG, "--file", str(work / ".git" / "config"), "--unset", "remote.origin.url"], cwd=work)`. `--local` is no way round it: with `GIT_CONFIG` set, git 2.43 exits 129 (only one config file at a time).
- **`work / ".git"` is a directory wherever `execute` reaches the clone:**
  - the clone is made by one `git clone -q <source> <work>`, and `work` is `work_root / <name>`, where `work_root` is `os.path.realpath(--work-dir)` or a `tempfile.mkdtemp` folder (`_work_root`): an absolute path;
  - an exported `GIT_DIR` does not move a clone (measured: the clone has its own `.git`);
  - an exported `GIT_WORK_TREE` does make the clone another shape (the git dir is `work` itself), but `execute` stops on it first, on main and on this head alike, at the legs' `git init --bare` (`REFUSED a git or gh command failed`);
  - a path whose directory is not there gives exit 255 (`could not lock config file ... No such file or directory`), which is refused.
- **Not changed:** filter-repo's own `git config --list` also reads the file `GIT_CONFIG` names (the `ValueError` above came from it), so under `GIT_CONFIG` it sees that file's settings, as on main. On this head the run ends `adoption verified` with the legs main makes.

**NIT-1, handled: a clone with no `origin` url.** A global `clone.defaultRemoteName = upstream` gives the clone a remote that is not `origin`, so `git config --unset remote.origin.url` has no key and exits 5 (measured, git 2.43). It can happen.
- **Before:** main fails at `mounting the legs` (`No such remote 'origin'`), after both legs are pushed. `825923c7` failed earlier, at the unset (exit 5, an empty output block), with nothing pushed.
- **Now:** `_extract_leg` goes on from exit 5 and from no other exit (`GIT_CONFIG_KEY_ABSENT = 5`). The run fails where main fails, not earlier and not worse. Any other exit of the unset (255 for a file git cannot lock, 129 for a usage it refuses) is refused as any failed command is.
- **Not mended here:** `clone.defaultRemoteName` itself. `execute` fails on it on main too. It is a follow-up for its own issue.
- Exit 5 also covers a config with more than one `remote.origin.url` (measured); that too goes on, as main does.

**NIT-2, corrected in this body:** the claim that the fix "depends on no locale and no variable" (it now names its config file, so `GIT_CONFIG` cannot redirect it); the work clone's leftovers, now listed under "What stays in each work clone" (the url-less `[remote "origin"]`, `branch.main.remote` and `branch.main.merge`); and the note that #1141 conflicts, dropped: it merges clean at `0b8fb320`.

## Rows

`tests/test_adopt_filter_repo_origin_url.py`, new, five rows:
- **`test_filter_repo_starts_with_no_origin_url_and_every_branch`** (any locale). `_extract_leg` itself runs over an ASCII source with a second branch, with `run` spied at the `git filter-repo` call. When filter-repo starts, the clone has no `remote.origin.url` and still has `refs/remotes/origin/main` and `refs/remotes/origin/beside`. filter-repo's output holds neither `Removing 'origin' remote` nor the source path. Afterwards the clone has `refs/heads/beside`, and the leg holds the clone's HEAD.
- **`test_execute_adopts_a_source_named_in_latin1_under_latin1`** (Latin-1). It runs `plan`, `check` and `execute --local-remote-dir` for #1247's source (a directory `caf` plus the byte 0xE9) in a Latin-1 locale the test builds as #1106 builds one. No `PYTHON*` variable is set except `PYTHONUTF8=0`. It asserts no traceback, `check` exit 0, `execute` exit 0 and `adoption verified`.
  - It skips, by name and with `localedef`'s words, where no Latin-1 locale can be built, and on Windows. It also skips where a name that is not UTF-8 cannot be made (`skip_where_a_name_cannot_be_not_utf_8`).
  - It builds its locale whatever the gate's locale is, so it fails on main under a UTF-8 gate too, which is what CI runs.
- **`test_execute_leaves_the_file_git_config_names_alone`** (new, SHOULD-1). `execute` with `GIT_CONFIG` naming a file that holds a `[remote "origin"]` url: `adoption verified`, and the file is byte for byte as it was. The child gets none of the gate's locale variables, so the row runs alike under a UTF-8 and a Latin-1 gate.
- **`test_a_clone_with_no_origin_url_goes_on_to_filter_repo`** (new, NIT-1). `_extract_leg` itself under a global `clone.defaultRemoteName = upstream`: the unset exits 5 (recorded as it is raised), the clone's `remote.upstream.url` is the source, and the leg is pushed.
- **`test_an_unset_that_fails_for_any_other_reason_stops_the_leg`** (new, NIT-1). A `config.lock` in the clone makes the unset exit 255: `CommandFailed`, before `git filter-repo` and with nothing pushed. It needs no git-filter-repo.
- The earlier rows' setup is now two helpers, `extract_the_code_leg` and `adopt_each_step`. The file is ASCII with no backslash escape; each non-ASCII character is built with `chr` or `bytes`.

**Counts** (5 rows; the same file in each tree):

| code | UTF-8 gate, pip and `-E` | Latin-1 gate, pip and `-E` |
|---|---|---|
| main (`_extract_leg` as on `dffc4226`; unchanged in `5926ccca`) | 4 failed, 1 passed (the GIT_CONFIG row passes: main leaves the file alone) | 4 failed, 1 passed |
| `825923c7` (the previous head) | 2 failed, 3 passed (the GIT_CONFIG row and the exit-5 row fail) | 2 failed, 3 passed |
| this head | 5 passed | 5 passed |

The two earlier rows fail on main and pass here. The GIT_CONFIG row fails on `825923c7` and passes here. The exit-5 row fails on `825923c7` and on main (no unset to record) and passes here. The lock row passes on `825923c7`, which refuses every exit; it guards the new `except`.

## Mutants

Each mutant was applied in a worktree at this head, the five rows plus the two `test_the_extraction_stops_at_a_source_branch_in_its_way` rows were run under UTF-8 and under Latin-1, and `adopt-project.py` was restored with its sha256 checked after each one (`RESTORED sha256 ok`, tree as before).

| mutant | UTF-8 | Latin-1 | killed by |
|---|---|---|---|
| M1: drop the unset | 4 failed, 3 passed | same | the two earlier rows, the exit-5 row and the lock row (no unset to see) |
| M2: the unset moved after `git filter-repo` | 2 failed, 5 passed | same | the two earlier rows |
| M3: `git remote remove origin` instead | 5 failed, 2 passed | same | the any-locale row, the exit-5 row, the lock row and both split-branch rows |
| M4: drop `--file <path>` | 1 failed, 6 passed | same | the GIT_CONFIG row |
| M5: `--local` instead of `--file <path>` (exit 129 under `GIT_CONFIG`) | 1 failed, 6 passed | same | the GIT_CONFIG row |
| M6: accept every exit of the unset | 1 failed, 6 passed | same | the lock row |
| M7: accept none (no `try`) | 1 failed, 6 passed | same | the exit-5 row |
| M8: `--file <work>/config`, a path with no file (exit 5, carried on) | 3 failed, 4 passed | same | the any-locale row, the Latin-1 row and the lock row |

## Gates

All runs used `/usr/bin/python3 -m pytest -p no:cacheprovider` with one reused basetemp per run kind and `-o tmp_path_retention_policy=failed`, at most 2 pytest runs at a time. The environment had a stub `gh` first on PATH, `GH_TOKEN=bogus`, an empty `GH_CONFIG_DIR`, `GIT_CONFIG_GLOBAL` an empty file and `GIT_CONFIG_NOSYSTEM=1`, and no `GIT_ALLOW_PROTOCOL`, `GIT_SSH_COMMAND`, `GIT_CONFIG`, `LANES_*` or `PYTHON*` variable. filter-repo was pip's launcher (`a40bce548d2c`) unless a row says otherwise.

**The tree gated is a scratch merge** of this head and `origin/main` `5926ccca` (built with `merge-tree` and `commit-tree`; its diff against main is this PR's two files). One hygiene row caught a path in the first attempt (`/home/someone/...` in the new test, `test_no_committed_file_names_a_host_absolute_path`); the row now names `https://example.invalid/keep-me.git`, and everything below ran on the final head.

**Gate A, UTF-8 (`LC_ALL=C.UTF-8`), one pytest, 23 files:** **1518 passed, 3 skipped** on the scratch merge.
- the new file, `test_adopt_template_values.py`, `test_adopt_split_branch.py` and `test_adopt_e2e.py`;
- the other four files that name `_extract_leg` or `_push_the_leg`: `test_adopt_filter_repo_failure.py`, `test_adopt_history_gitlinks.py`, `test_adopt_late_failure.py` and `test_adopt_plan_escapes.py`;
- `test_adopt_check_throwaway_repos.py`, `test_adopt_partial_clone_source.py`, `test_adopt_pins.py`, `test_adopt_git_rm_bytes.py` and `test_adopt_plan_partial_clone.py` (new on main);
- `test_repo_hygiene.py`, `test_path_guard_hygiene.py`, `test_cli_reference.py` and `test_scaffold_leg_path_collisions.py`;
- the files that scan every test file: `test_no_auto_maintenance.py`, `test_non_utf8_git_failures.py`, `test_git_output_decoding.py`, `test_setup_project_repo_misdrawn.py`, `test_shape_advisory.py` and `test_openreposhape_package.py`.
- **Main `5926ccca`, the same 22 files without the new one:** 1513 passed, 3 skipped. The head's extra 5 are the five rows.
- **Skips (`-rs`):** the same 3 on both, none for lack of git-filter-repo: `test_adopt_check_throwaway_repos.py:482` (a disk that keeps case) and `test_no_auto_maintenance.py:222` x2 (`GIT_CONFIG_GLOBAL` set).

**`tests/test_shape_doctor.py`, UTF-8, 4 shards from a bash script** (449 ids): **Head (scratch merge):** 118, 110, 118 and 100 passed, with 3 skipped in the last shard (the PowerShell line, Windows only): 446 passed, 3 skipped. RV1251 and W1247 measured 446 passed and 3 skipped on main at `11560eb9` and `b1f45fa7`; it was not re-run on `5926ccca`, because this PR touches nothing the doctor reads.

**Latin-1** (the locale built as above, `PYTHONUTF8=0`), the new file and `test_adopt_template_values.py`:

| run | main `5926ccca` | this head (scratch merge) |
|---|---|---|
| new file + `test_adopt_template_values.py`, pip launcher | 4 failed, 69 passed, 5 skipped (template values only) | 4 failed, 74 passed, 5 skipped |
| the same, `-E` launcher | 5 failed, 68 passed, 5 skipped (template values only) | 4 failed, 74 passed, 5 skipped |

- **The 4 failures are the same rows on main and on this head, under both launchers.** All four already fail on main and are #1214's to judge, not this PR's: `test_plan_refuses_a_flag_no_file_can_hold` at `--elected-by` and `--reference` with U+DCE9, `test_plan_and_check_say_the_same_sentence`, and `test_execute_refuses_a_local_source_directory_that_is_not_utf8` (under Latin-1 the byte 0xE9 is valid text).
- The fifth failure on main under `-E` is `test_a_utf8_source_directory_still_adopts_with_local_remotes`, which this head fixes.

## Merge-trees

`git merge-tree --write-tree` of this head against:
- **main:** clean against `5926ccca`, and the merged `adopt-project.py` keeps the `--file` call.
- **#1177 (`5208e874`) and #1225 (`73c9f57e`)**, the only open PRs that touch `_extract_leg`: both clean, each keeps the `--file` call.
- **#1141 (`0b8fb320`):** clean. The earlier note that it conflicts is stale and is dropped.
- RV1251 merge-tree'd the 27 other open PRs that touch `adopt-project.py` against the previous head: 26 clean, and #1237 conflicts in `tests/test_adopt_check_throwaway_repos.py` with main alone. This round re-ran main, #1177 and #1225 (the only PRs that touch `_extract_leg`) and #1141 (the one the earlier body got wrong).

## Not done, and limits

- Windows, another one-byte locale (cp1252 included) and a real pipx install were not run. The `-E` launcher here is a one-line copy of pip's.
- **Read from the code, not run:** filter-repo prints nothing else that holds the url. Its other prints of `origin` are under `--sensitive-data-removal`, which `execute` never passes.

Closes #1247

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.2.1

Development

Successfully merging this pull request may close these issues.

1 participant

@brettheap