shape-doctor.py: after #238 the doctor runs a root's OWN validator with the operator's PYTHONPATH still in the child's environment, so for an operator who exports one that reaches a repo_shape.py the manifest row of a root that lost its own scripts/repo_shape.py says ok ... passes again, the false pass #238 removed, where the project's own gate in CI fails on ModuleNotFoundError: No module named 'repo_shape' #306

Lane: openreposhape-4 (openRepoShape-4)

Found by this lane's re-check of #238's fix round (head 88c81e0; its nit N1), and re-verified twice on bytes that carry #238: on 88c81e0 itself, and on a scratch replay of #238's two commits (59724c9, 88c81e0) onto main d94d155 (#232's squash), which is clean (replay head 84bf5ff; the replay branch was not on origin when this was written). It exists only with #238, which has since landed as ede15ce (PR #287). The shape-doctor.py lines cited below are 88c81e0's; on main they sit 2 lines lower up to run_validator (:884-906 is :886-908, :937-942 is :939-944) and 4 lines lower from validator_for on (:1088-1089 is :1092-1093), because of the docstring commit that landed with #238. Rare, and for one kind of operator only: it needs a PYTHONPATH in the operator's own environment that reaches a repo_shape.py, and a root that has lost its own. Not claimed: that anything in the standard exports such a variable (nothing does: every validator inserts its own directory), and Windows.

What is wrong. #238 decides in one place whether a validator run gets the standard's scripts/ on PYTHONPATH: validator_scripts(shape, own_copy) returns None if own_copy else shape / "scripts" (shape-doctor.py:884-906 on 88c81e0), validator_row hands that to run_validator (:1088-1089), and run_validator sets PYTHONPATH only when it is given one (:937-942). The point, in validator_scripts's docstring: the root's own copy "is the project's own gate and the doctor's job is to run it as the project runs it -- as a path, importing repo_shape from beside itself. A root that has lost its own scripts/repo_shape.py has a gate that cannot import it, and the row must say what that gate says (ModuleNotFoundError as a finding), not a "passes" the project's own gate would never give." But "given nothing" is not "run with nothing": the child's environment starts as env = dict(os.environ) (:937), so a PYTHONPATH the operator exported rides into the own-copy run:

# $U: a clean checkout at 88c81e0 (the same on the replay, 84bf5ff). A project Atlas scaffolded from it, cloned to $C, with a VALID manifest,
# then `mv $C/scripts/repo_shape.py` away (a root that lost its own copy). gh is a shim; the git environment is isolated.
cd / && env -u PYTHONPATH python3 $U/shape-doctor.py --root $C --json     # rows `manifest` and `pins` shown
#   verdict: DRIFTED (1 copy-missing) exit 1
#   [manifest] FINDING :: the project's own scripts/validate-manifest.py reports a finding (exit 1): ModuleNotFoundError: No module named 'repo_shape'
#   [pins]     FINDING :: the project's own scripts/validate-pins.py reports a finding (exit 1): ModuleNotFoundError: No module named 'repo_shape'
cd / && PYTHONPATH=$U/scripts python3 $U/shape-doctor.py --root $C --json                  # the operator exports the standard's scripts/
#   verdict: DRIFTED (1 copy-missing) exit 1
#   [manifest] ok      :: the project's own scripts/validate-manifest.py passes
#   [pins]     FINDING :: the project's own scripts/validate-pins.py reports a finding (exit 1): FINDING shape-copy-missing: contracts/shape-pin.yaml: scripts/repo_shape.py is pinned but absent
cd $C && env -u PYTHONPATH python3 scripts/validate-manifest.py --root .                   # the project's own gate, as CI runs it
#   Traceback (most recent call last):
#     File ".../scripts/validate-manifest.py", line 64, in <module>
#       from repo_shape import (  # noqa: E402
#   ModuleNotFoundError: No module named 'repo_shape'                                       exit 1

The verdict is DRIFTED (1 copy-missing) both times, because shape currency ranks first and names the missing file, so what flips is the manifest row: a finding for everybody but this operator, and ok ... passes for them, for a gate that fails in CI. (The pins row is a finding either way, with a different message.) The test helper beside #238's tests knew this: doctor_with_no_pythonpath (tests/test_shape_doctor.py:5122-5129 on 88c81e0) blanks PYTHONPATH because an exported one "would reach the root's own validator through the child's inherited environment and hide the very import this is asking about".

Why. run_validator builds the child's environment from the doctor's own and adds to it; nothing removes. The three callers that pass no pythonpath (check_naming, check_not_a_root_naming, check_machine) run the standard's own scripts, which find repo_shape by themselves (validate-repository-naming.py:79 inserts its own directory; setup-project.py --preflight returns at :1820, before its only repo_shape import, which is deferred at :1377), so for them an inherited PYTHONPATH is harmless; for the root's own validator it is the one thing that can stand in for the file the root lost. #238's docstrings say the run and the pasted line apply PYTHONPATH under one condition, which holds, but "the row must say what that gate says" holds only for an operator with no such export.

What it should do. The row says what the project's own gate says, for every operator. The decisions are the author's:

1. How. Remove PYTHONPATH from the child's environment when the run is the root's own copy (env.pop("PYTHONPATH", None) when pythonpath is None and the script is the root's own), or for every caller that passes none (inert for the three above, per shape-doctor.py validator_row falls back to the standard's TEMPLATE validator when a root carries no copy of its own, and prints that path as the NEXT command without the PYTHONPATH it runs it under, so pasting the manifest, pins or family row's NEXT dies with ModuleNotFoundError: No module named 'repo_shape' and exit 1, the code a validator uses for a finding #238's review). Or document it instead, in validator_scripts's docstring: "the root's own copy runs under the operator's environment".
2. Which agreement to keep. A pasted NEXT runs in the reader's shell, with the reader's PYTHONPATH, so stripping it in the child makes the row agree with CI and no longer with that operator's own paste (the row says FINDING, the paste says ok). shape-doctor.py validator_row falls back to the standard's TEMPLATE validator when a root carries no copy of its own, and prints that path as the NEXT command without the PYTHONPATH it runs it under, so pasting the manifest, pins or family row's NEXT dies with ModuleNotFoundError: No module named 'repo_shape' and exit 1, the code a validator uses for a finding #238's property was "the paste agrees with detail.exit"; say which of the two the row is for, and say so in the row or the docstring.

How to test. The repro above. In the suite, next to doctor_with_no_pythonpath: run the doctor with PYTHONPATH=<standard>/scripts over the root test_an_own_validator_without_repo_shape_does_not_pass builds (a valid manifest, its own repo_shape.py removed; tests/test_shape_doctor.py:5169 on 88c81e0) and assert the manifest row is a FINDING quoting ModuleNotFoundError (today: ok), with the existing tests, which blank the variable, unchanged.

Related: #238 (its S1 closed by the fix round; this is that review's N1), #232 (the manifest and family rows), the issue on validator_row reading a validator's exit 1 as a finding whatever caused it (#278: the PYTHONPATH the template fallback runs under), and the issue on NEXT lines carrying NAME=value prefixes and # tails (#277: decision 4 there is the PYTHONPATH=<q> python3 ... spelling #238 introduces).