From e39b5d7e3f98dec48de04acffd539d95069c92f1 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 8 Nov 2026 18:51:36 +0000 Subject: [PATCH 1/4] repo_shape.py: `load_yaml`'s three refusals and `policy-unreadable` name the file as `shown_path` does, so a U+202E, an ESC or a line feed in a typed path is spelled, and validate-pins.py, family.py and shape-doctor.py read either spelling (#1168) `load_yaml` named the file with the bare `{path}` in `yaml-unreadable` (twice) and `yaml-unparsable`, so a bidi override, an ESC or a line feed in a `--plan`, `--path-policy`, `--policy`, `--project` or `--root` path reached the terminal raw from `adopt-project.py check` and `plan`, `validate-repository-naming.py`, `bump-leg.py`, `family.py bump`, `update-shape.py check` and `validate-manifest.py`. Each now names it as `shown_path` does: `repr` for a path that is not printable, byte for byte for one that is. `policy-unreadable`, the next line of `NamingPolicy.load`, does the same. Every reader that finds the path at the start of the detail by its text asks both the spelled and the raw prefix, so it reads a `repo_shape.py` from before this change or after it: `validate-pins.py`'s `_load` (a root re-copies its pinned files one row at a time), `family.py`'s member manifest (the swap to ` @ :project.yaml`), and in `shape-doctor.py` `naming_script_refusal` and the `manifest-kinds` row's `less_the_path`. `link_declarations`' own swap is unchanged and reads as it did. Rows: tests/test_load_yaml_refusals.py. Closes #1168 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 8 +- scripts/repo_shape.py | 20 +- shape-doctor.py | 22 +- .../assembly-root/scripts/validate-pins.py | 21 +- tests/test_load_yaml_refusals.py | 457 +++++++++++++++++- 5 files changed, 512 insertions(+), 16 deletions(-) diff --git a/scripts/family.py b/scripts/family.py index 2ce3635e..9cbdcb4a 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -1124,7 +1124,13 @@ def _read_member_project(path: Path, project: str, repository: str, except Refusal as exc: if shown is None: raise - raise Refusal(exc.code, exc.detail.replace(str(manifest_path), shown), + # In either spelling (#1168): `load_yaml` names the file as + # `shown_path` does, `repr`'d where the clone's path holds a + # character that is not printable (a `TMPDIR` that does), and a + # `repo_shape.py` older than #1168 named it raw. For a printable + # path the two are one. + detail = exc.detail.replace(shown_path(str(manifest_path)), shown) + raise Refusal(exc.code, detail.replace(str(manifest_path), shown), exc.remediation) from exc if not isinstance(manifest, dict): raise Refusal("member-manifest-unreadable", diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 00759f1e..fee1343f 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -1337,16 +1337,27 @@ def load_yaml(path: Path) -> Any: file it cannot open (#359), and the refusal is still raised `from` the `OSError`, which that validator reads to name the file relative to its root. + + THE FILE IS NAMED AS `shown_path` NAMES IT (#1168), in all three + refusals. The path is typed (`--plan`, `--policy`, `--root`), and named + raw, a bidi control in it reordered the line, an ESC reached the + terminal as the control it is, and a line feed began a line of the + path's choosing. A printable path is named byte for byte as it always + was. Each reader that finds the path at the start of the detail by its + text -- `validate-pins.py`'s `_load`, `family.py`'s member manifest and + two in `shape-doctor.py` -- asks this spelling and the raw one, so it + reads a `repo_shape.py` from before #1168 or after it alike. """ + named = shown_path(path) try: text = Path(path).read_text(encoding="utf-8") except OSError as exc: raise Refusal("yaml-unreadable", - f"{path}: cannot be read ({_os_reason(exc)})") from exc + f"{named}: cannot be read ({_os_reason(exc)})") from exc except UnicodeDecodeError as exc: raise Refusal( "yaml-unreadable", - f"{path}: not UTF-8 at byte {exc.start} ({exc.reason})", + f"{named}: not UTF-8 at byte {exc.start} ({exc.reason})", "Remediation: every YAML file this tool reads must be UTF-8. " f"Byte {exc.start}, counting from 0 at the start of the file, is " "the first one that is not: re-save the file as UTF-8, or replace " @@ -1359,7 +1370,7 @@ def load_yaml(path: Path) -> Any: # (#708): with neither, a plan that read as text before #392 and # #393 ended `check` with a refusal a person could not act on. where = "" if exc.line is None else f"line {exc.line}: " - raise Refusal("yaml-unparsable", f"{path}: {where}{exc}", + raise Refusal("yaml-unparsable", f"{named}: {where}{exc}", exc.remediation) from exc @@ -2235,7 +2246,8 @@ def __init__(self, data: dict): def load(cls, path: Path) -> "NamingPolicy": data = load_yaml(path) if not isinstance(data, dict): - raise Refusal("policy-unreadable", f"{path}: not a mapping") + raise Refusal("policy-unreadable", + f"{shown_path(path)}: not a mapping") if data.get("kind") != "repository-naming-policy": raise Refusal( "policy-wrong-kind", diff --git a/shape-doctor.py b/shape-doctor.py index e44b4ef3..d31f6648 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -2760,12 +2760,18 @@ def naming_script_refusal(shape: Path, whole: str) -> Refusal | None: file: a project whose own path holds this checkout's, a flag, the `project.yaml` the script was asked to read -- each is about what the script was asked, and is not this. + + IN EITHER SPELLING (#1168): `load_yaml` and `policy-unreadable` name the + file as `shown_path` does, `repr`'d where this checkout's path holds a + character that is not printable, and `policy-wrong-kind` names it raw. + For a printable path the two are one. """ policy = f"{shape / NAMING_COPY}: " + spelt = f"{shown_path(shape / NAMING_COPY)}: " for line in (line.strip() for line in whole.splitlines()): if line.startswith("REFUSED "): code, _, detail = line.removeprefix("REFUSED ").partition(": ") - if detail.startswith(policy): + if detail.startswith((policy, spelt)): return Refusal(code, detail) return None @@ -4248,6 +4254,18 @@ def taking_out(ctx: Context, names: list, states: dict, judged: list, return command, ". ".join([*said, *ways]) +def less_the_path(detail: str, path: Path) -> str: + """`detail`, a `load_yaml` refusal's, less the `: ` it opens with, + in either spelling (#1168): as `shown_path` names the path, `repr`'d + where it holds a character that is not printable, which `load_yaml` + gives since #1168, or raw, as a `repo_shape.py` older than that gave. + For a printable path the two are one.""" + for opening in (f"{shown_path(path)}: ", f"{path}: "): + if detail.startswith(opening): + return detail[len(opening):] + return detail + + def check_manifest_kinds(ctx: Context) -> Row: """Every `kind:`-bearing manifest in this root, against the registry. @@ -4383,7 +4401,7 @@ def check_manifest_kinds(ctx: Context) -> Row: continue unreadable.append(rel) refusals[rel] = {"code": exc.code, - "detail": exc.detail.removeprefix(f"{path}: ")} + "detail": less_the_path(exc.detail, path)} continue if not isinstance(data, dict): unreadable.append(rel) diff --git a/templates/assembly-root/scripts/validate-pins.py b/templates/assembly-root/scripts/validate-pins.py index acd44bfe..49886de8 100755 --- a/templates/assembly-root/scripts/validate-pins.py +++ b/templates/assembly-root/scripts/validate-pins.py @@ -650,18 +650,27 @@ def _load(root: Path, path: Path): start named relative to the root. Asked of the refusal and not of new words in `repo_shape`, so a root whose `scripts/repo_shape.py` is older than this file names the file the same way (#370, #610): `load_yaml` - has raised its refusal `from` the `OSError` since it was written.""" + has raised its refusal `from` the `OSError` since it was written. + + THE PATH AT ITS START IS LOOKED FOR IN BOTH SPELLINGS (#1168). Since + #1168 `load_yaml` names the file as `shown_path` names it, which is + `repr` for a path that is not printable (a root under a folder whose + name holds an ESC or a line feed), and before it, raw. A root re-copies + its pinned files one row at a time, so this file may meet either: the + raw prefix and `_shown`'s are both asked, and either is named relative + to the root. For a printable path the two are one.""" try: return load_yaml(path) except Refusal as exc: if isinstance(exc.__cause__, OSError): raise _unreadable_file("yaml-unreadable", _named(root, path), exc.__cause__) from exc.__cause__ - absolute = f"{path}: " - if not exc.detail.startswith(absolute): - raise - raise Refusal(exc.code, _named(root, path) + ": " - + exc.detail[len(absolute):], exc.remediation) from exc + for absolute in (f"{path}: ", f"{_shown(str(path))}: "): + if exc.detail.startswith(absolute): + raise Refusal(exc.code, _named(root, path) + ": " + + exc.detail[len(absolute):], + exc.remediation) from exc + raise def _workflows_unlistable(root: Path, workflows: Path, diff --git a/tests/test_load_yaml_refusals.py b/tests/test_load_yaml_refusals.py index 8cdcec89..0ce60530 100644 --- a/tests/test_load_yaml_refusals.py +++ b/tests/test_load_yaml_refusals.py @@ -17,19 +17,38 @@ `test_adopt_execute_refusals.py` (the adoption plan) and `test_manifest_validator.py` (`project.yaml`); this file is the function. -NO NETWORK AND NO GITHUB: a temporary file and an import. +THE FILE IS NAMED AS `shown_path` NAMES IT (#1168), in all three. A path is +typed (`--plan`, `--policy`, `--root`), and named raw, a bidi override in it +reordered the line, an ESC reached the terminal as the control it is, and a +line feed began a line of the path's choosing. The rows below hold each +refusal to that in process, then each tool that hands `load_yaml` a typed +path to it in a real run, and then every reader that finds the path in the +detail by its text: `validate-pins.py`'s `_load` in both version orders, +`link_declarations`, `family.py`'s member manifest and two in +`shape-doctor.py`. A printable path is named byte for byte as before. + +NO NETWORK AND NO GITHUB: temporary files, local repositories, imports, and +the session's `gh` guard first on PATH for every tool run. """ from __future__ import annotations +import subprocess import sys +from pathlib import Path import pytest -from conftest import REPO +from conftest import ( + ADOPT, PRINTABLE_NAME, REPO, UNSAFE_NAMES, assert_shown_safe, + copied_under, git, load_without_bytecode, make_source_repo, named_as, + run_script, spelt, unsafe_folder, +) sys.path.insert(0, str(REPO / "scripts")) -from repo_shape import Refusal, load_yaml # noqa: E402 +from repo_shape import ( # noqa: E402 + NamingPolicy, Refusal, link_declarations, load_yaml, shown_path, +) #: `(bytes, offset of the first byte that is not UTF-8)`. Each is a way a file #: reaches this state: a Latin-1 `é` where UTF-8 has two bytes, a UTF-16 byte @@ -104,3 +123,435 @@ def test_utf8_that_is_not_ascii_is_read_not_refused(tmp_path): path = tmp_path / "plan.yaml" path.write_bytes("name: café\nnote: \"—\"\n".encode("utf-8")) assert load_yaml(path) == {"name": "café", "note": "—"} + + +# --- #1168: the file is named as `shown_path` names it ----------------------- + +#: A folder name holding a bidi override, U+202E, which reorders the line +#: around it so that a path reads as another path. A Windows file name can +#: hold it, so it needs no mark. +RLO_NAME = "wd-pl" + chr(0x202E) + "an" + +#: The three the issue probed: U+202E, an ESC, and a line feed that begins a +#: line of the path's choosing (`conftest.UNSAFE_NAMES`, POSIX only). +UNPRINTABLE = [pytest.param(RLO_NAME, id="U+202E"), *UNSAFE_NAMES] + +#: `UNPRINTABLE`, and a name a line names byte for byte as it always did. +NAMES = [*UNPRINTABLE, pytest.param(PRINTABLE_NAME, id="printable")] + +#: Text this reader refuses, and what it says after the path about it. +UNPARSABLE = "a: 1\n: b\n" +NOT_PARSED = "line 2: empty keys are not supported" + +#: Each way `load_yaml` refuses: the bytes at the path (None: nothing is +#: there), and the code. +WAYS = { + "absent": (None, "yaml-unreadable"), + "not-utf-8": (b"name: caf\xe9\n", "yaml-unreadable"), + "unparsable": (UNPARSABLE.encode("utf-8"), "yaml-unparsable"), +} + + +def refused_under(tmp_path, name: str, way: str) -> tuple: + """`(path, refusal)` for a `plan.yaml` refused `way`, in a folder `name` + made under `tmp_path`.""" + body = WAYS[way][0] + path = unsafe_folder(tmp_path, name) / "plan.yaml" + if body is not None: + path.write_bytes(body) + return path, refusal_for(path) + + +def said_after_the_path(refusal: Refusal) -> str: + """The words a refusal says after `: `, from its own cause: the + system's reason, the byte and Python's reason, or the reader's line.""" + cause = refusal.__cause__ + if isinstance(cause, OSError): + return f"cannot be read ({cause.strerror})" + if isinstance(cause, UnicodeDecodeError): + return f"not UTF-8 at byte {cause.start} ({cause.reason})" + return NOT_PARSED + + +@pytest.mark.parametrize("way", sorted(WAYS)) +@pytest.mark.parametrize("name", UNPRINTABLE) +def test_each_refusal_names_a_path_that_is_not_printable_as_repr( + tmp_path, name, way): + """THE ISSUE'S THREE SITES, in process: the path as `repr` spells it, so + no character of the folder's name reaches the line raw and the detail + stays one line. On main each named it raw.""" + path, refusal = refused_under(tmp_path, name, way) + assert refusal.code == WAYS[way][1] + assert refusal.detail == f"{spelt(path)}: {said_after_the_path(refusal)}" + assert name not in str(refusal), ascii(str(refusal)) + assert_shown_safe(str(refusal)) + + +@pytest.mark.parametrize("way", sorted(WAYS)) +def test_a_printable_path_is_named_byte_for_byte_as_it_was(tmp_path, way): + """The control, which passes on main as well: a space and a letter past + ASCII are printable, so the path is named as it is, with no quotes.""" + path, refusal = refused_under(tmp_path, PRINTABLE_NAME, way) + assert refusal.detail == f"{path}: {said_after_the_path(refusal)}" + + +@pytest.mark.parametrize("name", NAMES) +def test_a_policy_that_is_not_a_mapping_is_named_as_shown_path_names_it( + tmp_path, name): + """`policy-unreadable`, the line after `load_yaml` in `NamingPolicy.load`, + names the file the same way.""" + policy = unsafe_folder(tmp_path, name) / "repository-naming.yaml" + policy.write_text("- a list\n", encoding="utf-8") + with pytest.raises(Refusal) as caught: + NamingPolicy.load(policy) + assert caught.value.code == "policy-unreadable" + assert caught.value.detail == f"{named_as(policy, name)}: not a mapping" + + +# --- each tool that hands `load_yaml` a path it was given ------------------- + +#: A commit nothing holds, for the two commands that ask one before they read. +A_COMMIT = "1" * 40 + +#: The template's validators, and the library a root keeps beside them. +TEMPLATE_SCRIPTS = REPO / "templates" / "assembly-root" / "scripts" +LIBRARY = REPO / "scripts" / "repo_shape.py" + + +def unparsable(path: Path) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(UNPARSABLE, encoding="utf-8") + return path + + +def a_root(folder: Path, gate: bytes, library: bytes, name: str) -> Path: + """A git repository at `/Atlas` carrying `scripts/` as + `gate` and `scripts/repo_shape.py` as `library`, as an assembly root + carries them, with a `project.yaml` that does not parse.""" + root = folder / "Atlas" + (root / "scripts").mkdir(parents=True) + git("init", "-q", str(root), cwd=folder) + (root / "scripts" / name).write_bytes(gate) + (root / "scripts" / "repo_shape.py").write_bytes(library) + unparsable(root / "project.yaml") + return root + + +def adopt_check(folder: Path, tmp_path: Path) -> tuple: + plan = unparsable(folder / "adoption-plan.yaml") + return plan, (ADOPT, "check", "--plan", str(plan)) + + +def adopt_plan(folder: Path, tmp_path: Path) -> tuple: + policy = unparsable(folder / "path-policy.yaml") + source = make_source_repo(tmp_path / "Thing", {"README.md": "# Thing\n"}, + edits=()) + return policy, (ADOPT, "plan", "--source", str(source), "--project", + "Northwind", "--org", "testorg", "--elected-by", + "Test Human", "--elected-on", "2026-09-02", "--out", + str(tmp_path / "adoption-plan.yaml"), "--path-policy", + str(policy)) + + +def naming_policy(folder: Path, tmp_path: Path) -> tuple: + policy = unparsable(folder / "repository-naming.yaml") + return policy, (REPO / "scripts" / "validate-repository-naming.py", + "--policy", str(policy), "Atlas") + + +def naming_project(folder: Path, tmp_path: Path) -> tuple: + manifest = unparsable(folder / "project.yaml") + return manifest, (REPO / "scripts" / "validate-repository-naming.py", + "--project", str(manifest)) + + +def bump_leg(folder: Path, tmp_path: Path) -> tuple: + return unparsable(folder / "project.yaml"), ( + REPO / "scripts" / "bump-leg.py", "--root", str(folder), "--leg", + "spec", "--to", A_COMMIT, "--dry-run") + + +def family_bump(folder: Path, tmp_path: Path) -> tuple: + return unparsable(folder / "family.yaml"), ( + REPO / "scripts" / "family.py", "bump", "--dry-run", "--family-root", + str(folder), "--member", "Atlas", "--to", A_COMMIT) + + +def update_shape(folder: Path, tmp_path: Path) -> tuple: + git("init", "-q", str(folder), cwd=tmp_path) + return unparsable(folder / "contracts" / "shape-pin.yaml"), ( + REPO / "update-shape.py", "check", "--root", str(folder)) + + +def validate_manifest(folder: Path, tmp_path: Path) -> tuple: + gate = "validate-manifest.py" + root = a_root(folder, (TEMPLATE_SCRIPTS / gate).read_bytes(), + LIBRARY.read_bytes(), gate) + return root / "project.yaml", (root / "scripts" / gate, "--root", + str(root)) + + +#: The issue's table: every tool that hands `load_yaml` a path typed, or +#: built from one typed, and how one run of it reaches that refusal. +TOOLS = { + "adopt-check--plan": adopt_check, + "adopt-plan--path-policy": adopt_plan, + "naming--policy": naming_policy, + "naming--project": naming_project, + "bump-leg--root": bump_leg, + "family-bump--family-root": family_bump, + "update-shape-check--root": update_shape, + "validate-manifest--root": validate_manifest, +} + + +def refused_lines(text: str) -> list[str]: + return [line for line in text.splitlines() if line.startswith("REFUSED ")] + + +@pytest.mark.parametrize("name", NAMES) +@pytest.mark.parametrize("tool", sorted(TOOLS)) +def test_each_tool_names_the_file_load_yaml_refuses_as_shown_path_names_it( + tmp_path, tool, name): + """THE ISSUE'S PROBE, through each tool: one `REFUSED yaml-unparsable` + line, exit 2, the file named as `shown_path` names it, and no U+202E, + ESC or forged line anywhere in the output. On main each tool printed + the path raw; a printable one is named as it always was.""" + named, argv = TOOLS[tool](unsafe_folder(tmp_path, name), tmp_path) + result = run_script(*argv) + said = result.stdout + result.stderr + assert result.returncode == 2, said + assert refused_lines(result.stderr) == [ + f"REFUSED yaml-unparsable: {named_as(named, name)}: {NOT_PARSED}"], ( + ascii(said)) + assert chr(0x202E) not in said, ascii(said) + assert_shown_safe(result.stdout, result.stderr) + + +# --- the readers that find the path in the detail by its text --------------- + +#: The commit before #1168: its `repo_shape.py` names the file raw, and its +#: `validate-pins.py` asks the raw prefix alone. +BEFORE_1168 = "51a9c5f103cd02c9b9e01e8a4c1acd95f791451e" +PINS = "validate-pins.py" + + +def before_1168(relative: str, holds: bytes) -> bytes: + """`relative` at `BEFORE_1168`, or a skip where this checkout does not + hold that history; it must hold `holds`, or the row would test nothing + older than this tree.""" + shown = subprocess.run(["git", "show", f"{BEFORE_1168}:{relative}"], + cwd=str(REPO), capture_output=True, check=False) + if shown.returncode != 0: + pytest.skip(f"needs {BEFORE_1168} in this checkout's history") + assert holds in shown.stdout, relative + return shown.stdout + + +def older_library() -> bytes: + return before_1168("scripts/repo_shape.py", + b'f"{path}: {where}{exc}"') + + +def older_gate() -> bytes: + return before_1168(f"templates/assembly-root/scripts/{PINS}", + b'absolute = f"{path}: "') + + +def gate_refused(folder: Path, gate: bytes, library: bytes) -> tuple: + """`(root, result)`: the root's own `validate-pins.py`, run over a root + under `folder` whose `project.yaml` does not parse.""" + root = a_root(folder, gate, library, PINS) + result = run_script(root / "scripts" / PINS, cwd=root) + assert result.returncode == 2, result.stdout + result.stderr + assert_shown_safe(result.stdout, result.stderr) + return root, result + + +#: What the gate says: the file named relative to the root, beside either +#: library. +RELATIVE = f"REFUSED yaml-unparsable: project.yaml: {NOT_PARSED}" + + +@pytest.mark.parametrize("name", NAMES) +def test_the_gate_names_the_file_from_the_root_beside_this_library( + tmp_path, name): + """`validate-pins.py` beside the `repo_shape.py` of this tree, which + spells a root that is not printable: the gate finds `_shown`'s prefix + and names the file `project.yaml`, as it always did. With main's gate + beside this library the prefix was not found, and the line named the + whole path.""" + _root, result = gate_refused(unsafe_folder(tmp_path, name), + (TEMPLATE_SCRIPTS / PINS).read_bytes(), + LIBRARY.read_bytes()) + assert refused_lines(result.stderr) == [RELATIVE], ascii(result.stderr) + + +@pytest.mark.parametrize("name", NAMES) +def test_the_gate_names_the_file_from_the_root_beside_an_older_library( + tmp_path, name): + """THE SKEW RULE, one way: a root that kept a `repo_shape.py` older than + #1168 (`update-shape.py apply --accept-local`), which names the path + raw. The gate still finds the raw prefix, and names `project.yaml`.""" + _root, result = gate_refused(unsafe_folder(tmp_path, name), + (TEMPLATE_SCRIPTS / PINS).read_bytes(), + older_library()) + assert refused_lines(result.stderr) == [RELATIVE], ascii(result.stderr) + + +@pytest.mark.parametrize("name", UNPRINTABLE) +def test_an_older_gate_beside_this_library_names_the_whole_path_spelled( + tmp_path, name): + """THE SKEW RULE, the other way: a root that kept a `validate-pins.py` + older than #1168 beside this `repo_shape.py`. That gate asks the raw + prefix alone, which a spelled path does not start with, so it says the + library's own line: the whole path, spelled, and nothing raw. Named + relative again once the gate is re-copied (the row above). A printable + root is named relative by either gate.""" + root, result = gate_refused(unsafe_folder(tmp_path, name), older_gate(), + LIBRARY.read_bytes()) + assert refused_lines(result.stderr) == [ + f"REFUSED yaml-unparsable: {spelt(root / 'project.yaml')}: " + f"{NOT_PARSED}"], ascii(result.stderr) + assert name not in result.stdout + result.stderr + + +def named_raw(path): + """`load_yaml` as a library older than #1168 is: the same data, and the + same refusal from the same cause with the path at its start raw.""" + try: + return load_yaml(path) + except Refusal as exc: + spelled, detail = f"{shown_path(path)}: ", exc.detail + if detail.startswith(spelled): + detail = f"{path}: {detail[len(spelled):]}" + raise Refusal(exc.code, detail, exc.remediation) from exc.__cause__ + + +#: The library a reader meets: this tree's, which spells a path that is not +#: printable, and one older than #1168, which names it raw. +LOADERS = [pytest.param(load_yaml, id="this-library"), + pytest.param(named_raw, id="an-older-library")] + + +@pytest.mark.parametrize("loader", LOADERS) +@pytest.mark.parametrize("name", NAMES) +def test_the_gates_load_takes_either_prefix(tmp_path, monkeypatch, name, + loader): + """Both halves of `_load`'s prefix test, in process and with no history: + the path `_shown` spells and the path raw are each named relative to + the root.""" + gate = load_without_bytecode(TEMPLATE_SCRIPTS / PINS, + "load_yaml_refusals_pins") + monkeypatch.setattr(gate, "load_yaml", loader) + root = unsafe_folder(tmp_path, name) + with pytest.raises(Refusal) as caught: + gate._load(root, unparsable(root / "project.yaml")) + assert caught.value.detail == f"project.yaml: {NOT_PARSED}" + + +@pytest.mark.parametrize("name", NAMES) +def test_a_link_manifest_is_named_in_why_as_it_was(tmp_path, name): + """`link_declarations` swaps the raw path for `shown_path`'s in a + refusal's detail (#610). The detail holds that spelling now, so the + swap finds nothing to do and `why` reads as it did.""" + tree = unsafe_folder(tmp_path, name) + manifest = unparsable(tree / "project.yaml") + assert link_declarations(tree) == ( + None, f"{named_as(manifest, name)}: {NOT_PARSED}") + + +#: What `family.py add` names a member's manifest that it read out of a +#: throwaway clone. +MEMBER_MANIFEST = "acme/Atlas @ 0123456789ab:project.yaml" + + +@pytest.mark.parametrize("loader", LOADERS) +@pytest.mark.parametrize("name", NAMES) +def test_family_names_a_members_manifest_as_its_caller_named_it( + tmp_path, monkeypatch, name, loader): + """`_read_member_project` names the clone's `project.yaml` by what was + cloned, for the clone is gone when the line is read (#203). Its clone + is under `TMPDIR`; one whose path is not printable is spelled in the + detail, and the swap looks for either spelling, so the line names the + member as it always did. With main's `family.py` beside this library + it named the deleted clone.""" + family = load_without_bytecode(REPO / "scripts" / "family.py", + "load_yaml_refusals_family") + monkeypatch.setattr(family, "load_yaml", loader) + clone = unsafe_folder(tmp_path, name) + unparsable(clone / "project.yaml") + with pytest.raises(Refusal) as caught: + family._read_member_project(clone, "Atlas", "acme/Atlas", + shown=MEMBER_MANIFEST) + assert caught.value.code == "yaml-unparsable" + assert caught.value.detail == f"{MEMBER_MANIFEST}: {NOT_PARSED}" + + +def doctor_module(): + return load_without_bytecode(REPO / "shape-doctor.py", + "load_yaml_refusals_doctor") + + +def policy_refusal(shape: Path, text: str) -> Refusal: + """The refusal this checkout's naming policy gives where it holds + `text`, as the naming script prints it.""" + policy = shape / "contracts" / "repository-naming.yaml" + policy.parent.mkdir(parents=True) + policy.write_text(text, encoding="utf-8") + with pytest.raises(Refusal) as caught: + NamingPolicy.load(policy) + return caught.value + + +@pytest.mark.parametrize("text, code", [ + pytest.param(UNPARSABLE, "yaml-unparsable", id="load-yaml"), + pytest.param("- a list\n", "policy-unreadable", id="not-a-mapping"), +]) +@pytest.mark.parametrize("name", NAMES) +def test_the_doctor_finds_its_own_policy_in_the_naming_scripts_refusal( + tmp_path, name, text, code): + """`naming_script_refusal` knows the checkout's own policy by the path + at the start of the detail. `load_yaml` and `policy-unreadable` spell + it now, and the doctor asks that spelling too.""" + shape = unsafe_folder(tmp_path, name) + refusal = policy_refusal(shape, text) + found = doctor_module().naming_script_refusal(shape, f"{refusal}\n") + assert found is not None, ascii(str(refusal)) + assert (found.code, found.detail) == (code, refusal.detail) + + +@pytest.mark.parametrize("name", [ + pytest.param(RLO_NAME, id="U+202E"), UNSAFE_NAMES[0], + pytest.param(PRINTABLE_NAME, id="printable"), +]) +def test_the_doctor_still_finds_its_own_policy_named_raw(tmp_path, name): + """`policy-wrong-kind` names the policy raw, as it always did, and the + doctor still finds it by that prefix. A line feed splits that line, so + it is not among the names here, as on main.""" + shape = unsafe_folder(tmp_path, name) + refusal = policy_refusal(shape, "kind: something-else\n") + assert refusal.code == "policy-wrong-kind" + found = doctor_module().naming_script_refusal(shape, f"{refusal}\n") + assert found is not None, ascii(str(refusal)) + assert found.detail == refusal.detail + + +@pytest.mark.parametrize("loader", LOADERS) +@pytest.mark.parametrize("name", NAMES) +def test_the_doctors_manifest_kinds_row_quotes_the_refusal_less_its_path( + project, tmp_path, monkeypatch, name, loader): + """`manifest-kinds` quotes `load_yaml`'s refusal of a file under + `contracts/` less the path it opens with, which the row has just named + relative to the root, in either spelling (`less_the_path`). With main's + doctor beside this library, a root that is not printable had its whole + path quoted again.""" + root = copied_under(project, tmp_path, name) + unparsable(root / "contracts" / "n1168.yaml") + doctor = doctor_module() + monkeypatch.setattr(doctor, "load_yaml", loader) + row = doctor.check_manifest_kinds(doctor.Context(root)) + assert row.detail["refusals"]["contracts/n1168.yaml"] == { + "code": "yaml-unparsable", "detail": NOT_PARSED}, row.detail + assert (f"unreadable as YAML: contracts/n1168.yaml (yaml-unparsable: " + f"{NOT_PARSED})") in row.reason, ascii(row.reason) From 136b38d1272f75b25e0ce2dedd8498fa5067a779 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 9 Nov 2026 16:09:54 +0000 Subject: [PATCH 2/4] test_load_yaml_refusals.py: a path typed relative to the folder a tool runs in is named as typed and spelled by `adopt-project.py check --plan`, and `_load`'s row builds its file before, not inside, `pytest.raises` (#1168) rv1274 and rv1274b found two things in the rows of #1274, each with no change to production code. Every tool row typed its path under `tmp_path`, which is absolute, so a `load_yaml` that named `Path(path).absolute()` (rv1274's X5) passed all of them. The new row, `test_a_path_typed_relative_is_named_as_typed_and_spelled` (U+202E, ESC and a line feed), types `/adoption-plan.yaml` and runs `adopt-project.py check --plan` with that folder's parent as the working directory: the refusal names the path as typed, still relative, as `repr` spells it. SonarCloud python:S5778 (MAJOR) flagged `unparsable(root / "project.yaml")` inside the `pytest.raises` block of `test_the_gates_load_takes_either_prefix` beside `gate._load`. The file is made first, so only `gate._load` runs inside the block; the row's ids and verdicts are the same. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_load_yaml_refusals.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/test_load_yaml_refusals.py b/tests/test_load_yaml_refusals.py index 0ce60530..df92d5b5 100644 --- a/tests/test_load_yaml_refusals.py +++ b/tests/test_load_yaml_refusals.py @@ -328,6 +328,26 @@ def test_each_tool_names_the_file_load_yaml_refuses_as_shown_path_names_it( assert_shown_safe(result.stdout, result.stderr) +@pytest.mark.parametrize("name", UNPRINTABLE) +def test_a_path_typed_relative_is_named_as_typed_and_spelled(tmp_path, name): + """The rows above type a path under `tmp_path`, which is absolute, so a + `load_yaml` that named `Path(path).absolute()` would pass every one of + them (rv1274's X5). This one types the file relative to the folder the + tool runs in: the refusal names it as typed, still relative, and spelled + as `repr` spells it. `adopt-project.py check --plan` is one of the tools + that names the file it was handed; the four that are given a root build + an absolute path of their own before `load_yaml` sees it.""" + plan = unparsable(unsafe_folder(tmp_path, name) / "adoption-plan.yaml") + typed = str(plan.relative_to(tmp_path)) + result = run_script(ADOPT, "check", "--plan", typed, cwd=tmp_path) + said = result.stdout + result.stderr + assert result.returncode == 2, said + assert refused_lines(result.stderr) == [ + f"REFUSED yaml-unparsable: {spelt(typed)}: {NOT_PARSED}"], ascii(said) + assert chr(0x202E) not in said, ascii(said) + assert_shown_safe(result.stdout, result.stderr) + + # --- the readers that find the path in the detail by its text --------------- #: The commit before #1168: its `repo_shape.py` names the file raw, and its @@ -445,8 +465,9 @@ def test_the_gates_load_takes_either_prefix(tmp_path, monkeypatch, name, "load_yaml_refusals_pins") monkeypatch.setattr(gate, "load_yaml", loader) root = unsafe_folder(tmp_path, name) + manifest = unparsable(root / "project.yaml") with pytest.raises(Refusal) as caught: - gate._load(root, unparsable(root / "project.yaml")) + gate._load(root, manifest) assert caught.value.detail == f"project.yaml: {NOT_PARSED}" From 771667f10b66d328002b1e973539c801d4123bdc Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 9 Nov 2026 16:50:28 +0000 Subject: [PATCH 3/4] repo_shape.py, path_classify.py, bump-leg.py, validate-repository-naming.py, update-shape.py and the root's validate-manifest.py: not-a-mapping and wrong-kind refusals name the file as `shown_path` does, so no U+202E, ESC or line feed is raw (#1279) #1168 (#1274) spelled the file in `load_yaml`'s three refusals and in `policy-unreadable`. The refusals on the next line of each reader, where the YAML parsed but is a list or declares another kind, still named the path raw: a bidi override reordered the line, an ESC reached the terminal, and a line feed began a line of the path's choosing. Each is spelled now: `policy-wrong-kind`, the naming gate's `manifest-unreadable`, `bump-leg-manifest-unreadable` (its other site was spelled already), `path-policy-unreadable`, `path-policy-wrong-kind`, `update-pin-unreadable` and the root's `manifest-unreadable`. The two copied gates use the `_shown` each already carries, so no pinned copy imports a new name. A printable path is named byte for byte as before. `adopt-project.py`'s `plan-unreadable` is left to #1177, which spells it with `_shown`; `validate-pins.py`'s `manifest-unreadable` names `project.yaml` and no path. `shape-doctor.py`'s `naming_script_refusal` docstring and two doctor rows in `tests/test_load_yaml_refusals.py` follow `policy-wrong-kind`: the raw row builds the line a root's older library prints. New rows in `tests/test_mapping_and_kind_refusals_shown.py`. Closes #1279 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/bump-leg.py | 2 +- scripts/path_classify.py | 7 +- scripts/repo_shape.py | 2 +- scripts/validate-repository-naming.py | 2 +- shape-doctor.py | 9 +- .../scripts/validate-manifest.py | 3 +- tests/test_load_yaml_refusals.py | 23 ++- tests/test_mapping_and_kind_refusals_shown.py | 166 ++++++++++++++++++ update-shape.py | 7 +- 9 files changed, 202 insertions(+), 19 deletions(-) create mode 100644 tests/test_mapping_and_kind_refusals_shown.py diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 5e86be7c..229fb9c8 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -234,7 +234,7 @@ def open(cls, root_spec) -> "Project": data = load_yaml(path) if not isinstance(data, dict): raise Refusal("bump-leg-manifest-unreadable", - f"{path}: not a mapping") + f"{shown_path(path)}: not a mapping") if data.get("kind") != "project-manifest": raise Refusal( "bump-leg-wrong-kind", diff --git a/scripts/path_classify.py b/scripts/path_classify.py index a61d5753..da32ebba 100644 --- a/scripts/path_classify.py +++ b/scripts/path_classify.py @@ -42,7 +42,7 @@ from pathlib import Path sys.path.insert(0, str(Path(__file__).resolve().parent)) -from repo_shape import Refusal, load_yaml # noqa: E402 +from repo_shape import Refusal, load_yaml, shown_path # noqa: E402 LEGS = ("spec", "code", "root", "ambiguous") #: The answers a rule that asks may name as its `default:` (#528). `drop` @@ -182,11 +182,12 @@ def __init__(self, data: dict): def load(cls, path: Path) -> "PathPolicy": data = load_yaml(path) if not isinstance(data, dict): - raise Refusal("path-policy-unreadable", f"{path}: not a mapping") + raise Refusal("path-policy-unreadable", + f"{shown_path(path)}: not a mapping") if data.get("kind") != "path-classification-policy": raise Refusal( "path-policy-wrong-kind", - f"{path}: kind is {data.get('kind')!r}, expected " + f"{shown_path(path)}: kind is {data.get('kind')!r}, expected " "'path-classification-policy'") return cls(data) diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index fee1343f..7da852d5 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -2251,7 +2251,7 @@ def load(cls, path: Path) -> "NamingPolicy": if data.get("kind") != "repository-naming-policy": raise Refusal( "policy-wrong-kind", - f"{path}: kind is {data.get('kind')!r}, expected " + f"{shown_path(path)}: kind is {data.get('kind')!r}, expected " "'repository-naming-policy'", ) return cls(data) diff --git a/scripts/validate-repository-naming.py b/scripts/validate-repository-naming.py index 4f3d7977..a2147469 100755 --- a/scripts/validate-repository-naming.py +++ b/scripts/validate-repository-naming.py @@ -1137,7 +1137,7 @@ def _project_manifest(path: Path) -> dict: """The `project.yaml` mapping `--project` names, or a `Refusal`.""" data = load_yaml(path) if not isinstance(data, dict): - raise Refusal("manifest-unreadable", f"{path}: not a mapping") + raise Refusal("manifest-unreadable", f"{_shown(path)}: not a mapping") return data diff --git a/shape-doctor.py b/shape-doctor.py index d31f6648..987b3ad5 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -2761,10 +2761,11 @@ def naming_script_refusal(shape: Path, whole: str) -> Refusal | None: `project.yaml` the script was asked to read -- each is about what the script was asked, and is not this. - IN EITHER SPELLING (#1168): `load_yaml` and `policy-unreadable` name the - file as `shown_path` does, `repr`'d where this checkout's path holds a - character that is not printable, and `policy-wrong-kind` names it raw. - For a printable path the two are one. + IN EITHER SPELLING (#1168): `load_yaml`, `policy-unreadable` and, since + #1279, `policy-wrong-kind` name the file as `shown_path` does, `repr`'d + where this checkout's path holds a character that is not printable, and + a root's `repo_shape.py` from before them names it raw. For a printable + path the two are one. """ policy = f"{shape / NAMING_COPY}: " spelt = f"{shown_path(shape / NAMING_COPY)}: " diff --git a/templates/assembly-root/scripts/validate-manifest.py b/templates/assembly-root/scripts/validate-manifest.py index 1f9df591..1410d1e6 100755 --- a/templates/assembly-root/scripts/validate-manifest.py +++ b/templates/assembly-root/scripts/validate-manifest.py @@ -1435,7 +1435,8 @@ def main(argv: list[str] | None = None) -> int: ) manifest = _load(root, manifest_path) if not isinstance(manifest, dict): - raise Refusal("manifest-unreadable", f"{manifest_path}: not a mapping") + raise Refusal("manifest-unreadable", + f"{_shown(manifest_path)}: not a mapping") policy_path = args.policy or (root / "contracts" / "repository-naming.yaml") policy = _load(root, policy_path, NamingPolicy.load) # Inside the `try`: a declared referent's pin file this user cannot diff --git a/tests/test_load_yaml_refusals.py b/tests/test_load_yaml_refusals.py index df92d5b5..fe3358e5 100644 --- a/tests/test_load_yaml_refusals.py +++ b/tests/test_load_yaml_refusals.py @@ -528,13 +528,17 @@ def policy_refusal(shape: Path, text: str) -> Refusal: @pytest.mark.parametrize("text, code", [ pytest.param(UNPARSABLE, "yaml-unparsable", id="load-yaml"), pytest.param("- a list\n", "policy-unreadable", id="not-a-mapping"), + pytest.param("kind: something-else\n", "policy-wrong-kind", + id="wrong-kind"), ]) @pytest.mark.parametrize("name", NAMES) def test_the_doctor_finds_its_own_policy_in_the_naming_scripts_refusal( tmp_path, name, text, code): """`naming_script_refusal` knows the checkout's own policy by the path - at the start of the detail. `load_yaml` and `policy-unreadable` spell - it now, and the doctor asks that spelling too.""" + at the start of the detail. `load_yaml`, `policy-unreadable` and, since + #1279, `policy-wrong-kind` spell it now, and the doctor asks that + spelling too. Named raw, a line feed split the wrong kind's line, and + the doctor did not find its own policy in it.""" shape = unsafe_folder(tmp_path, name) refusal = policy_refusal(shape, text) found = doctor_module().naming_script_refusal(shape, f"{refusal}\n") @@ -547,12 +551,17 @@ def test_the_doctor_finds_its_own_policy_in_the_naming_scripts_refusal( pytest.param(PRINTABLE_NAME, id="printable"), ]) def test_the_doctor_still_finds_its_own_policy_named_raw(tmp_path, name): - """`policy-wrong-kind` names the policy raw, as it always did, and the - doctor still finds it by that prefix. A line feed splits that line, so - it is not among the names here, as on main.""" + """A root's `repo_shape.py` from before #1279 names the policy raw in + `policy-wrong-kind`, and the doctor still finds it by that prefix. The + line is built as that library prints it, since this checkout's spells + it. A line feed splits that line, so it is not among the names here.""" shape = unsafe_folder(tmp_path, name) - refusal = policy_refusal(shape, "kind: something-else\n") - assert refusal.code == "policy-wrong-kind" + policy = policy_refusal(shape, "kind: something-else\n") + assert policy.code == "policy-wrong-kind" + refusal = Refusal(policy.code, + f"{shape / 'contracts' / 'repository-naming.yaml'}: " + "kind is 'something-else', expected " + "'repository-naming-policy'") found = doctor_module().naming_script_refusal(shape, f"{refusal}\n") assert found is not None, ascii(str(refusal)) assert found.detail == refusal.detail diff --git a/tests/test_mapping_and_kind_refusals_shown.py b/tests/test_mapping_and_kind_refusals_shown.py new file mode 100644 index 00000000..595c8a36 --- /dev/null +++ b/tests/test_mapping_and_kind_refusals_shown.py @@ -0,0 +1,166 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A file that reads as YAML but is not a mapping, or is a policy of another +kind, is refused naming the file as `shown_path` names it (#1279). + +#1168 gave that spelling to `load_yaml`'s three refusals and to +`policy-unreadable`. The refusals on the next line of each reader, where the +YAML parsed but is a list, or declares another `kind:`, still named the path +raw: a bidi override, U+202E, in a folder's name reordered the line, an ESC +reached the terminal as the control it is, and a line feed began a line of +the path's choosing. Each is spelled now: + +- `policy-wrong-kind`, in `repo_shape.NamingPolicy.load`; +- `manifest-unreadable`, the naming gate's `--project`; +- `bump-leg-manifest-unreadable`, `bump-leg.py --root`, where `project.yaml` + is not a mapping (a root it cannot search was spelled already); +- `path-policy-unreadable` and `path-policy-wrong-kind`, in + `path_classify.PathPolicy.load`, which `adopt-project.py plan + --path-policy` reads; +- `update-pin-unreadable`, `update-shape.py check --root`; +- the root's `manifest-unreadable`, its own `validate-manifest.py`. + +Each run is one of `test_load_yaml_refusals.py`'s, with the file it builds +holding YAML that parses. The naming gate and the root's +`validate-manifest.py` are copied into every project, beside a +`scripts/repo_shape.py` that may be older than they are, so each spells the +path with the `_shown` it already carries and imports nothing new: the rows +that run them beside a library from before #1168 hold that. A printable path +is named byte for byte as it always was. `adopt-project.py`'s +`plan-unreadable` is #1177's. + +NO NETWORK AND NO GITHUB: temporary files, local repositories, and the +session's `gh` guard first on PATH for every tool run. +""" + +from __future__ import annotations + +import shutil +from pathlib import Path + +import pytest + +from conftest import ( + REPO, assert_shown_safe, named_as, run_script, unsafe_folder, +) +from test_load_yaml_refusals import ( + NAMES, TEMPLATE_SCRIPTS, TOOLS, a_root, older_library, refused_lines, +) + +#: YAML that parses, to a list, and to a mapping of another kind. +A_LIST = "- a\n" +ANOTHER_KIND = "kind: other\n" + +#: What each refusal says after the path. +NOT_A_MAPPING = "not a mapping" +NOT_A_NAMING_POLICY = "kind is 'other', expected 'repository-naming-policy'" +NOT_A_PATH_POLICY = "kind is 'other', expected 'path-classification-policy'" + +#: The issue's table, less #1177's `plan-unreadable`: each refusal, as +#: `(code, the run in test_load_yaml_refusals.TOOLS that reaches it, what +#: its file holds, what the refusal says after the path)`. +SITES = { + "policy-wrong-kind": ( + "policy-wrong-kind", "naming--policy", ANOTHER_KIND, + NOT_A_NAMING_POLICY), + "manifest-unreadable-naming--project": ( + "manifest-unreadable", "naming--project", A_LIST, NOT_A_MAPPING), + "bump-leg-manifest-unreadable": ( + "bump-leg-manifest-unreadable", "bump-leg--root", A_LIST, + NOT_A_MAPPING), + "path-policy-unreadable": ( + "path-policy-unreadable", "adopt-plan--path-policy", A_LIST, + NOT_A_MAPPING), + "path-policy-wrong-kind": ( + "path-policy-wrong-kind", "adopt-plan--path-policy", ANOTHER_KIND, + NOT_A_PATH_POLICY), + "update-pin-unreadable": ( + "update-pin-unreadable", "update-shape-check--root", A_LIST, + NOT_A_MAPPING), + "manifest-unreadable-validate-manifest": ( + "manifest-unreadable", "validate-manifest--root", A_LIST, + NOT_A_MAPPING), +} + + +def holding(path: Path, text: str) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return path + + +def assert_refused_once(result, code: str, named: str, said: str) -> None: + """Exit 2, one `REFUSED` line, exactly `REFUSED : : + `, and no U+202E, ESC or forged line anywhere in the output.""" + output = result.stdout + result.stderr + assert result.returncode == 2, ascii(output) + assert refused_lines(result.stderr) == [ + f"REFUSED {code}: {named}: {said}"], ascii(output) + assert chr(0x202E) not in output, ascii(output) + assert_shown_safe(result.stdout, result.stderr) + + +@pytest.mark.parametrize("name", NAMES) +@pytest.mark.parametrize("site", sorted(SITES)) +def test_each_refusal_names_the_file_as_shown_path_names_it( + tmp_path, site, name): + """THE ISSUE'S PROBE, one real run per refusal: the file named as `repr` + spells it where its folder's name holds U+202E, an ESC or a line feed, + so nothing of that name reaches the terminal raw and the detail stays + one line. At #1274's head each named it raw. The printable row is the + control, named byte for byte as before, with no quotes.""" + code, tool, text, said = SITES[site] + path, argv = TOOLS[tool](unsafe_folder(tmp_path, name), tmp_path) + holding(path, text) + result = run_script(*argv, cwd=tmp_path) + assert_refused_once(result, code, named_as(path, name), said) + + +#: The naming gate, which every project carries as it is here. +NAMING = REPO / "scripts" / "validate-repository-naming.py" + +#: The root's own manifest gate. +MANIFEST_GATE = "validate-manifest.py" + + +def naming_gate(folder: Path, library: bytes) -> tuple: + """The naming gate as a root carries it, beside `library` and this + checkout's naming policy, asked of a `project.yaml` that is a list.""" + root = a_root(folder, NAMING.read_bytes(), library, NAMING.name) + policy = root / "contracts" / "repository-naming.yaml" + policy.parent.mkdir() + shutil.copyfile(REPO / "contracts" / "repository-naming.yaml", policy) + manifest = holding(folder / "n" / "project.yaml", A_LIST) + return manifest, (root / "scripts" / NAMING.name, "--project", + str(manifest)) + + +def manifest_gate(folder: Path, library: bytes) -> tuple: + """The root's `validate-manifest.py` beside `library`, over a root whose + `project.yaml` is a list.""" + root = a_root(folder, (TEMPLATE_SCRIPTS / MANIFEST_GATE).read_bytes(), + library, MANIFEST_GATE) + return holding(root / "project.yaml", A_LIST), ( + root / "scripts" / MANIFEST_GATE, "--root", str(root)) + + +#: The two copied gates whose own refusal changed. +COPIED_GATES = { + "naming-gate--project": naming_gate, + "validate-manifest--root": manifest_gate, +} + + +@pytest.mark.parametrize("name", NAMES) +@pytest.mark.parametrize("gate", sorted(COPIED_GATES)) +def test_a_copied_gate_spells_the_path_beside_an_older_library( + tmp_path, gate, name): + """THE SKEW RULE: each copied gate beside a root's `repo_shape.py` from + before #1168, which has none of #1279's lines. The gate spells the path + with its own `_shown`, so it runs, refuses `manifest-unreadable` by + name, and spells the path as it does beside this checkout's library.""" + path, argv = COPIED_GATES[gate](unsafe_folder(tmp_path, name), + older_library()) + result = run_script(*argv, cwd=tmp_path) + assert "Traceback" not in result.stderr, ascii(result.stderr) + assert_refused_once(result, "manifest-unreadable", named_as(path, name), + NOT_A_MAPPING) diff --git a/update-shape.py b/update-shape.py index 200cb843..8036ad9f 100755 --- a/update-shape.py +++ b/update-shape.py @@ -182,6 +182,10 @@ # as `parse_yaml` does, and put it back (#933). On a line of its own, for the # reason the branch rule's import below gives. from repo_shape import BYTE_ORDER_MARK # noqa: E402 +# How a refusal names a path it was handed (#1279): as it is, or as `repr` +# spells it where a character in it is not printable, as `load_yaml` names +# the same pin since #1168. On a line of its own, as the two above are. +from repo_shape import shown_path # noqa: E402 from shape_materialize import ( # noqa: E402 ATTRIBUTES, COPIED_FROM_SHAPE, COPIED_VERBATIM, FAMILY_COPIED_FROM_SHAPE, FAMILY_COPIED_VERBATIM, SHAPE_REPOSITORY, CommandFailed, check_program, @@ -838,7 +842,8 @@ def check_pin(pin, pin_path, root: Path) -> tuple[dict, list[Row]]: whose history a refusal's `git log` reads (#290). """ if not isinstance(pin, dict): - raise Refusal("update-pin-unreadable", f"{pin_path}: not a mapping") + raise Refusal("update-pin-unreadable", + f"{shown_path(pin_path)}: not a mapping") commit = str(pin.get("commit") or "").lower() if not COMMIT_RE.match(commit): raise Refusal( From 0270d48d7361ed599606ab0eae99eac479eba9d5 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 9 Nov 2026 22:20:11 +0000 Subject: [PATCH 4/4] test_mapping_and_kind_refusals_shown.py: a row pins the spelling of `bump-leg-manifest-unreadable` for a root this user cannot search; two docstrings name this checkout's older library, not a root's, as what prints a raw `policy-wrong-kind` (#1279) rv1284 (independent review of #1284, READY at 771667f10) found one SHOULD and three NITs, all fixed here and nothing else changed. SHOULD 1: `_manifest_unopened_refusal`, the second site of `bump-leg-manifest-unreadable`, was spelled with `shown_path` already, and no row ran it under a path holding U+202E, an ESC or a line feed: with it put back to a raw `{path}`, every file of the gate still passed. A new row over `NAMES` runs `bump-leg.py --root` over a root at mode 000, through `test_bump_leg_unsearchable.py`'s `unsearchable_root`, and asserts exactly `REFUSED bump-leg-manifest-unreadable: : cannot be read: Permission denied`. It passes on #1274's head too: it pins a spelling that was right. NIT 1: `naming_script_refusal` reads only the naming script of the checkout the doctor runs from, which imports that checkout's library, so a raw `policy-wrong-kind` prefix appears only where that library is older than its doctor. A root's library never reaches the function. The docstring of the function and of the changed raw row said "a root's `repo_shape.py`". NIT 2: the older-library rows of the new test file run beside a library that already has `shown_path`, so they cannot catch a gate that imports it; the file's docstring now credits `test_pinned_copy_text.py`'s `test_no_pinned_script_imports_what_an_older_library_lacks` for that import. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- shape-doctor.py | 8 ++-- tests/test_load_yaml_refusals.py | 11 ++++-- tests/test_mapping_and_kind_refusals_shown.py | 37 ++++++++++++++++--- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 987b3ad5..e8f122da 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -2763,9 +2763,11 @@ def naming_script_refusal(shape: Path, whole: str) -> Refusal | None: IN EITHER SPELLING (#1168): `load_yaml`, `policy-unreadable` and, since #1279, `policy-wrong-kind` name the file as `shown_path` does, `repr`'d - where this checkout's path holds a character that is not printable, and - a root's `repo_shape.py` from before them names it raw. For a printable - path the two are one. + where this checkout's path holds a character that is not printable. It + is named raw only where this checkout's own `repo_shape.py` is older + than its doctor: the script that prints the line is this checkout's, and + imports this checkout's library, so a root's library never reaches this + function. For a printable path the two are one. """ policy = f"{shape / NAMING_COPY}: " spelt = f"{shown_path(shape / NAMING_COPY)}: " diff --git a/tests/test_load_yaml_refusals.py b/tests/test_load_yaml_refusals.py index fe3358e5..da9734cb 100644 --- a/tests/test_load_yaml_refusals.py +++ b/tests/test_load_yaml_refusals.py @@ -551,10 +551,13 @@ def test_the_doctor_finds_its_own_policy_in_the_naming_scripts_refusal( pytest.param(PRINTABLE_NAME, id="printable"), ]) def test_the_doctor_still_finds_its_own_policy_named_raw(tmp_path, name): - """A root's `repo_shape.py` from before #1279 names the policy raw in - `policy-wrong-kind`, and the doctor still finds it by that prefix. The - line is built as that library prints it, since this checkout's spells - it. A line feed splits that line, so it is not among the names here.""" + """A `repo_shape.py` from before #1279 names the policy raw in + `policy-wrong-kind`, and the doctor still finds it by that prefix. Only + this checkout's own library can print that line: the naming script the + doctor reads is this checkout's and imports this checkout's library, so + the library is one older than its doctor, never a root's. The line is + built as that library prints it, since this checkout's spells it. A + line feed splits that line, so it is not among the names here.""" shape = unsafe_folder(tmp_path, name) policy = policy_refusal(shape, "kind: something-else\n") assert policy.code == "policy-wrong-kind" diff --git a/tests/test_mapping_and_kind_refusals_shown.py b/tests/test_mapping_and_kind_refusals_shown.py index 595c8a36..6172403c 100644 --- a/tests/test_mapping_and_kind_refusals_shown.py +++ b/tests/test_mapping_and_kind_refusals_shown.py @@ -12,7 +12,8 @@ - `policy-wrong-kind`, in `repo_shape.NamingPolicy.load`; - `manifest-unreadable`, the naming gate's `--project`; - `bump-leg-manifest-unreadable`, `bump-leg.py --root`, where `project.yaml` - is not a mapping (a root it cannot search was spelled already); + is not a mapping (a root it cannot search was spelled already, and the + last row below pins that spelling); - `path-policy-unreadable` and `path-policy-wrong-kind`, in `path_classify.PathPolicy.load`, which `adopt-project.py plan --path-policy` reads; @@ -23,10 +24,13 @@ holding YAML that parses. The naming gate and the root's `validate-manifest.py` are copied into every project, beside a `scripts/repo_shape.py` that may be older than they are, so each spells the -path with the `_shown` it already carries and imports nothing new: the rows -that run them beside a library from before #1168 hold that. A printable path -is named byte for byte as it always was. `adopt-project.py`'s -`plan-unreadable` is #1177's. +path with the `_shown` it already carries: the rows that run them beside a +library from before #1168 hold that for any name that library lacks. It has +`shown_path` already, so a gate that imported that name would pass those rows; +the import is held out by `test_pinned_copy_text.py`'s +`test_no_pinned_script_imports_what_an_older_library_lacks`. +A printable path is named byte for byte as it always was. +`adopt-project.py`'s `plan-unreadable` is #1177's. NO NETWORK AND NO GITHUB: temporary files, local repositories, and the session's `gh` guard first on PATH for every tool run. @@ -42,6 +46,8 @@ from conftest import ( REPO, assert_shown_safe, named_as, run_script, unsafe_folder, ) +from test_bump_leg_unopenable import needs_a_mode +from test_bump_leg_unsearchable import unsearchable_root from test_load_yaml_refusals import ( NAMES, TEMPLATE_SCRIPTS, TOOLS, a_root, older_library, refused_lines, ) @@ -164,3 +170,24 @@ def test_a_copied_gate_spells_the_path_beside_an_older_library( assert "Traceback" not in result.stderr, ascii(result.stderr) assert_refused_once(result, "manifest-unreadable", named_as(path, name), NOT_A_MAPPING) + + +@needs_a_mode +@pytest.mark.parametrize("name", NAMES) +def test_a_root_that_cannot_be_searched_is_named_as_shown_path_names_it( + tmp_path, name): + """THE SPELLING THIS CHANGE LEFT ALONE: a `--root` + this user cannot search is `bump-leg-manifest-unreadable` from + `_manifest_unopened_refusal`, which names `project.yaml` as `shown_path` + does and did before #1279. No other row ran it under a path holding + U+202E, an ESC or a line feed (`test_bump_leg_unsearchable.py` uses a + printable one), so it was one `{path}` from raw with every row green. + This row passes on #1274's head too: it pins a spelling, and fails + nothing first. The root's mode is put back in a `finally`.""" + path, argv = TOOLS["bump-leg--root"](unsafe_folder(tmp_path, name), + tmp_path) + with unsearchable_root(path.parent, 0): + result = run_script(*argv, cwd=tmp_path) + assert_refused_once(result, "bump-leg-manifest-unreadable", + named_as(path, name), "cannot be read: Permission " + "denied")