[brettheap]

Lane: openreposhape-1 (openRepoShape-1)
Claim: #415 (comment)

A v1.2.0 draft, authored on the v1.1.1 release commit 5017830. Lane 3 placed #415 in v1.2.0 (#473, issuecomment-6250991055). It changes tests/test_doctor_legs_members_next.py only. shape-doctor.py is unchanged.

N7: the "no character a pasted tail would break on" test held only by accident

The test asserted that the explanation holds none of ;, ', a backtick or a parenthesis. That held only because pytest's temporary roots need no quoting. Over a root like my atlas, the keep line is git -C '<root>/my atlas/spec' branch NAME, and the old test fails on the '. Lane 1's writer measured this over the detached scenario.

Since #415 was filed, #277 has landed. The sentence is the row's why, on its own WHY line, and is no longer a pasted # tail. submodule_update_next's docstring now says the no-;' rule "no longer binds it". So the old test claimed more than the code needs.

Now:

• A new helper, assert_fixed_words_are_plain(why, *spelled):
  • it asserts that each quote_arg-spelled path is in why, so the check cannot pass vacuously;
  • it replaces each path with PATH;
  • it holds the fixed words that remain clear of the five characters.
• The restated test, test_the_explanations_fixed_words_hold_no_shell_syntax_over_a_quoted_root:
  • it runs the five scenarios over two roots that need quoting, my atlas and do ct'or, each a copy of the project, so the doctor's real row is read;
  • the keep line is cut from why, and shlex.split of it must give ["git", "-C", "<mount>", "branch", "NAME"]. That check is skipped on Windows, where PowerShell doubles an apostrophe.
• Three more tests carried the same loop, and now use the helper: #413a and #413c, over a sentence that names a quoted path, and the members git line, whose sentence names no path. That last one passes no path, so there the helper reads the whole of why, as the loop it replaced did.

Mutants (in shape-doctor.py, restored after each):

Mutant	Restated test	Old test, over pytest's roots
M1: a ' in submodule_update_next's fixed words	15 fail, counting the converted sites	5 fail
M2: a ' in checkout_notes's keep-line sentence	2 fail (the detached scenario, both roots)	1 fails
M3: the keep line's path left unquoted	2 fail (No closing quotation, or a wrong split)	all pass

N6: the PowerShell form of the line is never run

This is documented, not tested.

• No CI job would run a Windows-only test here. tests.yml runs this file on ubuntu-latest only (ci: run the test suite on Linux only #597). Its one Windows job, tests-git-bash, runs three installer and launcher files. A skipif(os.name != "nt") test in this file would never run in CI.
• What the module docstring now says: "THE POWERSHELL FORM OF A LINE IS CHECKED AS TEXT AND NEVER RUN (tests/test_doctor_legs_members_next.py (#276, PR #354) never runs the PowerShell form of the git line a Windows reader is handed, and its test that the explanation holds "none of ;, ', a backtick or a parenthesis" passes only because pytest's temporary roots need no quoting: under a root with a space the same sentence's keep line is git -C '<root>/spec' branch NAME, as the file's own spelling test expects #415)".
• Related changes:
  • the claim that the text-level assertions "run on every OS" now says they run on whatever host runs the suite, bar two that give way on Windows for a reason of their own: a member named in bytes, and the shlex read of a quoted root's keep line;
  • the docstring of test_the_line_is_spelled_for_the_readers_shell says it is one of two checks of the PowerShell form, both of the text. The other is test_413a_the_kept_repositorys_keep_line_is_spelled_for_the_readers_shell.

Left alone: the precedent tests/test_shape_doctor.py::test_a_template_validators_next_runs_when_pasted_into_powershell (line 10493) is likewise never run by CI. Its docstring still names the tests-windows job that #597 dropped. That is outside #415.

Review

Lane 1's independent reviewer r415 (sonnet):

• CHANGES at 1ce9a8c8. SHOULD S1: the docstring called that test "THE ONLY CHECK OF THE POWERSHELL FORM", and there are two. NITs N1 to N5.
• The fix round is docstrings and comments only: 732696c8 (S1, N1, N2 and N3), and fbfcc9e8, which closes S1's sentence with a full stop.
• READY at fbfcc9e8. Outside docstrings, the AST is unchanged from 1ce9a8c8, and each new sentence was checked against the code it names.
• Left alone:
  • N4: CHECKOUT_SCENARIOS never reaches rerun_sentence, since rerun is set only when a pin is missing or differs from the gitlink. A ( there survives this test, as it survived the old one. So the title now claims the fixed words of the explanations these scenarios reach, not every explanation.
  • N5: the runtime of eight cases with no path in their sentence.

Tests

Run with /usr/bin/python3 -m pytest, a stub gh first on PATH, a bogus GH_TOKEN, an empty GH_CONFIG_DIR and GIT_CONFIG_GLOBAL=/dev/null:

File	Result
tests/test_doctor_legs_members_next.py	186 passed, 6 skipped. The skips are git 2.43 behaviours. The file has 192 tests, up from 187.
tests/test_repo_hygiene.py	78 passed
tests/test_doctor_paste_through_git.py and tests/test_no_auto_maintenance.py	79 passed, 2 skipped

No added line is over 79 columns, the change is ASCII, and git diff --check is clean.

Closes #415

🤖 Generated with Claude Code

Summary by Sourcery

Harden doctor explanation tests against shell syntax regressions across quoted roots and clarify the limits of PowerShell coverage.

Bug Fixes:

• Strengthen shell-safety coverage for doctor explanations by testing quoted repository roots and validating extracted keep commands as parsed arguments.

Enhancements:

• Replace the broad explanation-character checks with reusable assertions that distinguish quoted paths from fixed explanatory text.
• Document that PowerShell command forms are validated as text only because the relevant test file is not run by CI on Windows.

Tests:

• Expand checkout explanation tests to cover roots containing spaces and apostrophes, including POSIX shell parsing of keep lines.
• Apply the shared fixed-word shell-syntax checks across related explanation and members-git-line tests.

[sourcery-ai]

Reviewer's Guide

The PR revises the doctor explanation tests to distinguish fixed prose from quoted paths, exercising real rows over roots containing spaces and apostrophes and validating copied POSIX commands structurally. It also documents that PowerShell output is checked only as text because this test module has no Windows CI job.

File-Level Changes

Change	Details	Files
Strengthen shell-safety assertions for explanation text and copied keep commands when repository roots require quoting.	Add a helper that verifies expected quoted paths are present, masks path-specific shell syntax, and checks fixed words. Exercise checkout explanations across roots containing spaces and apostrophes. Parse each extracted POSIX keep command with shlex and verify the quoted path remains one argument. Apply the helper to the related members, 413a, and 413c explanation tests. Increase mutation-test coverage for fixed-word punctuation and unquoted paths.	tests/test_doctor_legs_members_next.py
Document that PowerShell command forms are validated as generated text rather than executed.	Clarify in the module documentation that this test file runs on Ubuntu and has no Windows CI coverage. State that the reader-shell test is the only check of the PowerShell form and does not invoke PowerShell. Update section headings and explanatory comments to distinguish cross-platform text assertions from executable POSIX-shell tests.	tests/test_doctor_legs_members_next.py
Update test setup and imports to support quoted-root scenarios.	Import shlex for POSIX command parsing. Reuse the awkward-root fixture value and define parametrized quoted roots. Copy the project into each quoted root before running scenarios. Capture the created leg path needed for validating the generated explanation.	tests/test_doctor_legs_members_next.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#415	Add a Windows-only test that pastes the PowerShell-form legs git line into PowerShell and verifies it succeeds and leaves the legs row healthy, including coverage for a root path with a space.	❌	The PR explicitly documents that the PowerShell form is checked only as text and is never run. It adds no PowerShell execution test, so the issue’s requested runtime verification remains unimplemented.
#415	Correct the shell-syntax assertion so it checks the fixed words of the explanation independently of quoted paths, and verify keep-line quoting under roots that require quoting.	✅

Possibly linked issues

• shape-doctor.py: the legs row's NEXT (and the members row's) runs <root>/scripts/bootstrap.py without asking whether the root has it, so a root that lost it pastes python3: can't open file, exit 2, and for an empty leg mount the legs NEXT would not cure the row even with the file there (bootstrap.py only prints Run git submodule update --init spec) #276: The PR addresses the reported gaps: it corrects the shell-syntax assertion for quoted paths and documents, rather than fixes, missing PowerShell execution.

---

Tips and commands

Interacting with Sourcery

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

Customizing Your Experience

Access your dashboard to:

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

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[brettheap]

(lane openRepoShape-1, 2026-10-18T15:11:52Z) Independent review of #899 (#415), by lane 1's reviewer r415 (sonnet), who is not its writer: READY at fbfcc9e8.

Round 1, at 1ce9a8c8: CHANGES.

• SHOULD S1. test_the_line_is_spelled_for_the_readers_shell called itself "THE ONLY CHECK OF THE POWERSHELL FORM". test_413a_the_kept_repositorys_keep_line_is_spelled_for_the_readers_shell also asserts the platform="nt" spelling.
• NIT N1. The shlex skip covers Git Bash too.
• NIT N2. The module docstring said "carry no skip".
• NIT N3. The helper's docstring overstated what a call with no path checks.
• NIT N4. rerun_sentence is never reached by these scenarios. That gap was already there.
• NIT N5. Eight cases add runtime only.
• Mutants: X2, X3, X4, X5, X8, X9, X11 and X12 were caught. X1, a ( in rerun_sentence, survives (N4).

The fix round is docstrings and comments only: 732696c8 took S1, N1, N2 and N3, and fbfcc9e8 closes S1's sentence with a full stop.

Round 2, at fbfcc9e8: READY, with no new finding.

• Every new sentence is true of the code it names: repo_shape.py's host_shell_dialect, the 36 WINDOWS_SKIP tests, and the two platform="nt" checks.
• With docstrings blanked, the AST is unchanged.
• No added line is over 78 columns, and git diff --check is clean.
• Runs:
  • -k "spelled or plain": 3 passed;
  • the restated and converted tests: 15 passed;
  • tests/test_repo_hygiene.py: 78 passed;
  • the whole file at 732696c8: 186 passed, 6 skipped.

N4 and N5 are left. The title now claims only the explanations the scenarios reach, and the body says why.

Lane: openreposhape-1 (openRepoShape-1)

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[sourcery-ai]

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

You can request another review in 1 day and 7 hours by commenting @sourcery-ai review. Upgrade to get a review now.