From 2bf402dd96fb17ca20fd63964d8dcc3660440026 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:44:09 +0000 Subject: [PATCH 01/10] shape-doctor.py: a validator that crashed says it crashed instead of "reports a finding", and is CANNOT ANSWER when it is the standard's template, which a standard whose path holds the `PYTHONPATH` separator no longer runs at all; a neutral product's digest `validate-pins.py` skipped is a `note` naming the product, from a child that can no longer reach `gh` or a proxy; and a root whose manifest does not load and whose `scripts/repo_shape.py` is gone is told to restore that file before its gate (#278, #297, #298) Three rows said more than the validator they ran had answered. #278. `validator_row` read a validator by its exit code alone, and an exception that reaches the top of a Python program exits 1, the code for findings. A root's own `validate-pins.py` reduced to `raise RuntimeError("boom")` read "reports a finding (exit 1)", and a standard checked out under a directory whose name holds `:` gave the template copy a `PYTHONPATH` that Python split in two, so the template died on `import repo_shape` and a valid manifest read INVALID (manifest). `crashed()`, beside `run_validator`, now tells a crash apart: a non-zero exit, no line starting FINDING or REFUSED, and a traceback header (R278-1). The root's own copy crashing stays a FINDING, since a gate that cannot run is a fact about the project; it says "crashed before reaching a verdict", keeps the quoted exception the #238 and #306 tests read, carries `detail.crashed`, and names a missing `scripts/repo_shape.py` when that is the cause. The template copy crashing is this checkout's failure: it also carries `detail.environment`, so the verdict is CANNOT ANSWER, exit 3. The NEXT stays the validator's own line, which shows the traceback. `pythonpath_unnamable()`, beside `validator_scripts`, says when the template's `scripts/` holds `os.pathsep` (R278-2, amended: no invented destination). `validator_row` must run the template to answer, so it runs nothing and is an environment row with no NEXT; `unloaded_manifest_row` already knows its answer, so its FINDING stands with no NEXT. Both say to run the doctor from a checkout of the standard whose path holds no separator, and no `PYTHONPATH=` value holding one is printed anywhere. It is `os.pathsep`, so Windows' `;` is claimed by the same helper, and a `;` directory on POSIX is an ordinary character and is not refused (R278-3). #297. A project that pins a neutral product, with no local checkout of it, has a `validate-pins.py` that asks `gh api`, prints a `skip` line when that fails, and exits 0. The doctor threw the output away and said "passes", under COMPLIANT, and the child had started a call to GitHub on the operator's login wherever there was one. Every validator child is now started `offline()` (R297-1 with its fallback, both walls): the four token variables `GH_TOKEN`, `GITHUB_TOKEN`, `GH_ENTERPRISE_TOKEN` and `GITHUB_ENTERPRISE_TOKEN` removed, `GH_CONFIG_DIR` the null device, `GH_NO_UPDATE_NOTIFIER=1`, and `HTTPS_PROXY`, `https_proxy`, `HTTP_PROXY` and `http_proxy` pointed at http://127.0.0.1:9 with `NO_PROXY` and `no_proxy` removed. On Windows, where a variable's case does not matter, only the upper-case proxy names are set. `check_machine` alone opts out, with `run_validator(..., reach_network=True)`: it is the documented exception. Exit 0 with ` skip ` lines is now a `note` (R297-2), "passes, but did not recheck: ", with `detail.skipped` and no NEXT, since the skip line names the three ways to give a source; a note moves no verdict. The validator's own exit-0-with-a-skip contract is unchanged (R297-3). #298. `unloaded_manifest_row` runs no gate, and promised that the gate "refuses it the same way until it loads" for a root that had also lost its `scripts/repo_shape.py`, where that gate dies on the import. The fact now lives in `imports_repo_shape()` beside `validator_scripts`, decided once for both rows and not as a fourth value of `validator_for`, whose arity three callers depend on (R298-1). When the root's own copy cannot import it, the NEXT is `git -C checkout HEAD -- scripts/repo_shape.py`, the bytes the project committed, and its why names the gate to run after it (R298-2, R298-3: restore first, as DRIFTED (copy-missing) already outranks INVALID; the verdict is unchanged). A deletion already committed is not in HEAD, so there the row has no NEXT and its reason says the file comes back from the parent of the commit that deleted it. The "refuses it the same way" promise is printed only where the gate can keep it. Tests, in the new tests/test_doctor_validator_rows.py, on what each row and command does and never on a NEXT's tail: a crashing own copy for each of the three gates and a crashing template (FINDING with `detail.crashed`; the template's CANNOT ANSWER, exit 3), and both NEXTs pasted show the traceback; a standard under `dir:with-colon` gives the environment row with nothing run and no NEXT, and with an unloadable manifest a FINDING with no NEXT, with no `PYTHONPATH=` in either report (WINDOWS_SKIP); a standard under `semi;colon` is refused on Windows and is the control on POSIX, where its pasted NEXT prints the template's finding; a MedxGlass descendant run with this file's own logging `gh` stub first on PATH is COMPLIANT with a `pins` note naming openGlass, the stub saw the validator child with no token, `GH_CONFIG_DIR` the null device and the proxy set, and saw the machine row's probes with the operator's values; a `run_validator` probe both ways on every platform; and for a project and a holder whose manifest has a tab and whose `scripts/repo_shape.py` is gone, the restore NEXT run and pasted exits 0, after which the gate the doctor names refuses `yaml-unparsable` with exit 2, beside the control (the file kept) and a committed deletion (no NEXT). Mutants, each run against the new file: a crash read as a finding again, the token strip dropped, the proxy dropped, the skip read as ok, the restore NEXT replaced by the old promise, and the separator check dropped each turn the rows that hold them red. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 280 ++++++++++++- tests/test_doctor_validator_rows.py | 609 ++++++++++++++++++++++++++++ 2 files changed, 883 insertions(+), 6 deletions(-) create mode 100644 tests/test_doctor_validator_rows.py diff --git a/shape-doctor.py b/shape-doctor.py index 3e5bbece..34db51ed 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -912,8 +912,159 @@ def validator_scripts(shape: Path, own_copy: bool) -> Path | None: return None if own_copy else shape / "scripts" +def pythonpath_unnamable(shape: Path, own_copy: bool) -> str | None: + """Why no `PYTHONPATH` can name the directory `validator_scripts` gives + this copy, or None when one can (#278). + + `PYTHONPATH` IS A LIST, AND A LIST HAS A SEPARATOR. Python splits the + variable at `os.pathsep` -- `:` on POSIX, `;` on Windows -- and nothing in + the variable can quote one, so a standard checked out under + `/srv/dir:with-colon/` gives the template copy two entries, neither of + which holds `repo_shape.py`. The doctor's child then died on the import + and the row read that crash as the project's finding: `INVALID + (manifest)` for a manifest that is valid, from one directory name. The + line a row prints has the same limit, because it is the same variable. + + ONE HELPER, FOR EVERY PLACE THAT WOULD GIVE OR PRINT IT: `validator_row` + must RUN the template to answer, so there it is an environment row and + nothing is run; `unloaded_manifest_row` already knows its answer, so + there only the printed line is withheld. Both ask this, with the + `own_copy` `validator_for` made, so the run and the line cannot disagree + about which directories are refused. The root's own copy is given no + `PYTHONPATH` at all, and is never refused here. A `;` on POSIX, or a `:` + on Windows, is an ordinary character in an entry and is not refused + either. + + THE SENTENCE NAMES NO DESTINATION. Where the standard should be checked + out is the person's choice, and a command with a made-up path in it is + one more thing to correct before it runs. + """ + scripts = validator_scripts(shape, own_copy) + if scripts is None or os.pathsep not in str(scripts): + return None + return (f"it imports repo_shape from {scripts}, which only PYTHONPATH " + f"can give it, and that path holds {os.pathsep!r}, the character " + "PYTHONPATH splits on, so no PYTHONPATH can name it; run the " + "doctor from a checkout of the standard whose path holds no " + f"{os.pathsep!r}") + + +def repo_shape_for(shape: Path, used: Path, own_copy: bool) -> Path: + """The `repo_shape.py` the validator `used` imports (#298): the one beside + the root's OWN copy, or the standard's for its TEMPLATE copy, which is + the directory `validator_scripts` gives that copy and no other. + """ + directory = validator_scripts(shape, own_copy) or used.parent + return directory / "repo_shape.py" + + +def imports_repo_shape(shape: Path, used: Path, own_copy: bool) -> bool: + """Whether the validator `used` can import `repo_shape` (#298, R298-1). + + DECIDED HERE, ONCE, for both rows that need it: `validator_row` adds it + to a crash's reason, and `unloaded_manifest_row`, which runs no gate, + uses it to decide what its NEXT may promise. NOT a fourth value of + `validator_for`, which three callers unpack: an arity change merges + cleanly and then fails at run time on whichever change lands second. + """ + return repo_shape_for(shape, used, own_copy).is_file() + + +def head_holds(root: Path, relative: str) -> bool: + """Whether the root's HEAD has a file at `relative`, so that `git + checkout HEAD -- ` puts it back (#298). `./` makes the path + the root's, as the restore line's pathspec is, and not the toplevel's. + A root that is no repository, or has no HEAD, holds nothing. + """ + try: + kind = git_out(["cat-file", "-t", f"HEAD:./{relative}"], cwd=root) + except (Refusal, OSError): + return False + return kind == "blob" + + +#: The four variables `gh` reads a token from, which no validator's child +#: keeps (#297). +GH_TOKEN_VARIABLES = ("GH_TOKEN", "GITHUB_TOKEN", "GH_ENTERPRISE_TOKEN", + "GITHUB_ENTERPRISE_TOKEN") +#: Where a validator's child is told its proxy is: the discard port on +#: loopback, which nothing a validator talks to answers on, so a request +#: that gets past `gh`'s missing login fails on this machine rather than +#: reaching a host (#297). +UNREACHABLE_PROXY = "http://127.0.0.1:9" +PROXY_VARIABLES = ("HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy") +NO_PROXY_VARIABLES = ("NO_PROXY", "no_proxy") + + +def offline(env: dict) -> dict: + """`env`, changed in place so a child started with it cannot reach a + network through `gh` (#297): no token, no `gh` configuration, and every + proxy variable pointed at `UNREACHABLE_PROXY` with no exception list. + + `gh` with no token and `GH_CONFIG_DIR` naming the null device knows no + host and no login, and stops at its own login check before a request; + `tree_digest_from_gh` turns that failure into `gh-unreadable`, the skip + `validate-pins.py` documents. The proxy is the second wall, for a + `gh` that sends a request anyway: it has nowhere to go. NOT A `gh` SHIM + ON `PATH`: that needs a directory written somewhere, which this command + never does, and a Windows child finds `gh.exe` past any shim that is + not one. + + ON WINDOWS A VARIABLE IS ONE VARIABLE WHATEVER ITS CASE, so only the + upper-case spelling of each proxy name is set there: the two spellings + are one name to the child, and one entry is what it should be handed. + """ + for name in GH_TOKEN_VARIABLES + NO_PROXY_VARIABLES: + env.pop(name, None) + env["GH_CONFIG_DIR"] = os.devnull + env["GH_NO_UPDATE_NOTIFIER"] = "1" + for name in PROXY_VARIABLES: + if os.name == "nt" and not name.isupper(): + continue + env[name] = UNREACHABLE_PROXY + return env + + +#: The line Python prints first when an exception reaches the top of a +#: program -- and one a validator that reached a verdict never prints (#278). +TRACEBACK_HEADER = "Traceback (most recent call last):" + + +def crashed(code: int, whole: str) -> str | None: + """The line to quote when a validator CRASHED before reaching a verdict, + or None when it reached one (#278). + + THE EXIT CODE CANNOT SAY. The validators' protocol is `0 valid, 1 + findings, 2 refusal`, and an exception that reaches the top of a Python + program exits 1 too -- so a validator that could not import `repo_shape`, + or raised on a line of its own, read as a project with a finding, quoting + the exception. A crash is told by three things together: a non-zero + exit, no line starting `FINDING` or `REFUSED` (every finding and refusal + a validator prints starts with one), and a `TRACEBACK_HEADER` line. The + line quoted is the output's last, which for a traceback is the exception + itself -- what the row quoted before, kept so a reader still sees why. + """ + lines = [line.strip() for line in whole.splitlines() if line.strip()] + if code == 0 or TRACEBACK_HEADER not in lines: + return None + if any(line.startswith(("FINDING", "REFUSED")) for line in lines): + return None + return lines[-1][:200] + + +def skipped(whole: str) -> list[str]: + """What each ` skip ` line a validator printed did not check, as + `` (#297): `validate-pins.py`'s line for a neutral product's digest + it found no source to recompute, which it reports and still exits 0. + Flattened by `ascii_text`, as the machine row's `report` is, so `detail` + says what the reason says.""" + return [ascii_text(line.strip()[len("skip "):]) + for line in whole.splitlines() if line.startswith(" skip ")] + + def run_validator(ctx: Context, script: Path, args: list[str], - pythonpath: Path | None = None) -> tuple: + pythonpath: Path | None = None, *, + reach_network: bool = False) -> tuple: """`(exit code, first line worth quoting, whole output)`. THE CHILD'S `PYTHONPATH` IS EXACTLY WHAT THE ROW'S NEXT PRINTS, AND THE @@ -968,12 +1119,26 @@ def run_validator(ctx: Context, script: Path, args: list[str], `repo_shape` can see a pasted own-copy line pass where the row says FINDING. That is their variable, and the gate has no such variable: the row is about the gate. + + AND NO CHILD REACHES A NETWORK UNLESS ITS CALLER SAYS SO (#297). A + project that pins a neutral product has a `validate-pins.py` that, with + no local checkout of the product to read, asks `gh api` for its tree -- + so the row this command calls offline started a call to GitHub, on the + operator's login wherever there was one. Every child is now started + `offline`: no token, no `gh` configuration, a proxy that goes nowhere. + `gh` fails, the validator prints its `skip` line and exits 0, and + `validator_row` reports that skip. `reach_network=True` is the ONE + exception, and its one caller is `check_machine`, whose preflight asks + `gh` who is logged in: the row the module docstring names as the + network's. """ env = dict(os.environ) if pythonpath is not None: env["PYTHONPATH"] = str(pythonpath) else: env.pop("PYTHONPATH", None) + if not reach_network: + offline(env) # UTF-8 whatever the console is: these validators print this repository's # own prose, and `ascii_text` flattens it for the report afterwards. env["PYTHONIOENCODING"] = "utf-8" @@ -1109,6 +1274,30 @@ def validator_row(ctx: Context, check_id: str, label: str, own: str, "does this project pass its gate" but "would it pass a gate it does not have". The template copy is the fallback for a root that has lost or never had one, and the row names which of the two answered. + + A VALIDATOR THAT CRASHED HAS NOT ANSWERED (#278). Exit 1 is a finding + AND an exception that reached the top, so the row asks `crashed` before + it believes the code. The root's OWN copy crashing is still a FINDING -- + a project whose gate cannot run has a broken gate, which is a fact about + the project (#237 draws the same line from the other side) -- but it + says it crashed, carries `detail.crashed`, and says when the cause is + the `repo_shape.py` it imports (`imports_repo_shape`). The TEMPLATE copy + crashing is this checkout's failure and not the project's: it carries + `detail.environment`, so the verdict is CANNOT ANSWER, exit 3. Either + way the NEXT is the validator's own line, which shows the traceback. + + AND A TEMPLATE THIS CHECKOUT CANNOT GIVE ITS `PYTHONPATH` IS NOT RUN + (#278): `pythonpath_unnamable` says why, the row is an environment row + with no NEXT, and no `PYTHONPATH=` line is printed, because no such line + could be pasted and work. + + A PASS THAT SKIPPED SOMETHING SAYS SO (#297). Exit 0 with ` skip ` + lines is `validate-pins.py` reporting a neutral product's digest it had + nothing to recompute from. That is its documented answer, never a + failure, so the row is a `note`, which moves no verdict -- and its + reason quotes what was not rechecked, because "passes" alone said a + digest was verified that nobody verified. The skip line already names + the three ways to give it a source, so there is no NEXT. """ used, which, own_copy = validator_for(ctx, own, template) if not used.is_file(): @@ -1120,12 +1309,41 @@ def validator_row(ctx: Context, check_id: str, label: str, own: str, f"{quote_arg(ctx.root)} # from a complete checkout of " "the standard; this one is missing that file", {"validator": None}) - code, quoted, _ = run_validator(ctx, used, args, - validator_scripts(ctx.shape, own_copy)) + unnamable = pythonpath_unnamable(ctx.shape, own_copy) + if unnamable is not None: + return Row(check_id, label, FINDING, f"{which} was not run: " + f"{unnamable}", None, + {"validator": used.as_posix(), "own_copy": own_copy, + "exit": None, "environment": True}) + code, quoted, whole = run_validator(ctx, used, args, + validator_scripts(ctx.shape, own_copy)) detail = {"validator": used.as_posix(), "own_copy": own_copy, "exit": code} if code == 0: - return Row(check_id, label, OK, f"{which} passes", None, detail) + skips = skipped(whole) + if not skips: + return Row(check_id, label, OK, f"{which} passes", None, detail) + detail["skipped"] = skips + return Row(check_id, label, NOTE, f"{which} passes, but did not " + f"recheck: {'; '.join(skips)}", None, detail) + crash = crashed(code, whole) + if crash is not None: + detail["crashed"] = True + reason = f"{which} crashed before reaching a verdict (exit {code}): " \ + f"{crash}" + if not imports_repo_shape(ctx.shape, used, own_copy): + lost = repo_shape_for(ctx.shape, used, own_copy) + reason += (f"; {lost.relative_to(ctx.root).as_posix()}, which it " + "imports from beside itself, is not there" + if own_copy else f"; {lost} is not there") + if not own_copy: + detail["environment"] = True + reason += ("; that copy is this checkout's, not the project's, " + "so nothing here is a statement about the project") + command = validator_command(ctx.shape, used, own_copy, args) + return Row(check_id, label, FINDING, reason, + f"{command} # read its output in full, it shows where " + "the validator stopped", detail) verb = "refuses" if code >= 2 else "reports a finding" reason = f"{which} {verb} (exit {code})" + (f": {quoted}" if quoted else "") # A COMMAND THAT RUNS WHEN IT IS PASTED. This row used to print @@ -1191,10 +1409,32 @@ def unloaded_manifest_row(ctx: Context, check_id: str, label: str, own: str, person's decision, and the line says that too. Never `adopt-project.py plan`, which is what this root was offered while the refusal was read as "no manifest". + + BUT THE GATE HAS TO BE ABLE TO RUN FOR THAT TO BE TRUE (#298). This row + runs no gate, so it asks `imports_repo_shape` instead: a root's own copy + with no `repo_shape.py` beside it dies on the import, pasted or not, and + the promise above would send the person to a traceback that says nothing + about the file. Such a root is told to put the file back FIRST, with the + one command that restores the copy HEAD holds -- the project's own + committed bytes, where `update-shape.py apply` would copy a newer + upstream, as `check_agent_files` restores a deleted copy -- and that the + gate comes after it. That is the + order the verdict already gives: `DRIFTED (copy-missing)` outranks + `INVALID`. A deletion already COMMITTED is not in HEAD, so the line + could not restore it: the row then has no NEXT, and says the file comes + back from the parent of the commit that deleted it. + + AND A TEMPLATE LINE THAT CANNOT CARRY ITS `PYTHONPATH` IS NOT PRINTED + (#278). The finding stands, since the manifest does not load either way, + but `pythonpath_unnamable` says why the standard's copy cannot be named + as a command, and the row has no NEXT rather than a line that splits + when pasted. Only the template copy can be refused that way: the root's + own copy needs no `PYTHONPATH`, and `validator_for` names it whenever + the root has one. """ refusal = ctx.manifest_refusal name = ctx.manifest_path.name - used, _which, own_copy = validator_for(ctx, own, template) + used, which, own_copy = validator_for(ctx, own, template) reason = f"{name} does not load: {refusal_line(refusal)}" detail = {"validator": used.as_posix(), "own_copy": own_copy, "manifest": name, @@ -1206,6 +1446,31 @@ def unloaded_manifest_row(ctx: Context, check_id: str, label: str, own: str, ["--root", str(ctx.root)]) return Row(check_id, label, FINDING, f"{reason}; {missing.reason}", missing.next_command, {**missing.detail, **detail}) + if own_copy and not imports_repo_shape(ctx.shape, used, own_copy): + lost = repo_shape_for(ctx.shape, used, own_copy).relative_to( + ctx.root).as_posix() + if not head_holds(ctx.root, lost): + return Row(check_id, label, FINDING, + f"{reason}; {lost} is gone too, and HEAD does not hold " + f"it, so {own}, the gate that owns {name}, cannot " + f"import repo_shape to refuse it: a deletion already " + "committed comes back from the parent of the commit " + f"that deleted it, and then {own} refuses {name} by " + "name until it loads", None, detail) + return Row(check_id, label, FINDING, reason, + f"git -C {quote_arg(ctx.root)} checkout HEAD -- " + f"{quote_arg(lost)} # it puts back the {lost} HEAD " + f"holds, the copy this project committed and {own} " + "imports, where update-shape.py apply would copy a newer " + f"one, then run {own} again: it refuses {name} by name " + "until it loads. " + "A deletion already committed comes back from the parent " + "of the commit that made it, not from HEAD", detail) + unnamable = pythonpath_unnamable(ctx.shape, own_copy) + if unnamable is not None: + return Row(check_id, label, FINDING, + f"{reason}; {which}, the gate that owns it, has no line " + f"here: {unnamable}", None, detail) command = validator_command(ctx.shape, used, own_copy, ["--root", ctx.root]) return Row(check_id, label, FINDING, reason, @@ -3686,7 +3951,10 @@ def check_machine(ctx: Context) -> Row: return Row("machine", "machine", NA, f"{entry} is not here, so the preflight could not be run", None, detail) - code, quoted, whole = run_validator(ctx, entry, ["--preflight"]) + # The ONE child that keeps the operator's `gh` login and proxies: every + # other is started `offline` (#297), and this row exists to ask `gh`. + code, quoted, whole = run_validator(ctx, entry, ["--preflight"], + reach_network=True) detail["exit"] = code # The preflight's own `[ok]`/`[!!]`/`[??]` lines, which are its report. marks = [ascii_text(line.strip()) for line in whole.splitlines() diff --git a/tests/test_doctor_validator_rows.py b/tests/test_doctor_validator_rows.py new file mode 100644 index 00000000..0e5719f0 --- /dev/null +++ b/tests/test_doctor_validator_rows.py @@ -0,0 +1,609 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`shape-doctor.py`'s validator rows: what a validator that did NOT answer +looks like (#278, #297, #298). + +THREE WAYS A ROW USED TO SAY MORE THAN THE VALIDATOR DID. + +- #278: a validator that CRASHED -- a traceback, exit 1 -- read as one that + reported a finding, because 1 is also the code for findings. A standard + checked out under a directory whose name holds `os.pathsep` gave the + template copy a `PYTHONPATH` Python split in two, so the template died on + `import repo_shape` and a valid manifest read `INVALID (manifest)`. +- #297: a project that pins a neutral product, with no local checkout of it, + has a `validate-pins.py` that asks `gh api`, gets nothing, prints a `skip` + line and exits 0. The doctor threw the output away and said `ok ... + passes`, and the child had reached for GitHub on the operator's login. +- #298: the unloaded-manifest row promised its gate "refuses it the same + way" for a root that had also lost `scripts/repo_shape.py`, where the gate + dies on the import instead. + +EVERY ASSERTION IS ON WHAT A COMMAND DOES -- the row's status, its `detail`, +the verdict, and a NEXT that is run -- and never on a NEXT's ` # ` tail, +whose text another change moves. The fixtures are those of +`tests/test_shape_doctor.py`, imported, so the standard, the project and the +holder are the ones every other doctor test reads. + +NOTHING HERE REACHES A NETWORK. The scaffolds use local bare remotes, the +`gh` the #297 rows see is this file's own stub, first on `PATH`, which runs +nothing and logs its environment, and no test here sets `GIT_ALLOW_PROTOCOL`. +""" + +from __future__ import annotations + +import json +import os +import shlex +import shutil +import subprocess +from pathlib import Path + +import pytest + +from conftest import FILE_PROTOCOL, WINDOWS_SKIP, git, run_script +#: The neutral product's stand-in, from the module that scaffolds a declared +#: descendant: a second way of building one would test a different one. +from test_scaffold_pin_and_reuse import product_repo +#: The standard, the project, the holder and the helpers every doctor test +#: reads, imported from the module that owns them. Importing a fixture gives +#: this module its own instance of it. +from test_shape_doctor import ( # noqa: F401 + BREAKAGES, DOCTOR, FAMILY_BREAKAGES, FAMILY_NAME, TEMPLATE_FALLBACK, + assert_next_runs_the_roots_own, break_manifest, doctor, doctor_module, + holder, loader_line, pasted_from_elsewhere, project, red_root, rows_of, + scaffolded_here, standard, +) + +#: The four variables `gh` takes a token from, the proxy every validator +#: child is pointed at, and the variables that point it -- spelled here, from +#: the issue, rather than read back from the code under test. +TOKENS = ("GH_TOKEN", "GITHUB_TOKEN", "GH_ENTERPRISE_TOKEN", + "GITHUB_ENTERPRISE_TOKEN") +UNREACHABLE = "http://127.0.0.1:9" +PROXIES = ("HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy") +NO_PROXIES = ("NO_PROXY", "no_proxy") +#: What the operator's environment holds in these tests: values that are +#: plainly not real, so a run that leaked one leaks nothing. +SENTINEL_TOKEN = "sentinel-token-not-a-real-one" +SENTINEL_HOST = "sentinel.invalid" + +PRODUCT = "openGlass" +CRASH = 'raise RuntimeError("boom")\n' + + +def payload_of(result) -> dict: + return json.loads(result.stdout) + + +def own_root(case: str, project: Path, holder: dict, tmp_path: Path) -> Path: + """The root `case` is a row of: the project, or a COPY of the holder, so + the session's stays intact. Resolved, as the doctor resolves `--root`.""" + if case != "family": + return project.resolve() + root = tmp_path / FAMILY_NAME + shutil.copytree(holder["root"], root, symlinks=True) + return root.resolve() + + +def standard_under(standard: Path, tmp_path: Path, directory: str) -> Path: + """A COPY of the whole standard, history and all, at + `//openRepoShape`: the same commit, a different path.""" + here = tmp_path / directory / "openRepoShape" + here.parent.mkdir() + shutil.copytree(standard, here, symlinks=True) + return here + + +def command_of(row: dict) -> list: + """The row's NEXT as the argv it runs, its ` # ` tail cut off. The + paths in these tests need no quoting on either platform, which is what + lets the same list run on Windows, where no `sh` pastes it.""" + return row["next"].split(" # ", 1)[0].split() + + +def run_argv(command: list) -> subprocess.CompletedProcess: + return subprocess.run(command, capture_output=True, text=True, + check=False, stdin=subprocess.DEVNULL, + env={**os.environ, "PYTHONDONTWRITEBYTECODE": "1"}) + + +# --- #278: a crash is not a finding ----------------------------------------- + +def crashing(setup: str, standard: Path, project: Path, holder: dict, + tmp_path: Path) -> tuple: + """`(standard to run, root, row id, own_copy)` for one way a validator + crashes: a root's OWN copy that raises (`own-manifest`, `own-pins`, + `own-family`), or the standard's TEMPLATE copy that raises, for a root + with none of its own (`template`).""" + if setup == "template": + here = standard_under(standard, tmp_path, "crashing") + (here / TEMPLATE_FALLBACK["manifest"]["template"]).write_text( + CRASH, encoding="utf-8") + root = project.resolve() + (root / TEMPLATE_FALLBACK["manifest"]["own"]).unlink() + return here, root, "manifest", False + case = setup.removeprefix("own-") + root = own_root(case, project, holder, tmp_path) + (root / TEMPLATE_FALLBACK[case]["own"]).write_text(CRASH, encoding="utf-8") + return standard, root, case, True + + +@pytest.mark.parametrize("setup", ["own-family", "own-manifest", "own-pins", + "template"]) +def test_a_validator_that_crashed_says_so_and_never_reports_a_finding( + standard, project, holder, tmp_path, setup): + """(b) of the issue: the row says it CRASHED, carries `detail.crashed`, + and still quotes the exception. + + THE ROOT'S OWN COPY IS STILL A FINDING: a project whose gate cannot run + has a broken gate, and the verdict is the one an edited copy gives + (DRIFTED), never CANNOT ANSWER. THE TEMPLATE COPY IS THIS CHECKOUT'S, so + its crash is `detail.environment` and the verdict is CANNOT ANSWER, exit + 3: nothing in it is a statement about the project. + """ + here, root, row_id, own_copy = crashing(setup, standard, project, holder, + tmp_path) + result = doctor(here, root, "--json") + payload = payload_of(result) + row = rows_of(result)[row_id] + assert row["status"] == "FINDING", row + assert row["detail"]["crashed"] is True, row + assert row["detail"]["own_copy"] is own_copy, row + assert row["detail"]["exit"] == 1, row + assert "crashed" in row["reason"], row + assert "reports a finding" not in row["reason"], row + assert "RuntimeError: boom" in row["reason"], row + if own_copy: + assert "environment" not in row["detail"], row + assert result.returncode == 1, result.stdout + result.stderr + assert not payload["verdict"].startswith("CANNOT ANSWER"), payload + assert_next_runs_the_roots_own( + row, root, Path(TEMPLATE_FALLBACK[row_id]["own"]).name) + else: + assert row["detail"]["environment"] is True, row + assert result.returncode == 3, result.stdout + result.stderr + assert payload["verdict"].startswith( + f"CANNOT ANSWER ({row['label']}: "), payload + shape = here.resolve() + expected = doctor_module(here).validator_command( + shape, shape / TEMPLATE_FALLBACK["manifest"]["template"], False, + ["--root", root]) + assert row["next"].startswith(expected + " # "), row["next"] + + +@WINDOWS_SKIP +@pytest.mark.skipif(shutil.which("sh") is None, reason="needs sh") +@pytest.mark.parametrize("setup", ["own-pins", "template"]) +def test_a_crashed_validators_next_shows_the_traceback_when_pasted( + standard, project, holder, tmp_path, setup): + """The NEXT of a crashed row is still the validator's own line, and + pasted from `/` with no `PYTHONPATH` it prints the traceback the row + quoted, with the row's exit code.""" + here, root, row_id, _own = crashing(setup, standard, project, holder, + tmp_path) + row = rows_of(doctor(here, root, "--json"))[row_id] + pasted = pasted_from_elsewhere(row["next"], tmp_path) + whole = pasted.stdout + pasted.stderr + assert pasted.returncode == row["detail"]["exit"], whole + assert "Traceback (most recent call last):" in whole, whole + assert "RuntimeError: boom" in whole, whole + + +@WINDOWS_SKIP +def test_a_standard_under_a_colon_is_cannot_answer_and_runs_nothing( + standard, project, tmp_path): + """(a) of the issue, and (i) of the ruling: the template copy cannot be + given its `PYTHONPATH`, so it is not run and no line is printed. + + The root has no `validate-manifest.py` of its own and a VALID manifest, + so the template is the only gate for it, and `:` splits the directory + that gate imports `repo_shape` from. The row is the doctor's own + environment failing -- CANNOT ANSWER, exit 3 -- with no `exit` (nothing + was run) and no NEXT, and not one `PYTHONPATH=` anywhere in either + report, because no such line could be pasted and work. `WINDOWS_SKIP`: + Windows refuses `:` in a file name; its separator is `;`, below. + """ + here = standard_under(standard, tmp_path, "dir:with-colon") + root = project.resolve() + (root / TEMPLATE_FALLBACK["manifest"]["own"]).unlink() + result = doctor(here, root, "--json") + payload = payload_of(result) + row = rows_of(result)["manifest"] + assert result.returncode == 3, result.stdout + result.stderr + assert payload["verdict"].startswith("CANNOT ANSWER (manifest: "), payload + assert row["status"] == "FINDING", row + assert row["detail"]["environment"] is True, row + assert row["detail"]["own_copy"] is False, row + assert row["detail"]["exit"] is None, row + assert row["next"] is None, row + assert "holds no ':'" in row["reason"], row + assert "ModuleNotFoundError" not in row["reason"], row + text = doctor(here, root) + assert text.returncode == 3, text.stdout + text.stderr + for said in (result.stdout, text.stdout, text.stderr): + assert "PYTHONPATH=" not in said, said + + +@WINDOWS_SKIP +def test_a_colon_standard_leaves_an_unloaded_manifest_its_finding_and_no_line( + standard, project, tmp_path): + """(ii) of the ruling: the manifest does not load, so the FINDING stands + -- exit 1, never CANNOT ANSWER -- but the template's line would carry a + `PYTHONPATH` that splits, so the row has no NEXT, quotes the loader as it + always did, and says why there is no line.""" + here = standard_under(standard, tmp_path, "dir:with-colon") + root = project.resolve() + (root / TEMPLATE_FALLBACK["manifest"]["own"]).unlink() + refusal = break_manifest(root / "project.yaml", *BREAKAGES["tab-indent"]) + result = doctor(here, root, "--json") + payload = payload_of(result) + row = rows_of(result)["manifest"] + assert result.returncode == 1, result.stdout + result.stderr + assert not payload["verdict"].startswith("CANNOT ANSWER"), payload + assert row["status"] == "FINDING", row + assert "environment" not in row["detail"], row + assert row["detail"]["own_copy"] is False, row + assert row["next"] is None, row + assert row["reason"].startswith( + f"project.yaml does not load: {loader_line(refusal)}; "), row + assert "holds no ':'" in row["reason"], row + text = doctor(here, root) + for said in (result.stdout, text.stdout, text.stderr): + assert "PYTHONPATH=" not in said, said + + +def test_a_standard_under_a_semicolon_is_refused_only_where_it_separates( + standard, project, tmp_path): + """R278-3: Windows' `;` is the same `os.pathsep` and is claimed by the + same helper, and on POSIX the same directory is the CONTROL -- an + ordinary character, quoted in the line, and nothing refused. + + The root's own `validate-manifest.py` is gone and its manifest is red by + a file that still loads, so the template is the gate. On Windows it is + the environment row with no NEXT; on POSIX it is the template's real + finding, and its NEXT, pasted, prints that finding. + """ + here = standard_under(standard, tmp_path, "semi;colon") + spec = TEMPLATE_FALLBACK["manifest"] + root = red_root("manifest", project, {}, tmp_path) + result = doctor(here, root, "--json") + row = rows_of(result)["manifest"] + assert row["status"] == "FINDING", row + assert row["detail"]["own_copy"] is False, row + if os.pathsep == ";": + assert row["detail"]["environment"] is True, row + assert row["next"] is None, row + assert result.returncode == 3, result.stdout + result.stderr + return + assert "environment" not in row["detail"], row + assert row["detail"]["exit"] == 1, row + assert spec["finding"] in row["reason"], row + assert row["next"].startswith("PYTHONPATH='"), row["next"] + if shutil.which("sh") is not None: + pasted = pasted_from_elsewhere(row["next"], tmp_path) + whole = pasted.stdout + pasted.stderr + assert pasted.returncode == 1, whole + assert f"FINDING {spec['finding']}" in whole, whole + + +# --- #297: no validator child reaches a network, and a skip is said --------- + +def logging_gh(where: Path) -> tuple: + """`(directory to put first on PATH, directory of calls)`: a `gh` that + runs NOTHING. Each call writes its argv and its whole environment to a + new file and exits 97, so a test can say which process called it and + what that process could have authenticated with. A shebang script, so + its tests are `WINDOWS_SKIP`.""" + bin_dir, calls = where / "bin", where / "calls" + bin_dir.mkdir(parents=True) + calls.mkdir() + stub = bin_dir / "gh" + stub.write_text( + "#!/bin/sh\n" + f"out=$(mktemp {shlex.quote(str(calls))}/call.XXXXXX)\n" + "printf '%s\\n' \"$*\" > \"$out\"\n" + "env >> \"$out\"\n" + "exit 97\n", encoding="utf-8") + stub.chmod(0o755) + return bin_dir, calls + + +def gh_calls(calls: Path) -> list: + """`[(argv as one string, {variable: value})]`, one per call.""" + found = [] + for path in sorted(calls.iterdir()): + lines = path.read_text(encoding="utf-8", + errors="replace").splitlines() + environment: dict = {} + for line in lines[1:]: + name, equals, value = line.partition("=") + if equals and name.isidentifier(): + environment.setdefault(name, value) + found.append((lines[0] if lines else "", environment)) + return found + + +@pytest.fixture(scope="module") +def descendant_run(standard, tmp_path_factory) -> dict: + """MedxGlass, pinning `openGlass` with NO source the validator can find, + and one doctor run of it with the stub first on `PATH`. + + Scaffolded from the session's standard with `--pin-source`, as + `tests/test_scaffold_pin_and_reuse.py` does, then cloned AWAY from the + stand-in, so neither `../openGlass` nor `--pin-source` answers and + `SHAPE_PIN_SOURCE_OPENGLASS` is blanked by `run_script`. The scaffold's + own `gh` calls go to a stub too, never a real one. The doctor's + environment carries a sentinel for every token, a `GH_CONFIG_DIR` and a + `NO_PROXY`, so what reached each `gh` call can be read back. + """ + base = tmp_path_factory.mktemp("descendant") + product, commit = product_repo(base / "standin", PRODUCT) + quiet, _ignored = logging_gh(base / "scaffold-gh") + made = run_script( + standard / "scaffold-project.py", "--org", "MedxSoft", "--project", + "MedxGlass", "--elected-by", "Test Human", "--elected-on", + "2026-09-10", "--pin", f"{PRODUCT}@{commit}", "--pin-source", + str(product), "--local-remote-dir", str(base / "remotes"), + "--work-dir", str(base / "work"), + env={"PATH": f"{quiet}{os.pathsep}{os.environ['PATH']}"}) + assert made.returncode == 0, made.stderr + made.stdout + clone = base / "clones" / "MedxGlass" + cloned = subprocess.run( + ["git", *FILE_PROTOCOL, "clone", "-q", "--recurse-submodules", + str(base / "remotes" / "MedxGlass.git"), str(clone)], + capture_output=True, text=True, check=False) + assert cloned.returncode == 0, cloned.stderr + assert not (clone.parent / PRODUCT).exists() + bin_dir, calls = logging_gh(base / "doctor-gh") + operators_config = str(base / "operators-gh-config") + result = run_script( + standard / DOCTOR, "--root", str(clone), "--json", + env={"PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + **{name: SENTINEL_TOKEN for name in TOKENS}, + "GH_CONFIG_DIR": operators_config, + **{name: SENTINEL_HOST for name in NO_PROXIES}}) + return {"result": result, "calls": gh_calls(calls), + "operators_config": operators_config} + + +@WINDOWS_SKIP +def test_a_skipped_digest_is_a_note_naming_the_product_and_still_compliant( + descendant_run): + """R297-2: exit 0 with a `skip` line is a `note`, not `ok ... passes`. + + The note moves no verdict, so the descendant is COMPLIANT, exit 0, as it + was; the row says which product's digest was not rechecked, keeps the + skip in `detail.skipped`, and has no NEXT -- the skip line names the + three ways to give the validator a source. + """ + result = descendant_run["result"] + payload = payload_of(result) + assert result.returncode == 0, result.stdout + result.stderr + assert payload["verdict"] == "COMPLIANT", payload + row = rows_of(result)["pins"] + assert row["status"] == "note", row + assert row["detail"]["exit"] == 0, row + assert row["next"] is None, row + assert PRODUCT in row["reason"], row + assert "did not recheck" in row["reason"], row + assert row["detail"]["skipped"], row + for skip in row["detail"]["skipped"]: + assert skip.startswith(f"{PRODUCT}: "), row + + +@WINDOWS_SKIP +def test_only_the_machine_rows_gh_keeps_the_login_and_the_proxies( + descendant_run): + """R297-1, both walls: the validator child's `gh` had no token, no + configuration and a proxy that goes nowhere; the machine row's probes + had the operator's environment exactly. + + THE CHILD IS FOUND BY WHAT IT ASKED: `api repos/.../git/trees/...` is + `validate-pins.py` recomputing the digest. The machine row's preflight + asks `--version`, `auth status` and `api user`. EVERY call that is not + one of the machine's is held to the offline environment, whichever + process made it, because the machine row is the one named exception. + """ + calls = descendant_run["calls"] + machine = [(argv, env) for argv, env in calls + if argv.startswith(("--version", "auth ", "api user"))] + others = [(argv, env) for argv, env in calls + if (argv, env) not in machine] + assert [argv for argv, _env in others + if "/git/trees/" in argv], calls + assert machine, calls + for argv, env in others: + for name in TOKENS + NO_PROXIES: + assert name not in env, (argv, name, env.get(name)) + assert env.get("GH_CONFIG_DIR") == os.devnull, (argv, env) + assert env.get("GH_NO_UPDATE_NOTIFIER") == "1", (argv, env) + for name in PROXIES: + assert env.get(name) == UNREACHABLE, (argv, name, env.get(name)) + for argv, env in machine: + for name in TOKENS: + assert env.get(name) == SENTINEL_TOKEN, (argv, name) + assert env.get("GH_CONFIG_DIR") == descendant_run["operators_config"] + for name in NO_PROXIES: + assert env.get(name) == SENTINEL_HOST, (argv, name) + for name in PROXIES: + assert env.get(name) != UNREACHABLE, (argv, name) + + +#: The variables the env probe below prints, in one list. +ASKED = TOKENS + ("GH_CONFIG_DIR", "GH_NO_UPDATE_NOTIFIER") + PROXIES + \ + NO_PROXIES + + +@pytest.mark.parametrize("reach_network", [False, True]) +def test_run_validator_starts_a_child_offline_unless_its_caller_says( + standard, tmp_path, monkeypatch, reach_network): + """The contract of `run_validator`'s `env` block, asked of the child + itself, as `test_run_validator_gives_the_child_exactly_the_pythonpath_it + _is_given` asks it of `PYTHONPATH`: by default no token, `GH_CONFIG_DIR` + the null device, the update notifier off, every proxy variable the + unreachable one and no exception list; with `reach_network=True` -- the + machine row's keyword -- the operator's values, untouched. + + ON EVERY PLATFORM, because the probe is a Python file and not a shebang: + the Windows half of R297-1 is held here. The operator's environment is + set through the upper-case names only, which on Windows are the same + variables as the lower-case ones. + """ + module = doctor_module(standard) + for name in TOKENS: + monkeypatch.setenv(name, SENTINEL_TOKEN) + for name in ("https_proxy", "http_proxy", "no_proxy"): + monkeypatch.delenv(name, raising=False) + monkeypatch.setenv("GH_CONFIG_DIR", str(tmp_path / "operators-gh-config")) + monkeypatch.setenv("HTTPS_PROXY", "http://operators-proxy.invalid:3128") + monkeypatch.setenv("NO_PROXY", SENTINEL_HOST) + root = tmp_path / "root" + root.mkdir() + probe = root / "probe.py" + probe.write_text( + "import json, os\n" + f"print(json.dumps({{n: os.environ.get(n) for n in {list(ASKED)!r}}}))" + "\n", encoding="utf-8") + ctx = module.Context(root) + if reach_network: + code, _quoted, whole = module.run_validator(ctx, probe, [], + reach_network=True) + else: + code, _quoted, whole = module.run_validator(ctx, probe, []) + assert code == 0, whole + seen = json.loads(whole) + if reach_network: + assert {name: seen[name] for name in TOKENS} == \ + {name: SENTINEL_TOKEN for name in TOKENS}, seen + assert seen["GH_CONFIG_DIR"] == str(tmp_path / "operators-gh-config") + assert seen["HTTPS_PROXY"] == "http://operators-proxy.invalid:3128" + assert seen["NO_PROXY"] == SENTINEL_HOST, seen + assert UNREACHABLE not in seen.values(), seen + return + assert {name: seen[name] for name in TOKENS + NO_PROXIES} == \ + dict.fromkeys(TOKENS + NO_PROXIES), seen + assert seen["GH_CONFIG_DIR"] == os.devnull, seen + assert seen["GH_NO_UPDATE_NOTIFIER"] == "1", seen + assert {name: seen[name] for name in PROXIES} == \ + dict.fromkeys(PROXIES, UNREACHABLE), seen + + +# --- #298: a gate that cannot import repo_shape is not promised ------------- + +#: Per kind of root: the manifest, the gate that owns it, and the row. +UNLOADED_ROOT = { + "project": ("project.yaml", "scripts/validate-manifest.py", "manifest", + BREAKAGES), + "family": ("family.yaml", "scripts/validate-family.py", "family", + FAMILY_BREAKAGES), +} +LOST = "scripts/repo_shape.py" + + +def unloaded(kind: str, project: Path, holder: dict, tmp_path: Path, + lose: str | None) -> tuple: + """`(root, row id, refusal)`: a root of `kind` whose manifest has a TAB, + so it does not load, and whose `scripts/repo_shape.py` is kept + (`lose=None`, the control), deleted in the tree (`"tree"`), or deleted by + a COMMIT (`"commit"`).""" + name, own, row_id, breakages = UNLOADED_ROOT[kind] + root = own_root("family" if kind == "family" else "manifest", project, + holder, tmp_path) + assert (root / own).is_file(), own + refusal = break_manifest(root / name, *breakages["tab-indent"]) + gone = root / LOST + assert gone.is_file(), gone + if lose == "tree": + gone.unlink() + elif lose == "commit": + git("rm", "-q", "--", LOST, cwd=root) + git("commit", "-q", "-m", "Drop repo_shape.py", "--", LOST, cwd=root) + return root, row_id, refusal + + +def assert_the_gate_refuses_the_file(root: Path, row: dict, kind: str, + refusal) -> None: + """The row's NEXT is the root's own gate, and run it refuses the file + with the loader's code and exit 2: the gate CAN say what the row said.""" + name, own, _row_id, _breakages = UNLOADED_ROOT[kind] + assert_next_runs_the_roots_own(row, root, Path(own).name) + ran = run_argv(command_of(row)) + said = ran.stdout + ran.stderr + assert ran.returncode == 2, said + assert f"REFUSED {refusal.code}: " in said, said + assert refusal.detail[len(str(root / name)):] in said, said + + +@pytest.mark.parametrize("kind", sorted(UNLOADED_ROOT)) +def test_a_root_that_lost_repo_shape_is_told_to_restore_it_first( + standard, project, holder, tmp_path, kind): + """R298-2 and R298-3: the reason is the loader's, unchanged, and the + NEXT puts the file back -- run, it exits 0 and the file is there -- after + which the doctor names the gate, and the gate refuses the manifest by its + code, exit 2. The order the verdict already gives: DRIFTED (copy-missing) + first, the gate after. + """ + root, row_id, refusal = unloaded(kind, project, holder, tmp_path, "tree") + name = UNLOADED_ROOT[kind][0] + result = doctor(standard, root, "--json") + row = rows_of(result)[row_id] + assert row["status"] == "FINDING", row + assert row["detail"]["own_copy"] is True, row + assert row["reason"] == f"{name} does not load: {loader_line(refusal)}" + assert payload_of(result)["verdict"].startswith("DRIFTED"), result.stdout + assert command_of(row)[:2] == ["git", "-C"], row["next"] + assert Path(command_of(row)[2]).resolve() == root, row["next"] + assert command_of(row)[3:] == ["checkout", "HEAD", "--", LOST], row["next"] + restored = run_argv(command_of(row)) + assert restored.returncode == 0, restored.stdout + restored.stderr + assert (root / LOST).is_file() + after = rows_of(doctor(standard, root, "--json"))[row_id] + assert_the_gate_refuses_the_file(root, after, kind, refusal) + + +@WINDOWS_SKIP +@pytest.mark.skipif(shutil.which("sh") is None, reason="needs sh") +@pytest.mark.parametrize("kind", sorted(UNLOADED_ROOT)) +def test_the_restore_line_pasted_lets_the_gate_print_its_refusal( + standard, project, holder, tmp_path, kind): + """The same two lines PASTED, whole, from `/` with no `PYTHONPATH`: the + restore, then the gate the doctor names next, which prints `REFUSED + yaml-unparsable` and exits 2 -- where the old promise, pasted, printed + `ModuleNotFoundError` and exited 1.""" + root, row_id, refusal = unloaded(kind, project, holder, tmp_path, "tree") + row = rows_of(doctor(standard, root, "--json"))[row_id] + restore = pasted_from_elsewhere(row["next"], tmp_path) + assert restore.returncode == 0, restore.stdout + restore.stderr + after = rows_of(doctor(standard, root, "--json"))[row_id] + gate = pasted_from_elsewhere(after["next"], tmp_path) + whole = gate.stdout + gate.stderr + assert gate.returncode == 2, whole + assert f"REFUSED {refusal.code}: " in whole, whole + assert "ModuleNotFoundError" not in whole, whole + + +@pytest.mark.parametrize("kind", sorted(UNLOADED_ROOT)) +def test_a_root_that_keeps_repo_shape_is_still_sent_to_its_gate( + standard, project, holder, tmp_path, kind): + """The control: the same manifest, `scripts/repo_shape.py` kept, and the + NEXT is the gate, which refuses the file the same way, exit 2.""" + root, row_id, refusal = unloaded(kind, project, holder, tmp_path, None) + row = rows_of(doctor(standard, root, "--json"))[row_id] + assert row["status"] == "FINDING", row + assert "checkout" not in command_of(row), row["next"] + assert_the_gate_refuses_the_file(root, row, kind, refusal) + + +def test_a_committed_deletion_of_repo_shape_gets_no_head_line( + standard, project, holder, tmp_path): + """HEAD does not hold a file whose deletion was COMMITTED, so `checkout + HEAD` could not restore it: the row has no NEXT, keeps the loader's + words first, and says the file comes back from the parent of the commit + that deleted it.""" + root, row_id, refusal = unloaded("project", project, holder, tmp_path, + "commit") + row = rows_of(doctor(standard, root, "--json"))[row_id] + assert row["status"] == "FINDING", row + assert row["next"] is None, row + assert row["reason"].startswith( + f"project.yaml does not load: {loader_line(refusal)}; "), row + assert "parent of the commit" in row["reason"], row From 8ad856ffe55f170fd4a3cb6c72c4c19e96b4dc2a Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:48:58 +0000 Subject: [PATCH 02/10] shape-doctor.py: the CANNOT ANSWER block prints `Remediation:` only for a row that has a NEXT, a skip keeps every line of `gh`'s stderr and the `--pin-source` advice after it, and a validator that printed a FINDING before it raised, and a crash for want of `scripts/repo_shape.py`, are held by tests: the fix round on the review of 2bf402d (#278, #297, #298) The review of 2bf402d was MERGEABLE WITH NITS, with three SHOULDs and the nits below; this is the one commit it asked for, on top of the head. S1 (#278). The separator environment row is a FINDING with no NEXT, by ruling, and `report` printed `Remediation: {next_command}` for it unconditionally, so the stderr block ended in a literal `Remediation: None`. The line is now printed only when the row has a NEXT; the reason already says what to do instead. The colon row asserts no `Remediation` at all, and the template-crash row, which has a NEXT, asserts the line is still there. S2 (#297). `skipped()` kept only the ` skip ` line, and `validate-pins.py`'s skip embeds `gh`'s stderr verbatim, which is two lines when `gh` is not logged in: the reason stopped at `gh auth login`, told the reader to do what the doctor's offline environment undoes, and lost the `--pin-source`, `SHAPE_PIN_SOURCE_` and `../` ways to give a source that are why the row has no NEXT. It now keeps the whole block, the ` skip ` line and every line after it up to the next item (`REPORT_ITEM`: ` ok`, ` skip`, ` --`, `FINDING`, `REFUSED`, `WARNING` or the validator's summary line) or the end, joined by a space. Tested with a logging `gh` stub that prints the two-line help, and on a constructed output on every platform. S3 (#278). The clause of `crashed()` that says a validator which printed a `FINDING` or `REFUSED` line reached a verdict had no test: deleting it left every row green. A validator that prints a finding and then raises is now asserted to be a FINDING that is neither `crashed` nor an environment row, for the root's own copy and the template; with the clause gone the own copy would say it crashed and the template would be CANNOT ANSWER, hiding the finding it printed. N3 (#298). The crash reason's "`scripts/repo_shape.py`, which it imports from beside itself, is not there" had no test either. Tested for a project and a holder whose gate dies on the import. N5. The module docstring's "offline by their own contract", which #297 quoted as false, says the doctor runs the validators offline, which `run_validator` now makes true. Mutants, each against the new file, each turning the rows that hold it red: `crashed()`'s no-FINDING/REFUSED clause deleted (the 2 finding-then-crash rows), the "is not there" fact not appended (the 2 missing-`repo_shape.py` rows), `Remediation:` printed unconditionally again (the colon row), never printed (the template-crash row), only the first line of a skip kept and the summary folded into it (the two skip rows, each). 26 tests in the file; the unmutated file passes them all. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 --- shape-doctor.py | 51 +++++++-- tests/test_doctor_validator_rows.py | 163 ++++++++++++++++++++++++++-- 2 files changed, 195 insertions(+), 19 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 34db51ed..1952b8d2 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -24,8 +24,10 @@ project's copies are compared against is THE CHECKOUT THIS FILE IS RUN FROM, which is the whole of why the verdict is offline: no clone, no authenticated call, and the same answer on a machine that has never spoken to github.com. -It runs the project's own validators, which are offline by their own -contract. ONE ROW IS THE EXCEPTION AND IT IS NAMED: `machine` asks `gh` who +It runs the project's own validators OFFLINE: `run_validator` starts each +child with no token, no `gh` configuration and a proxy that goes nowhere +(#297), so they are offline whatever their own contract says. ONE ROW IS THE +EXCEPTION AND IT IS NAMED: `machine` asks `gh` who it is logged in as (`setup-project.py --preflight` runs `gh --version`, `gh auth status` and `gh api user`), which `gh` answers by talking to its host. That row is `n/a` by status and cannot move the verdict -- a workstation missing `gh` @@ -1052,14 +1054,39 @@ def crashed(code: int, whole: str) -> str | None: return lines[-1][:200] +#: The first line of an ITEM in a validator's report, as against a line that +#: CONTINUES the item above it: an ` ok`, ` skip` or ` --` line, a +#: `FINDING`, `REFUSED` or `WARNING`, and the success summary a validator ends +#: on (`pins ok`, `manifest ok: ...`, `family ok: ...`). +REPORT_ITEM = re.compile( + r" (?:ok|skip|--)\s|(?:FINDING|REFUSED|WARNING)\b|[a-z]+ ok\b") + + def skipped(whole: str) -> list[str]: - """What each ` skip ` line a validator printed did not check, as + """What each ` skip ` block a validator printed did not check, as `` (#297): `validate-pins.py`'s line for a neutral product's digest it found no source to recompute, which it reports and still exits 0. - Flattened by `ascii_text`, as the machine row's `report` is, so `detail` - says what the reason says.""" - return [ascii_text(line.strip()[len("skip "):]) - for line in whole.splitlines() if line.startswith(" skip ")] + + THE WHOLE BLOCK, NOT ITS FIRST LINE: the skip embeds `gh`'s own stderr + verbatim, and `gh` prints its not-logged-in help over two lines (four + under `GITHUB_ACTIONS`), so the text `validate-pins.py` meant to say runs + on past the first newline -- to the `--pin-source`, + `SHAPE_PIN_SOURCE_` and `../` ways to give it a source, + which are why the row has no NEXT. Every line after the ` skip ` line is + kept, up to the next `REPORT_ITEM` or the end, and the lines are joined + with a space. Flattened by `ascii_text`, as the machine row's `report` + is, so `detail` says what the reason says.""" + skips: list[list[str]] = [] + inside = False + for line in whole.splitlines(): + if line.startswith(" skip "): + skips.append([line.strip()[len("skip "):]]) + inside = True + elif REPORT_ITEM.match(line): + inside = False + elif inside and line.strip(): + skips[-1].append(line.strip()) + return [ascii_text(" ".join(lines)) for lines in skips] def run_validator(ctx: Context, script: Path, args: list[str], @@ -4555,8 +4582,14 @@ def report(ctx: Context, rows: list[Row], verdict: str, code: int, print(note) blocked = cannot_answer(rows) if blocked is not None: - print(f"\nREFUSED shape-doctor-cannot-answer: {blocked.reason}\n" - f" Remediation: {blocked.next_command}", file=sys.stderr) + print(f"\nREFUSED shape-doctor-cannot-answer: {blocked.reason}", + file=sys.stderr) + # A ROW WITH NO NEXT HAS NO REMEDIATION TO PRINT (#278). The separator + # environment row is a FINDING with a NEXT of None by ruling -- no + # command could be pasted and work -- and its reason already says what + # to do instead; `Remediation: None` was a literal `None` after it. + if blocked.next_command: + print(f" Remediation: {blocked.next_command}", file=sys.stderr) #: The line NOT A SHAPE ROOT adds, and the one thing in this file that is diff --git a/tests/test_doctor_validator_rows.py b/tests/test_doctor_validator_rows.py index 0e5719f0..6a8bbb9d 100644 --- a/tests/test_doctor_validator_rows.py +++ b/tests/test_doctor_validator_rows.py @@ -68,6 +68,12 @@ PRODUCT = "openGlass" CRASH = 'raise RuntimeError("boom")\n' +#: A validator that REPORTS a finding, as the real ones do -- a `FINDING` line +#: on stderr -- and then dies on a line of its own (#278, R278-1). +FINDING_THEN_CRASH = ("import sys\n" + "print('FINDING pin-x: a made-up finding', " + "file=sys.stderr)\n" + 'raise RuntimeError("boom")\n') def payload_of(result) -> dict: @@ -109,21 +115,23 @@ def run_argv(command: list) -> subprocess.CompletedProcess: # --- #278: a crash is not a finding ----------------------------------------- def crashing(setup: str, standard: Path, project: Path, holder: dict, - tmp_path: Path) -> tuple: + tmp_path: Path, source: str = CRASH) -> tuple: """`(standard to run, root, row id, own_copy)` for one way a validator crashes: a root's OWN copy that raises (`own-manifest`, `own-pins`, `own-family`), or the standard's TEMPLATE copy that raises, for a root - with none of its own (`template`).""" + with none of its own (`template`). `source` is what the copy is made to + run.""" if setup == "template": here = standard_under(standard, tmp_path, "crashing") (here / TEMPLATE_FALLBACK["manifest"]["template"]).write_text( - CRASH, encoding="utf-8") + source, encoding="utf-8") root = project.resolve() (root / TEMPLATE_FALLBACK["manifest"]["own"]).unlink() return here, root, "manifest", False case = setup.removeprefix("own-") root = own_root(case, project, holder, tmp_path) - (root / TEMPLATE_FALLBACK[case]["own"]).write_text(CRASH, encoding="utf-8") + (root / TEMPLATE_FALLBACK[case]["own"]).write_text(source, + encoding="utf-8") return standard, root, case, True @@ -168,6 +176,11 @@ def test_a_validator_that_crashed_says_so_and_never_reports_a_finding( shape, shape / TEMPLATE_FALLBACK["manifest"]["template"], False, ["--root", root]) assert row["next"].startswith(expected + " # "), row["next"] + # A ROW WITH A NEXT STILL PRINTS ITS REMEDIATION (the control of the + # colon row below, which has none and prints no `Remediation:`). + stderr = doctor(here, root).stderr + assert "REFUSED shape-doctor-cannot-answer: " in stderr, stderr + assert f"\n Remediation: {row['next']}" in stderr, stderr @WINDOWS_SKIP @@ -188,6 +201,37 @@ def test_a_crashed_validators_next_shows_the_traceback_when_pasted( assert "RuntimeError: boom" in whole, whole +@pytest.mark.parametrize("setup", ["own-pins", "template"]) +def test_a_validator_that_reported_a_finding_before_it_died_still_reports_one( + standard, project, holder, tmp_path, setup): + """R278-1's other half: a `FINDING` line is a verdict, and an exception + after it does not take it back. + + `crashed` is true only of a validator that said nothing of its own before + the traceback. Without that clause a template that reported a real finding + and then raised would be CANNOT ANSWER -- "nothing here is a statement + about the project" -- and the finding it printed would be hidden. Both + copies are run: the root's own, whose row would say `crashed` and carry + `detail.crashed`, and the template, whose row would be an environment row + and exit 3. The row quotes the finding, as it always did. + """ + here, root, row_id, own_copy = crashing(setup, standard, project, holder, + tmp_path, FINDING_THEN_CRASH) + result = doctor(here, root, "--json") + payload = payload_of(result) + row = rows_of(result)[row_id] + assert row["status"] == "FINDING", row + assert row["detail"]["own_copy"] is own_copy, row + assert row["detail"]["exit"] == 1, row + assert "crashed" not in row["detail"], row + assert "environment" not in row["detail"], row + assert "reports a finding" in row["reason"], row + assert "crashed" not in row["reason"], row + assert "FINDING pin-x: a made-up finding" in row["reason"], row + assert result.returncode == 1, result.stdout + result.stderr + assert not payload["verdict"].startswith("CANNOT ANSWER"), payload + + @WINDOWS_SKIP def test_a_standard_under_a_colon_is_cannot_answer_and_runs_nothing( standard, project, tmp_path): @@ -221,6 +265,10 @@ def test_a_standard_under_a_colon_is_cannot_answer_and_runs_nothing( assert text.returncode == 3, text.stdout + text.stderr for said in (result.stdout, text.stdout, text.stderr): assert "PYTHONPATH=" not in said, said + # THE REFUSAL ON STDERR HAS NO REMEDIATION TO NAME: a row with no NEXT + # printed a literal `Remediation: None` after its reason. + assert "REFUSED shape-doctor-cannot-answer: " in text.stderr, text.stderr + assert "Remediation" not in text.stderr, text.stderr @WINDOWS_SKIP @@ -287,12 +335,14 @@ def test_a_standard_under_a_semicolon_is_refused_only_where_it_separates( # --- #297: no validator child reaches a network, and a skip is said --------- -def logging_gh(where: Path) -> tuple: +def logging_gh(where: Path, says: str = "", status: int = 97) -> tuple: """`(directory to put first on PATH, directory of calls)`: a `gh` that runs NOTHING. Each call writes its argv and its whole environment to a - new file and exits 97, so a test can say which process called it and - what that process could have authenticated with. A shebang script, so - its tests are `WINDOWS_SKIP`.""" + new file and exits `status` (97, which no real `gh` exits), so a test can + say which process called it and what that process could have + authenticated with. Given `says`, it prints that to stderr first, as a + `gh` that is not logged in prints its help. A shebang script, so its + tests are `WINDOWS_SKIP`.""" bin_dir, calls = where / "bin", where / "calls" bin_dir.mkdir(parents=True) calls.mkdir() @@ -302,7 +352,8 @@ def logging_gh(where: Path) -> tuple: f"out=$(mktemp {shlex.quote(str(calls))}/call.XXXXXX)\n" "printf '%s\\n' \"$*\" > \"$out\"\n" "env >> \"$out\"\n" - "exit 97\n", encoding="utf-8") + + (f"printf '%s\\n' {shlex.quote(says)} >&2\n" if says else "") + + f"exit {status}\n", encoding="utf-8") stub.chmod(0o755) return bin_dir, calls @@ -361,7 +412,7 @@ def descendant_run(standard, tmp_path_factory) -> dict: **{name: SENTINEL_TOKEN for name in TOKENS}, "GH_CONFIG_DIR": operators_config, **{name: SENTINEL_HOST for name in NO_PROXIES}}) - return {"result": result, "calls": gh_calls(calls), + return {"result": result, "calls": gh_calls(calls), "clone": clone, "operators_config": operators_config} @@ -428,6 +479,72 @@ def test_only_the_machine_rows_gh_keeps_the_login_and_the_proxies( assert env.get(name) != UNREACHABLE, (argv, name) +#: What a `gh` that is not logged in prints, over TWO lines (its auth help; +#: under `GITHUB_ACTIONS` it is four). +GH_NOT_LOGGED_IN = ( + "To get started with GitHub CLI, please run: gh auth login\n" + "Alternatively, populate the GH_TOKEN environment variable with a GitHub " + "API authentication token.") + + +@WINDOWS_SKIP +def test_a_two_line_gh_stderr_in_a_skip_keeps_the_ways_to_give_a_source( + descendant_run, standard, tmp_path): + """`validate-pins.py`'s skip embeds `gh`'s stderr verbatim, so a `gh` whose + help is two lines splits the skip in two: the row's reason used to stop at + the first, which told the reader to `gh auth login` -- a login the + doctor's own offline environment would then undo -- and dropped the + `--pin-source`, `SHAPE_PIN_SOURCE_` and `../` ways to + give the product a source, which are why the row has no NEXT. + + The same descendant, the same clone, with a stub that prints that help for + every call and exits 4. The summary line the validator ends on is not + folded into the reason. + """ + bin_dir, _calls = logging_gh(tmp_path / "gh-help", GH_NOT_LOGGED_IN, 4) + result = run_script( + standard / DOCTOR, "--root", str(descendant_run["clone"]), "--json", + env={"PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}"}) + assert result.returncode == 0, result.stdout + result.stderr + row = rows_of(result)["pins"] + assert row["status"] == "note", row + assert row["next"] is None, row + assert len(row["detail"]["skipped"]) == 1, row + for said in (row["reason"], row["detail"]["skipped"][0]): + assert "gh auth login" in said, row + assert "GH_TOKEN environment variable" in said, row + assert "--pin-source" in said, row + assert f"SHAPE_PIN_SOURCE_{PRODUCT.upper()}" in said, row + assert f"../{PRODUCT}" in said, row + assert said.index("GH_TOKEN") < said.index("--pin-source"), row + assert "pins ok" not in said, row + + +def test_skipped_keeps_the_whole_block_and_stops_at_the_next_item(standard): + """`skipped` on a constructed output, on every platform: a block is the + ` skip ` line and the lines after it up to the next item -- ` ok`, + ` skip`, `FINDING`, `WARNING` or the validator's summary -- joined by a + space, blank lines dropped; and a line before any skip is not one.""" + skipped = doctor_module(standard).skipped + whole = ("Notice: not part of a block\n" + " ok a: pin commit 0123456789ab digest recomputes\n" + " skip b: NOT rechecked -- no source (gh-unreadable: one\n" + "two\n" + "\n" + "three). A --pin-source would verify it offline.\n" + " ok c: fine\n" + " skip d: NOT rechecked (gh-unreadable: x). A --pin-source.\n" + "WARNING w: a note\n" + " skip e: NOT rechecked.\n" + "pins ok\n") + assert skipped(whole) == [ + "b: NOT rechecked -- no source (gh-unreadable: one two three). A " + "--pin-source would verify it offline.", + "d: NOT rechecked (gh-unreadable: x). A --pin-source.", + "e: NOT rechecked."] + assert skipped(" ok a: fine\nmanifest ok: x\n") == [] + + #: The variables the env probe below prints, in one list. ASKED = TOKENS + ("GH_CONFIG_DIR", "GH_NO_UPDATE_NOTIFIER") + PROXIES + \ NO_PROXIES @@ -607,3 +724,29 @@ def test_a_committed_deletion_of_repo_shape_gets_no_head_line( assert row["reason"].startswith( f"project.yaml does not load: {loader_line(refusal)}; "), row assert "parent of the commit" in row["reason"], row + + +@pytest.mark.parametrize("kind", sorted(UNLOADED_ROOT)) +def test_a_crash_for_want_of_repo_shape_says_which_file_is_not_there( + standard, project, holder, tmp_path, kind): + """R298-1's fact, in a CRASH reason (#298): a root's own gate that dies on + `import repo_shape` because `scripts/repo_shape.py` is gone says that file + is not there, after the exception it quotes -- so a reader is not left + with `ModuleNotFoundError` and a guess. The manifest is VALID here, so the + row runs the gate and the gate crashes, rather than the loader refusing. + + Only the root's own copy can be asked: the standard's template copy has no + `repo_shape.py` beside it by design, and a standard that lost its own + cannot start the doctor at all. + """ + _name, _own, row_id, _breakages = UNLOADED_ROOT[kind] + root = own_root("family" if kind == "family" else "manifest", project, + holder, tmp_path) + (root / LOST).unlink() + row = rows_of(doctor(standard, root, "--json"))[row_id] + assert row["status"] == "FINDING", row + assert row["detail"]["crashed"] is True, row + assert row["detail"]["own_copy"] is True, row + assert "ModuleNotFoundError" in row["reason"], row + assert (f"{LOST}, which it imports from beside itself, is not there" + in row["reason"]), row From 52e3eee70666b026fa093c86ff9b186dbdc3cec1 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:56:56 +0000 Subject: [PATCH 03/10] shape-doctor.py: a NEXT is only the command, and the sentence that ended sixteen of them after ` # ` is the row's `why`, on a `WHY` line of its own and `"why"` in `--json`, so a NEXT pasted into an interactive zsh -- which reads that `#` as a word -- no longer hands the comment to the command as arguments, runs what follows its `;`, or runs the `make bootstrap` in its backticks, and the template validator's line is `env PYTHONPATH=... python3 ...` (#277) The doctor built sixteen NEXTs as ` # ` and printed one prose sentence in the command's place. `sh -c`, bash and `zsh -c` read that `#` as a comment. An INTERACTIVE zsh without `interactivecomments` reads it as a word; that is zsh's default and macOS's login shell, the one a person pastes into. Pasted there, the comment's words reached the command as arguments, and whatever followed a `;` ran as a second command: the manifest NEXT exited 127, `zsh: command not found: it`, instead of with the validator's code. The `make bootstrap` that the `legs` and `members` comments put in backticks ran as command substitution, in whatever directory the person stood in. The tests could not see any of it, because they read a NEXT the way `sh` does. A NEXT is now the command and nothing else (R277-2: `next` is the bare command or null). The sentence moved verbatim, less its `#`, to a new keyword-only `Row` argument, `why` (R277-1). It is printed in three places: `"why"` on every `--json` row (null where there is none), a `WHY` line of its own under `NEXT` in the table, and ` Why: ...` on the line after ` Remediation: ...` in the CANNOT ANSWER block on stderr. The module docstring and `Row`'s say why the sentence never goes back on the command's line. The sites, re-counted on 3ae0109: 16 non-docstring string constants held ` # ` outside `placement_plan_leg` and `report`, plus one prose NEXT, the plan's 17: - `validator_row` (neither copy readable; the finding); - `unloaded_manifest_row`, whose neither-readable branch now passes on `validator_row`'s `why` together with its NEXT; - `check_shape_currency` (cannot answer; the pin alone behind; copies moved). The `then apply --at ... --branch ...` with its `--accept-local` and `--trailer 'Lane: X'` is the `why`, which `lane_trailer_command`'s docstring now says; - `check_legs`; - `missing_shape_file_fix`, which returns `(next, why)`. Its partly-copyable note is the `why`, and the all-rendered prose NEXT is now `python3 /scaffold-project.py --help`, with that prose as its `why`; - `placement_policy`, through `placement_row(why=...)`; - `check_placement`'s `--placement-plan` line, through `placement_verdict`'s new `plan_why`; - the three branches of `check_agent_files`; - `members_verdict`; - `check_machine`; - `check_the_way_in`. Out of scope as ruled: `check_manifest_kinds` (lane 2's #291) and `Refusal.remediation` used as a NEXT. `report`'s `placement plan written to ... # nothing moved` is printed text, not a NEXT. R277-4: `validator_command`'s `sh` branch spells the template's environment `env PYTHONPATH= python3 ...`. A bare `NAME=value` prefix is syntax in sh, bash and zsh only. By their documentation (not run here), csh and tcsh report `Command not found` for it and fish before 3.1 refuses it. `env` is a program, so the line runs in each of them and still sets the variable for that one command. The PowerShell `$env:` line is unchanged. Tests, in a new `tests/test_doctor_next_is_a_command.py`, importing `test_shape_doctor`'s fixtures: - A property over one red fixture per converted site (19 cases: the 17, plus the template-copy validator line and the unloaded-manifest neither-readable branch). The NEXT reads the same to `shlex` with and without comments, holds no backtick and no ` # `, and on POSIX no unquoted `;` (lexed with `#` not a comment, as zsh reads it). Its `why` opens with main's own words. - A `zsh -f -i` paste through a pty, skipped without zsh and on Windows. The manifest NEXT exits with the row's `detail.exit` and prints `FINDING manifest-topic`. The legs NEXT, pasted in a directory whose Makefile's `bootstrap` target touches a marker, leaves none. Each also pastes the line main printed, which exits 127 or creates the marker, so the harness is shown to see the defect. - An AST guard: no non-docstring string constant in `shape-doctor.py` holds ` # ` outside `placement_plan_leg` and `report`, allow-listed by function name. It proves its scanner on a sample first. A tail that any lane lands later is red in CI. - The table's `WHY` line sits under its own `NEXT` per row, and stderr has `Why:` right after `Remediation:`. `tests/test_shape_doctor.py` changes only where a test asserted tail text, the row key set (now with `why`) or the bare `PYTHONPATH=` prefix. That covers the way-in and the two unloaded-manifest file-to-fix assertions, the #111 lane tests (the trailer is read from `why`), the template and own-copy validator lines, the `VALIDATOR_COMMAND` rows and the `sh` read-back, and the PowerShell paste, which no longer cuts off a comment. The stale `RENDERED_NOTE` comments are corrected. Mutants: - A ` # x` tail put back on `check_legs`'s NEXT fails the AST guard and the property's `check_legs` case. - `why` dropped from `Row.as_dict` fails `test_every_row_of_a_compliant_project_is_ok_or_na`'s key set. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 248 ++++++---- tests/test_doctor_next_is_a_command.py | 656 +++++++++++++++++++++++++ tests/test_shape_doctor.py | 149 +++--- 3 files changed, 905 insertions(+), 148 deletions(-) create mode 100644 tests/test_doctor_next_is_a_command.py diff --git a/shape-doctor.py b/shape-doctor.py index 3e5bbece..40d6c7b4 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -186,6 +186,19 @@ reader improvises around -- which is the house rule the rest of this standard is built on. +AND THE NEXT IS THE COMMAND AND NOTHING ELSE (#277). The sentence saying why +it is the fix -- "read its output in full", "then apply --at ...", "what +`make bootstrap` runs" -- used to ride on the same line as ` # `, +because a terminal shows both at once. `sh`, bash and `zsh -c` read that `#` +as a comment. An INTERACTIVE zsh without `interactivecomments` -- zsh's +default, and macOS's login shell, the one a person pastes into -- reads it as +a word: the pasted line handed the comment to the command as arguments, ran +whatever followed a `;` as a second command, and ran whatever sat in +backticks, `make bootstrap` among them. So `next` and the `NEXT` line are the +bare command, pasted whole, and the sentence is the row's `why`: a `WHY` line +under `NEXT` in the table, `"why"` on every row of `--json` (null where there +is none), and a line after `Remediation:` when the report cannot answer. + AND ONLY A `FINDING` MOVES THE VERDICT. A row is `ok`, `note`, `FINDING`, `n/a` or -- the Triad row alone -- `advisory`, and `FINDING` means exactly "something ELSE asserts this" -- a @@ -651,10 +664,22 @@ def quote_arg(value, platform: str | None = None, *, class Row: - """One line of the report: what was asked, the answer, and the exit.""" + """One line of the report: what was asked, the answer, and the exit. + + `next_command` IS A COMMAND, WHOLE, AND `why` IS WHAT IS SAID ABOUT IT + (#277). The two were one string, ` # `, and the `#` is a + comment only to a shell that reads it as one: pasted into an interactive + zsh it was a word, the comment's text became arguments, a `;` in it ran a + second command and a pair of backticks ran a third. So a builder hands + the sentence over as `why`, a keyword of its own, and `next_command` + holds only what a reader pastes. `why` is never folded back into the + command line: `report` prints it on a `WHY` line of its own and `as_dict` + as its own key. + """ def __init__(self, check_id: str, label: str, status: str, reason: str, - next_command: str | None = None, detail: dict | None = None): + next_command: str | None = None, detail: dict | None = None, + *, why: str | None = None): self.id = check_id self.label = label self.status = status @@ -665,14 +690,18 @@ def __init__(self, check_id: str, label: str, status: str, reason: str, # ordinary job, flattening this repository's own typography, on the # literal words a caller wrote around those values. self.next_command = ascii_text(next_command) if next_command else None + #: The sentence that explains `next_command`, kept off its line. + self.why = ascii_text(why) if why else None #: Structured facts for `--json`, so a caller reads numbers rather #: than parsing the sentence a human is meant to read. self.detail = detail or {} def as_dict(self) -> dict: + # `why` ON EVERY ROW, null where there is none, so a caller reads a + # key rather than testing for its absence -- as `placement_plan` is. return {"id": self.id, "label": self.label, "status": self.status, "reason": self.reason, "next": self.next_command, - "detail": self.detail} + "why": self.why, "detail": self.detail} class Check: @@ -920,7 +949,7 @@ def run_validator(ctx: Context, script: Path, args: list[str], DOCTOR'S OWN IS NEVER PART OF IT (#306). `pythonpath` is the directory to put on it, and `validator_row` passes `validator_scripts`'s answer: given one, the child's `PYTHONPATH` is that directory ALONE, the way the printed - `PYTHONPATH= python3 ...` sets it; given none, the child has NO + `env PYTHONPATH= python3 ...` sets it; given none, the child has NO `PYTHONPATH`, and the variable is taken OUT of the copy of the doctor's environment this builds, not left in it. ONE PLACE DECIDES, and it is the `env` block below: it does not ask whether the script is a root's own copy @@ -1057,14 +1086,22 @@ def validator_command(shape: Path, used: Path, own_copy: bool, args: list, SO THE TEMPLATE'S LINE SETS IT, in the reader's dialect, which is the same two this file already speaks (`quote_arg`, `copy_command`, - `scaffold_command`): `PYTHONPATH=/scripts python3