From d8ccd4c7c64e58065540056f0bbec38c28042faa Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 13:58:48 +0000 Subject: [PATCH 1/4] adopt-project.py, shape-doctor.py and tests/test_shape_doctor.py: the verification row's colon, `submodule_config`'s reason, `run_validator`'s "costs no child an import" and the decoy test's `sitecustomize` claim say what main does, so four comments no longer describe code that is not there (#666) Comments and docstrings only. No behaviour, no `--help` text, no pinned copy, so `docs/cli.md` is unchanged. - adopt-project.py, module docstring: the verification table's `code` row reads `0 of N source paths (seeded from template)`, with no colon. That is what `AGENTS.md` and `README.md` say since #505, and what the row's print statement writes. - shape-doctor.py, `submodule_config`: the replacement decode is chosen for the comparison with `origin_url`'s reading of a clone's remote, and for nothing else. The row never printed a surrogate: the table goes through `ascii_text` and `--json` through `json.dumps`, neither of which raises on one. The docstring now says what `surrogateescape` would change (`--json`'s `mounted_from` would carry a lone-surrogate escape). - shape-doctor.py, `run_validator`: "it costs no child an import" holds for an unedited copy only. A root whose own validator copy imports from site-packages read `passes` before `-S` and now reads the crash its own CI gets. The docstring says so and calls the row a fair answer. - tests/test_shape_doctor.py, the decoy test's two comments: `-S` starts every child with no `site`, so no child imports a `sitecustomize`. The comments say that replace-not-prepend and the #306 review S1 case are held by `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given`, and that the decoy test holds the template row's real verdict. Proof that only comments and docstrings changed: the AST of each of the three files is identical before and after once docstrings are blanked. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 4 +-- shape-doctor.py | 16 ++++++++++-- tests/test_shape_doctor.py | 53 +++++++++++++++++++++----------------- 3 files changed, 45 insertions(+), 28 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index e7702d92..378e2642 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -137,8 +137,8 @@ HISTORY rather than an empty repository. So a leg no entry assigns a path to is seeded from `templates/-root/` as one initial commit, the plan records `seeded_from_template: true` for it, the split still mounts and pins -it, and the verification table reads `code: 0 of N source paths (seeded from -template)`. `check` WARNS; `execute` refuses without `--allow-empty-leg +it, and the verification table's `code` row reads `0 of N source paths (seeded +from template)`. `check` WARNS; `execute` refuses without `--allow-empty-leg `, in the plan or on its command line, because a plan that lost its code paths to a bad edit looks identical from here. diff --git a/shape-doctor.py b/shape-doctor.py index e4287748..192b8f3a 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -1572,6 +1572,14 @@ def run_validator(ctx: Context, script: Path, args: list[str], gate can import in CI either. ONE RULE FOR EVERY CHILD, as the `env` block is: the flag does not ask which script it starts. + THAT IS TRUE OF AN UNEDITED COPY ONLY. A root whose own copy of a + validator gained an `import yaml` read `passes` here while `site` was on, + wherever the doctor's interpreter could import it, and now reads + `crashed before reaching a verdict (exit 1): ModuleNotFoundError: No + module named 'yaml'`, which is the crash its own CI gets, because + `validate.yml` installs nothing. That row is a fair answer for that root, + and `-S` stays as it is: it does not ask whether a copy was edited. + WHICH AGREEMENT THE ROW KEEPS (#306, #342): the row reports the project's own GATE as CI runs it, with no inherited `PYTHONPATH` and no `site`; it does not predict what a pasted line prints in the reader's @@ -7018,8 +7026,12 @@ def submodule_config(root: Path) -> dict: is COMPARED with `origin_url`'s reading of a clone's remote, which is the replacement (#385): the same bytes read two ways are two strings, so a clone made from this very url would stop being the member's - working clone. It is also what the row PRINTS, and a lone surrogate is - a string no UTF-8 stream can write. + working clone. That comparison is the whole reason; what the row PRINTS + is not part of it, because the table goes through `ascii_text` and + `--json` through `json.dumps`, so neither would raise on a lone + surrogate. `surrogateescape` would change one thing there: `--json`'s + `mounted_from` would carry a `\\udce9` escape that a strict JSON reader + refuses, where the table shows `?` whichever way the byte is read. """ fields: dict = {} try: diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index d17e86b3..d0f1638d 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -10238,23 +10238,26 @@ def test_an_exported_pythonpath_does_not_stand_in_for_a_lost_repo_shape( #: `sitecustomize.py` that ends any VALIDATOR with a distinctive exit, and a #: `repo_shape.py` that raises when it is imported. #: -#: THE `sitecustomize.py` IS WHAT DOES THE WORK, and it catches every way the -#: directory can reach a validator. `PYTHONPATH` entries come ahead of the -#: standard library and of site-packages, so the first `sitecustomize` on a -#: child's path is this one wherever the directory sits in the value -- in -#: front of the standard's `scripts/`, behind it, or alone -- and it ends the -#: child at start-up, before a single import. Python imports ONE -#: `sitecustomize`, the first it finds, and the doctor is not that child: its -#: own script is not `validate-*`, so it runs on. +#: THE `sitecustomize.py` DOES NOTHING SINCE `-S` (#342). `run_validator` +#: starts every child as `[sys.executable, "-S", script, ...]`, so no child +#: runs `site` and none imports a `sitecustomize`, this one or any other. +#: Before `-S` it was what did the work: `PYTHONPATH` entries come ahead of +#: the standard library and of site-packages, so the first `sitecustomize` on +#: a child's path was this one wherever the directory sat in the value, and +#: it ended the child at start-up, before a single import. It would fire +#: again only for a child started without `-S`. #: -#: A `repo_shape.py` ALONE CANNOT TELL PREPENDING FROM REPLACING: the -#: standard's `scripts/` comes first in a prepended value, so the standard's -#: `repo_shape` wins the import and the decoy behind it is never read. The -#: raising `repo_shape.py` decides an outcome only where `sitecustomize` does -#: not load from the path (site customisation off, or an earlier -#: `sitecustomize` ahead of this one), and there it is what a child that put -#: the operator's directory FIRST would import. It is a fallback, and no test -#: here depends on it. +#: WHAT HOLDS REPLACE-NOT-PREPEND AND THE S1 CASE NOW IS +#: `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given`, +#: which asks the child what `PYTHONPATH` it got. A `run_validator` that puts +#: the standard's `scripts/` in front of the operator's value, or that keeps +#: the variable for the callers that pass none (#306 review S1), passes THIS +#: test, in all three cases, and is killed by that one. This test holds the +#: template row's real verdict under an exported `PYTHONPATH`, end to end. +#: The raising `repo_shape.py` is the part of it that still bites: a child +#: that put the operator's directory AHEAD of the standard's `scripts/` would +#: import it, because the template's own directory has no `repo_shape.py` +#: beside it, and that child fails this test. DECOY_REPO_SHAPE = ( "raise ImportError(\"the operator's repo_shape.py was imported by a " "validator\")\n") @@ -10299,15 +10302,17 @@ def test_an_exported_pythonpath_does_not_reach_a_template_validator( for leaked in ("DECOY", "operator's repo_shape", "ImportError"): assert leaked not in row["reason"], row - # AND NO OTHER ROW OF THE RUN REACHED THE DECOY. The row above is the one + # AND NO OTHER ROW OF THE RUN QUOTES THE DECOY. The row above is the one # that passes a directory; the `naming` rows and the machine's preflight - # pass none. On main every row whose validator the decoy reached said so, - # and this catches any row that reaches it, which is how it catches a - # `run_validator` that kept the variable for the callers that pass none - # (#306 review S1): popped only for a script inside the root, or with the - # `else` dropped, the `naming` row reads `FINDING ... (exit 86): DECOY` - # while `manifest` reads the real finding. So no row may quote the decoy - # or say it exited 86, whichever caller handed it the variable. + # pass none. Before `-S` this is how the test caught a `run_validator` + # that kept the variable for the callers that pass none (#306 review S1): + # popped only for a script inside the root, or with the `else` dropped, + # the `naming` row read `FINDING ... (exit 86): DECOY` while `manifest` + # read the real finding. Since `-S` no child imports the decoy's + # `sitecustomize`, so that case no longer shows here; the probe in + # `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given` + # holds it, beside the root as well as inside it. What this loop still + # holds is that no row quotes the decoy or says it exited 86. reached = [name for name, other in rows.items() if "DECOY" in other["reason"] or "(exit 86)" in other["reason"] From a2113cffb5e695ac3908898ab70aa32cfe053aad Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 14:00:30 +0000 Subject: [PATCH 2/4] shape-doctor.py: `submodule_config`'s docstring drops the sentence that gave printing as a reason, as lane 3's #668 does, so the two changes are one identical edit and merge without a conflict (#666) The sentence "It is also what the row PRINTS, and a lone surrogate is a string no UTF-8 stream can write" is not a reason: the table goes through `ascii_text` and `--json` through `json.dumps`, and neither raises on a lone surrogate. The comparison with `origin_url`'s reading of a clone's remote is the reason, and the paragraph already gives it. The previous commit on this branch replaced the sentence with a longer account of what `surrogateescape` would change in `--json`; that text conflicted with #668's deletion of the same two lines (merge-tree, `shape-doctor.py` around :7079), so this commit takes the issue's first direction, "drop the sentence", with the same bytes #668 leaves. Comment only: the AST of the file is unchanged. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- shape-doctor.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 192b8f3a..d5beece4 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -7026,12 +7026,7 @@ def submodule_config(root: Path) -> dict: is COMPARED with `origin_url`'s reading of a clone's remote, which is the replacement (#385): the same bytes read two ways are two strings, so a clone made from this very url would stop being the member's - working clone. That comparison is the whole reason; what the row PRINTS - is not part of it, because the table goes through `ascii_text` and - `--json` through `json.dumps`, so neither would raise on a lone - surrogate. `surrogateescape` would change one thing there: `--json`'s - `mounted_from` would carry a `\\udce9` escape that a strict JSON reader - refuses, where the table shows `?` whichever way the byte is read. + working clone. """ fields: dict = {} try: From 7f2bb36b8836fda5df548f51fbbf42394809944d Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 19:10:23 +0000 Subject: [PATCH 3/4] tests/test_shape_doctor.py: the probe test's docstring says the decoy of (b) could see a caller that passes no `pythonpath` only before `-S`, and the decoy loop's comment says it is a tripwire, so two comments no longer say a decoy holds what the probe holds (#666) Comments and docstrings only. The review of the first two commits found two sentences that still described the decoy test as it was before `-S`. - test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given, docstring: "so only a probe like this one, or (b)'s decoy, can see that they have none" was false under `-S`. A `run_validator` that keeps the variable for the callers that pass none passes the decoy test in all three cases and fails the probe test. It now says only a probe like this one can see that, and that the decoy of (b) could before `-S` (#342). - test_an_exported_pythonpath_does_not_reach_a_template_validator, the comment above `reached`: "What this loop still holds is that no row quotes the decoy or says it exited 86" overstated it. Under `-S` the loop cannot fire. It now says it is kept as a tripwire for a future removal of `-S`, and that `test_run_validator_starts_every_child_with_no_site` in `tests/test_doctor_validator_site.py` is the test that holds `-S`. Proof that only comments and docstrings changed: the AST of tests/test_shape_doctor.py is identical to the previous head's once docstrings are blanked. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_shape_doctor.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index d0f1638d..935dee49 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -10311,8 +10311,10 @@ def test_an_exported_pythonpath_does_not_reach_a_template_validator( # read the real finding. Since `-S` no child imports the decoy's # `sitecustomize`, so that case no longer shows here; the probe in # `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given` - # holds it, beside the root as well as inside it. What this loop still - # holds is that no row quotes the decoy or says it exited 86. + # holds it, beside the root as well as inside it. Under `-S` this loop + # cannot fire. It is kept as a tripwire for a future removal of `-S`, + # which `test_run_validator_starts_every_child_with_no_site` (in + # `tests/test_doctor_validator_site.py`) is the test that holds. reached = [name for name, other in rows.items() if "DECOY" in other["reason"] or "(exit 86)" in other["reason"] @@ -10349,8 +10351,8 @@ def test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given( caller sees it, and it holds the callers which pass no `pythonpath` by where its probe lies: beside the root, as the standard's own naming and preflight scripts do, as well as inside it. Those scripts need nothing - from the variable, so only a probe like this one, or (b)'s decoy, can see - that they have none. + from the variable, so only a probe like this one can see that they have + none (the decoy of (b) could before `-S`, #342). """ module = doctor_module(standard) if exported is None: From 426877bf7e71d09c02c038b87d53348129e78a59 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 19:19:17 +0000 Subject: [PATCH 4/4] tests/test_shape_doctor.py: the comment above the decoy loop's `reached` ends its tripwire sentence with a semicolon and says the named test holds `-S`, so the sentence parses (#666) Comment only. The fix-round commit 7f2bb36b ended the comment with "removal of `-S`, which `test_run_validator_starts_every_child_with_no_site` (in `tests/test_doctor_validator_site.py`) is the test that holds.", which does not parse. The last three lines now read: "It is kept as a tripwire for a future removal of `-S`; `test_run_validator_starts_every_child_with_no_site` (in `tests/test_doctor_validator_site.py`) is the test that holds `-S`." The AST of tests/test_shape_doctor.py is identical to the one at a2113cff once docstrings are blanked. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_shape_doctor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index 935dee49..e9fd577f 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -10312,9 +10312,9 @@ def test_an_exported_pythonpath_does_not_reach_a_template_validator( # `sitecustomize`, so that case no longer shows here; the probe in # `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given` # holds it, beside the root as well as inside it. Under `-S` this loop - # cannot fire. It is kept as a tripwire for a future removal of `-S`, - # which `test_run_validator_starts_every_child_with_no_site` (in - # `tests/test_doctor_validator_site.py`) is the test that holds. + # cannot fire. It is kept as a tripwire for a future removal of `-S`; + # `test_run_validator_starts_every_child_with_no_site` (in + # `tests/test_doctor_validator_site.py`) is the test that holds `-S`. reached = [name for name, other in rows.items() if "DECOY" in other["reason"] or "(exit 86)" in other["reason"]