Skip to content

adopt-project.py: the --plan path in check and execute, and the --local-remote-dir path in execute's bare, leg and split lines, are named with _shown, as #1154 names --out and --work-dir, so a bidi control, an escape, a line feed or a byte that is not UTF-8 in either no longer reaches the terminal as it was typed (#1152) - #1177

Draft
brettheap wants to merge 1 commit into
mainfrom
fix/1152-plan-and-remote-paths-spelled

Conversation

@brettheap

@brettheap brettheap commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1152

This PR lands only after the v1.2.0 tag.

What

#1154 (#1115) names --out and --work-dir in plan's and check's report with _shown. #1152 lists the two groups of paths it leaves raw: the --plan path in check and execute, and the --local-remote-dir path in execute. A bidi control (U+202E), an ESC or a line break in either reached the terminal as it was typed, where the other plan lines name a path with _shown.

This change spells each of the 21 sites with _shown(str(...)), in adopt-project.py only. A path with a character that is not printable is named as _spelled names it, between backticks. Every other path prints byte for byte as it did, and no refusal code is new.

The --plan path, 16 sites:

  • the five N finding(s) in <plan> lines: _refuse_a_mistyped_plan, _refuse_what_check_finds, the plan-split-branch-taken refusal, and cmd_check's two branches;
  • the <plan>: ... refusals: plan-wrong-kind, plan-unsupported-mode, plan-unreadable, plan-no-source, the three plan-bad-id ones, plan-not-the-source, adopt-topic-too-long, and the legs.assembly and org: ones.

The --local-remote-dir path, 5 sites:

  • the <bare> already exists refusal;
  • the bare <bare> line;
  • the two leg lines, in _extract_leg and _seed_leg, which end -> <url>;
  • the split <commit> on <branch> -> <url> line. It names the source's directory under --local-remote-dir.

The NEXT: clone command is not touched. It already quotes the url with shlex.quote, and a row pins that it still does.

Behaviour change

A path that is not UTF-8 now prints \udcff between backticks, as #1154's paths do, where it printed raw before. A plain path prints as it did.

Names

#1152 calls the wrong-kind refusal plan-bad-kind. The code's name for it is plan-wrong-kind, and that is the code this change spells. No refusal was renamed.

Not in this PR

repo_shape.load_yaml's own refusals, yaml-unreadable, still print a path raw. They live in scripts/repo_shape.py, and they are a separate follow-up: see #1168.

Proof

tests/test_adopt_plan_and_remote_paths_spelled.py is new, ASCII only and POSIX only, with every non-ASCII character written as an escape.

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

Summary by Sourcery

Prevent unsafe user-supplied plan and local remote paths from reaching terminal output unescaped.

Bug Fixes:

  • Safely spell --plan and --local-remote-dir paths in checks, refusals, and execution output so terminal-control characters and non-UTF-8 bytes cannot be emitted raw.

Tests:

  • Add comprehensive coverage for unsafe and printable plan, remote-directory, and split-path output across refusal and execution scenarios.

… `--local-remote-dir` path in `execute`'s `bare`, leg and `split` lines, are named with `_shown`, as #1154 names `--out` and `--work-dir`, so a bidi control, an escape, a line feed or a byte that is not UTF-8 in either no longer reaches the terminal as it was typed (#1152)

## What

#1154 (#1115) names `--out` and `--work-dir` in `plan`'s and `check`'s report with `_shown`. W1115 found two groups of paths it leaves raw (#1152): the `--plan` path, and the `--local-remote-dir` path. A bidi control (U+202E), an ESC or a line break in either reached the terminal as it was typed, where the other plan lines name a path with `_shown`. This change spells each of the 21 sites with `_shown(str(...))`, so a path with a character that is not printable is named as `_spelled` names it, between backticks, and every other path prints byte for byte as it did.

The `--plan` path, 16 sites in `check` and `execute`:
- the five `N finding(s) in <plan>` lines: `_refuse_a_mistyped_plan`, `_refuse_what_check_finds`, the `plan-split-branch-taken` refusal, and `cmd_check`'s two branches;
- the `<plan>: ...` refusals: `plan-wrong-kind` (the code's name for it; the issue calls it `plan-bad-kind`), `plan-unsupported-mode`, `plan-unreadable`, `plan-no-source`, the three `plan-bad-id` ones, `plan-not-the-source`, `adopt-topic-too-long`, and the `legs.assembly` and `org:` ones.

The `--local-remote-dir` path, 5 sites in `execute`:
- the `<bare> already exists` refusal;
- the `  bare  <bare>` line;
- the two leg lines, `_extract_leg` and `_seed_leg`, which end `-> <url>`;
- the `split <commit> on <branch> -> <url>` line, which names the source's directory under `--local-remote-dir`.

The `NEXT:` clone command is not touched: it already quotes the url with `shlex.quote`, and a row pins that it still does.

A path that is not UTF-8 now prints as `\udcff` between backticks, where it printed raw before. No refusal code is new, and none changes its name or its exit code.

The changed lines that ran past 79 columns are rewrapped; the message text is unchanged.

`repo_shape.load_yaml`'s own refusals still print a path raw, as `yaml-unreadable`. They live in another file and are a separate follow-up (see #1168).

## Test

`tests/test_adopt_plan_and_remote_paths_spelled.py` is new, ASCII only and POSIX only, with every non-ASCII character written as an escape. It has 148 rows:
- 135 plan rows: 14 refusal families (27 family and command pairs) by 5 path forms (U+202E, ESC, a line feed, the byte `\xff`, and a plain control);
- 5 `--local-remote-dir` rows for the `bare` and leg lines (the code leg is extracted, the spec leg seeded);
- 3 `split` rows. `execute` refuses a source path that holds U+202E, ESC, a line feed or `\xff` as `adopt-unwritable-value` before any leg exists, so these rows use U+200B in the parent and a name that ends in a space, and a plain control;
- 5 in-process rows for `{bare} already exists`, which call `_create_leg_remote`.

On main's `adopt-project.py`, 118 of the 148 fail, each for the raw path, and the 30 that pass are exactly the plain controls. On this commit all 148 pass. 23 mutants (each new spelling put back to raw, one at a time, and `_shown` swapped for `_spelled` at two sites) are each killed by the rows that name them.

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

sourcery-ai Bot commented Oct 22, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR hardens terminal-facing diagnostics in adopt-project.py by routing all previously raw --plan and --local-remote-dir path output through _shown, while preserving existing behavior for printable paths and the quoted NEXT: command; a broad POSIX regression suite validates all affected refusal and progress-message sites.

Sequence diagram for safely displaying plan and remote paths

sequenceDiagram
    participant User
    participant AdoptProject as adopt-project.py
    participant Shown as _shown
    participant Terminal

    User->>AdoptProject: check or execute with --plan / --local-remote-dir
    AdoptProject->>Shown: _shown(str(path))
    Shown-->>AdoptProject: escaped printable representation
    AdoptProject->>Terminal: print refusal or progress message
    Terminal-->>User: path shown safely between backticks
Loading

Flow diagram for path rendering in adopt-project.py

flowchart LR
    PlanPath[--plan path] --> PlanMessages[check and execute diagnostics]
    RemotePath[--local-remote-dir path] --> RemoteMessages[bare and split progress/refusals]
    PlanMessages --> Shown["_shown(str(path))"]
    RemoteMessages --> Shown
    Shown --> Terminal[Terminal output]
    Printable[Printable path] --> Same[Byte-for-byte output]
    Unsafe[Bidi, ESC, line feed, or non-UTF-8 byte] --> Escaped[Escaped path between backticks]
    Shown --> Printable
    Shown --> Unsafe
Loading

File-Level Changes

Change Details Files
Sanitize every targeted --plan path before it is emitted in check/execute findings and refusal messages.
  • Apply _shown(str(path)) to plan-loading, validation, finding-summary, and repository/source-related diagnostics.
  • Preserve existing refusal codes, wording, and byte-for-byte output for printable paths.
adopt-project.py
Sanitize --local-remote-dir-derived paths in execute progress, refusal, and split output.
  • Apply _shown to bare repository creation and already-exists messages.
  • Apply _shown to extracted, seeded, and split remote URL/path lines.
  • Leave the NEXT: clone command's existing shlex.quote behavior unchanged.
adopt-project.py
Add comprehensive regression coverage for unsafe and printable path forms across command paths.
  • Exercise bidi controls, ESC, line feeds, non-UTF-8 bytes, and plain paths across plan refusal/finding families.
  • Cover bare, extracted-leg, seeded-leg, already-existing, and split output cases.
  • Verify unsafe paths are absent raw while printable paths remain unchanged, with POSIX/environment skips where necessary.
tests/test_adopt_plan_and_remote_paths_spelled.py

Assessment against linked issues

Issue Objective Addressed Explanation
#1152 Spell the --plan path with _shown in the check and execute finding summaries and refusals so terminal-control characters and non-UTF-8 bytes are not printed raw. ✅
#1152 Spell the --local-remote-dir path with _shown in execute’s bare-directory, leg URL, already-exists refusal, and split output lines. ✅

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

Copy link
Copy Markdown

@brettheap

Copy link
Copy Markdown
Contributor Author

RV1177: independent review of #1177 at 5208e874: READY

Independent adversarial review for lane 4. No MUST and no SHOULD. There are five NITs, below; two of them (N3, N4) are follow-up issues outside #1152's words. Non-ASCII characters are written as escapes here.

Merge with current main

Completeness (#1152's two groups)

Probes: real runs of main's tool and the merge's tool on the same paths

  • --plan: 108 pairs (6 sites by 18 path forms). The sites are check uncovered, check mistyped, check wrong-kind, execute uncovered (with --local-remote-dir), execute bad org: and execute no source.
--plan path holds main PR
U+202E, U+2066, ESC, LF, CR, TAB raw (an LF splits the line) spelled between backticks; no raw character anywhere in stdout or stderr
byte 0xff \udcff with no backticks (stderr is backslashreplace) `.../pl\udcffan.yaml`
a space, a backtick inside, ", ', caf\u00e9, relative -plan.yaml, plain absolute, plain relative, ./plan.yaml as typed the whole of stdout and stderr byte for byte equal to main
a trailing space; relative `plan.yaml` as typed spelled (#900's rule; N1)
  • Same exits, and the strict run. Every pair exits the same on both tools. A second run under utf-8:strict (0xff, U+202E, LF, caf\u00e9, plain) gives the same results, with no traceback on either tool.
  • The two sites the rows leave out. A PLACEMENT plan's plan-wrong-kind and adopt-org-too-long, with U+202E or ESC in --plan: raw on main, spelled on the PR, and the plain path unchanged.
  • --local-remote-dir: 19 forms, each a full execute --allow-empty-leg spec that exits 0 on both tools. On main the bare and both leg lines carry a raw U+202E, a raw 0xff, or a line split at the LF. On the PR they are spelled, and a scan of the PR's entire output finds no raw U+202E, ESC, LF, 0xff or TAB from the path. Printable forms (a space, quotes, a backtick, caf\u00e9, a trailing space, -remote, ./remote) are byte for byte equal to main.

The split rows (point 4)

  • Confirmed. A source whose parent holds U+202E, ESC, LF, CR, TAB or 0xff is refused as adopt-unwritable-value on both tools: exit 2, before any leg, with no remote directory made.
  • What gets through. U+200B, U+200E and U+200F pass (BIDI_CONTROLS is the nine embeddings, overrides and isolates), and the PR's split line spells them.
  • No new row is needed. The refusal is pinned through a real execute in test_adopt_template_values.py (refused_then_moved: U+202E and byte 0xE9), and unwritable_reason has its own unit rows.

The behaviour change (point 3)

No test, doc or docs/cli.md text reads the raw form:

  • test_stdout_encoding.py's only check runs use a plain adoption-plan.yaml;
  • no doc shows sample output of these lines;
  • git grep of finding(s) in, bare , commits kept) ->, SEEDED from ... -> and three-repo-shape -> in *.md, *.html and docs/ finds nothing.

The wording is imprecise for the --plan sites (N2).

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

55 files, 4532 ids. They are the brief's seven (the new file, test_stdout_encoding.py, test_non_utf8_name_callers.py, test_adopt_plan_escapes.py, test_path_guard_hygiene.py, test_repo_hygiene.py, test_scaffold_leg_path_collisions.py) and every test file that holds finding(s) or already exists.

Shard Result
g0 564 passed
g1 (test_non_utf8_name_callers.py among them) 568 passed
g2 567 passed
g3 568 passed
g4 (test_stdout_encoding.py) 568 passed, 1 skipped (its Windows-default row)
g5 (test_adopt_plan_escapes.py, test_repo_hygiene.py) 567 passed
g6 (the new file, 148; test_path_guard_hygiene.py, 49) 567 passed
g7 (test_scaffold_leg_path_collisions.py) 562 passed

That is 4531 passed and 1 skipped, every shard exit 0. #1111's rule (test_path_guard_hygiene.py) passes on the merge with current main.

test_shape_doctor.py: 448 ids in 8 shards, run from a bash script with mapfile, every list non-empty, the lists together holding each collected id once (25 ids hold spaces), and no timeout under 600 s.

Shard Result
d0 56 passed
d1 56 passed
d2 55 passed, 1 skipped
d3 55 passed, 1 skipped
d4 55 passed, 1 skipped
d5 56 passed
d6 56 passed
d7 56 passed

That is 445 passed and 3 skipped (the Windows-only rows), every shard exit 0, the same as #1154's doctor count. d3 to d7 first errored at setup because the lane coordinator had removed the shards' basetemp parent folder to free disk; rerun with the folder restored, they passed as above.

Mutants (mine; each on adopt-project.py in its own worktree, restored with git checkout, sha256 checked after each)

W1152b's 23 are each a site put back raw or swapped to _spelled. These are new, on the changed lines and their call sites, run against the new file (148 rows):

Mutant Result
K0: check's last finding(s) in line raw again (sanity) killed: 24 failed
K1: that line spelled by isprintable() alone (no #900 rules) survives: 148 passed
K2: that line names plan.path.absolute() survives: 148 passed
K4: the PLACEMENT plan's plan-wrong-kind names the plan raw survives: 148 passed
K5: adopt-org-too-long names the plan raw (adopt-bad-org kept) survives: 148 passed
K6: the extracted leg line spells every non-ASCII url survives: 148 passed
K7: split names source.path, not urls['assembly'] survives: 171 passed (with test_adopt_github_exits.py)
K8: bare spelled by isprintable() alone survives: EQUIVALENT (see below)
K9: split spelled by isprintable() alone killed: 1 failed (the trailing-space row)
K10: already exists through _line_shown killed: 5 failed
K12: the seeded leg line spells only a line break killed: 3 failed
K13: the mistyped check line names plan.path.name killed: 5 failed

Weighed: why N1 and N5 stay NITs.

Merge-trees

git merge-tree --write-tree against all 18 open PRs that touch adopt-project.py, run both from this head and from the scratch merge: exit 0 every time. They are #1180 70f324859, #1178 fa56168ab, #1171 159815d75, #1170 24ee133aa, #1169 b8e63bcf0, #1154 8a7828332, #1153 ff33ba12a, #1143 7af5fabc3, #1141 7a2711d03, #1125 9549006fe, #1123 5c3908f8d, #1122 c01c4d73d, #1121 1c5173758, #1120 c55e55e33, #1119 bba168bdd, #1118 2307036ab, #1106 504a1dd65 and #1105 b28f40de0.

Findings

Lane: openreposhape-4 (openRepoShape-4)

This was referenced Oct 23, 2026
@brettheap brettheap added this to the v1.2.1 milestone Oct 23, 2026
This was referenced Oct 23, 2026
This was referenced Nov 8, 2026
brettheap added a commit that referenced this pull request Nov 8, 2026
…` 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

Labels

None yet

Projects

None yet

1 participant