Repository navigation
family.py and scaffold-project.py: add --local-remote-dir refuses a member clone URL .gitmodules cannot hold as text, --dry-run alike, each clone-URL remedy names a link's target and a lone surrogate, and the READMEs quote the clone URL, so the line under a b/ pastes as one command (#693) - #812
Conversation
… scaffold-project.py: `add --local-remote-dir` refuses a member clone URL `.gitmodules` cannot hold as text, `--dry-run` alike, each clone-URL remedy names a link's target and a lone surrogate, and the READMEs quote the clone URL, so the line under `a b/` pastes as one command (#693) #640 (#433) asked `unwritable_reason` of the clone URL the scaffold and `family.py init` write into their READMEs. Four things were left, each measured on main 8062810: - `family.py add --local-remote-dir <dir>/caf<e9>` exited 0, dry run and real, and committed the raw byte into the holder's `.gitmodules`, where the holder's `make siblings` (`siblings.submodule_urls`, `git config --list` read with `errors="replace"`) read the URL back as `caf\ufffd`; a line break was escaped as `\n` by git and read back cut short at it, and an escape and a bidi override were committed raw. `cmd_add` now asks `_refuse_an_unwritable_member_url` of `_member_url`'s result, after the branch question and before `_probe_member`'s throwaway clone, so the real run and `--dry-run` refuse alike, `family-unwritable-value`, with nothing read, mounted or written. - `--local-remote-dir link`, a UTF-8 name linked to a `caf<e9>` directory, was refused naming the RESOLVED URL by a remedy that said to pass a path that is UTF-8 text, which it was. Every clone-URL remedy now says the URL is the path the flag resolves to, and that where the path given is text and the URL is not, a link's target (or, for a relative path, the directory it was given from) is the one to rename. - `unwritable_reason` names a lone surrogate, which a Windows name can hold, and no clone-URL remedy did; both now say "or a lone surrogate", as #282's `UNWRITABLE_REMEDIATION` does. - `git clone --recurse-submodules {{CLONE_URL}}` (holder) and `{{CLONE_URL}}`/`{{ASSEMBLY_CLONE_URL}}` (assembly root, both legs) took a local path raw, so under `a b/` or `it's/` the line split when pasted. Both tools now render the value through `shell_quoted(url, "posix")`, POSIX whatever the host as #532 rules for pasted commands: a GitHub URL and an ordinary ASCII path stay bare, so the templates and their pins are unchanged, but a path holding `é` is quoted too, as `shell_quoted`'s conservative set has it, and `test_clone_url_bytes.py`'s two `café` README asserts now expect the quotes. The link sentence and the surrogate are static text in the remedies, not a branch on what was typed: it is true of every refusal (including #640's relative path under such a directory), and it leaves `cmd_init`'s and `_scaffold`'s call sites untouched. `add`'s remedy shares `init`'s words (`LOCAL_REMOTE_DIR_REMEDY`) with its own verb, GitHub URL and reasons: `.gitmodules` holds what `README.md` cannot, so its reasons say how the holder reads it back, not that it cannot be written. Closes #693 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main's 28 new commits since 8062810 merged with no conflict. Four of them touch a file this branch changes: #744, #728, #671 and #634 edit scaffold-project.py, and #728 and #671 edit scripts/family.py too, each in other functions than the clone URL's remedy, its refusal and the README values. Nothing this branch changes is a pinned copy, so nothing was re-pinned. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Reviewer's GuideThe PR hardens local clone-URL handling by refusing values that cannot round-trip through Sequence diagram for safe local member URL validationsequenceDiagram
participant User
participant Add as family.py add
participant Validate as _refuse_an_unwritable_member_url
participant Git as Git and filesystem
User->>Add: add --local-remote-dir path
Add->>Add: _member_url(repository, local_remote_dir)
Add->>Validate: _refuse_an_unwritable_member_url(local, url)
Validate->>Validate: unwritable_reason(url)
alt URL cannot be represented as text
Validate-->>Add: Refusal family-unwritable-value
Add-->>User: No git command or files changed
else URL is writable
Validate-->>Add: Continue
Add->>Git: Read member and update .gitmodules
end
Flow diagram for POSIX-quoted README clone URLsflowchart LR
URL[Resolved clone URL] --> Quote["shell_quoted(url, posix)"]
Quote --> README[Generated README git clone line]
README --> Shell[POSIX shell]
Shell --> Clone[One clone command argument]
Quote -.-> Safe[Quotes spaces and shell metacharacters]
Quote -.-> Bare[Leaves ordinary ASCII URLs bare]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
(lane openRepoShape-1, 2026-10-17T11:27:21Z) Lane 1's independent review of #812 at 831d76c: CHANGES NEEDED The code does what the issue asks. One remedy sentence sends How this was measured. The head's merge base with main is main's tip The issue's lines
Also checked:
Findings, ranked1. Must change:
|
…y says the directory must hold the member's bare repository and names no new directory as a way out, and the clone-URL remedies and `_template_values` say the apostrophe is escaped and a Windows path quoted, so none sends a person to a second refusal or says a README escapes no character (#693) Lane 1's review of #812 (comment 6233341619) measured three texts of this branch that were false for a person following them. - `LOCAL_REMOTE_DIR_REMEDY` is shared by `init` and `add`, and said the directory could be "renamed or made anew". For `init` and the scaffold that holds: they make the bare repository there. For `add` the directory must already hold the member's `<Project>.git` (`_probe_member` clones it), so a new, empty one is refused again with `repository ... does not exist`. The remedy now takes the caller's words (`{holding}`): `init` keeps its text byte for byte, `add` says "and that holds the member's bare repository `<Project>.git`, its directory renamed or the repository moved". `MEMBER_REMEDIATION` in the new test file follows, and a new test makes both ways out of `add`'s refusal exit 0 with the member mounted. - "with no character escaped" was false for the PR's own `it's` row, which prints `'<P>/it'\''s/...'`. Both remedies, the two comments above them and `QUOTED_NOT_ESCAPED` (renamed `QUOTED_ONE_WORD`) now say "every character but an apostrophe written as itself". - `shell_quoted` has no `\` in its bare set, so a Windows path is quoted (`'C:\Users\me\remotes\Atlas.git'`, read by `sh` as `C:UsersmeremotesAtlas.git` on main). `_template_values` said an ordinary path is left bare; it now says an ordinary POSIX path is, and the test comment above `FOLDERS` says `\`, which `sh` reads as an escape, not `\` and `:`. Two test nits: the composite assert in `assert_one_command` is two asserts, and `refused_text` binds the refusal before the `pytest.raises` block so it holds one call. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
(lane openRepoShape-1, 2026-10-17T11:47:24Z) Lane 1's re-verification of #812 at 372196a: PR-READY I built a local merge of
Runs on the merge (
Checks at 372196a:
New: nothing. The checklist items from my first review stay outside #693. Lane: openreposhape-1 (openRepoShape-1) |
|
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 12 hours and 7 minutes by commenting |
|
…mand-scope, and tests/test_local_remote_dir_urls.py calls `allow_file_protocol()`, so #812's test file still imports once #758 lands (#758) The retarget after #748 landed as 431c8cf. This merge has three parents: b6e1559 (this branch), 8368d1f (#748's final head, the restack of deb029c this branch stood on) and fa04d2c (main). Merging #748's head first gives git the history #748's squash dropped, so main's copy of #713 and #748 meets its own lines here. Main was d17564d when the conflicts below were resolved; its next two landings, #809 and #816, then merged without a conflict. Conflicts, and how each was resolved: - tests/test_adopt_empty_symlinks.py, against 8368d1f, which carries #388 through main 741679e. Both sides kept: #388's two targets (`with_each_target`, `SAID`, the `target` parameters) and #758's `symlinks_off()`, built at the call and appended to the command scope the environment holds. The `env` parameter is `off`, and `test_the_link_given_a_target_adopts_end_to_end`, which git did not flag, calls `symlinks_off()` too: left as it was, the file fails at collection. - tests/test_clone_just_pushed.py, tests/test_doctor_legs_members_next.py and tests/test_family_bump_remove_rollback.py, against main d17564d. Main's copy of each was 8368d1f's byte for byte (#748 squashed as 431c8cf, and nothing since had touched them), and this branch's copy is 8368d1f's plus #758's own hunks, so this branch's side is taken. Against main, the merged tree differs in #758's 25 files and in tests/test_local_remote_dir_urls.py. Each of the 25 differs by #758's own added and removed lines, tests/test_adopt_empty_symlinks.py aside. rv818's MUST 1 (lane 2): tests/test_local_remote_dir_urls.py came with #812 and imported `ALLOW_FILE_PROTOCOL` from test_family, which #758 replaces with `allow_file_protocol()`, so the file failed at collection and none of its 12 tests (27 cases) ran. It now imports `allow_file_protocol` and calls it at the paste. No other file on main uses a name #758 removes (`SYMLINKS_OFF`, `SYMLINKS_ON`, `CONFIG_COUNT`, `CONFIG_VALUE`, `PASTE_ENV`, `ALLOW_FILE_PROTOCOL`), and `NO_MAINTENANCE`, `QUOTE_PATH_OFF` and `tool_env` are used only in their own files. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
main gained #713 (#645), #812 (#693), #767 (#685), #745 (#704, #694) and #805 (#690) since 2000488. No conflict, and none of them touches README.md, the handbook, AGENTS.md, docs/ or the README cap's ledger, so every handbook marker reads as before. README.md is 1475 lines, its cap; AGENTS.md 494, its cap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…notes entries for the five pull requests landed since 2000488, current to main 40e678e (#563, #683) RV630's review of rounds A3, A4 and B1 (#630 issuecomment-6233641684, READY), its four optional nits: - 1: README.md's drift paragraph ended on the one-word line "finished.". "exactly " and "about the shape " are cut, and the paragraph re-wraps to seven lines with both bold leads whole. README.md is 1474 lines, its cap (1422 + 2 + 5 + 12 + 22 + 10 + 1); tests/readme-cap.d/683.txt is +1, and its text says why. Both `Keeping a project's shape current` markers move to df780eb5; the other 21 read current. - 2: #682's lead reads "A holder's `make bootstrap` that left a member unchecked-out". - 3: #730's entry says "Comments and docstrings", not "Four": its squash makes three claims, comments only. - 4: the doctor's `members` row has one bullet, #705's sentence and #624's together. Wave B2 so far, `git log --first-parent 2000488..40e678e`, each entry from its squash diff: - #713 (#645): the test session turns git's automatic maintenance off (Install, CI and the docs). - #812 (#693): `add --local-remote-dir`'s member clone URL, the remedies and the READMEs' quoted clone URL (the `--local-remote-dir` bullet). - #767 (#685): `members/` renamed with `git mv` is renamed back in two steps, and the git a printed `--sparse` needs (family). - #745 (#704, #694): lane 5's line (#630 issuecomment-6227129879), with v1.0.0's known limit at v1.0.0.md:463 (no git on `PATH`) said closed (Bootstrap). - #805 (#690): `make siblings`'s inputs (family). The counts (111 pull requests; 122 of 135 milestone issues closed at 16:14Z), "current to main" and the open list (13, each with a pull request open) are at main 40e678e, still placeholders. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s and #817's entries, #642's upgrade note for a holder's `cd` line, and v1.0.0's `--into` limit kept open, current to main 64ec47c (#563) RV630's pre-review of the B2 stage (lane 3's session): - SHOULD-1: the CI bullet said every test that clones a just-pushed local remote clones it safely (#554), but five plain-path `git submodule update --init` runs are left on main (tests/test_doctor_legs_members_next.py:631, :987, :1054; tests/test_family_bump_remove_rollback.py:337, :908). It now says "runs `git clone` on", which is what #554 changed. - NIT-1: #812's READMEs quote a local clone URL "that needs it". - NIT-2: #682's first sentence no longer repeats the lead ("there:"). Landed since 40e678e, each entry checked against its squash: - #747 (#681), 6626a3c, whose tree equals `merge-tree 40e678e ff7c59b`: a new family bullet, with RV630's NIT ("`member-gitlink-absent`'s remedy puts the gitlink back by hand"); v1.0.0's limits at v1.0.0.md:466 and :467 are said closed. - #817 (#708), 64ec47c, whose tree equals `merge-tree 6626a3c d63a267`, and whose own lines equal those of 8fd2fa6, the head RV630 read: RV630's text, appended to the YAML reader bullet. #738 (#695) is v1.1.1 (lane 5, #473 issuecomment-6236564004), so its docs entry is not taken, and v1.0.0's `--into` limit (v1.0.0.md:492) is listed as still open under Known limits. Its upgrade note is true of main since #642 (0ed1b84), so it goes in now, under "Install and upgrade": lane 5's sentence verbatim, and the README's `Y/Y` layout. Counts and the open list at main 64ec47c: 113 pull requests since bfed143; 124 of 135 milestone issues closed at 16:42Z; 11 open, each with a pull request open, #681 and #708 out, #695 marked as planned for v1.1.1. All still placeholders. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd what they tell git to the `GIT_CONFIG_COUNT` entries the environment holds, so a pasted line keeps the session's maintenance settings and the Windows job's two (#758) (#818) Against main. #748 landed as `431c8cf8`; this branch merged #748's final head and main (see Fix round 1). After that merge, #723 (`bad05d21`) and #746 (`fc867207`) landed. The branch is refreshed over main `fc867207` as `4a553e17`, one merge commit. Its only conflict was `tests/test_non_utf8_git_reads.py`'s `from conftest import` block, resolved by keeping both sides' names, as rv818 asked. The merged tree differs from main in #818's own 27 files, and #818's count scan flags no file. Tests only: 25 files under `tests/`, no shipped code, no pinned file, no README, handbook, `AGENTS.md` or `docs/cli.md` change. Two new commits on `b6e15591`: the retarget merge `2908e46f`, then the fix commit `eaa72d9e`. Tests only, as before. ### The retarget: merge commit `2908e46f` #748 landed as `431c8cf8` from its final head `8368d1f8`, and GitHub moved this PR's base to main. The merge commit has three parents: `b6e15591`, `8368d1f8` and main `fa04d2cd`. #748's head goes in first, so git sees the history the squash dropped. - **#748's head did not merge clean**, as rv818 found. `tests/test_adopt_empty_symlinks.py` conflicts, because `8368d1f8` holds main `741679e7`, which carries #388. - I took w758's earlier resolution of that file, which rv818 checked. Main's side of the file is the same at `974d4827`, `8368d1f8` and `fa04d2cd`, so it still fits. - Both sides stay: #388's two targets (`with_each_target`, `SAID`, the `target` parameters) and #758's `symlinks_off()`, built at the call. - `test_the_link_given_a_target_adopts_end_to_end` calls `symlinks_off()` too. Git does not flag it, and left as it was the file fails at collection. - **Main then conflicted in three files** (main was `d17564d9` when I resolved them): `tests/test_clone_just_pushed.py`, `tests/test_doctor_legs_members_next.py` and `tests/test_family_bump_remove_rollback.py`. - Main's copy of each was `8368d1f8`'s byte for byte. - This branch's copy is `8368d1f8`'s plus #758's own hunks, so this branch's side is taken. - Main's next two landings, #809 and #816, merged with no conflict. - **rv818 MUST 1.** `tests/test_local_remote_dir_urls.py` came with #812 and imported `ALLOW_FILE_PROTOCOL` from `test_family`. - Without a change, the file fails at collection: `ImportError: cannot import name 'ALLOW_FILE_PROTOCOL' from 'test_family'`, and none of its 27 cases run. - It now imports `allow_file_protocol` and calls `pasted(lines, paste, allow_file_protocol())`. - `git grep` finds no other file on main that uses a name #758 removes (`SYMLINKS_OFF`, `SYMLINKS_ON`, `CONFIG_COUNT`, `CONFIG_VALUE`, `PASTE_ENV`, `ALLOW_FILE_PROTOCOL`). `NO_MAINTENANCE`, `QUOTE_PATH_OFF` and `tool_env` change shape, and each is used only in its own file. - **Checked:** - Against main, the merged tree differs in #758's 25 files and in `tests/test_local_remote_dir_urls.py`. - Each of the 25 differs by exactly #758's own added and removed lines, `tests/test_adopt_empty_symlinks.py` aside. - Collecting the whole suite at the head finds 12,320 tests and no error. ### The fix commit `eaa72d9e` - **SHOULD 2:** `configure` in `tests/test_adopt_reencoded_files.py`, the seventh append from #758's line 2, now sets what `appended_to_command_scope(os.environ, pairs)` returns. It gives the same entries in the same order, so no test can fail on the old form. - **SHOULD 3:** the scan also reads a name the module binds to `"GIT_CONFIG_COUNT"` (`CONFIG_COUNT = "GIT_CONFIG_COUNT"`, annotated or not) as the count. It does so in a dict key, a setter's first argument and a subscript. - rv818's repro: append the base's two lines to `tests/test_adopt_root_symlinks.py`. - The old scan passes. The new one names line 2137. - **SHOULD 4:** `counts_set_to_a_number` is split into one reader per node type: `in_a_dict`, `in_a_call` and `by_subscript`. The walk takes each node's reader from `READERS`. By hand, no function scores more than 5 on S3776. I could not run Sonar. - **NIT 5:** `conftest.py` is no longer skipped. It scans clean, at the head and on today's main. - **NIT 6:** - New "sees" cases: `os.putenv("GIT_CONFIG_COUNT", "1")` and a bare `setenv("GIT_CONFIG_COUNT", "1")`. - The walk is now `files_off_their_count(directory)`. A new test runs it over a folder of its own, and checks three things: - `conftest.py` and a helper module are read; - a `.txt` file is not; - a file `OWN_COUNTS` allows is named for fewer counts as well as for more. - Four of rv818's six surviving mutants now fail a case: `putenv` dropped, the bare-name setter dropped, `!=` changed to `>`, and `glob("test_*.py")`. - The fifth, `conftest.py` scanned, is now how the code works, and putting the skip back fails the new test. - The sixth, the count's value taking any string, still survives: it is a stricter scan. Git refuses a count that is not a number (`bogus count in GIT_CONFIG_COUNT`, exit 128), so a test that holds one fails on its own. - **NIT 7, in part:** - An empty count (`""`) is now a count of its own. Git 2.43 and 2.47.1 read it as no entries at all. Measured: `GIT_CONFIG_COUNT= GIT_CONFIG_KEY_0=a.b GIT_CONFIG_VALUE_0=c git config --get a.b` exits 1. - The docstring says the scan reads the count's value as a constant only. - Left out: - a value built by a call (`str(len(pairs))`) or held in a variable; - an int; - `dict()`, or `environ={}` by keyword, handed to the helper; - `setenv` with keyword arguments; - an annotated subscript. rv818 judged only `str(len(entries))` likely, and telling it from a count worked out needs what the call means, not how it is spelled. - **NIT 8:** the paste case deletes `GIT_CONFIG_PARAMETERS` first. Measured on the merge before the fix commit, with `GIT_CONFIG_PARAMETERS="'probe.param'='p'"`, the case failed (`Left contains one more item: 'probe.param=p'`). It passes now. - **NIT 9, left as it is:** main's three appends built by hand each work the count out right: `tests/test_family_head_lost_gitmodules.py:97`, `tests/test_family_bootstrap_gate_order.py:135` and `tests/test_family_add_spellings_remedies.py:137`. Each of those files is also changed by another open PR (#828 `fix/761-add-move-back-gitmodules`, #820 `fix/762-validate-family-remedies-texts`, `fix/763-holder-docs-agree`), so converting them here would only add conflicts. - **NIT 10:** the comment on `NO_MAINTENANCE` now says its `maintenance.auto` and `gc.auto` repeat the session's two, which reach these gits in the command scope. Only `gc.autoDetach` adds a setting. The two stay, so the constant alone still keeps maintenance off for these gits. - **NIT 11:** the count was in lane 5's #473 summary, not in this body. The seven sets held 2,401 distinct passes, not 2,609: the 208-test run repeats three files of the 256-test run. ### Tests at `eaa72d9e` All runs went through `run9.sh`, with the real `gh` unreachable. All passed, none skipped: - **The 26 test files the PR changes against main**, in one full-file run each: 2,206 passed (1,108 + 1,098). - These are #758's 24 test files, `tests/test_local_remote_dir_urls.py` and `tests/test_adopt_reencoded_files.py`. - The count matches the collection exactly. - **The 43 files that import any of those 26, with `tests/test_repo_hygiene.py`, `tests/test_agents_md_codes.py` and `tests/test_cli_reference.py`:** 1,646 passed (825 + 821). - **The same merge made on main `d17564d9`** (a local commit, `c16a7e1a`, never pushed), before the fix commit: `tests/test_local_remote_dir_urls.py` and `tests/test_no_auto_maintenance.py`, 55 passed. ### Fails on the base The new tests were run against the base's scan and walk, as `b6e15591` has them, with `GIT_CONFIG_PARAMETERS` set: 6 failed, 30 passed. The six: - the paste case; - the `nothing`, `a-name-bound-to-it`, `an-annotated-name` and `a-name-by-subscript` cases; - the walk test. The three other new cases (`a-bare-setenv`, `putenv`, `a-name-bound-to-another-key`) are there to kill mutants, and they pass on any correct scan. MUST 1 fails on main's form at collection, as above. SHOULD 2 and NIT 10 change no behaviour. ### Mutants I made 26 mutants. 25 are caught, and one survives as rv818 said it would: - **22 on the scan and the paste case**, each run against `tests/test_no_auto_maintenance.py`: - the delenv dropped; - `putenv` dropped from `SETTERS`, or the bare-name setter; - the name branch of `is_the_count` dropped, or any name taken as the count; - `""` no longer a count; - the `Assign` names dropped, or the `AnnAssign` names; - no names collected; - each reader ignoring the names; - each of the three readers dropped from `READERS`; - `conftest.py` skipped; - `!=` changed to `>` or to `<`; - `glob("test_*.py")`, or `glob("*")`; - `by_subscript` taking any value; - the walk pointed at an empty folder. The last passes on a clean tree, like any change to an assertion that nothing is found. It is caught by rv818's SHOULD 3 repro, where the real walk fails and the mutant passes. - **The survivor:** the count's value taking any string. It makes the scan stricter (see NIT 6). - **`configure`:** with `appended_to_command_scope({}, pairs)`, the scan names the file. With the pairs dropped, three of its tests fail. - **MUST 1:** main's import put back fails collection with `ImportError`. ### Merge checks at `eaa72d9e` I ran `git merge-tree --write-tree` against `origin/main` (`fa04d2cd`) and the 53 other `fix/*` and `l3-*` heads on origin: - **Clean:** `origin/main` and 43 heads. - **#792 (`l3-785-non-ascii-paths`)** merges clean, but `tests/test_doctor_non_ascii_paths.py` imports `PASTE_ENV`. Whichever PR lands second changes `:50` (the import), the `:266` docstring and `:268` to `paste_env()`. - **#723 (`fix/663-non-utf8-name-tests`), #746 (`fix/672-bump-leg-branch-and-push-line`, which now holds #723) and #806 (`fix/757-non-utf8-test-dirs`)** conflict only in the `from conftest import` block of `tests/test_non_utf8_git_reads.py` (and of `tests/test_adopt_git_reads.py`, for #806). Both sides add names, so keep them all. - **Seven heads conflict with main itself, not with this PR:** - `l3-732-latin1-section` and `l3-733-ignored-sections` (`tests/test_doctor_legs_members_next.py`); - `fix/712-validate-pins-contracts`, `fix/766-spelled-paths` and `fix/798-adopt-legs-asked-first`; - `l3-677-pins-hand-step-wip` and `l3-678-naming-checkout-wip`. - I scanned each clean merged tree's changed test files for a name #758 removes, or a count the scan flags. Only #792 shows up. - **Main moved after the push.** #723 landed as `bad05d21`, and the two now conflict in one place: the `from conftest import` block of `tests/test_non_utf8_git_reads.py`. The fix is to keep both sides' names: `ORG, PROJECT, REPO, appended_to_command_scope, cloned_just_pushed, git, run_script, skip_message, skip_where_fsencode_gives_a_surrogate_no_byte, windows_fsencode`. Every other change merges in. On that merged tree, with the block resolved that way, the scan flags no file, and no file uses a name #758 removes. On `deb029c9` a probe test imported `PASTE_ENV` and pasted `git config --show-origin --list` through `paste()`: - `PASTE_ENV` held `GIT_CONFIG_COUNT=1` (`protocol.file.allow`), a stale `GIT_CONFIG_KEY_1=user.email` left from collection, and no `GIT_CONFIG_GLOBAL`. - `os.environ` at the call held a count of 4 and the session's global file. - The pasted git listed one command-line entry, `protocol.file.allow=always`, and `git config --get maintenance.auto` exited 1. So a pasted line ran with git's automatic maintenance on, in both scopes, as the issue said. `paste_env()` replaces it. It reads `os.environ` at each call (minus `PYTHONPATH`), adds the identity and `PYTHONDONTWRITEBYTECODE`, and appends `protocol.file.allow` with `appended_to_command_scope`. The six uses call it. The import-time copy also predated the session's `gh` guard on `PATH`, as `test_doctor_next_is_a_command.py`'s docstring says of its own environment; a pasted line now gets the `PATH` of the call. That docstring now says `paste_env()` reads at the call too. - **The seven.** Six are on this base, each now one call to `appended_to_command_scope`: `test_adopt_git_reads.py`, `test_adopt_nested_attributes.py`, `test_adopt_ignored_shape_files.py`, `test_adopt_root_symlinks.py`, `test_bootstrap_after_bump.py`, `test_family_bump_remove_rollback.py`. The seventh, `test_adopt_reencoded_files.py`, came with #743 and is converted in fix round 1. `test_clone_just_pushed.py:96` stays, as the issue says: it is the helper's own test. - **The other literals.** Each sets a count of its own and is laid over `os.environ`, where the session's two settings and the Windows job's two sit, so each replaced them (`NO_MAINTENANCE` put the two maintenance keys back and dropped the job's two). I read each; each is converted, none left: - `test_family.py`: `ALLOW_FILE_PROTOCOL` is now `allow_file_protocol()`; its two importers (`test_siblings_family_name.py`, `test_family_siblings_placement.py`) and the four uses in `test_family.py` call it. - `test_family_dry_run.py`: `NO_MAINTENANCE` is a tuple of entries, appended by `isolated()`. - `test_non_utf8_git_reads.py`: `QUOTE_PATH_OFF` is a tuple of entries, which `tool_env()` appends; `user_name()` appends too. - `test_adopt_empty_symlinks.py`: `SYMLINKS_OFF` is `symlinks_off()`. - `test_adopt_root_symlinks.py`: `SYMLINKS_OFF` and `SYMLINKS_ON` are gone; the four tests that set the first call `links_as_files` and the two that set the second call `told_git`, which already appended. - one-off environments: `test_scaffold_reuse_seeded_legs.py`, `test_adopt_split_branch.py`, `test_scaffold_free_text_values.py`, `test_adopt_plan_yaml_quoting.py`, `test_setup_project_py.py`, `test_setup_sh.py`, `test_doctor_next_is_a_command.py`. - The two tests that build a held environment on purpose to see it kept (`test_adopt_git_reads.py`, `test_clone_just_pushed.py`) now build it with `appended_to_command_scope({}, ...)`. Measured with a probe on a throwaway bare remote after a push (18 loose objects): the hand-built options and `clone_just_pushed`'s give the same clone, one pack and no loose object, with `upload-pack` and `pack-objects` in `GIT_TRACE`, and `origin` keeps the plain path. A plain local clone of the same remote keeps all 18 loose objects. For a clone that is not recursive the helper's `insteadOf` rewrite is inert, because no submodule is cloned; the helper adds one place for the options, not a different read. - `test_bump_leg_dry_run_truth.py:98`, the private bare copy of the session's spec remote: a session remote. `cloned_just_pushed(..., bare=True)`; the fixture now asserts the copy is bare. - `test_bump_leg_dry_run_truth.py:110`, `private_work`, a clone of that private remote, which tests push tags into: `cloned_just_pushed(..., recurse=False)`. - `test_doctor_legs_members_next.py:1828` and `:1929`, `Ghost`, a clone of the first member's checkout in the test's own holder copy: not a remote, and the hand-built `-c protocol.file.allow=always` was dead weight on a direct clone. Both call `clone_member`, one helper over `cloned_just_pushed(..., recurse=False)` that asserts the legs stay empty. - `test_family_bump_to_checks.py:235`, a clone of `a_private_remote`, a bare repository filled by a push: `cloned_just_pushed(remote, work, "main", recurse=False, env=env)`. - `test_doctor_pin_restore_merges.py:714` is **left as it is**: a `--filter=blob:none` clone of a file URI. The helper takes a path, and has no way to pass `--filter`. - `test_family_dry_run.py`: the comment on `NO_MAINTENANCE` says what the entries are for, the gits `isolated()` hands an environment to, whose global file `excluding()` replaces, and that the session fixture covers the module's own setup commits, which pass no `env=`. - `tests/conftest.py`: the two i645 replacements, "repacks once its estimate, taken from `objects/17` alone, is above 256 loose objects, so two loose objects there are enough", and the `objects changed` sentence. **#713's body holds the matching sentence and needs the same two replacements; I cannot edit a body.** `tests/test_no_auto_maintenance.py` has 17 new cases (28 in the file, 11 before): - `test_a_pasted_line_keeps_the_entries_the_environment_holds_at_the_call`: the environment holds two entries as the Windows job does, then the two settings as the session fixture appends them, set after the doctor tests were imported. `git config --show-origin --list` through `paste()` lists the four, in order, then `protocol.file.allow`, and reads both settings from the session's global file. - `test_no_test_sets_a_git_config_count_of_its_own`: reads every `tests/*.py` but `conftest.py` with `ast` and fails a `GIT_CONFIG_COUNT` set to a number (dict, keyword, `setenv`/`setdefault`/`putenv`, subscript) and an `appended_to_command_scope` call handed a dict written out. Three files are allowed the ones that build a held environment on purpose, with a count each in `OWN_COUNTS`. - 15 cases that hold the scan itself: seven spellings it must see, eight it must leave (`env.get("GIT_CONFIG_COUNT", "0")` is a read). Runs at the head `b6e15591` through `run9.sh` (the real `gh` unreachable), all passed, none skipped: - `test_no_auto_maintenance.py`, `test_doctor_legs_members_next.py`, `test_doctor_next_is_a_command.py`: 208 passed. - `test_clone_just_pushed.py` and `test_doctor_validator_rows.py` (it imports the doctor file), with those three: 256 passed, run one commit earlier (the later commit only moved two blank lines and reworded one failure message). - The sweep files: `test_adopt_git_reads.py`, `test_adopt_nested_attributes.py`, `test_adopt_root_symlinks.py`, `test_adopt_ignored_shape_files.py`, `test_bootstrap_after_bump.py`, `test_family_bump_remove_rollback.py`, `test_adopt_empty_symlinks.py`, `test_adopt_split_branch.py`: 445 passed. - `test_non_utf8_git_reads.py`, `test_family.py`, `test_siblings_family_name.py`, `test_family_siblings_placement.py`, `test_scaffold_reuse_seeded_legs.py`, `test_scaffold_free_text_values.py`, `test_adopt_plan_yaml_quoting.py`: 999 passed. - `test_family_dry_run.py`, the five files that import its `isolated` (`test_family_bump_to_a_tag.py`, `test_family_bump_to_checks.py`, `test_family_bump_to_local_only.py`, `test_family_refuse_up_front.py`, `test_family_remove_gitmodules.py`) and `test_bump_leg_dry_run_truth.py`: 216 passed. - `test_setup_project_py.py`, `test_setup_sh.py` and their importers (`test_adopt_shape_pin_and_work_dir.py`, `test_dry_run_asks_remotes.py`, `test_clone_url_bytes.py`, `test_setup_by_hand_commands.py`): 313 passed. - `test_repo_hygiene.py`, `test_agents_md_codes.py`, `test_cli_reference.py`, `test_doctor_pin_restore_merges.py`, `test_git_helper_identity.py`: 172 passed. `render-cli-reference.py --check` is ok. **On the base (`deb029c9`)**, with the new `test_no_auto_maintenance.py` copied in: 2 of the 17 fail, the paste case and the scan (it finds 12 files). The other 15 hold the scan itself and pass on any tree. Each of the new hunks was broken in a throwaway and run against its tests: - `paste_env`: a literal count of 1, a snapshot taken at import, and `appended_to_command_scope({}, ...)`: each fails the paste case, and the first and third the scan too. - The 18 sites that call `appended_to_command_scope(os.environ, ...)` (or `env`), each with `{}` in place of the environment: each fails the scan. Sites: the six of line 2, `allow_file_protocol`, `isolated`, `tool_env`, `user_name` (non-UTF-8), `symlinks_off`, `git_reads_github_from`, `github_at`, `user_name` (free text), `test_adopt_plan_yaml_quoting.py`, the two setup tests, `test_doctor_next_is_a_command.py`. - The scan: eight changes (ignore dicts, keywords, `setenv` calls, subscripts, `appended_to_command_scope({}`; flag `os.environ`; flag a read; allow fewer in `OWN_COUNTS`) each fail a scan case. - The clone sites: dropping `bare=True` from the private remote, `recurse=False` from `clone_member`, and `"main"` from the checks clone each fail. Two **survive as equivalents**: `recurse=False` in `private_work` (the spec leg has no `.gitmodules`, so a recursive clone of it is the same clone) and `env=env` in the checks clone (a clone reads no config the isolated environment changes). - The two comments cannot be mutated. `git merge-tree --write-tree` of this head against `origin/main` (`974d4827`), #748 (`deb029c9`), #713 (`979bb394`) and every `fix/*` and `l3-*` head on origin (46 refs): - **Clean:** #748, #713, and 24 of the 46 heads: 21 `fix/*` heads (every lane-5 PR but #723 and #756, and most of lane 4's) and `l3-677`, `l3-678`, `l3-rehearsal-v1.1.0`. - **`origin/main`: one file, `tests/test_adopt_empty_symlinks.py`** (four hunks), against #388's landing (`5c990708`). The parent #748 merges clean, so this arrives with the retarget. Resolve by keeping main's lines and my `symlinks_off()`. Two more spots need the same change and git does not flag them: - `tests/test_adopt_empty_symlinks.py::test_the_link_given_a_target_adopts_end_to_end` uses `SYMLINKS_OFF`, which this change removes: give it the `off` parametrization. - `tests/test_adopt_reencoded_files.py::configure`, the seventh hand-built append, goes to `appended_to_command_scope(os.environ, pairs)`. - `tests/test_family_head_lost_gitmodules.py::on_disk` (on main) is one more hand-built append: `{**env, **appended_to_command_scope(env, [("core.ignorecase", ignorecase)])}`. The scan does not read for hand-built appends, so it is not required, but it is the same thing. I made that merge in a scratch tree (main at `bf1f099b`), resolved it as above, and ran the files this change touches that main also changed, with `test_adopt_empty_symlinks.py`, `test_adopt_reencoded_files.py`, `test_family_head_lost_gitmodules.py`, `test_no_auto_maintenance.py` and the files that import `isolated` or the doctor helpers: 653 and 245 passed, in two runs. The scan finds no other file of main's with a count set by hand. - **#723 (`fix/663-non-utf8-name-tests`):** `tests/test_non_utf8_git_reads.py`, the `from conftest import` block, where both add names. - **`l3-732`, `l3-733`:** `tests/test_doctor_legs_members_next.py` (the `from conftest import` block) and `tests/test_adopt_empty_symlinks.py`. - **19 more** (`fix/664`, `fix/729` (#756) and 17 `l3-*` heads): `tests/test_adopt_empty_symlinks.py` only, because they carry main's #388 as main does. - **Not a textual conflict, but it breaks on merge:** `l3-785-non-ascii-paths` imports `PASTE_ENV` (`tests/test_doctor_non_ascii_paths.py:47` and `:223`); it must call `paste_env()`. #812 (`fix/693-local-remote-dir`) landed first, with `tests/test_local_remote_dir_urls.py` importing `ALLOW_FILE_PROTOCOL` (`:51`, `:349`). This PR's retarget merge makes it call `allow_file_protocol()` (rv818 MUST 1). - A pasted `git submodule update --init` still clones a local path: `paste()` takes no remotes, so no `insteadOf` rewrite reaches it (rv662's "build it from `read_through_git`"). It runs under the session's maintenance settings now, which is what the issue asks. - Three more appends built by hand beside `appended_to_command_scope`, in files this base does not hold: `tests/test_family_head_lost_gitmodules.py:97` (main), `tests/test_family_bootstrap_gate_order.py:135` (#682), `tests/test_family_add_spellings_remedies.py:137` (#767). The scan reads for counts set to a number, not for appends worked out by hand. Review: lane 2's independent review (issuecomment-6236874291) and its re-verification of eaa72d9, READY (issuecomment-6238781272); its new SHOULD is taken in #848, and 4a553e1 merges main over eaa72d9. CI (Brett Heap's ruling 27): H0 is the head 4a553e1 itself, which passed tests, tests-git-bash and SonarCloud; main has touched none of its files since its base fc86720; and the local gate passed on its predicted squash tree 9d6c099 over main d8b8eb3: the adopt_empty_symlinks, adopt_git_reads, adopt_ignored_shape_files, adopt_nested_attributes, adopt_plan_yaml_quoting, adopt_reencoded_files, adopt_root_symlinks, adopt_split_branch, bootstrap_after_bump, bump_leg_dry_run_truth, clone_just_pushed, doctor_legs_members_next, doctor_next_is_a_command, family, family_bump_remove_rollback, family_bump_to_checks, family_dry_run, family_siblings_placement, local_remote_dir_urls, no_auto_maintenance, non_utf8_git_reads, scaffold_free_text_values, scaffold_reuse_seeded_legs, setup_project_py, setup_sh, siblings_family_name, repo_hygiene, agents_md_codes, cli_reference sets, 2359 passed. Landed on Brett Heap's word of 2026-10-17T21:47:28Z, in lane 5's session (posted on #473 as issuecomment-6241758069): "merge lane 5's v1.1.1 PRs once reviewed and green". Closes #758 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>



Lane: openreposhape-4 (openRepoShape-4)
Claim: #693 (comment)
What changed and why
#640 (#433) asked
shape_materialize.unwritable_reasonof the clone URL that the scaffold andfamily.py initwrite into their READMEs. This PR asks the same question offamily.py addand fixes what the remedy and the README line still got wrong. All four of #693's lines were re-measured on main8062810d, with local bare remotes, aghshim andGIT_ALLOW_PROTOCOL=file:add --local-remote-dir <dir>/caf<e9>.gitmodulesholds the raw0xE9; the holder'ssiblings.submodule_urlsreads it back ascaf\ufffd. A line break was written as\nand read back cut off at the break; an escape and U+202E were committed rawfamily-unwritable-value, both dry run and real, before_probe_member's throwaway clone; the holder is byte for byte as it was--local-remote-dir link, wherelinkis a UTF-8 name pointing at acaf<e9>directorycaf\udce9URL; the remedy says only "Pass a --local-remote-dir whose path is one line of UTF-8 text"unwritable_reasonnames it; no clone-URL remedy doesUNWRITABLE_REMEDIATIONdoesa b/orit's/git clone --recurse-submodules <P>/a b/remotes/Atlas.git, written raw into the assembly root's README (twice), each leg's README and the holder's README'<P>/a b/remotes/Atlas.git'and'<P>/it'\''s/remotes/Atlas.git'add(scripts/family.py).cmd_addcalls the new_refuse_an_unwritable_member_url(local, url)on_member_url's result. The call comes after the branch question and before the member is read, so--dry-runrefuses the same way. It usesinit's question andinit's refusal code. The refusal now reads:Remedies (
LOCAL_REMOTE_DIR_REMEDY, shared byinitandadd, and the scaffold'sCLONE_URL_REMEDIATION). They gain the resolved-path sentence and the lone surrogate. What the directory must hold is each caller's:initand the scaffold make the bare repository there, so they keep "a directory renamed or made anew", byte for byte;addreads the member's<Project>.gitfrom it, so a new, empty directory is refused again (repository ... does not exist), and its remedy says the path "holds the member's bare repository<Project>.git, its directory renamed or the repository moved" (LOCAL_REMOTE_DIR_REMEDY's{holding}; lane 1's review of this PR measured the second refusal). Each README remedy now says the README prints the URL "in agit cloneline, in quotes where a shell needs them, every character but an apostrophe written as itself", in place of "as it stands".README clone line.
family.py's_template_valuesandscaffold-project.py's values table both renderCLONE_URL(andASSEMBLY_CLONE_URL) throughshell_quoted(url, "posix"), as the issue directs.Tests
tests/test_local_remote_dir_urls.py, 27 tests, on the harnesses oftest_clone_url_bytes.pyandtest_family_add_refusals.py, importing their helpers rather than copying them.add, fourBAD_DIRECTORIESrows × {dry run, real}: exit 2, empty stdout, stderr in full,assert_unchangedon the holder, no throwaway clone, noghcall.addthrough a link, both runs; acafécontrol that still mounts.add's own remedy followed: from the refusedcaf<e9>directory, renaming it, or moving the bare repository into a directory of text, each makesaddexit 0 with the member mounted.initthrough a link, both runs, with no process started.\ud800) asked of each of the three refusal functions.a b,it'sand an ordinaryplain(left bare on POSIX, quoted on Windows) for the scaffold's three READMEs and the holder's. Each line is checked withshlex.split, and the holder's is pasted intosh, where it clones.tests/test_clone_url_bytes.pyspells the two remedies in full, so its fixed text gains the new sentences, and itsQUOTED_NOT_ESCAPEDconstant, whose words were false for theit'srow, isQUOTED_ONE_WORD. Its twocaféREADME assertions now expect the quotes.GIT_ALLOW_PROTOCOL.bf1f099b(head831d76c6): 57 files (the new file, the 53 existing files that runfamily.py, the scaffold, the README rendering or the clone-URL remedies,tests/test_repo_hygiene.py,tests/test_cli_reference.pyandtests/test_agents_md_codes.py): 3565 passed, 23 skipped, 1 xfailed in 35:22, exit 0. The skips, by name: 16 intest_family_init_values.py:219("this template quotes the placeholder") and 3 at:291("this template quotesid:"), both pre-existing and not about this change; 1 intest_stdout_encoding.py("the default this is about is Windows' own"); 3 intest_shape_doctor.py(the PowerShell line, which only a Windows host prints). The xfail istest_shape_doctor.py's strict one for siblings.py (make siblings): a FILE, or a directory it cannot list, at../<Project>ends in a traceback (NotADirectoryErrororPermissionErroratsiblings.py:558), exit 1, where the doctor calls the slot NOT A CLONE #735. No test of this PR is skipped.372196a7, lane 1's review): the new file,test_clone_url_bytes.py,test_family.py,test_family_init_values.py,tests/test_repo_hygiene.py,tests/test_cli_reference.pyandtests/test_agents_md_codes.py: 415 passed, 19 skipped in 4:07, exit 0 (the 16 + 3 skips above). The round changed remedy text, two comments, a docstring and the new file's tests only, and no other test file spells a remedy it changed (git grepfor "made anew" and "no character escaped" finds only the two files run).tests/test_local_remote_dir_urls.py, and is restored.bf1f099b: all five killed.addno longer asks the question (thecmd_addcall removed): 10 failed, 15 passed. Killed bytest_add_from_a_directory_no_file_can_name_is_refused(8 rows) andtest_add_through_a_link_to_such_a_directory_says_the_target_is_the_fault(2).LOCAL_REMOTE_DIR_REMEDY: 14 failed, 11 passed.CLONE_URLandASSEMBLY_CLONE_URLraw again: 2 failed, 23 passed. Killed bytest_the_scaffolds_readmes_print_a_clone_line_that_pastes_whole.CLONE_URLraw again (_template_values): 2 failed, 23 passed. Killed bytest_the_holders_readme_prints_a_clone_line_that_pastes_whole. M5 is beyond the 2-4 the wave brief asks; it is there because M3 covers only the scaffold's README.372196a7: the four mutants that touch the changed text, all killed. M1, M3 and M5 mutate code this round did not change, so they were not re-run.LOCAL_REMOTE_DIR_REMEDY: 14 failed, 13 passed. Killed by theaddrefusal rows (8), theaddlink test (2), theinitlink test (2), and the two lone-surrogate tests of the family remedies (2).test_the_scaffold_explains_a_lone_surrogateand the scaffold's link test (2).add's{holding}back to "a directory renamed or made anew": 11 failed, 16 passed. Killed by theaddrefusal rows (8), theaddlink test (2) andtest_add_explains_a_lone_surrogate.test_the_scaffold_explains_a_lone_surrogateand the scaffold's link test (2).What it cannot tell
.gitmodulesreads back "another path" on Windows is reasoned, not run, and CI has no Windows job since ci: run the test suite on Linux only #597.shell_quotedleaves bare only[A-Za-z0-9_./:@%+=,-], so a remote undercafé/now prints in quotes, and so does a Windows path, which is spelled with\('C:\Users\me\remotes\Atlas.git'; bare on main, whereshread it asC:UsersmeremotesAtlas.git). A GitHub URL and an ordinary ASCII POSIX path stay bare, so no template and no pin changes, and an existing README is never re-rendered.--local-remote-dirwhose clone URL is not text a README can hold is refused before any remote or folder exists,--dry-runalike, so a directory named in a legacy code page no longer ends in aUnicodeEncodeErrortraceback after the bare remotes are made (#433) #640's relative-path case included, and the call sites incmd_initand_scaffoldstay untouched.adopt-project.pyis not changed (read from the code, not run). Itsexecute --local-remote-dirrenders"CLONE_URL": urls["assembly"]and"ASSEMBLY_CLONE_URL"raw (adopt-project.py:11874-11875), so a source undera b/prints the same split line in the READMEs it writes, and itsadopt-unwritable-valueremedy says nothing of a link's target or a lone surrogate. The issue names the scaffold andfamily.pyonly, so both are left, for Tracking: adopt-project.py, converting an existing repository in place #457's checklist.Shared files
Main has moved since this PR opened. It is
1eeef3b6now (#670, #703 and #706 landed). The branch holds the merge ofbf1f099b(831d76c6) and one fix commit on top of it (372196a7), and merges clean with1eeef3b6, which changedscripts/family.py(#670'sadd) and two lines oftests/test_family_add_refusals.py, a file whose fixtures this PR's new file imports. The refresh and its gate are the coordinator's.Earlier, on the merge of
bf1f099b: #744, #728, #671 and #634 editscaffold-project.py, and #728 and #671scripts/family.py, each in other functions than this PR's hunks. #806 editstests/test_clone_url_bytes.py'sdirectory_namedand its imports; this PR's new file importsdirectory_namedby the name and signature #806 keeps. #688's head, before its own refresh, conflicted with main alone infamily.py's import block (#728's import), not with this branch.Each merge-tree below used
git merge-tree --write-treewith head372196a7, and every head was read from GitHub at 2026-10-17T11:40Z (ls-remotefor main, a fetch of each branch):1eeef3b6: clean, treefd6f8f05041dfix/684-family-holder-statesat772105aa: clean, tree42384f251489fix/766-spelled-pathsat0e516bba: clean, treeb1d60f53f737fix/689-next-push-refspecatb5a7a7f1: clean, tree3df6255778e0fix/688-family-init-valuesat6958dfbc: clean, treef9499a421df4fix/757-non-utf8-test-dirsat6b5e9074: clean, treedf8efde8394dfix/686-bump-reachabilityat444934d9: clean, tree9889e2edc91dl3-786-ci-gitmodules-rulesat31c3792c: clean, tree7a237b4c4564fix/685-spellings-remediesatac764679: clean, tree1ebb9232a9bafix/695-family-docsatc42286de: clean, treebce54e899c0cfix/692-raw-folder-pathsatab55c29f: clean, treea9784ae5a6e7fix/690-siblings-inputsat5ba1f8b3: clean, treeb0e18add8719fix/681-validate-family-remediesat58d6c50f: clean, tree0753855de2b4Closes #693
🤖 Generated with Claude Code
Summary by Sourcery
Prevent unsafe local clone URLs from being written while making generated README clone commands safe to paste.
Bug Fixes:
.gitmodules, including during dry runs, before reading or modifying the member.Enhancements:
Tests: