Skip to content

setup-project.py: a passed-through --dry-run makes none of the preflight's offers and --shape-ref is held to git's rule part by part, so a rehearsal no longer writes git's global config and a.lock/b is refused before the clone (#857) - #873

Merged
brettheap merged 16 commits into
mainfrom
fix/857-setup-dry-run-offers
Oct 18, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

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

Stacked on #811 (fix/777-typed-name-edges); retarget to main once it lands.

#756 has landed as 4dea85ee, and #811's head is now b6f56cad: #756's squash merged in, and a test-only change to tests/test_setup_project_name_edges.py. This branch stays on 050abea6, the merge base of the two, so its diff against #811's branch is its two commits: the fix (79a33f11) and fix round 1 (ac570069), which changes two test files and nothing else. Lane 5 restacks it after #811 lands.

Line 1: a passed-through --dry-run makes none of the preflight's offers

Direction: the silent reading. A run passed -- --dry-run reports what is missing and asks nothing that would change the machine. Where a terminal would have been offered a command, it is told not offered: --dry-run was passed through. A run without it offers this here, and so does --preflight., and asked nothing. The plan's last line now ends: The preflight made none of its offers; a run without --dry-run makes each one marked `not offered` above. --preflight still makes every offer, whatever follows --: it is the run that exists for them (#59), and it returns before anything reads the passthrough.

Why. Measured at 050abea6, what each offer does on a typed yes, and all three change the machine:

  • an install (offer_missing): git or gh, through the package manager;
  • gh auth login (_offer_login): gh's own sign-in;
  • gh auth setup-git (_check_credential_helper): gh writes a credential helper into the person's global git config.

A pty run of setup-project.py Sample --org demoorg --elected-by Tester --into <dir> -- --dry-run, under a signed-in stub gh and a git with no helper, asked Run `gh auth setup-git` now? Type yes to continue:. The typed yes ran it, and the stub, which writes what the real one writes, added [credential "https://github.com"] to the file GIT_CONFIG_GLOBAL names. The run then ended at the plan saying nothing was created and nothing was cloned. With gh signed out, the same run asked Run `gh auth login` now?, and the yes signed the stub in. The help said a --dry-run there "ends this run at the plan, creating nothing". And #811's CI run 39027235147 showed five rehearsals that typed only the project name waiting at that question until they were killed. Nothing argued for the offering reading: the offers stay one flag away (--preflight), and the real run makes them.

What changed. preflight passes offering = not passes_a_dry_run(opts.passthrough) to preflight_checks, and it goes down through _check_git, _check_gh, offer_missing, offer_install, _offer_login and _check_credential_helper to offer. offer tests it after can_ask(), so a run with no terminal prints what it printed before, and it is a bool, never Options, so test_the_offer_helpers_cannot_see_the_yes_flag still holds. The help says it, and docs/cli.md is regenerated. AGENTS.md, README.md and the handbook were grepped for dry-run, creating nothing and the offers: no sentence there is made false, so none is touched.

The git install offer cannot be reached by a dry run end to end: _main refuses a machine with no git (GIT_MISSING) before the preflight runs. It is held in process, through preflight, as the other two are.

Tests (tests/test_setup_project_dry_run_offers.py):

  • test_a_dry_run_makes_none_of_the_offers_preflight_makes[install|login|helper], once for --dry-run and once for its short spelling --dry (passes_a_dry_run reads both), in process on every platform. A fake terminal types yes to anything and a recorder runs nothing. preflight on Sample --org demoorg -- <flag> makes no prompt and runs no offered command, says not offered once per offer (2, 1, 1), and refuses or passes as before. preflight_only on --preflight -- <flag> then makes the same 2, 1, 1 offers.
  • test_a_dry_run_with_no_terminal_says_what_a_run_without_it_says[install|login|helper], in process: with no terminal (stdin is not one and there is none to open), preflight on -- --dry-run prints no not offered line, and what it prints is what the same run prints without --dry-run. It holds the order in offer, can_ask() before offering.
  • test_a_dry_run_leaves_git_with_no_helper_as_it_found_it[py|piped|py with --dry], a pty run (PTY_SKIP). A signed-in stub gh, an empty system config and a global config of the run's own, with no GIT_CONFIG_COUNT and run from a folder that is no repository. The run asks nothing, exits 0 at the plan, prints the [??] warning with the not offered line under it and the new last line, never calls gh auth setup-git, and leaves the global config the bytes it was.
  • test_a_dry_run_does_not_sign_gh_in: the same with gh signed out (git has a helper, so the login is the one offer at stake). It asks nothing, the [!!] line has the not offered line under it, it refuses one or more prerequisites are missing, and gh is still signed out.

Line 2: --shape-ref is held to git's rule part by part

Direction: finish the rule, in a new why_not_a_ref beside why_refused, which checked_ref asks after REF_RE. Git's rule is per part: no part is empty (//, a trailing /), none starts with ., none ends in .lock, and the whole holds no .. and does not end in .. Each failure has its own sentence (WHY_EMPTY_PART, WHY_DOT_PART, WHY_LOCK_PART, WHY_DOT_END). WHY_RANGE and WHY_LOCK keep #811's words, so its ref-range and ref-lock cases are unchanged. REF_RE's comment and checked_ref's docstring say what is asked.

Evidence. At 050abea6, checked_ref beside git check-ref-format --branch disagreed on seven of the issue's ten names (a.lock/b, x/y.lock/z, release.lock/v1, a/.b, a//b, a/b/, a.lock/), and on a. and a/b., under git 2.43.0 and 2.47.1 alike. Over all 3110 names of up to five parts from {a, 0, ., /, -, .lock} that REF_RE takes, they disagreed on 1122 before and on none now, under both versions. In self-bootstrap mode --shape-ref a.lock/b cloned the standard and ended in error: pathspec 'a.lock/b' did not match any file(s) known to git, exit 1. Now it is refused before (0) self-bootstrap prints.

Tests (tests/test_setup_project_ref_parts.py):

  • test_the_tool_and_git_give_one_answer[<name>]: the issue's ten names and fourteen around them (a., a/b., a./b, a.lockb, a/b.lock/, a/.lock, a..b, refs/tags/v1.0, v1.0.0, and five from fix round 1). Each is fed to checked_ref and to git check-ref-format --branch, which must give one answer, and the tool's reason is pinned. No network. Fix round 1's five are a/.b/c and a/b/.c/d, a part that starts with . that is not the last part; and three names that break two rules, which hold the order why_not_a_ref asks them in (its docstring says the sentence names the first one the value failed): a//..b is a range, not an empty part; a//.b an empty part, not a dot part; a.lock/b. a lock part, not a . at the end. With a/.lock and a/b.lock, that holds each of the five pairs of neighbours in the order.
  • test_a_ref_git_refuses_is_refused_before_the_clone[a.lock/b|a/.b|a//b]: a self-bootstrap run exits 2 with the reason, prints nothing on stdout and clones nothing.

Runs

Through run9s.sh, SHARDS=3, at 79a33f11: the two setup files, every other test file that names setup-project (git grep -l 'setup-project' -- tests), the two new files and the trio, 25 files.

  • git 2.43.0: 1606 passed, 3 skipped, 1 xfailed in 746.00s, rc 0 (shards 428 passed, 3 skipped, 1 xfailed; 546 passed; 632 passed)
  • git 2.47.1: 1606 passed, 3 skipped, 1 xfailed in 677.22s, rc 0 (shards 428 passed, 3 skipped, 1 xfailed; 546 passed; 632 passed)

Before the commit, unsharded on git 2.43.0: test_setup_project_py.py, test_setup_project_name_edges.py, test_setup_project_typed_name.py and the two new files, 377 passed. test_setup_sh.py and the trio, 235 passed.

Fix round 1, at ac570069 (setup-project.py is unchanged, so every other file's result stands):

  • git 2.43.0, SHARDS=3: test_setup_project_py.py, test_setup_project_name_edges.py, test_setup_project_typed_name.py, the two new files and the trio, 535 passed in 174.38s, rc 0.
  • the two new files, 40 tests: 40 passed on git 2.43.0 (3.74s) and 40 passed on git 2.47.1 (2.64s).

Fail on base

The two new files, run against 050abea6's code: at ac570069, 40 tests, 28 failed and 12 passed (at 79a33f11, 28 tests: 20 failed, 8 passed). All ten dry-run cases that make an offer fail: the three machines in both spellings, the three terminal runs and the signed-out run. Line 2's fifteen table rows that 050abea6 gets wrong fail: ten names git refuses that it passed, a/.lock, which it refused for the .lock and not for the part that starts with ., and fix round 1's a/.b/c, a/b/.c/d, a//.b and a.lock/b.. So do the three self-bootstrap cases. The 12 that pass are the three no-terminal cases (050abea6 made no offer there either), and nine table rows: eight where 050abea6 already agreed with git (a.lock, a/b.lock, topic.locked, a./b, a.lockb, a..b, refs/tags/v1.0, v1.0.0) and a//..b, which it refused for its .., with the sentence the order gives. They hold the table to both directions, topic.locked passing among them.

Mutants

23 mutants, each run against its tests, and all 23 caught:

  • the gate in offer removed;
  • offering not passed on in each of offer_install, offer_missing, _check_git, _check_gh (install, login), _offer_login, _check_credential_helper, and each of preflight_checks's three calls;
  • preflight always offering;
  • preflight_checks silencing --preflight -- --dry-run too;
  • the plan's new sentence removed;
  • the not offered words changed;
  • the help reverted (tests/test_cli_reference.py);
  • checked_ref's old two checks put back (text.endswith(".lock"));
  • each of the four new parts of the rule dropped;
  • why_not_a_ref's answer ignored;
  • the .lock part sentence changed.

The comment and docstring hunks have no mutant: they are prose. None survived.

An independent review then made 14 mutants of its own at 79a33f11. Nine were caught. Five survived the tests as first pushed (the two new files and the setup tests): R1, the dry-run check put before can_ask() in offer; R3, the .. and empty-part checks swapped; R4, parts[-1].startswith(".") only; R7, a literal "--dry-run" not in opts.passthrough in preflight; and R13, parts[:2] only. Fix round 1 closes each, rebuilt from the review's descriptions and run at ac570069 against the two new files on git 2.43.0:

  • R1 fails the no-terminal test on all three machines (3);
  • R3 fails a//..b (1);
  • R4 fails a/.b/c and a/b/.c/d (2);
  • R7 fails the three --dry cases in process and the --dry terminal run (4);
  • R13 fails a/b/.c/d (1).

Run against the two files as first pushed (79a33f11), the same five all survived, which is what the review found. Two more mutants for the new order rows are caught: a//.b fails with an empty part checked after a dot part, and a.lock/b. fails with a . at the end checked before a lock part. The other nine of the review's mutants were caught before and were not rerun.

Merge checks

git merge-tree --write-tree of 79a33f11, and of ac570069 for the first line:

Left out

  • setup.sh's shim keeps its own copy of the old rule (*..*|*.lock), and its comment says it mirrors setup-project.py. Under cat setup.sh | bash -s -- --shape-ref a.lock/b the shim clones the standard and ends in git's pathspec 'a.lock/b' did not match, exit 1, before this file's check runs. Measured at this head, it is outside setup-project.py: a passed-through -- --dry-run still offers gh auth setup-git, which writes the person's global git config on a typed yes, and checked_ref passes a.lock/b, which git refuses #857's lines and is reported to lane 5 as a checklist candidate.
  • The plan's last line prints on every dry run, including where the preflight marked no offer not offered, where "each one marked not offered above" points at none. It is true as written. Printing it only where an offer was marked would take a count that offer writes and the plan reads, held in the module: offer is three to five calls below preflight, and each caller between returns a bool that the next one reads. That is state in a file whose offer takes its inputs as arguments and nothing else, for a sentence most dry runs would then not print, so it is left.
  • No Windows or macOS run: CI is Linux only (ci: run the test suite on Linux only #597). The in-process test runs on every platform; the pty tests are POSIX only.

Closes #857

Summary by Sourcery

Make dry-run rehearsals non-mutating and enforce Git-compatible shape references before any clone occurs.

Bug Fixes:

  • Prevent passed-through --dry-run executions from making preflight offers that could install software, authenticate GitHub CLI, or modify global Git configuration.
  • Reject --shape-ref values that violate Git's per-component reference rules before cloning begins.

Enhancements:

  • Keep preflight diagnostics and warnings visible during dry runs while explicitly reporting skipped offers.
  • Align reference validation and refusal reasons with Git's complete branch naming rules.

Documentation:

  • Update CLI documentation to explain that passed-through dry runs skip preflight offers while --preflight still performs them.

Tests:

  • Add coverage for dry-run offer suppression, unchanged global Git configuration, and preserved unauthenticated state.
  • Add cross-checks against git check-ref-format and verify invalid references are rejected before self-bootstrap cloning.

Brett Heap and others added 11 commits October 16, 2026 23:38
…-project` is and an empty line says no name was typed, so `../x`, `a/b`, `..` and `--help` are refused by name, not by an `--id` hint, a help page or a probe write (#729)

The name typed at the `project name` prompt was used as typed. The same
value as `--project` or as the positional goes through `checked_value` at
parse time and is refused there. Typed, it reached step (3), which builds
the clone path from it and asks the disk about that path: `..` leaves
`--into`, an absolute name replaces it, and #608's write probe is made in
the nearest folder above it. Step (4) did not catch it either: the naming
validator reads only the last part of what it is handed (`../x` passes as
`x`), has no length bound and reads a leading `-` as its own option, so
`--help` printed its help and exited 0. The scaffold's plan then refused
`../x` as `scaffold-bad-id: --id '../x'`, an id nobody passed, and `-h`
with a usage dump.

Now `checked_value("project name", typed)` runs right after the `ask`, the
check `take_positional` makes, so the refusal is the tool's own wording
with `project name` as its subject, exit 2, before anything reads or writes
a path. `a b` and `Bad_Name` still go to step (4): `NAME_RE` is permissive
on purpose and the policy rules on those. Nothing in step (3) changes, and
the validator's basename rule stays as it is.

An empty typed line (and Ctrl-D) now says "no project name was typed;
nothing was created.", exit 2, with no usage text. It said "no --project
given, and no terminal to ask on." after the full usage text, which is the
refusal for no terminal, and `can_ask()` still gives that one.

tests/test_setup_project_typed_name.py types each name at a pseudo-terminal
(POSIX only, as the other terminal tests are).

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… and check are one helper, and the tests pin where the check sits, so a check made after step (3) has asked the disk fails them (#729)

Review round on 9f429f4 (rv729: approve with nits, two SHOULDs).

SHOULD-1. SonarCloud reported python:S3776 on resolve_project_and_elector
(cognitive complexity 16 of 15). The prompt, the empty-line refusal and the
`checked_value("project name", typed)` call move into
`ask_for_the_project_name()`, with their comments; the call site is
`opts.project = ask_for_the_project_name()`. No behaviour changes. The
function is back to 14 by rv729's estimator, and the helper is 1.

SHOULD-2. The tests did not pin where the check sits: a check moved to just
before `check_names`, or to after `resolve_family`, passed all 16 of them.
Now every refused name is typed with a file standing where the folder above
`--into` goes, so a check made after step (3) asks the disk would end in "is
a file" and not "project name is"; and `[ok] project` and `[ok] legs` must
not print. Only the seeded file may be on the disk afterwards, unchanged.
Both moved checks now fail 12 tests each.

Nits. The empty line and Ctrl-D are also typed through the one-liner's shape
(stdin a pipe) and through `setup.sh`, 6 tests in place of 2. `(4) naming
policy` is one constant. The module docstring no longer says `--help`
"exited 0" of the run: the naming validator exited 0 at step (4), and the
run ended at step (5) with exit 2.

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…t name over 95 characters is refused before step (4) where GitHub will hold the repositories, and the prompt takes the spaces off a typed name, so `../x` is no longer told about a leading `-`, a 100-character name no longer ends at step (5) or at GitHub, and `Atlas ` no longer hides in step (4) (#777)

`refuse_value` gave every refusal the one closing sentence, and the sentence
explained only a leading `-`: `../x`, `a/b`, ` Atlas`, `Atlas/` and a name with
an accented letter were told about a `-` none of them has. It now takes the
reason, and `why_refused` names the first check the value failed, in the order
empty, leading `-` (which keeps its "git reads its own arguments" sentence),
control character, then the pattern, said in words and as the pattern, as
`repo_shape.checked_value` prints its own. `checked_ref` says `..` and `.lock`
in their own sentences and `checked_passthrough` its control character.
`repo_shape.checked_value` is not changed: it is a pinned copy, so a wording
change there is a re-pin, and it already prints its pattern.

A project name over 95 characters is refused before step (1) for `--project`
and the positional, and before step (3) prints for the prompt, unless
`--local-remote-dir` is given. GitHub keeps a repository name to at most 100
characters (its documentation: nothing here creates a real repository, so the
number is not measured) and `-spec` and `-code` add five. Under
`--local-remote-dir` the limit is the disk's and step (5) still asks it as
`scaffold-name-too-long`. The check runs at the end of the parse, because
`--local-remote-dir` can come after `--project` on the line.

The prompt takes the spaces off the ends of what was typed. `Atlas ` passed
`NAME_RE` where ` Atlas` was refused, and step (4) judged `Atlas -spec` with no
space shown anywhere. A line of spaces is now an empty line. An argv value is
deliberate and is held as typed.

tests/test_setup_project_name_edges.py is new. #756's file moves its
three-spaces case from the refusals to the empty answers, and a comment in
tests/test_setup_project_py.py stops saying the two entry points refuse in the
same words.

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…dential helper for github.com, and one run empties git's own config, so a GitHub run no longer waits on CI at a `gh auth setup-git` offer nothing answers (#777)

On CI (run 39027235147, git 2.55.0) five of this file's tests were killed
after 60 seconds: the four over-the-bound names typed at the prompt
(96, 100, 101, 255) and the 95-character name typed in a GitHub run. Each
screen ended in step (1):

    [??] git has no credential helper for github.com, so the first push
    over HTTPS will fail. Fix it once: gh auth setup-git
    Run `gh auth setup-git` now? Type yes to continue:

A GitHub run (no `--local-remote-dir`) makes that offer in its preflight
when git has no credential helper for github.com, before step (3) asks for
the project. The tests type only at the project prompt, so nothing
answered the offer, and the run waited. The bound played no part: the same
names in local runs passed, and the 95-character GitHub run hung too. On
the writer's machine /etc/gitconfig and ~/.gitconfig both name a helper,
so the offer never came there. CI's runner names none. With git's system
file skipped and its global file empty, the same five hang on that machine
with CI's screen.

The tool is right: the offer is `_check_credential_helper`'s documented
warning and offer. The tests depended on the machine's git config.

- `gh_that_is_signed_in` also gives git a credential helper for github.com,
  in git's command scope (`GIT_CONFIG_COUNT`, appended to what the
  environment holds): `!<the stub> auth git-credential`, the line
  `gh auth setup-git` writes, pointed at the stub. The preflight finds a
  helper on every machine. No run here pushes, so git never asks it.
- A new test, `test_a_github_run_here_is_not_held_by_a_git_with_no_helper`
  (95 and 96 characters), runs with git's system and global files empty, as
  CI's runner has them. A run that waits at the offer now fails on every
  machine, not on CI alone.

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd the name and ref sentences say ASCII, so a typed tab, form feed or no-break space is refused as `--project` refuses it, and `Caf\xe9` is no longer described as spellable (#777)

Lane 2's review of #811 (rv811, at 854f6ed), SHOULDs 2 and 3 and NITs 4
and 5. Its MUST 1 is b5def3f.

- The prompt's `.strip()` took off every kind of whitespace, not only the
  spaces its docstring names: a tab, VT, FF, `\x1c` to `\x1f`, NEL, NBSP and
  U+3000. Typed, each gave `[ok] project      Atlas` and exit 0, where
  `--project` refuses the same value as a control character or by the
  name pattern, and where #756's head refused it at the prompt too. It is
  now `.strip(" ")`. A typed line holds no newline here (`ask` drops it),
  so nothing else needs taking off. The test file's typed-refusal test
  takes the nine values, with stdin the terminal and with stdin a pipe,
  and the argv refusals take them as `--project`. The same reason is
  given on every way in. Each control character is typed after ^V, so
  the terminal takes it as text (`\x1c` alone is the quit key).
- The name sentence said "letters" and "spaces", but `NAME_RE` takes only
  ASCII letters and U+0020, so its words described `Caf\xe9` and a no-break
  space as values that pass. Both sentences now say "an ASCII letter or
  digit first, then ASCII letters, digits, the space and ..." (the ref
  sentence without the space), as `adopt-project.py`'s help says "ASCII
  letters, digits and ...". The drift guards also walk every character
  above ASCII and find none taken. README, the handbook, AGENTS.md and
  docs/cli.md do not state either sentence (`render-cli-reference.py
  --check` is ok).
- A ref is refused for `.lock` only where it ends in it: a new test passes
  `topic.locked` and `a.lockb` through `checked_ref`.
- `a_run_for`'s list comprehension that only copied is gone.

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… for: setup-project.py tests/test_setup_project_typed_name.py

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… is appended through `appended_to_command_scope`, as #848 asks of every hand-worked count, so the scan in `test_no_auto_maintenance.py` passes again (#777)

`gh_that_is_signed_in` worked its `GIT_CONFIG_COUNT` out by hand from the
environment's own count (`held + 1`, `GIT_CONFIG_KEY_{held}` and
`GIT_CONFIG_VALUE_{held}`). That sum was right, and #848's scan
(`test_no_test_sets_a_git_config_count_of_its_own`) refuses it in a test
file now: it named `test_setup_project_name_edges.py:190`. #848 landed first,
and its second-lander note gives this conversion to whichever of #848 and
#811 lands second.

The three entries now come from `appended_to_command_scope(os.environ, ...)`,
merged into the returned dict beside `PATH`. The same three variables get the
same values, under an environment that holds two entries, none, or an empty
count. The docstring names the helper. No other line of the file changes.

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…light's offers and `--shape-ref` is held to git's rule part by part, so a rehearsal no longer writes git's global config and `a.lock/b` is refused before the clone (#857)

A `--dry-run` passed through after `--` ends the run at the plan, and the
help and the plan's last line say nothing was created. The preflight before
it still offered, on a terminal, every command that fixes what it found
missing: an install of git or gh, `gh auth login` and `gh auth setup-git`.
Each changes the machine on a typed `yes` (software, a gh login, a helper in
the person's global git config), and a rehearsal that typed only the project
name waited at the question until it was killed, as five of #777's tests did
on CI.

`preflight` now passes `offering` False down to `offer` for such a run, and
`offer` says `not offered: --dry-run was passed through. A run without it
offers this here, and so does --preflight.` where it would have asked, and
asks nothing. Every check still runs and reports what is missing. The plan's
last line adds that the preflight made none of its offers. `--preflight`
still makes every offer, whatever follows `--`: it is the run that exists
for them (#59). The help says so, and docs/cli.md is regenerated.

`checked_ref` asked only `..` and a `.lock` at the very end, where git's
rule is per part: `why_not_a_ref` refuses an empty part (`//`, a trailing
`/`), a part that starts with `.`, a part that ends in `.lock`, and a `.` at
the end, each in a sentence of its own. `a.lock/b`, `x/y.lock/z`,
`release.lock/v1`, `a/.b`, `a//b`, `a/b/`, `a.lock/` and `a.` are refused
before the standard is cloned; `topic.locked` and `a./b` go on. Over the
3110 names of up to five parts from {a, 0, ., /, -, .lock} that `REF_RE`
takes, the tool and `git check-ref-format --branch` now give one answer,
under git 2.43.0 and 2.47.1 (1122 disagreed before).

New tests: tests/test_setup_project_dry_run_offers.py (each offer in
process; a terminal run under a stub gh that leaves the global git config's
bytes as they were, and one that leaves gh signed out) and
tests/test_setup_project_ref_parts.py (the issue's ten names and nine more,
each beside git, and three refused before a self-bootstrap clone).

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

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-5, 2026-10-18T03:53Z) @codex review

@sourcery-ai

sourcery-ai Bot commented Oct 18, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR makes passed-through --dry-run a non-mutating rehearsal by suppressing all preflight offers while retaining --preflight behavior, and strengthens --shape-ref validation to agree with git part by part before repository setup begins.

Sequence diagram for non-mutating dry-run preflight

sequenceDiagram
    participant User
    participant SetupProject
    participant Preflight
    participant Offer
    participant GitConfig
    User->>SetupProject: run -- --dry-run
    SetupProject->>Preflight: preflight(opts)
    Preflight->>Preflight: passes_a_dry_run(opts.passthrough)
    Preflight->>Offer: offer(command, offering=false)
    Offer-->>Preflight: print NOT_OFFERED, do not prompt
    Preflight->>Offer: offer(gh auth login, offering=false)
    Offer-->>Preflight: print NOT_OFFERED, do not run
    Preflight->>Offer: offer(gh auth setup-git, offering=false)
    Offer-->>GitConfig: no global config change
    Preflight-->>SetupProject: report prerequisites and continue
    SetupProject-->>User: plan, nothing created or cloned
Loading

Sequence diagram for preflight offers

sequenceDiagram
    participant User
    participant SetupProject
    participant Preflight
    participant Offer
    participant GitHubCLI
    participant GitConfig
    User->>SetupProject: run --preflight
    SetupProject->>Preflight: preflight_only(opts)
    Preflight->>Offer: offer(gh auth login, offering=true)
    Offer->>User: prompt for yes
    User-->>Offer: yes
    Offer->>GitHubCLI: gh auth login
    Preflight->>Offer: offer(gh auth setup-git, offering=true)
    Offer->>User: prompt for yes
    User-->>Offer: yes
    Offer->>GitHubCLI: gh auth setup-git
    GitHubCLI->>GitConfig: write credential helper
    Preflight-->>SetupProject: return after checks
Loading

Sequence diagram for rejecting invalid shape-ref before clone

sequenceDiagram
    participant User
    participant SetupProject
    participant CheckedRef
    participant Git
    User->>SetupProject: run --shape-ref a.lock/b
    SetupProject->>CheckedRef: checked_ref(--shape-ref, a.lock/b)
    CheckedRef->>CheckedRef: why_not_a_ref(a.lock/b)
    CheckedRef-->>SetupProject: WHY_LOCK_PART
    SetupProject-->>User: refuse value with reason
    Note over Git: clone_and_bootstrap is not called
Loading

Flow diagram for git-compatible shape-ref validation

flowchart TD
    A[--shape-ref value] --> B[checked_ref]
    B --> C[checked_value with REF_RE]
    C --> D[why_not_a_ref]
    D --> E{Git rule checks}
    E -->|empty part| F[refuse with WHY_EMPTY_PART]
    E -->|part starts with dot| G[refuse with WHY_DOT_PART]
    E -->|part ends in .lock| H[refuse with WHY_LOCK_PART]
    E -->|whole contains ..| I[refuse with WHY_RANGE]
    E -->|whole ends in .lock| J[refuse with WHY_LOCK]
    E -->|whole ends in dot| K[refuse with WHY_DOT_END]
    E -->|valid| L[continue to repository setup]
Loading

File-Level Changes

Change Details Files
Propagate dry-run state through preflight offer handling so rehearsal runs report missing prerequisites without prompting or executing machine-changing commands, while preserving full offers for --preflight.
  • Add an offering gate to the offer helper and all install, authentication, and credential-helper call paths.
  • Emit per-offer not offered diagnostics and extend dry-run help and plan output.
  • Add in-process and POSIX terminal tests covering suppressed prompts, unchanged git config, and unchanged gh authentication.
setup-project.py
docs/cli.md
tests/test_setup_project_dry_run_offers.py
Complete --shape-ref validation to match git's per-component ref rules before any clone or checkout occurs.
  • Add validation for empty slash-separated parts, dot-prefixed parts, intermediate .lock parts, and trailing dots with dedicated refusal messages.
  • Route checked_ref through the new validator while retaining existing range and terminal .lock handling.
  • Add parity tests against git and self-bootstrap tests proving invalid refs are rejected before cloning.
setup-project.py
tests/test_setup_project_ref_parts.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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T03:56:25.473394Z 79a33f1 Manual request
ℹ️ About Codex in GitHub

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

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

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 79a33f112d

ℹ️ About Codex in GitHub

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

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

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

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

Comment thread setup-project.py
Comment on lines +530 to +532
why = why_not_a_ref(text)
if why:
refuse_value(flag, text, why)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the ref-part validation in the bootstrap shim

When an invalid ref such as a.lock/b is supplied through the documented curl … setup.sh entry point, this new validation is reached only after setup.sh has cloned the repository and attempted the checkout. The shim's guard at setup.sh:235-268 still accepts these values, so the run performs the clone and terminates with Git's raw pathspec error instead of the promised pre-clone refusal; update the shim's mirrored check as well.

AGENTS.md reference: AGENTS.md:L55-L61

Useful? React with 👍 / 👎.

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-5, 2026-10-18T04:18:25Z) Independent review of #873 (#857) at 79a33f11, for lane openRepoShape-5: READY

No MUST findings. Both lines work as the body says, and every sentence the PR adds about a dry run is true. There is one test hole worth a short fix (S1). The setup.sh rule (S2) belongs in its own issue.

Findings, most severe first

S1. SHOULD: the table never checks a dot-starting part anywhere but last.

  • Where: tests/test_setup_project_ref_parts.py:46,56, against setup-project.py:508. The only dot-part rows are a/.b and a/.lock, and both put that part last.
  • Failure: two of my mutants survive the new files and also test_setup_project_py.py, test_setup_project_name_edges.py, test_setup_project_typed_name.py and test_refusal_default_remediation.py (369 passed each):
    • R4 checks parts[-1].startswith(".") only;
    • R13 checks parts[:2] only.
  • What gets through: under R4, a/.b/c, x/.y/z/w and a/b/.c/d pass the tool while git check-ref-format --branch exits 128 on each. Under R13, a/b/.c/d passes. That is the issue's own defect, back for those names.
  • Fix: add a row such as a/.b/c and one with three or more parts (a/b/.c/d), both DOT_PART.

S2. SHOULD, as its own issue and not in this PR: the old ref rule lives on in three bash copies.

  • Where: setup.sh:236, openRepoShape:588 (ref_fault) and install.sh:151 all keep *..*|*.lock. setup.sh's comment (:213) still says it "Mirrors setup-project.py's REF_RE".
  • Measured at this head: cat setup.sh | bash -s -- --org demoorg --project Sample --shape-ref <ref> --local-remote-dir …, with OPENREPOSHAPE_REPO set to a local clone:
    • a.lock/b, a/.b and a//b each print (0) self-bootstrap, clone the standard, and end error: pathspec '<ref>' did not match any file(s) known to git, exit 1;
    • topic.lock is refused by the shim, exit 2.
  • Why it matters: the installed openRepoShape command also fetches and runs setup.sh. So on Linux and macOS both ways in still pay for the clone. The PR title's "refused before the clone" holds only for setup-project.py run directly or downloaded.
  • Why its own issue: setup-project.py: a passed-through -- --dry-run still offers gh auth setup-git, which writes the person's global git config on a typed yes, and checked_ref passes a.lock/b, which git refuses #857 scopes checked_ref. Three bash files carry their own tests (test_setup_sh.py:898), and the fix would grow this PR. The body's "Left out" could also say that the installed command goes through the shim.

N1. NIT: nothing tests that the dry-run check comes after can_ask().

  • Where: setup-project.py:1015-1019. Mutant R1 puts if not offering first and survives all six files above.
  • Effect of that mutant: a run with no terminal would print not offered: … A run without it offers this here, which is false there.
  • Head is correct: a setsid run with no terminal and no helper printed [??] and no not offered line. The body states this guarantee, but no test holds it.

N2. NIT: the plan's new sentence prints even when nothing is marked.

  • Where: setup-project.py:2495.
  • Seen: in that same run with no terminal, the plan still says "a run without --dry-run makes each one marked not offered above", though nothing is marked. It is true only because there is nothing to point at.

N3. NIT: nothing tests the check order the docstring claims.

  • Mutant R3 swaps the .. and empty-part checks and survives. On a//..b the reason changes from WHY_RANGE to WHY_EMPTY_PART.
  • The docstring says "the sentence names the first one the value failed", but no row has both faults.

N4. NIT: nothing tests the short spellings of --dry-run.

  • passes_a_dry_run takes --dry, --dr and --dry-r. Head handles them: a pty run with -- --dry made no offer.
  • Mutant R7, a literal "--dry-run" not in opts.passthrough, survives all six files.

Checked and true

1. Line 1, measured in my own pty probe.

  • Setup: a stub gh first on PATH, PATH=stub:/usr/bin:/bin so the real gh at /usr/local/bin is unreachable, an empty system config, its own global file, no GIT_CONFIG_COUNT, and yes typed at every prompt. For the install case it typed no.
  • At head, every case made 0 prompts, left the global config's sha256 unchanged, and left gh signed out:
    • signed in, no helper, entered four ways: py, piped, cat setup.sh | bash -s -- and a downloaded copy;
    • signed out: py and setup.sh;
    • gh not installed: py.
  • Each case printed the not offered line under its [??] or [!!], and the new last line where the run reached the plan.
  • At base 050abea6:
    • helper: 1 prompt, the yes ran auth setup-git, the config changed, and the run still said "nothing was created";
    • login: 2 prompts, signed in, the config changed;
    • install: 1 prompt.
  • --preflight -- --dry-run at head still offers: 1 prompt with the helper missing, 2 when signed out, and the yes answers ran.
  • No offer is reachable under a dry run: every caller of offer (offer_install, _offer_login, _check_credential_helper) starts from preflight_checks. Its only callers are preflight, which passes offering, and preflight_only.
  • Docs: --help and docs/cli.md agree (test_cli_reference.py passes). Nothing in AGENTS.md, README.md or the handbook describes setup's -- --dry-run, and no sentence there became false.

2. Line 2, checked_ref beside git check-ref-format --branch, on git 2.43.0 and 2.47.1 with identical results.

3. Fail on base: with the two new files copied onto 050abea6, 20 failed and 8 passed. The 8 are the rows w857 named (a.lock, a/b.lock, topic.locked, a./b, a.lockb, a..b, refs/tags/v1.0, v1.0.0). Each failure is the intended one: "a dry run made an offer", git exit 128, or exit 1 against an expected 2.

4. Mutants: I made 14 of my own (R1 to R14). 9 were caught (R2, R5, R6, R8 to R12, R14) and 5 survived: R1, R3, R4, R7 and R13, above.

5. Runs at 79a33f11:

Run Git Result
3 setup files plus the 2 new files 2.43.0 377 passed in 270.80s, rc 0
test_setup_sh.py plus the trio 2.43.0 235 passed in 265.63s, rc 0
the 2 new files 2.47.1 28 passed, rc 0

6. The restack's view. simrestack.sh printed:

#811: simulated update-branch fee7ef6a
#873 79a33f11 over #811 fee7ef6a: main d6475f96, predicted squash 9691990c (tree 5226ab9a)
resolved ours-side: setup-project.py
#873: simulated restack 4d8ace08 (tree 076a9a0b), clean
  • Main had moved to d6475f96 during this review.
  • The restack's diff against the predicted squash is the PR's 4 files, and its changed lines are identical to 050abea6..79a33f11.
  • On 4d8ace08: the 2 new files plus test_cli_reference.py, 51 passed; test_no_auto_maintenance.py, 60 passed.
  • The scan allows dropping the command scope: it flags only a count set to a number. Rehearsal deletes the key through a filtered dict, and the walk test passes with neither new file in OWN_COUNTS.
  • And it is right to:
    • trace2 shows the rehearsal's gits are all read-only (remote twice, rev-parse twice, config --get-all twice, check-ref-format, ls-tree, status, --version), so no maintenance can start;
    • a helper reset appended to the command scope cannot empty config --get-all, so dropping the scope is the only way to be sure git has no helper.

7. setup.sh's copy of the old rule: confirmed, along with two more copies (S2).

8. Sonar-style checks:

  • Duplication: no 6-line window of either new file repeats anywhere in tests/.
  • Asserts: none of the 28 joins conditions with and or or.
  • Non-ASCII and whitespace: no non-ASCII byte in the diff, and git diff --check is clean.
  • Cognitive complexity, counted by hand against 15: _check_gh 10, offer_install 7, offer 6, why_not_a_ref 6 (8 if Sonar counts the generators), _check_credential_helper 5, _check_git 3, _offer_login 3, and 1 each for checked_ref, offer_missing, preflight_checks and preflight.

Could not verify

  • git 2.55.0 (CI's version): no build here.
  • Windows and macOS.
  • The real gh's auth setup-git bytes: my stub copies them with git config --global --add.
  • w857's 23 mutants: not re-run; I read its mutants.out, where all were caught.
  • w857's 25-file sharded gates: I ran only the brief's subset.

Scratch left, and processes

  • Clone: /workspace/projects/openRepoShape-worktrees/_lane5-scratch/rv873-head, with private refs refs/rv873/{pr873,b811,main} in that clone only.
  • Its worktrees:
    • /workspace/projects/openRepoShape-worktrees/_lane5-scratch/rv873-base, holding the 2 copied test files, untracked;
    • /workspace/projects/openRepoShape-worktrees/_lane5-scratch/rv873-mut, clean;
    • /workspace/projects/openRepoShape-worktrees/_lane5-scratch/rv873-mut2, clean;
    • /workspace/projects/openRepoShape-worktrees/_lane5-scratch/rv873-restack, clean.
  • Logs, probes, and the pty and shim run folders: /tmp/claude-1000/-workspace-projects-openRepoShape/50cede29-ab82-4ca3-9f67-d98e1a58c8d2/scratchpad/v118/rv873/.
  • Basetemps: /tmp/claude-1000/-workspace-projects-openRepoShape/50cede29-ab82-4ca3-9f67-d98e1a58c8d2/scratchpad/v110/bt/rv873-{A,B,C,D,E,F,m,mut,mut2}.
  • Probe temp folders: /tmp/rv873-refs-* (5) and /tmp/rv873-mprobe-dr60ibyi.
  • In the shared repository: simrestack.sh wrote the unreferenced commit objects fee7ef6a, 9691990c, cf7238ae and 4d8ace08 into its object store, with no ref.
  • Nothing changed, pushed or posted. No process of mine is running.

Next, from lane 5: a small round for S1 and NITs 1, 3 and 4, then rv873 re-checks it. S2 goes to its own v1.1.1 issue.

Lane: openreposhape-5 (openRepoShape-5)

…y_run_offers.py: a part that starts with `.` is held anywhere in a ref, a name that breaks two rules is told the first, and a dry run with no terminal and one spelled `--dry` are held, so five mutants of #857's fix that passed every test are caught (#857)

An independent review of #873 made 14 mutants of the fix; five survived the
two new files and the setup tests. This closes each one with a test.

- `a/.b/c` and `a/b/.c/d` join the table. The only dot-part rows put the part
  last, so a check of `parts[-1]` alone (R4) and of `parts[:2]` alone (R13)
  passed, and `git check-ref-format --branch` refuses both names.
- Three names that break two rules hold `why_not_a_ref`'s order, which its
  docstring claims: `a//..b` is a range and not an empty part (R3), `a//.b`
  an empty part and not a dot part, and `a.lock/b.` a lock part and not a
  `.` at the end. With `a/.lock` and `a/b.lock` that holds all five pairs of
  neighbours.
- A dry run with no terminal prints no `not offered` line and reads as it does
  without `--dry-run`, on each of the three machines (R1, which puts the
  dry-run check before `can_ask()`).
- The in-process dry-run test and one terminal run take `--dry`, the short
  spelling `passes_a_dry_run` reads, so `preflight` cannot test the literal
  `--dry-run` (R7).

setup-project.py is unchanged. The plan's last line still prints where nothing
was marked `not offered`: telling the plan whether the preflight marked one
takes a count written by `offer`, four calls below `preflight`, and the line
is true as written.

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

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-5, 2026-10-18T04:31:48Z) Fix round 1 of #873 (#857): 79a33f11..ac570069.

One new commit on 79a33f11, in two files: tests/test_setup_project_ref_parts.py and tests/test_setup_project_dry_run_offers.py. setup-project.py is not changed, so --help and docs/cli.md are as they were. Pushed as a plain fast-forward (79a33f11..ac570069), and git ls-remote reads ac570069 back. rv873 found no MUST, and every point it raised against the tests is closed. Each point, and what was done:

S1: "the table in tests/test_setup_project_ref_parts.py never checks a dot-starting part anywhere but last". Done.

  • Two rows join the table, both held to git check-ref-format --branch and to the dot-part sentence, like every row: a/.b/c (a dot part in the middle) and a/b/.c/d (the third of four parts).
  • Both pass at ac570069 on git 2.43.0 and 2.47.1, so the tool and git give one answer on them. They fail on 050abea6's code, which passes both names that git refuses.
  • R4 (if parts[-1].startswith("."):) now fails [a/.b/c] and [a/b/.c/d].
  • R13 (any(... for part in parts[:2])) now fails [a/b/.c/d]. Its first two parts are a and b, so [a/.b/c] still passes under R13, and the three-part-plus row is the one that kills it.

NIT 1: "nothing tests that the dry-run check comes after can_ask()". Done.

  • New test test_a_dry_run_with_no_terminal_says_what_a_run_without_it_says[install|login|helper], in process on every platform. It uses the suite's _no_terminal (stdin is not a terminal and open_terminal returns nothing), runs preflight on each of the three machines without a passthrough and then with -- --dry-run, and asserts that the second prints no not offered line and prints what the first printed. The helper machine is the one the review named, git with no credential helper. Both runs refuse or pass alike.
  • R1 (if not offering put before if not can_ask() in offer) now fails all three cases.
  • To share the machine set-up between this test and the first one, the module has a small helper, a_machine, which both use, so the new test duplicates no block.

NIT 3: "nothing tests the check order the docstring claims". Done, with two more rows than the review asked for.

  • a//..b joins the table, expected RANGE (the .. is asked first). R3 (the .. and empty-part checks swapped) now fails [a//..b]: the sentence becomes the empty part's.
  • The docstring's order is .., an empty part, a part that starts with ., a name that ends in .lock, a part that ends in .lock, a . at the end. The table already held two pairs of neighbours apart (a/.lock: a dot part before a lock at the end; a/b.lock: a lock at the end before a lock part). Two rows hold the other two, so the order is held pair by pair:
    • a//.b, expected the empty part's sentence, and not the dot part's;
    • a.lock/b., expected the lock part's sentence, and not the . at the end's.
  • Two mutants of mine for those rows: X1 (the dot-part check before the empty-part check) fails [a//.b]; X2 (the . at the end before the lock part) fails [a.lock/b.].
  • The module docstring says it in two sentences, and a comment over the rows names the order.

NIT 4: "nothing tests the short spellings of --dry-run". Done.

  • The in-process test test_a_dry_run_makes_none_of_the_offers_preflight_makes now runs for --dry-run and --dry, on every machine (6 cases, from 3). --dry is a spelling passes_a_dry_run reads (PASSED_FLAGS["--dry-run"] is --d), and --preflight -- --dry makes the same offers as --preflight -- --dry-run.
  • The terminal run takes it too: test_a_dry_run_leaves_git_with_no_helper_as_it_found_it is [py|piped|py with --dry]. Rehearsal has a flag argument, default --dry-run. A run at a real pty, with --dry, asks nothing and leaves the global git config's bytes as they were.
  • R7 (a literal "--dry-run" not in opts.passthrough in preflight) now fails the three in-process --dry cases and the --dry terminal run.

NIT 2 (optional): "the plan's new sentence prints even when nothing is marked". Left, as the brief allows.

  • Printing it only where an offer was marked means the plan has to know that offer marked one. plan_and_confirm runs after preflight, which returns nothing (preflight_checks returns a bool). offer is three to five calls below preflight (preflight_checks, then _check_credential_helper or _check_git or _check_gh, then offer_missing, offer_install or _offer_login), and every caller on the way returns a bool that the next one reads. No return value can carry it without changing each of them, and offer must not take Options (test_the_offer_helpers_cannot_see_the_yes_flag).
  • What is left is a count written by offer and read by the plan, held in the module. That is shared state in a file whose offer takes its inputs as arguments and nothing else, for a sentence most dry runs would then not print: a machine with a helper and a login marks nothing, so its dry run would lose a line.
  • The sentence is true as written: it says the preflight made none of its offers, and each one marked not offered above is a run without --dry-run's to make, and there are none to point at. The body's "Left out" now says it, with the reason.

S2 (setup.sh, openRepoShape and install.sh keep the old rule): not touched. The review put it in its own issue, and the brief did not ask for it. The body's "Left out" already reports the setup.sh copy. The coordinator may want to add that the installed openRepoShape command fetches and runs setup.sh, so the PR title's "refused before the clone" holds for setup-project.py run directly or downloaded; the review's S2 text has the measurements.

Evidence

Mutants, at ac570069, against the two new files on git 2.43.0. Each was rebuilt from final.md, applied to setup-project.py by an exact one-place replacement (a script that asserts one match), and restored byte for byte after each run (cmp against a backup, and git diff of setup-project.py empty). Logs: mut/head-<R>.log.

Mutant Result at ac570069 Tests that fail
R1: if not offering before can_ask() killed, 3 failed 37 passed `test_a_dry_run_with_no_terminal_says_what_a_run_without_it_says[install
R3: .. and empty part swapped killed, 1 failed 39 passed test_the_tool_and_git_give_one_answer[a//..b]
R4: parts[-1] only killed, 2 failed 38 passed [a/.b/c], [a/b/.c/d]
R7: literal "--dry-run" not in opts.passthrough killed, 4 failed 36 passed `test_a_dry_run_makes_none_of_the_offers_preflight_makes[install
R13: parts[:2] only killed, 1 failed 39 passed [a/b/.c/d]
X1: empty-part check after the dot-part check killed, 1 failed [a//.b]
X2: . at the end before the lock part killed, 1 failed [a.lock/b.]

To check that my rebuilt R-mutants are the review's, each was also run against copies of the two files as pushed at 79a33f11 (made untracked in the worktree for the run and deleted after). All five survived them, as the review found, and failed only in the new files (mut/mut-<R>.log).

Runs at ac570069, through run9.sh or run9s.sh, gh stub first on PATH, GH_TOKEN=bogus, empty GH_CONFIG_DIR, dirty=0:

  • git 2.43.0, run9s.sh with SHARDS=3: test_setup_project_py.py, test_setup_project_name_edges.py, test_setup_project_typed_name.py, the two new files, test_repo_hygiene.py, test_agents_md_codes.py and test_cli_reference.py: 535 passed in 174.38s, rc 0 (shards 77, 230 and 228 passed). Log run-gate243.log, shard logs run-gate243-s1.log to -s3.log.
  • git 2.43.0, the two new files: 40 passed in 3.74s, rc 0 (run-new1.log, run on the same content before the commit).
  • git 2.47.1 (PATH with v115/w764/git247/bin first; git --version there reads 2.47.1), the two new files: 40 passed in 2.64s, rc 0 (run-new247.log).
  • The two new files against 050abea6's setup-project.py (put in place for the run, then restored byte for byte): 28 failed, 12 passed (run-base.log). The 12 are the three no-terminal cases, and nine table rows (eight where the base already agrees with git, and a//..b, which it refuses for its ..).

Merge check: git merge-tree --write-tree HEAD b6f56cad (#811's head) is clean, rc 0, tree 3e369754b5820289d74bcb896555d7f31eaa4abd; the merge base is 050abea6. Against the local origin/main (d6071c7d) it conflicts in setup-project.py and tests/test_setup_project_typed_name.py, the same two files 79a33f11 conflicts in, inherited from #811's history (the restack resolves them ours-side, as simrestack.sh showed). The other lanes' branches were not re-run; this commit touches only the two test files this PR added.

Hygiene: body-guard.py is clean on both test files, git diff --check is clean, no non-ASCII byte in the diff, no line over 79 characters. The two new files hold no assert that joins conditions with and or or.

Note on the commit message: it says offer is "four calls below preflight". It is three to five. The message is in the pushed commit and is not amended.

Next, from lane 5: rv873 re-checks this round. S2 is filed as #874.

Lane: openreposhape-5 (openRepoShape-5)

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-5, 2026-10-18T04:37:03Z) Re-check of #873 (#857) fix round 1 at ac570069, for lane openRepoShape-5: READY

All five mutants that survived my review are now caught, the runs pass, and the merge with #811's head is clean. There are no new findings.

Findings

None.

NIT 2, left unfixed: the reason holds. The plan's last line prints even when nothing above is marked not offered. The sentence is true as it stands. Printing it only when something was marked would need one of two things:

  • a counter shared across the module, which offer writes and the plan reads; or
  • a changed return value in each bool-returning function between preflight and offer. offer may not take Options (test_the_offer_helpers_cannot_see_the_yes_flag).

A wording-only change, such as adding "if any", would avoid both. It stays optional and does not block.

A small error in a commit message, no action needed. The fix commit's message says offer is "four calls below preflight". It is three to five. f873 already notes this in fixround.md, and the squash takes the PR's own message.

Checked and true

1. Scope.

  • git diff --stat 79a33f11 ac570069 shows only tests/test_setup_project_dry_run_offers.py (+63/−28) and tests/test_setup_project_ref_parts.py (+15/−1).
  • git diff --quiet for setup-project.py and docs/cli.md exits 0, so neither changed.
  • refs/pull/873/head and fix/857-setup-dry-run-offers both read ac570069.

2. Each point is closed.

  • S1: the table has the rows a/.b/c and a/b/.c/d, both expecting DOT_PART.
  • NIT 1: there is a new test, test_a_dry_run_with_no_terminal_says_what_a_run_without_it_says[install|login|helper].
    • It uses the suite's _no_terminal: stdin is not a terminal and open_terminal returns nothing.
    • It checks that a dry run prints no not offered line, and prints exactly what the same run without --dry-run prints.
  • NIT 3: the table has a//..b (expects RANGE), a//.b (expects EMPTY_PART) and a.lock/b. (expects LOCK_PART).
    • With the existing a/.lock and a/b.lock, each neighbouring pair of checks in why_not_a_ref's order now has a row that tells them apart.
    • I checked by hand that each new row breaks both rules of its pair.
  • NIT 4: the in-process test takes --dry-run and --dry on all three machines, and the terminal test now runs [py|piped|py with --dry].

3. Mutants. I reran my own five with my mutants.py in rv873-mut, moved to ac570069. Each is caught:

Mutant Result Tests that fail
R1 (dry-run check before can_ask()) 3 failed, 37 passed the no-terminal test, all three machines
R3 (.. and empty-part checks swapped) 1 failed, 39 passed [a//..b]
R4 (dot part checked in the last part only) 2 failed, 38 passed [a/.b/c], [a/b/.c/d]
R7 (literal "--dry-run" check in preflight) 4 failed, 36 passed the three in-process --dry cases and the --dry terminal run
R13 (dot part checked in the first two parts only) 1 failed, 39 passed [a/b/.c/d]

The tree read clean again after the runs.

4. Runs at ac570069.

Run Git Result
the two new files, test_setup_project_py.py and test_cli_reference.py 2.43.0 292 passed in 171.23s, rc 0
the two new files 2.47.1 (confirmed by git --version) 40 passed in 2.60s, rc 0

5. Merge. git merge-tree --write-tree b6f56cad ac570069 is clean (rc 0, tree 3e369754), which is the same tree f873 reported. The merge base is 050abea6.

Other checks.

  • The diff has no non-ASCII byte, and git diff --check is clean.
  • No assert in either new file joins conditions with and or or.
  • No six-line window in either file repeats anywhere in tests/. The shared setup moved into a new helper, a_machine.

Could not verify

  • I did not run git 2.55.0 (the version CI uses), Windows or macOS.
  • I did not rerun f873's own X1 and X2 mutants. My rows-by-hand check above covers what they target.
  • I did not rerun the sharded gate or the base-failure run. I read f873's results for those (535 passed; 28 failed and 12 passed on base).

Scratch left

  • Removed: the /tmp/rv873-* probe folders, file by file with rm -f and rmdir, no rm -rf. ls /tmp/rv873-* now finds nothing.
  • The clone and its worktrees:
    • /workspace/projects/openRepoShape-worktrees/_lane5-scratch/rv873-head (the clone), now at ac570069, with private refs refs/rv873/{pr873,b873,b811,main} in this clone only;
    • /workspace/projects/openRepoShape-worktrees/_lane5-scratch/rv873-mut and /workspace/projects/openRepoShape-worktrees/_lane5-scratch/rv873-mut2, both at ac570069 and clean;
    • /workspace/projects/openRepoShape-worktrees/_lane5-scratch/rv873-restack, at 4d8ace08, clean;
    • /workspace/projects/openRepoShape-worktrees/_lane5-scratch/rv873-base, at 050abea6, holding two untracked copies of the old test files.
  • Logs and probes: /tmp/claude-1000/-workspace-projects-openRepoShape/50cede29-ab82-4ca3-9f67-d98e1a58c8d2/scratchpad/v118/rv873/. The new ones are runG-243-fix1.log, runH-247-fix1.log and mutR1/.
  • Basetemps: /tmp/claude-1000/-workspace-projects-openRepoShape/50cede29-ab82-4ca3-9f67-d98e1a58c8d2/scratchpad/v110/bt/rv873-{A,B,C,D,E,F,G,H,m,mut,mut2}.
  • In the shared repository: unreferenced commit objects fee7ef6a, 9691990c, cf7238ae and 4d8ace08, left by simrestack.sh in the first review.
  • Running: nothing.
  • Changed, pushed or posted: nothing.

From lane 5: #873 is READY at ac570069. It lands after #811, by restack3.sh onto #811's squash.

Lane: openreposhape-5 (openRepoShape-5)

brettheap pushed a commit that referenced this pull request Oct 18, 2026
…le's sentence says ASCII letters as setup-project.py's does, three rows hold the patterns off a part that starts upper-case, `_` or `-`, and two comments say what is true (#874)

rv874's review of #875 (READY, no MUST), fix round 1.

The shared sentence said "a letter or digit" and "letters", which `Café`
satisfies under a UTF-8 locale although the patterns refuse it; #873's
REF_RE words say "an ASCII letter or digit" and "ASCII letters" (#777,
rv811). The three copies' refusals and `REF_RULE_WORDS` now say the same,
byte for byte, and so does setup.sh's comment.

`REF_PARTS` had no part that starts upper-case, `_` or `-`, so three
over-refusing mutants passed all 33 rows of their copy's table test:
`*/[-_]*` in setup.sh, `*/[A-Z]*` in install.sh and `[!a-z0-9]*` for
`[!A-Za-z0-9]*` in openRepoShape. `Feature/ABC-123`, `fix/_tmp` and
`fix/-wip` join the names that must pass, and each mutant is killed by them.
`a/b.` and `a/.` move up to the names the old line took, where the
docstring already counted them, from "refused before #874 as well".

setup.sh's comment names `why_not_a_ref` as #857's, held here since #874,
and says each rule has one or two patterns (an empty part and a `.lock` part
have two). install.sh's note says a refused version is refused before the
launcher is fetched: with no version named, `releases/latest` is asked first.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Base automatically changed from fix/777-typed-name-edges to main October 18, 2026 07:50
Brett Heap and others added 3 commits October 18, 2026 07:50
Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… for: setup-project.py

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

Copy link
Copy Markdown

@brettheap
brettheap marked this pull request as ready for review October 18, 2026 09:30
@sourcery-ai

sourcery-ai Bot commented Oct 18, 2026

Copy link
Copy Markdown

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 3 days and 9 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap merged commit 1ef1a01 into main Oct 18, 2026
5 checks passed
@brettheap
brettheap deleted the fix/857-setup-dry-run-offers branch October 18, 2026 09:31
brettheap pushed a commit that referenced this pull request Oct 18, 2026
…and its round 2, and run to main 33c5128 (#883)

Round 2b of #883. It answers RV883's full NIT list on ede678c (lane 3,
2026-10-18T09:29:28Z) and RV883's round 2 on 86baec5 (#883
issuecomment-6248362246). Each finding was checked against the code at
main 33c5128 before the text changed:

- The #843 limit is two: what `adopt-work-dir-shared` leaves open (group
  write, Windows, root as the other user, this user's own processes, the
  default work directory), and what it now refuses that v1.1.0 used
  (another user's directory, a run as root included, a 0777 directory
  without the sticky bit, a drive mounted without ownership), with the
  way out. "What a script sees" names `adopt-work-dir-shared` and
  `adopt-work-dir-occupied`.
- The doctor's NEXT lines, not its `members` row, read no url from an
  unmerged `.gitmodules`; the `make siblings` limit names that row.
  `siblings.py` has no unmerged rule; the `validate.yml` templates do.
- `validate-family.py`'s `family ok` is scoped to `--no-members` and to a
  holder whose members are checked out.
- The wording points: `update-pr-no-repository` under `--pr`, family.py's
  `git rm -r --cached` line, `MSYSTEM` set and not empty, `..` and
  `naming-unclassified`, the keys git refuses with no value, the url the
  root's config keeps, `contracts: cannot be listed`, install.sh's newest
  tag and redirect location, the scan test's name,
  `update-rewrite-symlink`, `os.access` for root, `LC_ALL=C`,
  `update-validators-red`, `heads/main` in both keys, where
  `plan written to` came, junctions on Python 3.12, the approval click and
  the run's approvals, and the back-port route's one run.
- #457's L1-17 is narrowed by #825; #853 and #854 join #834 as test
  limits.

The refresh past c5359df adds #870 (#864), #876 (#839), #791 (#787),
#859 (#838), #780 (#776), #873 (#857) and #875 (#874): 57 pull requests.
It drops the limits for #776, #839 and #874's ref rule, rewrites the #838
clauses, and adds the limits those squashes name (#863, #879, #884, an
older git without `GIT_NO_LAZY_FETCH`, family.py's link guard). The open
list is the 14 that the 09:42Z milestone read leaves after #875: #675,
#707, #739, #755, #764, #834, #840, #853, #854, #856, #863, #878, #879
and #884.

The six-and-six shape-copy lists stand: from c5359df to 33c5128 the
templates and repo_shape.py changed only in the family root's
bootstrap.py and validate-family.py, both listed. No other file changes.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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