From a30091f0bd1c2053194e3183d33fe646b84ecc17 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:29:05 +0000 Subject: [PATCH 1/9] scaffold-project.py, validate-pins.py, repo_shape.py: two neutral products whose names differ only in `-`, `.` or `_` share one `SHAPE_PIN_SOURCE_`, so `--pin` refuses the pair and `validate-pins.py` reports it as `neutral-pin-source-collides` and reads the variable for neither, where one checkout answered for both pins and the other was a `neutral-pin-source-unreadable` finding (#275) `pin_source_env_name` maps every character outside `[A-Z0-9_]` to `_`, so `open-glass`, `open_glass` and `open.glass` are three legal names, three pin files and ONE variable. `scaffold-project.py --pin open-glass@ --pin open_glass@` scaffolded the pair (exit 0); the project's `validate-pins.py` then gave whichever checkout `SHAPE_PIN_SOURCE_OPEN_GLASS` named to BOTH pins and reported the one it was not a checkout of as `neutral-pin-source-unreadable` (exit 1, a red `validate.yml`); with no source, both skip notes offered the same variable as the way to verify either. Every variable name is kept: an operator may have set one and the docs name them. The pair is refused where it is declared instead. `repo_shape.pin_source_env_collisions(products)` is the one pure answer, `{variable: [products]}` for each variable two or more DISTINCT products share, distinct after a casefold (the fold of the pin file, `--pin-source` and `--link-source`), so one product typed twice or in two cases is never a collision (that is #241's file collision). `pin_source_env_shared` is the set of casefolded names it implies. - `scaffold-project.py`: `_refuse_colliding_pins` names each variable's `--pin` arguments as typed after #241's sentences, in one `scaffold-pin-collides` refusal, dry run and real run alike, before any process starts and ahead of the leg-path refusal. #241's own wording and remediation are unchanged when only a file collides. - `validate-pins.py`: one `neutral-pin-source-collides` finding per variable, naming every product and the variable. A product whose variable is shared does not read it (rule 2 of `resolve_neutral_pin_source`) and its skip hint stops offering it, so the pin the variable was not a checkout of is no longer reported against the project. A variable one product has to itself reads and is offered exactly as before. - `repo_shape.py` chain readers: `link_pins_from_trees` does not read a variable two links of the chain share, and the `declared-unverified` hint stops naming it. Only a hand-written chain can have two such links. `adopt-project.py` is not changed: its `--pin` half arrives with #273, which routes adopt through `pin_source_env_collisions`. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- scaffold-project.py | 74 ++- scripts/repo_shape.py | 82 ++- .../assembly-root/scripts/validate-pins.py | 67 +- tests/test_pin_source_env_collisions.py | 604 ++++++++++++++++++ 4 files changed, 810 insertions(+), 17 deletions(-) create mode 100644 tests/test_pin_source_env_collisions.py diff --git a/scaffold-project.py b/scaffold-project.py index 4836a526..eda1fd07 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -58,6 +58,9 @@ COMMIT_RE, NEUTRAL_PRODUCT_OWNER, PROJECT_ID_RE, TREE_DIGEST_DEFINITION, VISIBILITY_CHOICES, NamingPolicy, Refusal, accepts_role, checked_value, git_out, link_pins_from_trees, repo_basename, tree_digest, + # Which `--pin` products share a `SHAPE_PIN_SOURCE_` variable: the + # one answer, which the project's own `validate-pins.py` reports on (#275). + PIN_SOURCE_ENV_PREFIX, pin_source_env_collisions, ) from shape_materialize import ( # noqa: E402 RULESET_HINT, SHAPE_REPOSITORY, CommandFailed, @@ -449,6 +452,19 @@ def _exists_remedy(role: str, empty: bool, reuse_flag: bool) -> str: "once, by the name of the repository it pins." ) +#: What a refused pair of `--pin`s whose products share a source variable is +#: told to do (#275), after what `PIN_COLLISION_REMEDIATION` says when a file +#: collides too. +PIN_VARIABLE_REMEDIATION = ( + f"`{PIN_SOURCE_ENV_PREFIX}` names the local checkout of a " + "product, and is its name upper-cased with every character outside `A-Z`, " + "`0-9` and `_` turned into `_`, so names that differ only in `-`, `.` or " + "`_` share one. The project's own `validate-pins.py` reports such a pair " + "as `neutral-pin-source-collides`, and a variable two pins share is read " + "for neither. Pass one product of each such group, or products whose " + "names differ by more." +) + def _refuse_colliding_pins(pins, owner: str) -> None: """No `--pin` is written where the shape writes a file of its own, and no @@ -489,6 +505,13 @@ def _refuse_colliding_pins(pins, owner: str) -> None: contracts` was told it contains that file "and 4 more" where there were three (#197's re-review, N1). Refused first, no pin path it lists is the shape's or listed twice. + + TWO PINS THAT SHARE A SOURCE VARIABLE ARE REFUSED HERE TOO (#275), in the + same refusal and after the file collisions, by `_colliding_pin_variables`: + `--pin open-glass@ --pin open_glass@` are two files and one + `SHAPE_PIN_SOURCE_OPEN_GLASS`. The refusal is the one the project's own + `validate-pins.py` would make as a finding, said before three repositories + exist. """ # Folded on both sides, as `path_overlap` folds the leg paths: a future # mixed-case shape file under `contracts/` is still one path with a @@ -496,10 +519,12 @@ def _refuse_colliding_pins(pins, owner: str) -> None: shape_files = {shape_file.casefold() for shape_file in assembly_root_paths()} written: dict[str, list[tuple[str, str]]] = {} + typed_pins: list[tuple[str, str]] = [] for raw in pins: product = parse_pin(raw, owner)[0] path = neutral_pin_path(product) written.setdefault(path, []).append((raw, product)) + typed_pins.append((raw, product)) collisions = [] for path, typed in written.items(): in_shape = path.casefold() in shape_files @@ -519,9 +544,52 @@ def _refuse_colliding_pins(pins, owner: str) -> None: collisions.append( f"{said} at one file, `{path}`, which holds one pin: pass " "each product once.") - if collisions: - raise Refusal("scaffold-pin-collides", " ".join(collisions), - PIN_COLLISION_REMEDIATION) + shared = _colliding_pin_variables(typed_pins) + if collisions or shared: + raise Refusal("scaffold-pin-collides", " ".join(collisions + shared), + _pin_collision_remediation(bool(collisions), + bool(shared))) + + +def _colliding_pin_variables(typed_pins) -> list[str]: + """One sentence for each `SHAPE_PIN_SOURCE_` variable that two or + more of the products `--pin` names share, each argument as it was typed, + in the order each variable's first `--pin` was (#275). + + `open-glass`, `open_glass` and `open.glass` are three pin files and three + legal `--pin` names, and one variable: whichever checkout it names answers + for all of them, and the project's own `validate-pins.py` reported the + pin it was not a checkout of as `neutral-pin-source-unreadable`. The + question is `repo_shape.pin_source_env_collisions`'s, so it is the one + `validate-pins.py` answers, and it takes products as their pin files do: + the same product typed twice is #241's `one file`, said above, and is + never a collision here. + """ + typed = {} + for raw, product in typed_pins: + typed.setdefault(product.casefold(), []).append(raw) + out = [] + for variable, products in pin_source_env_collisions( + product for _, product in typed_pins).items(): + arguments = [f"--pin {raw!r}" for product in products + for raw in typed[product.casefold()]] + out.append( + " and ".join(arguments) + " name the products " + + " and ".join(f"`{product}`" for product in products) + + f", and one variable, `{variable}`, names the local checkout of " + "each: it cannot say which it holds.") + return out + + +def _pin_collision_remediation(file_collision: bool, + variable_collision: bool) -> str: + """The remediation for the collisions a refusal names: #241's alone for a + file, the variable's alone, or both said once each.""" + if not variable_collision: + return PIN_COLLISION_REMEDIATION + if not file_collision: + return f"Remediation: nothing was created. {PIN_VARIABLE_REMEDIATION}" + return f"{PIN_COLLISION_REMEDIATION} {PIN_VARIABLE_REMEDIATION}" def _leg_collision_remediation(neutral_pins=()) -> str: diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 38b45290..439b6cce 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -1092,6 +1092,14 @@ def recorded_gitlink(repo: Path, path: str) -> str | None: #: ONE variable answers both questions asked of such a checkout: the digest #: `validate-pins.py` recomputes from it, and the chain-link declaration #: `resolve_referent` verifies against it. +#: +#: THE NAME IS NOT INJECTIVE (#275). `.`, `-` and `_` are all legal in a +#: product's name and all become `_`, so `open-glass`, `open_glass` and +#: `open.glass` share `SHAPE_PIN_SOURCE_OPEN_GLASS` while naming three pin +#: files. Every existing name is kept, because an operator may have set it and +#: the docs name it; what is refused instead is DECLARING two products that +#: share one (`pin_source_env_collisions`), and a variable two declared +#: products share answers for neither. PIN_SOURCE_ENV_PREFIX = "SHAPE_PIN_SOURCE_" @@ -1099,6 +1107,49 @@ def pin_source_env_name(product: str) -> str: return PIN_SOURCE_ENV_PREFIX + re.sub(r"[^A-Z0-9_]", "_", product.upper()) +def pin_source_env_collisions(products) -> dict[str, list[str]]: + """The variables two or more DISTINCT products share, each with them: + `{variable: [product, ...]}` (#275). + + PURE, and the one place the question is answered, so `scaffold-project.py`, + `validate-pins.py` and the chain readers below cannot disagree about it. + `products` is any iterable of product NAMES (`openGlass`, never + `opensoft/openGlass@`): an entry that is not a non-empty string is + not a name and is left out, as `validate-pins.py` reports it as + malformed on its own. + + DISTINCT MEANS DISTINCT AFTER A CASEFOLD, the fold every other lookup of a + product makes (`contracts/-pin.yaml`, the key of + `--pin-source` and `--link-source`). `openGlass` and `OpenGlass` are one + product and name one variable rightly: whether the same product is + declared twice is a different question, which `scaffold-project.py` + answers as `scaffold-pin-collides` (#241). A product listed twice is + therefore never a collision here, and two that differ in more than case + are, when `pin_source_env_name` maps them to one name. + + Variables are in the order their first product was given, and each + group's products are in that order too, each in the spelling it was first + given in. A variable only one product names is not in the result. + """ + by_variable: dict[str, dict[str, str]] = {} + for product in products or (): + if not isinstance(product, str) or not product: + continue + group = by_variable.setdefault(pin_source_env_name(product), {}) + group.setdefault(product.casefold(), product) + return {variable: list(group.values()) + for variable, group in by_variable.items() if len(group) > 1} + + +def pin_source_env_shared(products) -> set[str]: + """The casefolded names, among `products`, whose variable another of them + shares (#275): the products a variable must not answer for, because it + does not say which.""" + return {product.casefold() + for group in pin_source_env_collisions(products).values() + for product in group} + + def form_id(family_id: str, role_id: str | None) -> str: """`("project-leg", "assembly")` -> `"project-leg/assembly"`.""" return f"{family_id}/{role_id}" if role_id else family_id @@ -1252,15 +1303,20 @@ def _chain_link_findings(chain, link_pins) -> tuple: available = _link_declarations(link_pins) warnings: list[str] = [] unverified: list[str] = [] + # A link whose variable another link of the chain shares is never offered + # that variable: it would be read for both (#275). + shared = pin_source_env_shared(chain) for holder, held in zip(chain, chain[1:]): declared = available.get(holder.casefold()) if declared is None: unverified.append(holder) + offered = ("a sibling checkout or" if holder.casefold() in shared + else f"a sibling checkout, {pin_source_env_name(holder)}, " + "or") warnings.append( f"{CHAIN_UNVERIFIED}: {holder} declaring a pin on {held} " f"is a fact in {holder}'s own tree, which is not " - f"reachable here (a sibling checkout, " - f"{pin_source_env_name(holder)}, or --link-source " + f"reachable here ({offered} --link-source " f"{holder}= would let it be read). The recorded " "chain still classifies.") continue @@ -2355,20 +2411,26 @@ def declared_neutral_pins(manifest_path, def resolve_link_source(product: str, root: Path | None = None, keyed_sources: dict | None = None, - default_source=None) -> tuple[Path | None, str | None]: + default_source=None, + env_shared: bool = False + ) -> tuple[Path | None, str | None]: """Where `product`'s own manifest can be read, if anywhere at all. 1. `--link-source =` (case-insensitive on the name) 2. `SHAPE_PIN_SOURCE_` in the environment 3. a checkout sitting BESIDE this project (`../openXdox`) 4. a bare `--link-source ` with no `product=` prefix + + `env_shared` says another product asked about alongside this one names + the same variable, so rule 2 is skipped: a variable that does not say + which product it holds is not read for either (#275). """ candidates: list[tuple[Path, str]] = [] keyed = (keyed_sources or {}).get(product.casefold()) if keyed is not None: candidates.append((Path(keyed), f"--link-source {product}=...")) env_name = pin_source_env_name(product) - env_value = os.environ.get(env_name) + env_value = None if env_shared else os.environ.get(env_name) if env_value: candidates.append((Path(env_value), f"${env_name}")) if root is not None: @@ -2390,12 +2452,22 @@ def link_pins_from_trees(links, root: Path | None = None, A link whose tree is not on the disk is simply ABSENT from the mapping, which is what `NamingPolicy.resolve_referent` reads as declared-unverified. + + A link whose `SHAPE_PIN_SOURCE_` another of `links` names too is + not read through it (#275): one tree would answer for two links, and a + chain verified against the wrong tree is worse than one left + declared-unverified. Only a hand-written chain can hold two such links: + `scaffold-project.py` takes every link but the first from the naming + policy's `open` form, which has no `-`, `.` or `_` to fold. """ found: dict[str, set] = {} + links = list(links) + shared = pin_source_env_shared(repo_basename(str(link)) for link in links) for link in links: name = repo_basename(str(link)) path, _how = resolve_link_source(name, root, keyed_sources, - default_source) + default_source, + name.casefold() in shared) if path is None: continue pins = declared_neutral_pins(path, key) diff --git a/templates/assembly-root/scripts/validate-pins.py b/templates/assembly-root/scripts/validate-pins.py index bcc43bd4..af86f880 100755 --- a/templates/assembly-root/scripts/validate-pins.py +++ b/templates/assembly-root/scripts/validate-pins.py @@ -57,7 +57,8 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) from repo_shape import ( # noqa: E402 COMMIT_RE, SHA256_RE, TREE_DIGEST_DEFINITION, Refusal, file_sha256, - find_repo_root, git_out, load_yaml, pin_source_env_name, recorded_gitlink, + find_repo_root, git_out, load_yaml, pin_source_env_collisions, + pin_source_env_name, pin_source_env_shared, recorded_gitlink, repo_basename, tree_digest, tree_digest_from_gh, ) @@ -352,6 +353,7 @@ def _check_shape_pin(root: Path, manifest: dict, report: Report) -> None: def resolve_neutral_pin_source(root: Path, product: str, repository: str, keyed_sources: dict[str, Path], default_source: Path | None, + env_shared: bool = False, ) -> tuple[Path | None, str | None]: """Where to read `product`'s tree OFFLINE, if anywhere at all. @@ -367,6 +369,11 @@ def resolve_neutral_pin_source(root: Path, product: str, repository: str, to whichever pin does not already have a more specific answer — the common case is exactly one declared neutral-product pin + Rule 2 is skipped when `env_shared` is true, which is when another + declared pin names the same variable (#275): a variable that cannot say + which product it holds answers for neither, and `neutral-pin-source- + collides` is reported for the pair. + Returns `(None, None)` when nothing above answers; the caller's next step is `gh api`, and after that, a SKIP. """ @@ -374,7 +381,7 @@ def resolve_neutral_pin_source(root: Path, product: str, repository: str, if keyed is not None: return keyed, f"--pin-source {product}=..." env_name = pin_source_env_name(product) - env_value = os.environ.get(env_name) + env_value = None if env_shared else os.environ.get(env_name) if env_value: return Path(env_value), f"${env_name}" sibling = root.parent / repo_basename(repository) @@ -469,7 +476,9 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, commit: str, rel: str, keyed_sources: dict[str, Path], default_source: Path | None, - report: Report) -> tuple[str, str] | None: + report: Report, + shared: frozenset[str] = frozenset() + ) -> tuple[str, str] | None: """Recompute the referent's tree digest: offline where a source can be found, else from `gh api`, else a named SKIP that never fails the run. @@ -478,8 +487,10 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, already reported in that case, exactly as the original loop's `continue` left it. """ + env_shared = product.casefold() in shared source, how = resolve_neutral_pin_source(root, product, repository, - keyed_sources, default_source) + keyed_sources, default_source, + env_shared) if source is not None: try: return tree_digest(source, commit), how @@ -493,11 +504,14 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, try: return tree_digest_from_gh(repository, commit), "gh-api-tree-recursive" except Refusal as exc: + # A variable another pin shares is not offered (#275). + offered = ("A --pin-source or" if env_shared else + f"A --pin-source, {pin_source_env_name(product)}, or") report.skip( f"{product}: pin commit {commit[:12]} digest NOT " f"rechecked — no local checkout ({exc.code}: " - f"{exc.detail}). A --pin-source, {pin_source_env_name(product)}, " - f"or a checkout at ../{repo_basename(repository)} beside " + f"{exc.detail}). {offered} a checkout at " + f"../{repo_basename(repository)} beside " "this assembly root would verify it offline.", ) return None @@ -505,7 +519,9 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, def _check_one_neutral_product_pin(root: Path, product, report: Report, keyed_sources: dict[str, Path], - default_source: Path | None) -> None: + default_source: Path | None, + shared: frozenset[str] = frozenset() + ) -> None: """One `neutral_product_pins:` entry, start to finish. Split from `_check_neutral_product_pins` for #132 so the loop in that @@ -523,7 +539,7 @@ def _check_one_neutral_product_pin(root: Path, product, report: Report, commit, recorded = fields recomputed = _recompute_neutral_pin_digest(root, product, repository, commit, rel, keyed_sources, - default_source, report) + default_source, report, shared) if recomputed is None: return actual, how = recomputed @@ -541,6 +557,29 @@ def _check_one_neutral_product_pin(root: Path, product, report: Report, f"recomputes ({how})") +def _report_pin_source_collisions(products, report: Report) -> None: + """One `neutral-pin-source-collides` finding for each variable two or more + declared products share (#275), naming every one of them and the + variable. `pin_source_env_collisions` is `repo_shape`'s, the one answer + `scaffold-project.py` refuses on too: products are distinct by casefold, + so a product listed twice is not a collision here. The names are quoted + with `repr`, as `neutral-pin-entry-malformed` quotes an entry: they are + the manifest's own text and may hold a newline.""" + for variable, group in pin_source_env_collisions(products).items(): + names = " and ".join(repr(product) for product in group) + report.finding( + "neutral-pin-source-collides", + f"project.yaml neutral_product_pins declares {names}, and one " + f"variable, {variable}, names the local checkout of each: it " + "cannot say which it holds, so it is read for none of them.\n" + " Remediation: `SHAPE_PIN_SOURCE_` is the product's " + "name upper-cased with every character outside `A-Z`, `0-9` and " + "`_` turned into `_`, so names that differ only in `-`, `.` or " + "`_` share one. Declare one product of each such group, or " + "products whose names differ by more.", + ) + + def _check_neutral_product_pins(root: Path, manifest: dict, report: Report, keyed_sources: dict[str, Path], default_source: Path | None) -> None: @@ -553,11 +592,21 @@ def _check_neutral_product_pins(root: Path, manifest: dict, report: Report, local checkout when one can be found, else from `gh api`. Neither available is a SKIP, never a finding and never a refusal: the absence of a way to answer is not evidence that the pin is wrong. + + TWO PRODUCTS THAT SHARE ONE VARIABLE ARE A FINDING (#275), and neither + reads it. `SHAPE_PIN_SOURCE_` is the product's name upper-cased + with every character outside `[A-Z0-9_]` turned into `_`, so `open-glass`, + `open_glass` and `open.glass` are three pin files and one variable. + Whichever checkout it named answered for all of them, and the pin it was + not a checkout of was reported as `neutral-pin-source-unreadable`: drift + in the project, said about a variable. """ products = manifest.get("neutral_product_pins") or [] + _report_pin_source_collisions(products, report) + shared = frozenset(pin_source_env_shared(products)) for product in products: _check_one_neutral_product_pin(root, product, report, keyed_sources, - default_source) + default_source, shared) def _parse_pin_sources(pin_source_args: list[str] diff --git a/tests/test_pin_source_env_collisions.py b/tests/test_pin_source_env_collisions.py new file mode 100644 index 00000000..26cf852d --- /dev/null +++ b/tests/test_pin_source_env_collisions.py @@ -0,0 +1,604 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Two neutral products must not share one `SHAPE_PIN_SOURCE_` (#275). + +`pin_source_env_name` turns every character outside `[A-Z0-9_]` into `_`, so +`open-glass`, `open_glass` and `open.glass` are three legal product names, with +three pin files (`contracts/-pin.yaml`), and ONE variable, +`SHAPE_PIN_SOURCE_OPEN_GLASS`. A project that pinned two of them got whichever +checkout the variable named for BOTH, and a `neutral-pin-source-unreadable` +FINDING (exit 1, a red `validate.yml`) for the one it was not a checkout of; +with no source given the same variable was offered as the way to verify either. + +Every existing variable name is kept: an operator may have set one and the docs +name them. What is refused instead is DECLARING a set of products whose +variables collide, in each place that declares or reads pins: + +- `scaffold-project.py --pin`, beside #241's file collisions, dry run and real + run alike, before any process starts; +- the project's own `validate-pins.py`, as the finding + `neutral-pin-source-collides`, after which neither product reads the + variable; +- the chain readers in `repo_shape.py`, which never read a variable two links + of one chain share and never offer it. + +`adopt-project.py --pin` is not here: its half arrives with #273's change, +which routes adopt through the one function this file pins. + +The tests that call `main()` refuse every process start, which shows the +refusal comes before `git` and `gh`. The validator runs against a copy of the +session's scaffolded project, whose own `validate-pins.py` and `repo_shape.py` +are the copies under test, with a `gh` that exits 97 and records its arguments +where the run would otherwise have reached the forge. +""" + +from __future__ import annotations + +import os +import stat +import sys +import textwrap +from pathlib import Path + +import pytest + +from conftest import ( + REPO, SCAFFOLD, WINDOWS_SKIP, clear_ambient_pin_sources, git, run_script, +) +from test_scaffold_pin_and_reuse import product_repo +# Fixtures and constants of #241's file, shared rather than re-made: the +# stand-in `openGlass` with two commits, the module-level `scaffold`, the +# refusal of every process start, and the remote and `gh` world. +from test_scaffold_pin_collisions import ( # noqa: F401 + BASE_ARGS, LEG_REFUSED, ONE_FILE, REFUSED, REMEDIATION, SHAPE_WRITES, + glass, listing, nothing_was_created, no_process, pin_args, pins_of, + scaffold, world, +) + +sys.path.insert(0, str(REPO / "scripts")) +import repo_shape # noqa: E402 +from repo_shape import NamingPolicy, tree_digest # noqa: E402 + +UTF8 = "utf-8" +POLICY_PATH = REPO / "contracts" / "repository-naming.yaml" + +#: The two refusals' remediation for the variable, word for word. Alone it +#: stands after `Remediation: nothing was created.`, and beside a file +#: collision it follows #241's remediation after a space. +VARIABLE_REMEDIATION = ( + "`SHAPE_PIN_SOURCE_` names the local checkout of a product, and " + "is its name upper-cased with every character outside `A-Z`, `0-9` and " + "`_` turned into `_`, so names that differ only in `-`, `.` or `_` share " + "one. The project's own `validate-pins.py` reports such a pair as " + "`neutral-pin-source-collides`, and a variable two pins share is read " + "for neither. Pass one product of each such group, or products whose " + "names differ by more.") +ALONE = f"Remediation: nothing was created. {VARIABLE_REMEDIATION}" +BESIDE_A_FILE = f"{REMEDIATION} {VARIABLE_REMEDIATION}" + + +def shared(variable: str) -> str: + """What the refusal says after it has named the products.""" + return (f", and one variable, `{variable}`, names the local checkout of " + "each: it cannot say which it holds.") + + +# --- the one function, and the names it must not change ----------------------- + +@pytest.mark.parametrize("product, variable", [ + pytest.param("openGlass", "SHAPE_PIN_SOURCE_OPENGLASS", id="camel"), + pytest.param("open-glass", "SHAPE_PIN_SOURCE_OPEN_GLASS", id="hyphen"), + pytest.param("open_glass", "SHAPE_PIN_SOURCE_OPEN_GLASS", id="underscore"), + pytest.param("open.glass", "SHAPE_PIN_SOURCE_OPEN_GLASS", id="dot"), + pytest.param("OpenGlass", "SHAPE_PIN_SOURCE_OPENGLASS", id="capital"), + pytest.param("openXdox", "SHAPE_PIN_SOURCE_OPENXDOX", id="openXdox"), +]) +def test_every_existing_variable_name_is_kept(product, variable): + """The refusal is the fix, so no name an operator may have set moves.""" + assert repo_shape.pin_source_env_name(product) == variable + + +def test_the_variable_two_products_share_is_found_with_all_of_them(): + found = repo_shape.pin_source_env_collisions( + ["open-glass", "openGlass", "open_glass", "open.glass"]) + assert found == { + "SHAPE_PIN_SOURCE_OPEN_GLASS": ["open-glass", "open_glass", + "open.glass"]} + + +def test_each_variable_and_each_product_keeps_the_order_first_given(): + found = repo_shape.pin_source_env_collisions( + ["x.y", "a-b", "lone", "a_b", "x_y", "x-y"]) + assert list(found) == ["SHAPE_PIN_SOURCE_X_Y", "SHAPE_PIN_SOURCE_A_B"] + assert found == {"SHAPE_PIN_SOURCE_X_Y": ["x.y", "x_y", "x-y"], + "SHAPE_PIN_SOURCE_A_B": ["a-b", "a_b"]} + + +@pytest.mark.parametrize("products", [ + pytest.param([], id="none"), + pytest.param(["openGlass"], id="one"), + pytest.param(["openGlass", "openPane"], id="two-that-differ"), + pytest.param(["open-glass", "openGlass"], id="hyphen-and-none"), + pytest.param(["openGlass", "OpenGlass", "OPENGLASS"], id="case-only"), + pytest.param(["open-glass", "open-glass", "Open-Glass"], id="repeats"), + pytest.param(["open-glass", None, "", 7, ["open_glass"]], + id="not-names-are-left-out"), +]) +def test_products_that_do_not_share_a_variable_are_no_collision(products): + """The same product declared twice, in any case, is ONE product: whether + that is wrong is #241's `scaffold-pin-collides` and not this function's. + An entry that is not a name is not one for this either.""" + assert repo_shape.pin_source_env_collisions(products) == {} + assert repo_shape.pin_source_env_shared(products) == set() + + +def test_what_is_shared_is_every_product_a_variable_cannot_tell_apart(): + assert repo_shape.pin_source_env_shared( + ["open-glass", "openGlass", "Open_Glass", "openPane"] + ) == {"open-glass", "open_glass"} + + +def test_it_reads_a_generator_once(): + """`link_pins_from_trees` and the scaffold hand it generators.""" + products = (p for p in ("a-b", "a_b")) + assert repo_shape.pin_source_env_collisions(products) == { + "SHAPE_PIN_SOURCE_A_B": ["a-b", "a_b"]} + + +# --- scaffold-project.py --pin: refused, dry and real, before any process ---- + +#: `(the --pin values, what the refusal says, its remediation)`, `{A}` and +#: `{B}` being two commits of the `--pin-source` clone. +REFUSED_CASES = [ + pytest.param( + ["open-glass@{A}", "open_glass@{B}"], + "--pin 'open-glass@{A}' and --pin 'open_glass@{B}' name the products " + "`open-glass` and `open_glass`" + shared("SHAPE_PIN_SOURCE_OPEN_GLASS"), + ALONE, id="hyphen-underscore"), + pytest.param( + ["open.glass@{A}", "open-glass@{B}"], + "--pin 'open.glass@{A}' and --pin 'open-glass@{B}' name the products " + "`open.glass` and `open-glass`" + shared("SHAPE_PIN_SOURCE_OPEN_GLASS"), + ALONE, id="dot-hyphen"), + pytest.param( + ["open-glass@{A}", "openGlass@{A}", "open.glass@{B}", "open_glass@{A}"], + "--pin 'open-glass@{A}' and --pin 'open.glass@{B}' and --pin " + "'open_glass@{A}' name the products `open-glass` and `open.glass` and " + "`open_glass`" + shared("SHAPE_PIN_SOURCE_OPEN_GLASS"), + ALONE, id="three-with-a-bystander"), + pytest.param( + ["opensoft/open-glass@{A}", "open_glass@{B}"], + "--pin 'opensoft/open-glass@{A}' and --pin 'open_glass@{B}' name the " + "products `open-glass` and `open_glass`" + + shared("SHAPE_PIN_SOURCE_OPEN_GLASS"), + ALONE, id="owner-qualified-as-typed"), + pytest.param( + ["Open-Glass@{A}", "open_glass@{B}"], + "--pin 'Open-Glass@{A}' and --pin 'open_glass@{B}' name the products " + "`Open-Glass` and `open_glass`" + shared("SHAPE_PIN_SOURCE_OPEN_GLASS"), + ALONE, id="capitals"), + pytest.param( + ["a-b@{A}", "x.y@{A}", "a_b@{B}", "x_y@{B}"], + "--pin 'a-b@{A}' and --pin 'a_b@{B}' name the products `a-b` and " + "`a_b`" + shared("SHAPE_PIN_SOURCE_A_B") + + " --pin 'x.y@{A}' and --pin 'x_y@{B}' name the products `x.y` and " + "`x_y`" + shared("SHAPE_PIN_SOURCE_X_Y"), + ALONE, id="two-groups-in-the-order-typed"), + pytest.param( + ["spec@{A}", "open-glass@{A}", "open_glass@{B}"], + "--pin 'spec@{A}' writes the pin of the product `spec` at " + "`contracts/spec-pin.yaml`, " + SHAPE_WRITES + + " --pin 'open-glass@{A}' and --pin 'open_glass@{B}' name the " + "products `open-glass` and `open_glass`" + + shared("SHAPE_PIN_SOURCE_OPEN_GLASS"), + BESIDE_A_FILE, id="and-a-file-the-shape-writes"), + pytest.param( + ["open-glass@{A}", "Open-Glass@{B}", "open_glass@{A}"], + "--pin 'open-glass@{A}' and --pin 'Open-Glass@{B}' write the pins of " + "the products `open-glass` and `Open-Glass` at one file, " + "`contracts/open-glass-pin.yaml`, " + ONE_FILE + + " --pin 'open-glass@{A}' and --pin 'Open-Glass@{B}' and --pin " + "'open_glass@{A}' name the products `open-glass` and `open_glass`" + + shared("SHAPE_PIN_SOURCE_OPEN_GLASS"), + BESIDE_A_FILE, id="and-one-product-typed-twice"), +] + + +def refusal(scaffold, capsys, said: str, remediation: str, *args: str) -> str: + """`main()` with `args`: the pin refusal it printed, checked whole.""" + code = scaffold.main([*BASE_ARGS, *args]) + out, err = capsys.readouterr() + assert code == 2, out + err + assert out == "", "the refusal comes before the plan is printed" + assert err == f"{REFUSED}: {said}\n{remediation}\n", err + return err + + +@pytest.mark.parametrize("dry", [pytest.param(True, id="dry-run"), + pytest.param(False, id="real-run")]) +@pytest.mark.parametrize("values, said, remediation", REFUSED_CASES) +def test_pins_sharing_a_variable_are_refused_before_any_process( + scaffold, glass, no_process, capsys, tmp_path, values, said, + remediation, dry): + """The sentence names each argument as typed, the products and the + variable. A real run that got past it would read each pin's digest from + the `--pin-source` with `git`, which `no_process` refuses; and the remote + and work directories it was given were never made.""" + remotes, work = tmp_path / "remotes", tmp_path / "work" + _, first, second = glass + refusal(scaffold, capsys, said.format(A=first, B=second), remediation, + *pin_args(glass, values), "--local-remote-dir", str(remotes), + "--work-dir", str(work), *(("--dry-run",) if dry else ())) + assert not remotes.exists() and not work.exists() + + +@pytest.mark.parametrize("values, said, remediation", REFUSED_CASES) +def test_a_dry_run_refuses_exactly_as_the_real_run(glass, world, values, said, + remediation): + """On main through c7c02fa the dry run said yes and the real run made + three repositories holding two pins one variable named. Now both say the + same no, word for word, and nothing exists afterwards.""" + args = (*BASE_ARGS, *pin_args(glass, values), + "--local-remote-dir", str(world["remotes"]), + "--work-dir", str(world["work"])) + dry = run_script(SCAFFOLD, *args, "--dry-run", env=world["env"]) + real = run_script(SCAFFOLD, *args, env=world["env"]) + assert (dry.returncode, real.returncode) == (2, 2), ( + dry.stdout + dry.stderr + real.stdout + real.stderr) + assert dry.stderr == real.stderr + assert dry.stdout == real.stdout == "" + _, first, second = glass + assert real.stderr == (f"{REFUSED}: {said.format(A=first, B=second)}\n" + f"{remediation}\n") + nothing_was_created(world) + + +def test_the_variable_refusal_comes_before_the_leg_path_refusal( + scaffold, glass, no_process, capsys, monkeypatch): + """Where #241's file refusal runs, in `_refuse_colliding_pins`, and so + ahead of `_refuse_colliding_leg_paths`: a leg path over one of the two + pins' files is the pin refusal, and the leg-path question is never asked. + The control is the same leg path with one pin: the leg-path refusal.""" + asked = [] + original = scaffold._refuse_colliding_leg_paths + + def spy(*args, **kwargs): + asked.append(args) + return original(*args, **kwargs) + + monkeypatch.setattr(scaffold, "_refuse_colliding_leg_paths", spy) + legs = ("--code-path", "contracts/open-glass-pin.yaml") + _, first, second = glass + err = refusal( + scaffold, capsys, + f"--pin 'open-glass@{first}' and --pin 'open_glass@{second}' name " + "the products `open-glass` and `open_glass`" + + shared("SHAPE_PIN_SOURCE_OPEN_GLASS"), ALONE, *legs, + *pin_args(glass, ["open-glass@{A}", "open_glass@{B}"]), "--dry-run") + assert LEG_REFUSED not in err and asked == [] + code = scaffold.main([*BASE_ARGS, *legs, "--dry-run", + *pin_args(glass, ["open-glass@{A}"])]) + assert code == 2 + assert capsys.readouterr().err.startswith(f"{LEG_REFUSED}: ") + assert len(asked) == 1 + + +@WINDOWS_SKIP # the stub `gh` is a `#!` script; Windows executes none +def test_no_gh_command_runs_before_the_variable_refusal(world): + """Pointed at GitHub, with no `--pin-source`, so `gh api` would read each + pin's digest, the refusal comes first and `gh` is never run. The control + is the same run with one of the two pins: it reaches `gh`.""" + commit = "1" * 40 + common = (*BASE_ARGS, "--pin", f"open-glass@{commit}", + "--work-dir", str(world["work"])) + refused = run_script(SCAFFOLD, *common, "--pin", f"open_glass@{commit}", + env=world["env"]) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refused.stderr.startswith(f"{REFUSED}: --pin 'open-glass@{commit}' " + f"and --pin 'open_glass@{commit}' ") + nothing_was_created(world) + + control = run_script(SCAFFOLD, *common, env=world["env"]) + assert control.returncode == 2, control.stdout + control.stderr + assert "pin-unreadable" in control.stderr + assert world["log"].read_text(encoding=UTF8).startswith( + "api repos/opensoft/open-glass") + + +@pytest.mark.parametrize("values, files", [ + pytest.param(["open_glass@{A}"], ["contracts/open_glass-pin.yaml"], + id="one-punctuated-pin"), + pytest.param(["open-glass@{A}", "openGlass@{B}"], + ["contracts/open-glass-pin.yaml", + "contracts/openglass-pin.yaml"], id="variables-differ"), + pytest.param(["open-glass@{A}", "open-glass-2@{B}", "open.pane@{A}"], + ["contracts/open-glass-pin.yaml", + "contracts/open-glass-2-pin.yaml", + "contracts/open.pane-pin.yaml"], id="three-that-differ"), +]) +def test_pins_whose_variables_differ_still_scaffold(glass, world, values, + files): + """Nothing the refusal does touches a set whose variables differ: the + plan names each pin's file, and the pushed root holds each pin at its own + file with its own commit.""" + args = (*BASE_ARGS, *pin_args(glass, values), + "--local-remote-dir", str(world["remotes"]), + "--work-dir", str(world["work"])) + dry = run_script(SCAFFOLD, *args, "--dry-run", env=world["env"]) + assert dry.returncode == 0, dry.stdout + dry.stderr + planned = [line for line in dry.stdout.splitlines() + if line.startswith("pin ")] + assert [line.split(" -> ")[1].split(" (")[0] for line in planned] == files + result = run_script(SCAFFOLD, *args, env=world["env"]) + assert result.returncode == 0, result.stdout + result.stderr + assert REFUSED not in result.stderr + root = world["remotes"] / "Northwind.git" + for path, value in zip(files, pins_of(glass, values)): + pin = git("show", f"main:{path}", cwd=root).stdout + assert f'commit: "{value.rsplit("@", 1)[1]}"' in pin, pin + assert not world["log"].exists() + + +# --- validate-pins.py: a finding naming each product and the variable --------- + +PIN_FILE = textwrap.dedent("""\ + schema_version: 1 + kind: pinned_contract_manifest + pin_role: neutral-product + product: {name} + source_repository: opensoft/{name} + materialization: referenced + commit: "{commit}" + revision_kind: commit + digest_algorithm: sha256 + digest_definition: sorted-ls-tree-r-v1 + digests: + tree_sha256: "{digest}" + digest_source: stand-in + declared_in: project.yaml + verify_pin: scripts/validate-manifest.py + """) + + +def write(path: Path, text: str) -> None: + """LF on every platform, as the shape's own writers do.""" + with open(path, "w", encoding=UTF8, newline="\n") as handle: + handle.write(text) + + +def declare_pins(project: Path, base: Path, names) -> dict[str, Path]: + """Declare a pin on each of `names` in the scaffolded `project`, by hand: + a stand-in repository per name under `base`, a pin file written from it + and a `neutral_product_pins:` entry, as a hand-edited manifest, or one an + older scaffold wrote, holds them. `{name: the stand-in}`; `base` is no + sibling of `project`, so rule 3 of the lookup finds none of them.""" + stand_ins = {} + for name in names: + path, commit = product_repo(base, name) + stand_ins[name] = path + write(project / "contracts" / f"{name.lower()}-pin.yaml", + PIN_FILE.format(name=name, commit=commit, + digest=tree_digest(path, commit))) + manifest = project / "project.yaml" + text = manifest.read_text(encoding=UTF8) + assert "neutral_product_pins: []" in text, "fixture drift" + write(manifest, text.replace( + "neutral_product_pins: []", + "neutral_product_pins:\n" + "\n".join(f" - {name}" for name in names), + 1)) + return stand_ins + + +def validate(project: Path, *args: str, env=None): + return run_script(project / "scripts" / "validate-pins.py", *args, + cwd=project, env=env) + + +@pytest.fixture +def gh_shim(tmp_path): + """A `gh` that records its arguments and fails, first on PATH: the run + that reaches the forge says so in `log`, and a project whose pins were + all answered locally leaves it absent. Windows executes no `#!` script, + so the tests that need it are skipped there.""" + bin_dir, log = tmp_path / "shim", tmp_path / "gh.log" + bin_dir.mkdir() + gh = bin_dir / "gh" + write(gh, textwrap.dedent("""\ + #!/usr/bin/env python3 + import os, sys + with open(os.environ["GH_STUB_LOG"], "a", encoding="utf-8") as log: + log.write(" ".join(sys.argv[1:]) + "\\n") + print("gh shim: refused", file=sys.stderr) + sys.exit(97) + """)) + gh.chmod(gh.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) + return {"log": log, "env": { + "PATH": f"{bin_dir}{os.pathsep}{os.environ.get('PATH', '')}", + "GH_STUB_LOG": str(log)}} + + +COLLIDES = "FINDING neutral-pin-source-collides: " + + +def test_two_pins_sharing_a_variable_are_one_finding_naming_both( + project, tmp_path): + """Every source named by product answers, so what is reported is the + declaration and nothing else: one finding, both products, the variable.""" + stand_ins = declare_pins(project, tmp_path / "products", + ["open-glass", "open_glass"]) + result = validate(project, "--pin-source", + f"open-glass={stand_ins['open-glass']}", + "--pin-source", f"open_glass={stand_ins['open_glass']}") + assert result.returncode == 1, result.stdout + result.stderr + assert result.stderr.count("FINDING") == 1, result.stderr + assert (f"{COLLIDES}project.yaml neutral_product_pins declares " + "'open-glass' and 'open_glass', and one variable, " + "SHAPE_PIN_SOURCE_OPEN_GLASS, names the local checkout of each: " + "it cannot say which it holds, so it is read for none of them." + ) in result.stderr + assert "Remediation: `SHAPE_PIN_SOURCE_`" in result.stderr + assert "1 finding(s)" in result.stderr + for name in ("open-glass", "open_glass"): + assert f" ok {name}: pin commit " in result.stdout + assert f"digest recomputes (--pin-source {name}=...)" in result.stdout + + +def test_three_products_one_variable_are_one_finding_naming_all_three( + project, tmp_path): + names = ["open-glass", "open.glass", "open_glass"] + stand_ins = declare_pins(project, tmp_path / "products", + [*names, "openGlass"]) + result = validate(project, *(arg for name in names + for arg in ("--pin-source", + f"{name}={stand_ins[name]}")), + "--pin-source", f"openGlass={stand_ins['openGlass']}") + assert result.returncode == 1, result.stdout + result.stderr + assert result.stderr.count("FINDING") == 1, result.stderr + assert (f"{COLLIDES}project.yaml neutral_product_pins declares " + "'open-glass' and 'open.glass' and 'open_glass', and one " + "variable, SHAPE_PIN_SOURCE_OPEN_GLASS,") in result.stderr + assert "openGlass'" not in result.stderr.split("Remediation")[0] + + +@WINDOWS_SKIP # the stub `gh` is a `#!` script; Windows executes none +@pytest.mark.parametrize("named", ["open-glass", "open_glass"]) +def test_a_shared_variable_answers_for_neither_pin(project, tmp_path, gh_shim, + named): + """THE ISSUE'S PROBE. The variable names one product's checkout and, on + main through c7c02fa, answered for both: the other was reported + `neutral-pin-source-unreadable`, a finding about the project made by a + checkout of another. Now it answers for neither, so both fall through to + `gh` (stubbed, and asked about each) and the only finding is the + declaration's; the hint no longer offers the variable.""" + stand_ins = declare_pins(project, tmp_path / "products", + ["open-glass", "open_glass"]) + env = {**gh_shim["env"], + "SHAPE_PIN_SOURCE_OPEN_GLASS": str(stand_ins[named])} + result = validate(project, env=env) + assert result.returncode == 1, result.stdout + result.stderr + assert "neutral-pin-source-unreadable" not in result.stderr + assert result.stderr.count("FINDING") == 1, result.stderr + assert COLLIDES in result.stderr + calls = gh_shim["log"].read_text(encoding=UTF8).splitlines() + assert [call.split("/git/trees")[0] for call in calls] == [ + "api repos/opensoft/open-glass", "api repos/opensoft/open_glass"] + skips = [line for line in result.stdout.splitlines() + if line.startswith(" skip ")] + assert len(skips) == 2 + for line in skips: + assert "SHAPE_PIN_SOURCE" not in line + assert "A --pin-source or a checkout at ../open" in line + + +@WINDOWS_SKIP # the stub `gh` is a `#!` script; Windows executes none +def test_a_pin_no_other_shares_a_variable_with_is_offered_it(project, tmp_path, + gh_shim): + """The control: one punctuated pin keeps its variable in the hint, as it + always did, and the run is green with no source (a skip is no finding).""" + declare_pins(project, tmp_path / "products", ["open-glass"]) + result = validate(project, env=gh_shim["env"]) + assert result.returncode == 0, result.stdout + result.stderr + assert "FINDING" not in result.stderr + skip = next(line for line in result.stdout.splitlines() + if line.startswith(" skip open-glass")) + assert ("A --pin-source, SHAPE_PIN_SOURCE_OPEN_GLASS, or a checkout at " + "../open-glass beside this assembly root") in skip + + +def test_pins_whose_variables_differ_each_read_their_own(project, tmp_path): + """The names are kept: `open-glass` is still `SHAPE_PIN_SOURCE_OPEN_GLASS` + and `openGlass` is still `SHAPE_PIN_SOURCE_OPENGLASS`, two variables for + two pins, and each answers for its own product.""" + stand_ins = declare_pins(project, tmp_path / "products", + ["open-glass", "openGlass"]) + result = validate(project, env={ + "SHAPE_PIN_SOURCE_OPEN_GLASS": str(stand_ins["open-glass"]), + "SHAPE_PIN_SOURCE_OPENGLASS": str(stand_ins["openGlass"])}) + assert result.returncode == 0, result.stdout + result.stderr + assert "FINDING" not in result.stderr + assert ("open-glass: pin commit" in result.stdout + and "digest recomputes ($SHAPE_PIN_SOURCE_OPEN_GLASS)" + in result.stdout) + assert "digest recomputes ($SHAPE_PIN_SOURCE_OPENGLASS)" in result.stdout + assert "pins ok" in result.stdout + + +def test_a_product_declared_twice_is_not_a_shared_variable(project, tmp_path): + """`open-glass` and `Open-Glass` are one pin file and so one product, and + one variable rightly: no collision, and the variable answers.""" + stand_ins = declare_pins(project, tmp_path / "products", ["open-glass"]) + manifest = project / "project.yaml" + write(manifest, manifest.read_text(encoding=UTF8).replace( + " - open-glass", " - open-glass\n - Open-Glass", 1)) + result = validate(project, env={ + "SHAPE_PIN_SOURCE_OPEN_GLASS": str(stand_ins["open-glass"])}) + assert result.returncode == 0, result.stdout + result.stderr + assert "neutral-pin-source-collides" not in result.stderr + assert result.stdout.count("digest recomputes ($SHAPE_PIN_SOURCE_OPEN_GLASS)" + ) == 2 + + +# --- the chain readers: a variable two links share is read for neither -------- + +def tree(path: Path, *pins: str) -> Path: + """A stand-in for a link's checkout: a manifest and its declaration.""" + path.mkdir(parents=True, exist_ok=True) + write(path / "project.yaml", + "schema_version: 1\nkind: project-manifest\nneutral_product_pins:\n" + + "".join(f" - {pin}\n" for pin in pins)) + return path + + +def test_a_variable_two_links_share_is_read_for_neither(tmp_path, monkeypatch): + """`open-x` and `open_x` are two links of one hand-written chain and one + variable. On main through c7c02fa the tree it named answered for both, so + `open_x` was read as declaring what `open-x`'s tree declares.""" + clear_ambient_pin_sources(monkeypatch) + only = tree(tmp_path / "open-x", "openDox") + monkeypatch.setenv("SHAPE_PIN_SOURCE_OPEN_X", str(only)) + assert repo_shape.link_pins_from_trees( + ("open-x", "open_x", "openDox"), root=None) == {} + + +def test_a_link_no_other_shares_a_variable_with_still_reads_it(tmp_path, + monkeypatch): + """The control: the same variable answers for `open-x` in a chain with no + other link it names, which is what every chain read before.""" + clear_ambient_pin_sources(monkeypatch) + only = tree(tmp_path / "open-x", "openDox") + monkeypatch.setenv("SHAPE_PIN_SOURCE_OPEN_X", str(only)) + assert repo_shape.link_pins_from_trees( + ("open-x", "openY", "openDox"), root=None) == {"open-x": {"openDox"}} + + +def test_links_sharing_a_variable_are_still_read_by_name(tmp_path, + monkeypatch): + """`--link-source =` is not the variable, and answers each.""" + clear_ambient_pin_sources(monkeypatch) + first = tree(tmp_path / "one", "open_x") + second = tree(tmp_path / "two", "openDox") + assert repo_shape.link_pins_from_trees( + ("open-x", "open_x", "openDox"), root=None, + keyed_sources={"open-x": first, "open_x": second} + ) == {"open-x": {"open_x"}, "open_x": {"openDox"}} + + +def test_a_shared_variable_is_not_offered_as_the_way_to_read_a_link(): + """The `declared-unverified` hint names the variable for a link that has + it to itself, as before, and names `--link-source` alone for two links + that share one.""" + policy = NamingPolicy.load(POLICY_PATH) + apart = policy.resolve_referent( + "MedxGlass", {"openXglass"}, ["openXglass", "openGlass"], {}) + assert list(apart.warnings) == [ + "declared-unverified: openXglass declaring a pin on openGlass is a " + "fact in openXglass's own tree, which is not reachable here (a " + "sibling checkout, SHAPE_PIN_SOURCE_OPENXGLASS, or --link-source " + "openXglass= would let it be read). The recorded chain still " + "classifies."] + together = policy.resolve_referent( + "MedxGlass", {"open-x"}, ["open-x", "open_x", "openGlass"], {}) + assert [w.split(" (")[1].split(" would")[0] + for w in together.warnings] == [ + "a sibling checkout or --link-source open-x=", + "a sibling checkout or --link-source open_x="] + assert "SHAPE_PIN_SOURCE" not in " ".join(together.warnings) From 53fb96e0f7ba8f6ca6630e02da7819b4e44dabc4 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:51:13 +0000 Subject: [PATCH 2/9] adopt-project.py: `plan --pin` takes `[owner/]product@<40 hex>` as the scaffold does and records the commit and the digest, and `execute` writes `contracts/-pin.yaml`, so an adoption that declares a pin no longer says `adoption verified` over a root whose own `validate-pins.py` refuses it as `neutral-pin-missing` (#273) Brett Heap ruled for #273's decision 1, first option ("go with your recommendation on #273"): adopt takes the scaffold's pin and writes the scaffold's pin file. This reverses the NAME-only `--pin` of #6. - scripts/shape_materialize.py: the scaffold's pin machinery moved here, beside `neutral_pin_path` and the materializer that writes the file: `PIN_ARG_RE`, `parse_pin`, `pin_digest_from_source`, `pin_digest_from_gh`, `declared_pin_values`, `declared_pin_products`, `PIN_COLLISION_REMEDIATION`, and `_refuse_colliding_pins`, split into the pure `pin_collisions` and `refuse_colliding_pins(refusal=...)`. Moved verbatim; `scaffold-project.py` imports what it calls under its old names, and its refusals and their words are unchanged. - adopt-project.py `plan`: `--pin`, `--pin-owner` and `--pin-source` mean what they mean for the scaffold. A name with no `@` is `pin-malformed`; `spec`, `code` and `shape` in any case, and one product twice, are `adopt-pin-collides` (#273 decisions 4 and 5); a leg mounted over a pin's file is `adopt-leg-path-collides`. The digest is read at `plan` time and the plan records product, repository, commit, digest and how it was read, so `check` and `execute` read no network for a pin. - `check` finds, and `execute` refuses before any leg exists (one list, `_entry_findings`), a pin the plan does not record whole: `plan-pin-unrecorded` (a name alone, from an old plan or an edit, or a mapping missing its commit or digest), `plan-pin-malformed`, `plan-pin-collides`, and `plan-root-holds-pin` (a kept source path at the pin's file, which `validate-pins.py` would read in place of the shape's). - `execute` writes each pin through `materialize_assembly_root(neutral_pins=)` and lists the product, owner stripped, in `neutral_product_pins:` (#273 decision 3). - `--help` and docs/cli.md say so; tests/test_adopt_pins.py covers every row of #273's table end to end, with the adopted clone's own validators and bootstrap, and every refusal and finding. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 437 +++++++++++++++--- docs/cli.md | 30 +- scaffold-project.py | 241 +--------- scripts/shape_materialize.py | 288 +++++++++++- tests/test_adopt_pins.py | 730 ++++++++++++++++++++++++++++++ tests/test_adopt_plan.py | 60 ++- tests/test_git_output_decoding.py | 7 +- 7 files changed, 1456 insertions(+), 337 deletions(-) create mode 100644 tests/test_adopt_pins.py diff --git a/adopt-project.py b/adopt-project.py index 3ed2498e..96b0de77 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -23,7 +23,12 @@ path holding one of the source's own submodules, carry `leg: null`, `review_required: true` and the QUESTION to ask. A leg path over a path it keeps in the root itself is a NOTE, - not a refusal: the answer is the person's to change. + not a refusal: the answer is the person's to change. A + `--pin @<40 hex>` is read as `scaffold-project.py + --pin` reads it, its digest included, and recorded in the plan, + and `execute` writes its `contracts/-pin.yaml`; `check` + finds, and `execute` refuses, a pin the plan does not record + whole (#273). check validates a plan against the source: each entry path bound for a leg one `execute` will extract, every path covered exactly once, no unresolved legs, leg names conforming to the naming @@ -132,29 +137,258 @@ NAMING_POLICY = SHAPE_ROOT / "contracts" / "repository-naming.yaml" PATH_POLICY = SHAPE_ROOT / "contracts" / "path-classification.yaml" -#: A declared pin here is a NAME, `[owner/]openProduct` — never `@`. -#: Adopting a project records no commit for a neutral-product pin at plan -#: time (there is no `contracts/-pin.yaml` for one, unlike -#: `scaffold-project.py --pin`), so a value carrying `@` is almost always -#: that OTHER tool's syntax pasted in by habit, and is refused rather than -#: silently written into `neutral_product_pins:` as a name nothing matches. -PIN_NAME_RE = re.compile(r"^[A-Za-z0-9_.-]+(?:/[A-Za-z0-9_.-]+)?$") +#: A DECLARED PIN IS `[owner/]openProduct@<40 hex commit>`, as +#: `scaffold-project.py --pin` takes it, and `execute` writes +#: `contracts/-pin.yaml` for it, as the scaffold does +#: (#273; Brett Heap, 2026-10-12). This reverses the NAME-only `--pin` of #6 +#: (2026-09-03), which recorded no commit and wrote no file, so the adopted +#: root's own `validate-pins.py` refused the declaration as +#: `neutral-pin-missing` on its first run after `execute` said `adoption +#: verified`. `plan` reads each pin with the scaffold's own functions, which +#: live beside the materializer that writes the file: the argument, `--pin-owner` +#: and `--pin-source` mean what they mean there, the digest is read at `plan` +#: time (from `--pin-source`, or `gh api`), and a name with no `@`, a +#: product named `spec`, `code` or `shape`, or one product twice is refused in +#: the scaffold's words. The plan RECORDS product, repository, commit, digest +#: and how the digest was read, so `check` and `execute` read no network for a +#: pin; `check` finds, and `execute` refuses before any leg exists, a pin the +#: plan does not record whole, a name alone among them (an old plan, or an +#: edit). The manifest lists each product with its owner stripped, as the +#: file is named. +from shape_materialize import ( # noqa: E402 + PIN_ARG_RE, declared_pin_products, declared_pin_values, neutral_pin_path, + one_printable_line, pin_collisions, refuse_colliding_pins, +) +from repo_shape import SHA256_RE, repo_basename # noqa: E402 + +#: The keys `plan` records for each pin, in the order it writes them, and the +#: value of `contracts/neutral-product-pin.yaml` each one fills. +PIN_FIELDS = (("product", "PIN_PRODUCT"), ("repository", "PIN_REPOSITORY"), + ("commit", "PIN_COMMIT"), ("tree_sha256", "PIN_TREE_SHA256"), + ("digest_source", "PIN_DIGEST_SOURCE")) +#: The two a pin is a fact by. A pin without either is a name. +PIN_RECORDED = ("commit", "tree_sha256") + +#: `check`'s findings about the plan's `pins:`. +PIN_UNRECORDED = "plan-pin-unrecorded" +PIN_MALFORMED = "plan-pin-malformed" +PIN_COLLIDES = "plan-pin-collides" +ROOT_HOLDS_PIN = "plan-root-holds-pin" + +#: The comment `plan` writes above `pins:`. +PINS_COMMENT = ( + "# Neutral products this project declares a pin on. A", + "# `` assembly root is a DESCENDANT only when", + "# its `open` is listed here (2026-09-02). Each pin was", + "# read by `plan --pin @<40 hex commit>`: the commit, and", + "# the `sorted-ls-tree-r-v1` digest of the product's tree there.", + "# `execute` writes `contracts/-pin.yaml` from", + "# them, as `scaffold-project.py --pin` does, and reads no network", + "# for them. To change a pin, re-run `plan`: a commit or a digest", + "# edited here is one nobody read (#273).", +) -def _checked_pin_name(raw: str) -> str: - name = raw.strip() - if not PIN_NAME_RE.match(name): - raise Refusal( - "adopt-pin-malformed", - f"--pin {raw!r} is not `[owner/]openProduct`", - "Remediation: pass --pin openGlass, or name the owner explicitly " - f"with --pin {NEUTRAL_PRODUCT_OWNER}/openGlass — every neutral " - f"open lives under {NEUTRAL_PRODUCT_OWNER} by the " - "family's rule. Adopting a project declares no commit for a " - "neutral-product pin at plan time, so drop `@` if you " - "copied it from `scaffold-project.py --pin`.", - ) - return name +def _pin_lines(pins: dict) -> list[str]: + """`pins:` as `plan` writes it, from `declared_pin_values`: one mapping + per product, its commit and digest quoted so that no reader takes forty + digits for a number.""" + if not pins: + return ["pins: []"] + lines = ["pins:"] + for values in pins.values(): + for index, (key, placeholder) in enumerate(PIN_FIELDS): + value = values[placeholder] + lines.append((" - " if index == 0 else " ") + f"{key}: " + + (double_quoted(value) if key in PIN_RECORDED + else y(value))) + return lines + + +def _pin_products(rows) -> list[str]: + """The products a plan's `pins:` declares, in its order: each mapping's + `product:`, or the name an old plan's entry was. The naming policy + classifies the root by them; whether each entry is a pin `execute` can + write is `_read_pins`'s question, asked by `check` and `execute`.""" + if not isinstance(rows, list): + return [] + products = [] + for row in rows: + if isinstance(row, dict): + row = row.get("product") + if isinstance(row, str) and row: + products.append(row) + return products + + +def _pin_row_problem(row: dict) -> tuple[str, str] | None: + """`(code, why)` for a `pins:` mapping `execute` cannot write a pin file + from, or None. `why` finishes a sentence about the entry. + + Each value is held to what `plan` writes there: the commit to + `COMMIT_RE`, the repository and the commit together to the scaffold's own + `PIN_ARG_RE`, owner-qualified, with the product its name, the digest to + `SHA256_RE`, and the digest's source to one printable line.""" + missing = [key for key, _ in PIN_FIELDS if row.get(key) in (None, "")] + unrecorded = [key for key in PIN_RECORDED if key in missing] + if unrecorded or missing: + return ((PIN_UNRECORDED if unrecorded else PIN_MALFORMED), + "records no " + " and no ".join( + f"`{key}:`" for key in (unrecorded or missing))) + not_text = [key for key, _ in PIN_FIELDS + if not isinstance(row.get(key), str)] + if not_text: + return PIN_MALFORMED, "records " + ", ".join( + f"`{key}: {row[key]!r}`" for key in not_text) + ", which " + ( + "is" if len(not_text) == 1 else "are") + " not text" + product, repository, commit, digest, how = ( + row[key] for key, _ in PIN_FIELDS) + if not COMMIT_RE.match(commit): + return PIN_MALFORMED, ( + f"records `commit: {commit!r}`, which is not 40 hex: a tag, a " + "branch or an abbreviated oid can be moved, and a commit cannot") + if not (PIN_ARG_RE.match(f"{repository}@{commit}") and "/" in repository + and repo_basename(repository) == product): + return PIN_MALFORMED, ( + f"records `product: {product!r}` and `repository: " + f"{repository!r}`, and `plan` records the repository as " + "`/`, its name the product's") + if not SHA256_RE.match(digest): + return PIN_MALFORMED, (f"records `tree_sha256: {digest!r}`, which is " + "not 64 hex") + if not one_printable_line(how): + return PIN_MALFORMED, (f"records `digest_source: {how!r}`, which is " + "not one printable line") + return None + + +def _pin_fix(name=None) -> str: + """How every pin finding ends: the `plan` that records the pin whole, + naming the product where the entry names one `--pin` would take.""" + if not (isinstance(name, str) and PIN_ARG_RE.match(f"{name}@{'0' * 40}")): + name = "" + return (f"re-run `plan` with `--pin {name}@<40 hex commit>`, which reads " + "the digest there (from `--pin-source ` with no network, " + "or `gh api`) and records both, as `scaffold-project.py --pin` " + "takes it") + + +def _read_pins(plan: "Plan") -> tuple[list, list[str]]: + """`(pins, findings)` for the plan's `pins:`. + + `pins` is `(said, product, values)` for each entry `execute` can write a + pin file from, in the plan's order and with a repeat kept, `said` naming + the entry and `values` the `PIN_*` values its file is rendered from. + `findings` is one FINDING for each entry it cannot: a name alone (an old + plan's, or an edit's) or a mapping missing its commit or digest is + `plan-pin-unrecorded`, and anything else `plan` never writes there is + `plan-pin-malformed`.""" + rows = plan.data.get("pins") + if rows is None: + return [], [] + if not isinstance(rows, list): + return [], [f"FINDING {PIN_MALFORMED}: `pins:` is {rows!r}, not a " + f"list of pins: {_pin_fix()}."] + pins, findings = [], [] + for number, row in enumerate(rows, 1): + if isinstance(row, str): + # The file is named only for a name `--pin` would take, so that + # a line break or an escape in an edited entry is never printed + # raw; `repr` spells the entry itself. + path = (f"`{neutral_pin_path(repo_basename(row))}` for this one" + if PIN_ARG_RE.match(f"{row}@{'0' * 40}") else "one") + findings.append( + f"FINDING {PIN_UNRECORDED}: `pins:` entry {number}, " + f"{row!r}, is a name with no commit and no digest. `execute` " + f"writes each declared pin's file, {path}, from the commit " + "and the digest `plan` records, and without it the root's " + "own `validate-pins.py` refuses the declaration " + f"(`neutral-pin-missing`): {_pin_fix(row)}.") + continue + if not isinstance(row, dict): + findings.append( + f"FINDING {PIN_MALFORMED}: `pins:` entry {number} is " + f"{row!r}, not a pin: {_pin_fix()}.") + continue + product = row.get("product") + said = f"`pins:` entry {number}" + ( + f" ({product!r})" if isinstance(product, str) and product else "") + problem = _pin_row_problem(row) + if problem: + code, why = problem + findings.append( + f"FINDING {code}: {said} {why}. `execute` writes the pin file " + "from the commit and the digest `plan` records, and nothing " + f"else: {_pin_fix(row.get('repository') or product)}.") + continue + pins.append((said, product, { + placeholder: (row[key].lower() if key in PIN_RECORDED + else row[key]) + for key, placeholder in PIN_FIELDS})) + findings += [f"FINDING {PIN_COLLIDES}: {collision} `plan` refuses such " + "`--pin`s as `adopt-pin-collides`: re-run `plan` with each " + "product once, by the name of the repository it pins." + for collision in pin_collisions( + [(said, product) for said, product, _ in pins])] + return pins, findings + + +def _declared_pins(plan: "Plan") -> list[str]: + """The products of the pins `execute` would write, repeats kept: the + files a leg mount and a path the root keeps are asked about.""" + return [product for _, product, _ in _read_pins(plan)[0]] + + +def _recorded_pins(plan: "Plan") -> dict[str, dict[str, str]]: + """`{product: PIN_* values}` for the materializer, as + `declared_pin_values` hands it the scaffold's. Read only after `execute` + has refused every finding `_read_pins` makes.""" + return {product: values for _, product, values in _read_pins(plan)[0]} + + +def _kept_pin_findings(plan: "Plan", tree: list) -> list[str]: + """A FINDING for each declared pin whose file a path the plan keeps in + the root already holds. The materializer writes the shape's pin beside + such a path, under `COLLISION_DIR`, as it writes any shape file the root + holds; for a pin that is no copy to merge but the wrong file, because the + root's own `validate-pins.py` reads `contracts/-pin.yaml` and + would read the source's. Asked of a disk that keeps case and, where that + finds nothing, of one that ignores it.""" + products = list(dict.fromkeys(_declared_pins(plan))) + kept = _kept_in_root(plan.entries, tree) if products else [] + if not kept: + return [] + disks = ((_KeptTree(kept, fold=False), ""), + (_KeptTree(kept, fold=True), _CASE_ASIDE)) + out = [] + for product in products: + rel = neutral_pin_path(product) + for disk, aside in disks: + at = disk.at(rel) + rows = [row for row in ([at] if at else disk.under(rel)) + if row[1] != GITLINK_MODE] + if not rows: + continue + path, mode, entry = rows[0] + under = "" if entry == path else f" under the entry {_spelled(entry)}" + where = "at" if at else "inside" + out.append( + f"FINDING {ROOT_HOLDS_PIN}: the plan keeps {_kind_of(mode)} " + f"{_spelled(path)} in the assembly root{under}, {where} " + f"`{rel}`{aside}, where `execute` writes the pin the plan " + f"declares on `{product}`. The shape's pin would be written " + f"beside it, under `{COLLISION_DIR}/`, and the root's own " + "`validate-pins.py` would read the source's file as that " + f"pin: {_fix_the_kept_path(entry)}.") + break + return out + + +def _pin_findings(plan: "Plan", tree: list) -> list[str]: + """Every FINDING about the plan's pins: each entry `execute` cannot + write a pin file from, two pins at one file or one at a file the shape + writes, and a path the root keeps where a pin is written.""" + return _read_pins(plan)[1] + _kept_pin_findings(plan, tree) PLAN_KIND = "adoption-plan" @@ -1175,7 +1409,7 @@ def assigned_paths(entries) -> dict[str, list[str]]: def render_plan(args, source: Source, entries: list[Entry], names: dict, - pins: list[str], follow_ups: list[str], + pins: dict, follow_ups: list[str], seeded: list[str]) -> str: lines = [ "schema_version: 1", @@ -1220,11 +1454,7 @@ def render_plan(args, source: Source, entries: list[Entry], names: dict, emit(lines, "code", names["code"], 2) emit(lines, "spec_path", args.spec_path, 2) emit(lines, "code_path", args.code_path, 2) - lines += ["", - "# Neutral products this project declares a pin on. A", - "# `` assembly root is a DESCENDANT only when", - "# its `open` is listed here (2026-09-02).", - "pins: [" + ", ".join(pins) + "]", ""] + lines += ["", *PINS_COMMENT, *_pin_lines(pins), ""] lines += [ "# A leg that NO entry below assigns a path to is SEEDED from the", @@ -1360,24 +1590,30 @@ def _plan_leg_paths(plan: Plan) -> tuple[tuple, tuple]: SHAPE_DIR_RESERVED = (f"`{COLLISION_DIR}/` is reserved for the shape's own " "copies of files the root already holds") -LEG_COLLISION_REMEDIATION = ( - "Remediation: nothing here picks a path for you, so pass another one " - "yourself. A leg path neither is, holds nor lies inside, whatever the " - "case, a path the shape writes into the assembly root (" - + ", ".join(f"`{path}`" for path in assembly_root_paths()) - + f") or `{COLLISION_DIR}`. {SHAPE_DIR_RESERVED}, so a leg path there is " - "refused even where this source needs no copy. The defaults `spec` and " - "`code` pass. `check` then asks the same of every source path the plan " - "keeps in the root." -) - - -def _shape_collisions(what: str, value: str) -> list[str]: +def _leg_collision_remediation(neutral_pins=()) -> str: + """What `plan` tells a person whose leg path collides. The paths are + `assembly_root_paths(neutral_pins)`, the materializer's own list, each + declared pin's file among them (#273), as the scaffold lists its own.""" + return ( + "Remediation: nothing here picks a path for you, so pass another one " + "yourself. A leg path neither is, holds nor lies inside, whatever the " + "case, a path the shape writes into the assembly root (" + + ", ".join(f"`{path}`" for path in assembly_root_paths(neutral_pins)) + + f") or `{COLLISION_DIR}`. {SHAPE_DIR_RESERVED}, so a leg path there " + "is refused even where this source needs no copy. The defaults `spec` " + "and `code` pass. `check` then asks the same of every source path the " + "plan keeps in the root." + ) + + +def _shape_collisions(what: str, value: str, neutral_pins=()) -> list[str]: """Where the mount path `value` overlaps what the shape writes: the - paths of `assembly_root_paths`, asked through the definition this tool - shares with `scaffold-project.py` (#197), then this tool's own - `COLLISION_DIR`, which only an adoption writes into.""" - out = shape_path_collisions(what, value, COLLISION_DIR) + paths of `assembly_root_paths`, each declared pin's file among them + (#273), asked through the definition this tool shares with + `scaffold-project.py` (#197), then this tool's own `COLLISION_DIR`, + which only an adoption writes into.""" + out = shape_path_collisions(what, value, COLLISION_DIR, + neutral_pins=neutral_pins) beside = _overlap(value, COLLISION_DIR) if beside: relation, aside = beside @@ -1427,7 +1663,7 @@ def _kept_collisions(what: str, value: str, kept: list) -> list[str]: return out -def leg_path_collisions(legs: tuple, kept=()) -> list[str]: +def leg_path_collisions(legs: tuple, kept=(), neutral_pins=()) -> list[str]: """Everything each leg's mount path overlaps in the assembly root, as sentences; `[]` when nothing. @@ -1435,11 +1671,12 @@ def leg_path_collisions(legs: tuple, kept=()) -> list[str]: already a canonical path: an overlap is a statement about two paths. `kept` is `(path, mode, entry)` for each source path the plan keeps in the root (`_kept_in_root`); with none, only the shape's own paths are asked about, - which is all `plan` knows. + which is all `plan` knows. `neutral_pins` are the products whose pin + files `execute` writes (#273), asked about with the shape's own paths. """ out: list[str] = [] for what, value in legs: - out += _shape_collisions(what, value) + out += _shape_collisions(what, value, neutral_pins) out += _kept_collisions(what, value, kept) return out @@ -1466,7 +1703,8 @@ def _leg_collision_findings(plan: Plan, tree) -> list[str]: prints them. Asked only of a plan whose mount paths are canonical.""" return [f"FINDING {LEG_COLLIDES}: {collision}" for collision in leg_path_collisions( - _plan_leg_paths(plan), _kept_in_root(plan.entries, tree))] + _plan_leg_paths(plan), _kept_in_root(plan.entries, tree), + _declared_pins(plan))] def _flag_leg_paths(args) -> tuple[tuple, tuple]: @@ -1507,7 +1745,8 @@ def _kept_collision_notes(args, entries: list, tree: list) -> list[str]: def _checked_plan_inputs(args, source: Source, naming: NamingPolicy) -> tuple[dict, list]: """Every value the plan will RECORD, defaulted and checked before one of - them is written. Returns `(names, pins)`. + them is written. Returns `(names, pins)`, `pins` being + `declared_pin_values`'s `{product: PIN_* values}`, each digest read. Split out of `cmd_plan` for #138, and the order inside it is the order the refusals came in: a run that got as far as naming the three legs before it @@ -1524,14 +1763,23 @@ def _checked_plan_inputs(args, source: Source, # is the gate that hands back the text. spec, code = legs = _flag_leg_paths(args) _refuse_bad_leg_paths("adopt-bad-leg-path", *legs) + # EACH `--pin`'S FILE AGAINST THE SHAPE'S OWN AND AGAINST EACH OTHER, from + # the arguments as typed, in the scaffold's words (#241, #273), and + # before the leg paths, which are asked about every pin's file and would + # miscount one written over the shape's or listed twice. A name with no + # `@` is `parse_pin`'s `pin-malformed`. No digest is read yet. + args.pin_owner = checked_value("--pin-owner", args.pin_owner) + refuse_colliding_pins(args.pin, args.pin_owner, "adopt-pin-collides") + products = list(declared_pin_products(args)) # Then the paths the shape writes, which are known before any plan is - # (#172). A path the plan keeps in the root is `check`'s to find: the - # plan's answers are edited after this writes them. `plan` only notes - # the ones it wrote itself (`_kept_collision_notes`). - collisions = leg_path_collisions(legs) + # (#172), each pin's file among them (#273). A path the plan keeps in the + # root is `check`'s to find: the plan's answers are edited after this + # writes them. `plan` only notes the ones it wrote itself + # (`_kept_collision_notes`). + collisions = leg_path_collisions(legs, neutral_pins=products) if collisions: raise Refusal("adopt-leg-path-collides", " ".join(collisions), - LEG_COLLISION_REMEDIATION) + _leg_collision_remediation(products)) args.spec_path = checked_value(*spec) args.code_path = checked_value(*code) args.id = args.id or args.project.lower() @@ -1553,7 +1801,7 @@ def _checked_plan_inputs(args, source: Source, args.reference = args.reference or default_reference(args.elected_on) args.elected_by = args.elected_by or _elector() names = _names(args.project) - pins = [_checked_pin_name(p) for p in (args.pin or []) if p.strip()] + pins = declared_pin_values(args) _check_names(naming, names, set(pins)) return names, pins @@ -1607,7 +1855,8 @@ def _print_triad_advisory(path: Path, commit: str | None = None) -> None: def _print_plan_report(args, source: Source, tree: list, entries: list, - follow_ups: list, seeded: list, out: Path) -> None: + follow_ups: list, seeded: list, out: Path, + pins: dict | None = None) -> None: """What `plan` says to the terminal once the file is on disk. Split out of `cmd_plan` for #138. THE UNRESOLVED PATHS ARE LAST on @@ -1619,6 +1868,14 @@ def _print_plan_report(args, source: Source, tree: list, entries: list, f"{source.commit[:12]} ({source.branch}), " f"{len(tree)} files, {source.commit_count()} commits") print(f"project {args.project} ({args.id}) in {args.org}, mode in-place") + # Each pin as the scaffold's plan prints it: what is pinned, at which + # commit and digest, the file `execute` writes, and how the digest was + # read (#273). + for product, values in (pins or {}).items(): + print(f"pin {values['PIN_REPOSITORY']} @ " + f"{values['PIN_COMMIT'][:12]} tree " + f"{values['PIN_TREE_SHA256'][:12]}… -> " + f"{neutral_pin_path(product)} ({values['PIN_DIGEST_SOURCE']})") _print_triad_advisory(source.path, source.commit) _print_entries(entries) _print_summary(entries) @@ -1666,7 +1923,8 @@ def cmd_plan(args) -> int: write_lf(out, render_plan(args, source, entries, names, pins, follow_ups, seeded)) - _print_plan_report(args, source, tree, entries, follow_ups, seeded, out) + _print_plan_report(args, source, tree, entries, follow_ups, seeded, out, + pins) return 0 @@ -1784,7 +2042,9 @@ def __init__(self, path: Path, data: dict): self.entries = [e for e in (data.get("paths") or []) if isinstance(e, dict)] self.legs = data.get("legs") or {} - self.pins = [str(p) for p in (data.get("pins") or []) if p] + # The products only: what the naming policy classifies by. Whether + # each entry is a pin `execute` can write is `_read_pins`'s (#273). + self.pins = _pin_products(data.get("pins")) @classmethod def load(cls, path: Path) -> "Plan": @@ -1973,6 +2233,12 @@ def _leg_findings(plan: Plan, tree: list | None) -> list[str]: f"one of {list(LEG_VALUES)}") if isinstance(plan.legs, dict): findings += _root_shape_findings(plan, tree) + # THE PINS, LAST (#273): each `pins:` entry `execute` cannot write a + # pin file from, two pins at one file, and a path the root keeps + # where a pin is written. Each finding names the `plan` that records + # the pin whole. A leg mounted over a pin's file is + # `plan-leg-path-collides`, above, with the shape's own paths. + findings += _pin_findings(plan, tree) return findings @@ -2748,6 +3014,10 @@ def _entry_findings(plan: Plan, source: Source THE PATHS COME FIRST (#196), so that `_refuse_what_check_finds` wears `plan-unsafe-path` for a plan that has one: a path that is a `git` option is refused as that, and never as the uncovered file it also leaves. + + THE PINS COME AFTER THE LEGS (#273), from `_leg_findings`: each `pins:` + entry `execute` cannot write a pin file from, two pins at one file, and + a path the root keeps where a pin is written (`_pin_findings`). """ tree = source.tree() unsafe, unsafe_remediation = _unsafe_path_findings(plan) @@ -2792,6 +3062,13 @@ def _print_what_will_happen(plan: Plan, source: Source, names: dict, f"{_plan_leg_path(plan, 'code')}/") print(f" {len(stays)} path(s) stay in the assembly root; " f"{len(drops)} dropped") + # Each pin `execute` writes, as the plan records it (#273): a pin it + # cannot write is a finding, printed after this. + for product, values in _recorded_pins(plan).items(): + print(f" write {neutral_pin_path(product)}: " + f"{values['PIN_REPOSITORY']} @ {values['PIN_COMMIT'][:12]} tree " + f"{values['PIN_TREE_SHA256'][:12]}… " + f"({values['PIN_DIGEST_SOURCE']})") print(" the source is never deleted, never renamed, never force-pushed") print(_topics_line(topic, local)) for item in plan.get("follow_ups", []): @@ -3599,9 +3876,14 @@ def _commit_the_split(plan: Plan, source: Source, assembly: Path, _mount_the_legs(assembly, work_root, names, urls, paths_for, spec_path, code_path) + # Each declared pin's file, `contracts/-pin.yaml`, is written by + # the materializer as the scaffold's is, from the values the plan + # records (#273); `_refuse_what_check_finds` has refused a pin it does + # not record whole, and one whose file a kept path holds. materialized = materialize_assembly_root( SHAPE_ROOT, assembly, values, collision_dir=COLLISION_DIR, - append={"Makefile": ADOPT_MAKEFILE_BLOCK}) + append={"Makefile": ADOPT_MAKEFILE_BLOCK}, + neutral_pins=_recorded_pins(plan)) for intended, actual in materialized.collisions: print(f" beside {actual} (the source already has {intended}; nothing " "was overwritten)") @@ -4235,8 +4517,9 @@ def _check_names(policy: NamingPolicy, names: dict[str, str], f"({found.reason})", "Remediation: --project takes one CamelCase token. Two forms " "beyond the bare one MAY be the assembly root: a declared " - "descendant (declare the pin with `--pin open` and it " - "classifies as one, 2026-09-02), and an `open` " + "descendant (declare the pin with `--pin " + "open@<40 hex commit>` and it classifies as one, " + "2026-09-02), and an `open` " "neutral product, which may elect the shape and be its own " "root (2026-09-05). Neither may be the spec or code leg, and " "an `-Install` may be no leg at all.") @@ -4288,12 +4571,30 @@ def main(argv: list[str] | None = None) -> int: plan.add_argument("--spec-path", default="spec") plan.add_argument("--code-path", default="code") plan.add_argument("--pin", action="append", default=[], - help="a neutral product this project declares a pin on " - "— a NAME only, e.g. --pin openGlass or --pin " - f"{NEUTRAL_PRODUCT_OWNER}/openGlass to name the " - "owner explicitly. Adopting a project pins no " - "commit at plan time, so a trailing @ " - "(scaffold-project.py's syntax) is refused.") + metavar="openProduct@COMMIT", + help="declare a neutral-product pin, as " + "scaffold-project.py --pin does, e.g. --pin " + "openGlass@<40 hex>. `plan` reads the digest of " + "the product's tree at that commit (from " + "--pin-source, or `gh api`) and records it in the " + "plan; `execute` writes " + "contracts/-pin.yaml from it and " + "lists the product in the manifest. A name with " + "no @, a product named spec, code or " + "shape, and one product twice are refused. " + "Repeatable.") + plan.add_argument("--pin-owner", default=NEUTRAL_PRODUCT_OWNER, + help="the organisation an UNQUALIFIED --pin name " + "resolves under (default: " + f"{NEUTRAL_PRODUCT_OWNER}, where every neutral " + "open lives — never the adopted " + "repository's own --org). Name the owner in --pin " + "itself (owner/openProduct@) to override it " + "for one pin.") + plan.add_argument("--pin-source", type=Path, default=None, + help="a local clone to compute --pin digests from, " + "instead of reading the forge with `gh api` (the " + "TEST path; no network)") plan.add_argument("--allow-empty-leg", action="append", default=[], choices=EXTRACTED_LEGS, help="record in the plan that this leg having NO path " diff --git a/docs/cli.md b/docs/cli.md index b646ba8d..3169eac5 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -340,7 +340,9 @@ usage: adopt-project.py plan [-h] --source SOURCE --project PROJECT [--elected-on ELECTED_ON] [--reference REFERENCE] [--tracking-branch TRACKING_BRANCH] [--spec-path SPEC_PATH] [--code-path CODE_PATH] - [--pin PIN] [--allow-empty-leg {spec,code}] + [--pin openProduct@COMMIT] + [--pin-owner PIN_OWNER] [--pin-source PIN_SOURCE] + [--allow-empty-leg {spec,code}] [--path-policy PATH_POLICY] [--out OUT] [--work-dir WORK_DIR] @@ -364,12 +366,26 @@ options: --tracking-branch TRACKING_BRANCH --spec-path SPEC_PATH --code-path CODE_PATH - --pin PIN a neutral product this project declares a pin on — a - NAME only, e.g. --pin openGlass or --pin - opensoft/openGlass to name the owner explicitly. - Adopting a project pins no commit at plan time, so a - trailing @ (scaffold-project.py's syntax) is - refused. + --pin openProduct@COMMIT + declare a neutral-product pin, as scaffold-project.py + --pin does, e.g. --pin openGlass@<40 hex>. `plan` + reads the digest of the product's tree at that commit + (from --pin-source, or `gh api`) and records it in the + plan; `execute` writes + contracts/-pin.yaml from it and lists the + product in the manifest. A name with no @, a + product named spec, code or shape, and one product + twice are refused. Repeatable. + --pin-owner PIN_OWNER + the organisation an UNQUALIFIED --pin name resolves + under (default: opensoft, where every neutral + open lives — never the adopted repository's + own --org). Name the owner in --pin itself + (owner/openProduct@) to override it for one pin. + --pin-source PIN_SOURCE + a local clone to compute --pin digests from, instead + of reading the forge with `gh api` (the TEST path; no + network) --allow-empty-leg {spec,code} record in the plan that this leg having NO path is intended, so it may be SEEDED from the shape's diff --git a/scaffold-project.py b/scaffold-project.py index 4836a526..f0c4044b 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -45,7 +45,6 @@ import argparse import datetime as _dt import json -import re import subprocess import sys import tempfile @@ -76,11 +75,14 @@ ) #: The pin argument: `openGlass@<40 hex>`, optionally organisation-qualified. -PIN_ARG_RE = re.compile(r"^(?P[A-Za-z0-9_.-]+(?:/[A-Za-z0-9_.-]+)?)@(?P[0-9a-fA-F]{40})$") - -#: How a neutral product's tree is read when the pin is taken from the forge -#: rather than from a clone on this machine. -GH_TREE_API = "repos/{repo}/git/trees/{commit}?recursive=1" +#: It, its digest and its collisions are `shape_materialize`'s since #273, +#: because `adopt-project.py plan --pin` takes the same argument and writes +#: the same file; imported under the names this file has always called them +#: by, so what it refuses, and in which words, is unchanged. +from shape_materialize import ( # noqa: E402 + declared_pin_products, declared_pin_values, + refuse_colliding_pins as _refuse_colliding_pins, +) #: The naming-policy form every link between a chain's first and last must #: classify as (#251). Each such link is an entry of the link before its @@ -89,106 +91,6 @@ CHAIN_LINK_FORM = "neutral-product" -def parse_pin(raw: str, owner: str = NEUTRAL_PRODUCT_OWNER) -> tuple[str, str, str]: - """`openGlass@` -> (product, `/openGlass`, commit). - - THE COMMIT IS 40 HEX OR IT IS NOT A PIN. An abbreviated oid, a branch name - or a tag cannot pass here for the same reason `contracts/-pin.yaml` - refuses them: a tag can be moved and a commit cannot. - - AN UNQUALIFIED NAME RESOLVES UNDER `owner` (opensoft, unless `--pin-owner` - said otherwise) — NEVER under the pinning project's own `--org`. Domain - repositories never author a neutral product; they only pin the opensoft - original, so `--org MedxSoft --pin openGlass@` must not go looking - for `MedxSoft/openGlass`, which does not exist and was never the claim. - """ - match = PIN_ARG_RE.match(raw.strip()) - if not match: - raise Refusal( - "pin-malformed", - f"--pin {raw!r} is not `[owner/]@<40 hex commit>`", - "Remediation: pass the full 40-character commit, e.g. " - "--pin openGlass@0123456789abcdef0123456789abcdef01234567 " - f"(resolved under {NEUTRAL_PRODUCT_OWNER}/openGlass by default), " - "or name the owner explicitly with --pin MedxSoft/openGlass@ " - "or --pin-owner MedxSoft. A tag or an abbreviated oid is refused: " - "a tag can be moved.", - ) - name = match.group("name") - product = name.rsplit("/", 1)[-1] - repository = name if "/" in name else f"{owner}/{product}" - return product, repository, match.group("commit").lower() - - -def pin_digest_from_source(source: Path, commit: str, repository: str) -> str: - """The `sorted-ls-tree-r-v1` digest, read from a clone on this machine.""" - try: - return tree_digest(source, commit) - except Refusal as exc: - raise Refusal( - "pin-source-unreadable", - f"{source} could not answer for {repository} @ {commit}: " - f"{exc.detail}", - "Remediation: point --pin-source at a clone of that repository " - "that HAS the pinned commit (`git fetch --all` first), or drop " - "--pin-source and let `gh api` read the forge.", - ) from exc - - -def pin_digest_from_gh(repository: str, commit: str) -> str: - """The same digest, computed from the forge's own recursive tree listing. - - A SHALLOW READ, not a clone: `git/trees/?recursive=1` returns one - row per object with `mode`, `type`, `sha` and `path` — exactly the four - columns `git ls-tree -r -z` emits, which is what makes the two readings - the same number. Tree rows are dropped because `-r` emits none; a - submodule arrives as `type: commit` with mode `160000` and is kept, as - `ls-tree` keeps it. - """ - proc = subprocess.run( - ["gh", "api", GH_TREE_API.format(repo=repository, commit=commit)], - # UTF-8 and STRICT, not `errors="replace"`: this JSON becomes a - # digest, and a replaced byte would be a wrong digest with a - # confident tone (#217). - capture_output=True, text=True, encoding="utf-8", check=False) - if proc.returncode != 0: - raise Refusal( - "pin-unreadable", - f"`gh api` could not read {repository} @ {commit}: " - f"{proc.stderr.strip()}", - "Remediation: check the commit exists in that repository and that " - "`gh auth status` can see it. An unqualified --pin resolves under " - f"{NEUTRAL_PRODUCT_OWNER} by default (never the project's own " - "--org) — if this product actually lives under a different " - "owner, name it explicitly with --pin /@ " - "or --pin-owner . You can also pass --pin-source to compute the digest with no network at all.", - ) - try: - payload = json.loads(proc.stdout) - except json.JSONDecodeError as exc: - raise Refusal("pin-unreadable", - f"{repository} @ {commit}: {exc}") from exc - if payload.get("truncated"): - raise Refusal( - "pin-tree-truncated", - f"the forge truncated its tree listing for {repository} @ " - f"{commit}, so the digest would be computed over a PARTIAL tree", - "Remediation: clone that repository and pass --pin-source . " - "A digest over a partial tree is a wrong answer with a confident " - "tone.", - ) - import hashlib - records = sorted( - f"{row['mode']} {row['type']} {row['sha']}\t{row['path']}".encode() - for row in payload.get("tree") or [] if row.get("type") != "tree") - digest = hashlib.sha256() - for record in records: - digest.update(record) - digest.update(b"\n") - return digest.hexdigest() - - def remote_is_empty(repository: str) -> bool | None: """Does `/` exist on GitHub with ZERO commits? @@ -345,47 +247,6 @@ def main(argv: list[str] | None = None) -> int: return 2 -def declared_pin_values(args) -> dict[str, dict[str, str]]: - """The template values for every `--pin`, with its digest resolved. - - The digest is the same `sorted-ls-tree-r-v1` number every other pin here - carries, read either from a clone on this machine (`--pin-source`, and no - network at all) or from the forge's own recursive tree listing. - """ - pins: dict[str, dict[str, str]] = {} - for raw in args.pin: - product, repository, commit = parse_pin(raw, args.pin_owner) - if args.pin_source is not None: - digest = pin_digest_from_source(args.pin_source.resolve(), commit, - repository) - digest_source = f"local-clone ({args.pin_source})" - else: - digest = pin_digest_from_gh(repository, commit) - digest_source = "gh-api-tree-recursive" - pins[product] = { - "PIN_PRODUCT": product, - "PIN_REPOSITORY": repository, - "PIN_COMMIT": commit, - "PIN_TREE_SHA256": digest, - "PIN_DIGEST_SOURCE": digest_source, - } - return pins - - -def declared_pin_products(args) -> dict[str, None]: - """The products `declared_pin_values` keys its pins by, in its order and - read by the same `parse_pin`, with no digest: so no `git` and no `gh`. - - #197: `_build_assembly_root` hands those pins to the materializer, which - writes `neutral_pin_path(product)` for each, and the leg paths are asked - about that file before `_identity` and `_declaration` run, which is - before any digest is read. A malformed `--pin` is therefore refused - here, by `parse_pin`'s own `pin-malformed`, rather than in - `_declaration`; it was refused before anything was created either way. - """ - return dict.fromkeys(parse_pin(raw, args.pin_owner)[0] for raw in args.pin) - - def _reusable_remotes(names: dict, urls: dict, repositories: dict, local: bool, reuse_flag: bool) -> set[str]: """Which remotes already exist, and whether that is allowed. @@ -438,92 +299,6 @@ def _exists_remedy(role: str, empty: bool, reuse_flag: bool) -> str: "--force.") -#: What every refused `--pin` is told to do. The file a pin is written at is -#: `neutral_pin_path`'s, spelled for a placeholder product, never by hand. -PIN_COLLISION_REMEDIATION = ( - "Remediation: nothing was created. A neutral product is named for the " - "repository it pins, as `--pin openGlass@<40 hex commit>` names " - f"{NEUTRAL_PRODUCT_OWNER}/openGlass by default, and its pin is written at " - f"`{neutral_pin_path('')}`, the name lowercased, beside the " - "pins the shape writes for its two legs and for itself. Pass each product " - "once, by the name of the repository it pins." -) - - -def _refuse_colliding_pins(pins, owner: str) -> None: - """No `--pin` is written where the shape writes a file of its own, and no - two `--pin`s are written at one path, or this refuses (#241). - - `_build_assembly_root` hands the pins to the materializer, which writes - each at `neutral_pin_path(product)` after the shape's own files and - before the copy pin, and a second file at one path is - `materialize-collision`: a pin over a leg's pin, or the copy pin over a - pin named `shape`, or a pin over another pin. So each was met only after - `_create_remotes` and `_seed_legs`. On main through 39ac59c, `--pin - spec@` (`Spec@`, `code@`, `MedxSoft/code@`, `shape@`) and `--pin - openGlass@ --pin OpenGlass@` planned under `--dry-run`, then - created all three repositories, pushed both legs, left the root with no - commit and exited 2, and the corrected re-run was refused as - `scaffold-remote-exists`. Against GitHub, each left three repositories - for a run that made no project. - - THE QUESTION IS ASKED THROUGH THE MATERIALIZER'S OWN TWO DEFINITIONS: a - pin's file is `neutral_pin_path(product)`, and the shape's own files are - `assembly_root_paths()` with no pins. `neutral_pin_path` lowercases, so - `Spec` is `spec` and `openGlass` and `OpenGlass` are one file, with no - case rule of this tool's own. The product is read by `parse_pin` alone, - so a malformed `--pin` is still its `pin-malformed`, and no digest is - read: no `git` and no `gh`. - - THE RAW `--pin` LIST, NEVER `declared_pin_products`, which collapses a - repeated product, as `declared_pin_values` does by keeping the last: - `--pin openGlass@ --pin openGlass@` planned one pin, wrote `` - and dropped `` without a word. A declaration is a claim about what - the project descends from, so a repeat is refused whether or not its - commits agree: either way it is a typing error, and one of the two - would be lost. Every argument is named as it was typed. - - IT RUNS BEFORE `_refuse_colliding_leg_paths`, which asks about - `assembly_root_paths()`. With `--pin spec@` - that list held `contracts/spec-pin.yaml` twice, and `--spec-path - contracts` was told it contains that file "and 4 more" where there - were three (#197's re-review, N1). Refused first, no pin path it lists - is the shape's or listed twice. - """ - # Folded on both sides, as `path_overlap` folds the leg paths: a future - # mixed-case shape file under `contracts/` is still one path with a - # lowercased pin on the case-insensitive disk of macOS and Windows. - shape_files = {shape_file.casefold() - for shape_file in assembly_root_paths()} - written: dict[str, list[tuple[str, str]]] = {} - for raw in pins: - product = parse_pin(raw, owner)[0] - path = neutral_pin_path(product) - written.setdefault(path, []).append((raw, product)) - collisions = [] - for path, typed in written.items(): - in_shape = path.casefold() in shape_files - if not in_shape and len(typed) == 1: - continue - products = list(dict.fromkeys(product for _, product in typed)) - said = (" and ".join(f"--pin {raw!r}" for raw, _ in typed) - + (" writes the pin" if len(typed) == 1 else " write the pins") - + " of the product" + ("s" if len(products) > 1 else "") + " " - + " and ".join(f"`{product}`" for product in products)) - if in_shape: - collisions.append( - f"{said} at `{path}`, a file the shape writes into the " - "assembly root itself: name the product for the repository it " - "pins.") - else: - collisions.append( - f"{said} at one file, `{path}`, which holds one pin: pass " - "each product once.") - if collisions: - raise Refusal("scaffold-pin-collides", " ".join(collisions), - PIN_COLLISION_REMEDIATION) - - def _leg_collision_remediation(neutral_pins=()) -> str: """What a leg-path collision tells the person to do. The paths are `assembly_root_paths(neutral_pins)`, the materializer's own list for this diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index b84e2606..8a0f6376 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -32,6 +32,7 @@ import argparse import codecs import datetime as _dt +import json import os import re import shutil @@ -46,6 +47,12 @@ double_quoted, file_sha256, ) +# What a declared neutral-product pin is read with (#273): the forge's tree +# listing, the owner an unqualified name resolves under, and the digest. +from repo_shape import ( # noqa: E402 + GH_TREE_API, NEUTRAL_PRODUCT_OWNER, tree_digest, +) + SHAPE_REPOSITORY = "opensoft/openRepoShape" @@ -1523,8 +1530,8 @@ def shape_path_collisions(what: str, value: str, `neutral_pins` IS THE SAME KIND OF ARGUMENT: the products whose pins the caller hands the materializer, so the paths asked about are `assembly_root_paths(neutral_pins)`, each pin's file among them. The - scaffold passes its `--pin` products (#197); adopt hands the materializer - no pin and passes none, so its list and its sentences are unchanged. + scaffold passes its `--pin` products (#197), and adopt the products + its plan declares a pin on, whose files `execute` writes since #273. """ out: list[str] = [] files = [path for path in assembly_root_paths(neutral_pins) @@ -1549,6 +1556,283 @@ def shape_path_collisions(what: str, value: str, return out +# --------------------------------------------------------------------------- +# A declared neutral-product pin (#273) +# --------------------------------------------------------------------------- +# +# `scaffold-project.py --pin @<40 hex>` read the argument, the digest +# and the collisions with functions of its own, and `adopt-project.py plan +# --pin` took a NAME only and wrote no pin file, so the adopted root's own +# `validate-pins.py` refused it as `neutral-pin-missing` on its first run. +# Brett Heap ruled on 2026-10-12 that adopt takes the argument scaffold takes +# and writes the file scaffold writes, so the reading moved here, beside the +# materializer that writes the file (`neutral_pin_path`), and both tools +# import it: one argument, one digest, one set of refusals, in one set of +# words. `scaffold-project.py` imports each under the name it always called it +# by, and its messages are these, unchanged. + +#: The pin argument: `openGlass@<40 hex>`, optionally organisation-qualified. +PIN_ARG_RE = re.compile(r"^(?P[A-Za-z0-9_.-]+(?:/[A-Za-z0-9_.-]+)?)@(?P[0-9a-fA-F]{40})$") + + +def parse_pin(raw: str, owner: str = NEUTRAL_PRODUCT_OWNER) -> tuple[str, str, str]: + """`openGlass@` -> (product, `/openGlass`, commit). + + THE COMMIT IS 40 HEX OR IT IS NOT A PIN. An abbreviated oid, a branch name + or a tag cannot pass here for the same reason `contracts/-pin.yaml` + refuses them: a tag can be moved and a commit cannot. + + AN UNQUALIFIED NAME RESOLVES UNDER `owner` (opensoft, unless `--pin-owner` + said otherwise) — NEVER under the pinning project's own `--org`. Domain + repositories never author a neutral product; they only pin the opensoft + original, so `--org MedxSoft --pin openGlass@` must not go looking + for `MedxSoft/openGlass`, which does not exist and was never the claim. + + A NAME WITH NO `@` IS REFUSED HERE TOO, by both tools: since #273 + `adopt-project.py plan --pin` writes the pin file as the scaffold does, + and a name alone is a declaration with no referent. + """ + match = PIN_ARG_RE.match(raw.strip()) + if not match: + raise Refusal( + "pin-malformed", + f"--pin {raw!r} is not `[owner/]@<40 hex commit>`", + "Remediation: pass the full 40-character commit, e.g. " + "--pin openGlass@0123456789abcdef0123456789abcdef01234567 " + f"(resolved under {NEUTRAL_PRODUCT_OWNER}/openGlass by default), " + "or name the owner explicitly with --pin MedxSoft/openGlass@ " + "or --pin-owner MedxSoft. A tag or an abbreviated oid is refused: " + "a tag can be moved.", + ) + name = match.group("name") + product = name.rsplit("/", 1)[-1] + repository = name if "/" in name else f"{owner}/{product}" + return product, repository, match.group("commit").lower() + + +def pin_digest_from_source(source: Path, commit: str, repository: str) -> str: + """The `sorted-ls-tree-r-v1` digest, read from a clone on this machine.""" + try: + return tree_digest(source, commit) + except Refusal as exc: + raise Refusal( + "pin-source-unreadable", + f"{source} could not answer for {repository} @ {commit}: " + f"{exc.detail}", + "Remediation: point --pin-source at a clone of that repository " + "that HAS the pinned commit (`git fetch --all` first), or drop " + "--pin-source and let `gh api` read the forge.", + ) from exc + + +def pin_digest_from_gh(repository: str, commit: str) -> str: + """The same digest, computed from the forge's own recursive tree listing. + + A SHALLOW READ, not a clone: `git/trees/?recursive=1` returns one + row per object with `mode`, `type`, `sha` and `path` — exactly the four + columns `git ls-tree -r -z` emits, which is what makes the two readings + the same number. Tree rows are dropped because `-r` emits none; a + submodule arrives as `type: commit` with mode `160000` and is kept, as + `ls-tree` keeps it. + """ + proc = subprocess.run( + ["gh", "api", GH_TREE_API.format(repo=repository, commit=commit)], + # UTF-8 and STRICT, not `errors="replace"`: this JSON becomes a + # digest, and a replaced byte would be a wrong digest with a + # confident tone (#217). + capture_output=True, text=True, encoding="utf-8", check=False) + if proc.returncode != 0: + raise Refusal( + "pin-unreadable", + f"`gh api` could not read {repository} @ {commit}: " + f"{proc.stderr.strip()}", + "Remediation: check the commit exists in that repository and that " + "`gh auth status` can see it. An unqualified --pin resolves under " + f"{NEUTRAL_PRODUCT_OWNER} by default (never the project's own " + "--org) — if this product actually lives under a different " + "owner, name it explicitly with --pin /@ " + "or --pin-owner . You can also pass --pin-source to compute the digest with no network at all.", + ) + try: + payload = json.loads(proc.stdout) + except json.JSONDecodeError as exc: + raise Refusal("pin-unreadable", + f"{repository} @ {commit}: {exc}") from exc + if payload.get("truncated"): + raise Refusal( + "pin-tree-truncated", + f"the forge truncated its tree listing for {repository} @ " + f"{commit}, so the digest would be computed over a PARTIAL tree", + "Remediation: clone that repository and pass --pin-source . " + "A digest over a partial tree is a wrong answer with a confident " + "tone.", + ) + import hashlib + records = sorted( + f"{row['mode']} {row['type']} {row['sha']}\t{row['path']}".encode() + for row in payload.get("tree") or [] if row.get("type") != "tree") + digest = hashlib.sha256() + for record in records: + digest.update(record) + digest.update(b"\n") + return digest.hexdigest() + + +def declared_pin_values(args) -> dict[str, dict[str, str]]: + """The template values for every `--pin`, with its digest resolved. + + The digest is the same `sorted-ls-tree-r-v1` number every other pin here + carries, read either from a clone on this machine (`--pin-source`, and no + network at all) or from the forge's own recursive tree listing. + + `args` is either tool's command line: `pin`, `pin_owner` and `pin_source` + are the same three flags in `scaffold-project.py` and in + `adopt-project.py plan`, which records these values in the plan so that + `check` and `execute` read no network for them (#273). + """ + pins: dict[str, dict[str, str]] = {} + for raw in args.pin: + product, repository, commit = parse_pin(raw, args.pin_owner) + if args.pin_source is not None: + digest = pin_digest_from_source(args.pin_source.resolve(), commit, + repository) + digest_source = f"local-clone ({args.pin_source})" + else: + digest = pin_digest_from_gh(repository, commit) + digest_source = "gh-api-tree-recursive" + pins[product] = { + "PIN_PRODUCT": product, + "PIN_REPOSITORY": repository, + "PIN_COMMIT": commit, + "PIN_TREE_SHA256": digest, + "PIN_DIGEST_SOURCE": digest_source, + } + return pins + + +def declared_pin_products(args) -> dict[str, None]: + """The products `declared_pin_values` keys its pins by, in its order and + read by the same `parse_pin`, with no digest: so no `git` and no `gh`. + + #197: `_build_assembly_root` hands those pins to the materializer, which + writes `neutral_pin_path(product)` for each, and the leg paths are asked + about that file before `_identity` and `_declaration` run, which is + before any digest is read. A malformed `--pin` is therefore refused + here, by `parse_pin`'s own `pin-malformed`, rather than in + `_declaration`; it was refused before anything was created either way. + `adopt-project.py plan` asks its leg paths about the same files, from + the same products, before it reads a digest (#273). + """ + return dict.fromkeys(parse_pin(raw, args.pin_owner)[0] for raw in args.pin) + + +#: What every refused `--pin` is told to do. The file a pin is written at is +#: `neutral_pin_path`'s, spelled for a placeholder product, never by hand. +PIN_COLLISION_REMEDIATION = ( + "Remediation: nothing was created. A neutral product is named for the " + "repository it pins, as `--pin openGlass@<40 hex commit>` names " + f"{NEUTRAL_PRODUCT_OWNER}/openGlass by default, and its pin is written at " + f"`{neutral_pin_path('')}`, the name lowercased, beside the " + "pins the shape writes for its two legs and for itself. Pass each product " + "once, by the name of the repository it pins." +) + + +def pin_collisions(typed) -> list[str]: + """One sentence for each file that a pin would be written over: a file + the shape writes itself, or a file another pin is written at (#241). + `[]` when no pin collides. + + `typed` is `(said, product)` for each pin, in the order the person gave + them: `said` names the pin as they gave it (`--pin 'spec@'` on a + command line, or the `pins:` entry of an adoption plan), and `product` + is the name its file is spelled from. ONE LIST OF EVERY PIN, NEVER ONE + KEYED BY PRODUCT, which collapses a repeat: each pin given is named, and + a repeat is a collision whether or not its commits agree. + + THE QUESTION IS ASKED THROUGH THE MATERIALIZER'S OWN TWO DEFINITIONS: a + pin's file is `neutral_pin_path(product)`, and the shape's own files are + `assembly_root_paths()` with no pins. `neutral_pin_path` lowercases, so + `Spec` is `spec` and `openGlass` and `OpenGlass` are one file, with no + case rule of this function's own. + """ + # Folded on both sides, as `path_overlap` folds the leg paths: a future + # mixed-case shape file under `contracts/` is still one path with a + # lowercased pin on the case-insensitive disk of macOS and Windows. + shape_files = {shape_file.casefold() + for shape_file in assembly_root_paths()} + written: dict[str, list[tuple[str, str]]] = {} + for said, product in typed: + written.setdefault(neutral_pin_path(product), []).append( + (said, product)) + collisions = [] + for path, pins in written.items(): + in_shape = path.casefold() in shape_files + if not in_shape and len(pins) == 1: + continue + products = list(dict.fromkeys(product for _, product in pins)) + said = (" and ".join(said for said, _ in pins) + + (" writes the pin" if len(pins) == 1 else " write the pins") + + " of the product" + ("s" if len(products) > 1 else "") + " " + + " and ".join(f"`{product}`" for product in products)) + if in_shape: + collisions.append( + f"{said} at `{path}`, a file the shape writes into the " + "assembly root itself: name the product for the repository it " + "pins.") + else: + collisions.append( + f"{said} at one file, `{path}`, which holds one pin: pass " + "each product once.") + return collisions + + +def refuse_colliding_pins(pins, owner: str, + refusal: str = "scaffold-pin-collides") -> None: + """No `--pin` is written where the shape writes a file of its own, and no + two `--pin`s are written at one path, or this refuses (#241). + + `scaffold-project.py`'s `_build_assembly_root` hands the pins to the + materializer, which writes each at `neutral_pin_path(product)` after the + shape's own files and before the copy pin, and a second file at one path + is `materialize-collision`: a pin over a leg's pin, or the copy pin over + a pin named `shape`, or a pin over another pin. So each was met only + after `_create_remotes` and `_seed_legs`. On main through 39ac59c, `--pin + spec@` (`Spec@`, `code@`, `MedxSoft/code@`, `shape@`) and `--pin + openGlass@ --pin OpenGlass@` planned under `--dry-run`, then + created all three repositories, pushed both legs, left the root with no + commit and exited 2, and the corrected re-run was refused as + `scaffold-remote-exists`. Against GitHub, each left three repositories + for a run that made no project. `adopt-project.py plan` asks the same of + its `--pin`s before it writes a plan (#273), as `refusal` + `adopt-pin-collides`, and `check` asks it of the plan's `pins:` through + `pin_collisions`. + + The product is read by `parse_pin` alone, so a malformed `--pin` is still + its `pin-malformed`, and no digest is read: no `git` and no `gh`. + + THE RAW `--pin` LIST, NEVER `declared_pin_products`, which collapses a + repeated product, as `declared_pin_values` does by keeping the last: + `--pin openGlass@ --pin openGlass@` planned one pin, wrote `` + and dropped `` without a word. A declaration is a claim about what + the project descends from, so a repeat is refused whether or not its + commits agree: either way it is a typing error, and one of the two + would be lost. Every argument is named as it was typed. + + IT RUNS BEFORE THE LEG-PATH COLLISION REFUSAL, which asks about + `assembly_root_paths()`. With `--pin spec@` + that list held `contracts/spec-pin.yaml` twice, and `--spec-path + contracts` was told it contains that file "and 4 more" where there + were three (#197's re-review, N1). Refused first, no pin path it lists + is the shape's or listed twice. + """ + collisions = pin_collisions([(f"--pin {raw!r}", parse_pin(raw, owner)[0]) + for raw in pins]) + if collisions: + raise Refusal(refusal, " ".join(collisions), PIN_COLLISION_REMEDIATION) + + def materialize_family_root(shape_root: Path, target: Path, values: dict[str, str]) -> Materialized: """Write the FAMILY-root skeleton into `target`. diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py new file mode 100644 index 00000000..2e664861 --- /dev/null +++ b/tests/test_adopt_pins.py @@ -0,0 +1,730 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`adopt-project.py plan --pin` takes a pin and `execute` writes its file (#273). + +On main through c7c02fa, `adopt-project.py plan --pin` took a NAME only +(`--pin openGlass`, `--pin opensoft/openGlass`) and refused a trailing +`@` as `adopt-pin-malformed`. `plan`, `check` and `execute` each exited +0, `execute` said `adoption verified`, and the manifest listed the name in +`neutral_product_pins:` with no `contracts/-pin.yaml` beside it, so +the adopted root's own `validate-pins.py` refused it as `neutral-pin-missing` +on its first run. `--pin opensoft/openGlass` put the owner in the manifest, +naming a file in a directory; `--pin spec` and `--pin shape` passed the gate +on the leg's or the shape's own pin; a repeated `--pin` was listed twice. + +Brett Heap ruled on 2026-10-12 for the scaffold's contract: `--pin +[owner/]product@<40 hex>`, with `--pin-owner` and `--pin-source` meaning what +they mean for `scaffold-project.py`, the digest read at `plan` time and +recorded in the plan with the product, the repository and the commit, so that +`check` and `execute` read no network for a pin; `execute` writes the pin file +through the materializer the scaffold uses, and the manifest lists the product +with its owner stripped. The refusals the scaffold makes (#241) are adopt's: +`spec`, `code` and `shape` in any case, and one product twice. A plan that +holds a name alone, or a pin missing its commit or digest, is a `check` finding +and an `execute` refusal before any leg exists. + +NO NETWORK AND NO GITHUB. The neutral product is a stand-in repository made +here; `--pin-source` points at it, and a `gh` on PATH records its arguments and +exits 97, so a run that called it would say so (Windows runs no `#!` script, +so there the record stays empty whatever happens). The end-to-end tests need +`git filter-repo`, as `tests/test_adopt_e2e.py` does, and skip without it. +""" + +from __future__ import annotations + +import importlib.util +import json +import os +import shutil +import stat +import subprocess +import sys +import textwrap +from pathlib import Path + +import pytest + +from conftest import (ADOPT, FILE_PROTOCOL, REPO, SCAFFOLD, SYNTHETIC_TREE, + WINDOWS_SKIP, findings, git, make_source_repo, planned, + refused_before_any_leg, resolve, run_script) + +sys.path.insert(0, str(REPO / "scripts")) +import shape_materialize # noqa: E402 +from repo_shape import load_yaml, tree_digest # noqa: E402 + +UTF8 = "utf-8" +PROJECT = "MedxGlass" +ORG = "testorg" +BRANCH = "adopt/three-repo-shape" +ANSWERS = (("examples/", "spec"), (".claude/", "root"), + ("release.yaml", "root")) +#: What `conftest.planned` answers, for a plan written with no pin. +PLANNED = dict(ANSWERS) +PIN_FILE = "contracts/openglass-pin.yaml" +#: How the gate this test vouches for says the pin's digest was RECOMPUTED +#: from the stand-in, never skipped (#272). +RECOMPUTES = "openGlass: pin commit {commit} digest recomputes" + +needs_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git filter-repo is not installed: `pip install git-filter-repo`") + + +@pytest.fixture(scope="module") +def adopter(): + """`adopt-project.py` loaded as a module, so `cmd_execute` runs here.""" + module_spec = importlib.util.spec_from_file_location("adopt_pins", ADOPT) + module = importlib.util.module_from_spec(module_spec) + module_spec.loader.exec_module(module) + return module + + +def make_product(path: Path) -> tuple[Path, str, str]: + """A local stand-in for `openGlass` with two commits: `(path, first, + second)`. Its files are written as bytes, so the tree is the same on + every platform.""" + path.mkdir(parents=True) + git("init", "-q", "-b", "main", cwd=path) + commits = [] + for name, body in (("README.md", b"# openGlass\n"), + ("contracts/glass.yaml", b"schema_version: 1\n")): + (path / name).parent.mkdir(parents=True, exist_ok=True) + (path / name).write_bytes(body) + git("add", "-A", cwd=path) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + name, cwd=path) + commits.append(git("rev-parse", "HEAD", cwd=path).stdout.strip()) + return (path, *commits) + + +@pytest.fixture(scope="module") +def glass(tmp_path_factory): + return make_product(tmp_path_factory.mktemp("glass") / "openGlass") + + +def stub_gh(directory: Path) -> dict: + """A `gh` that records its arguments in `gh.log` beside it and exits 97, + and the environment that puts it first on PATH; with `GH_STUB_TREE` + set, it prints that file and exits 0 instead, as `gh api` prints a + tree listing.""" + bin_dir = directory / "fakebin" + bin_dir.mkdir() + gh = bin_dir / "gh" + gh.write_text(textwrap.dedent("""\ + #!/usr/bin/env python3 + import os, sys + with open(os.environ["GH_STUB_LOG"], "a", encoding="utf-8") as log: + log.write(" ".join(sys.argv[1:]) + "\\n") + tree = os.environ.get("GH_STUB_TREE") + if tree: + with open(tree, encoding="utf-8") as listing: + sys.stdout.write(listing.read()) + sys.exit(0) + print("HTTP 404: Not Found", file=sys.stderr) + sys.exit(97) + """), encoding=UTF8) + gh.chmod(gh.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) + return {"PATH": f"{bin_dir}{os.pathsep}{os.environ.get('PATH', '')}", + "GH_STUB_LOG": str(directory / "gh.log")} + + +def gh_calls(env: dict) -> str: + log = Path(env["GH_STUB_LOG"]) + return log.read_text(encoding=UTF8) if log.exists() else "" + + +@pytest.fixture +def gh_env(tmp_path): + return stub_gh(tmp_path) + + +def plan_with(source: Path, out: Path, *pins: str, extra: tuple = (), + env: dict | None = None, + project: str = PROJECT) -> subprocess.CompletedProcess: + """`plan` for `source` with a `--pin` for each of `pins`.""" + return run_script(ADOPT, "plan", "--source", str(source), + "--project", project, "--org", ORG, + "--elected-by", "Test Human", + "--elected-on", "2026-09-02", "--out", str(out), + *(arg for pin in pins for arg in ("--pin", pin)), + *extra, env=env) + + +def pin_entry(product: str = "openGlass", + repository: str = "opensoft/openGlass", commit: str = "", + digest: str = "", source: str = "local-clone (openGlass)", + omit: tuple = ()) -> str: + """One `pins:` mapping as `plan` writes it, its keys in `plan`'s order, + with each key in `omit` left out.""" + fields = (("product", product), ("repository", repository), + ("commit", f'"{commit}"'), ("tree_sha256", f'"{digest}"'), + ("digest_source", f'"{source}"')) + lines = [f"{key}: {value}" for key, value in fields if key not in omit] + return "".join((" - " if index == 0 else " ") + line + "\n" + for index, line in enumerate(lines)) + + +def with_pins(plan: Path, block: str) -> None: + """The plan's `pins: []` replaced by `block` (which starts with + `pins:`), as a hand edit would leave it: bytes, so LF everywhere.""" + text = plan.read_bytes().decode(UTF8) + assert text.count("pins: []\n") == 1, "the plan was written with no pin" + plan.write_bytes(text.replace("pins: []\n", block, 1).encode(UTF8)) + + +def a_good_pin(glass) -> str: + path, commit, _ = glass + return pin_entry(commit=commit, digest=tree_digest(path, commit)) + + +# --- the two spellings of #273's table, end to end -------------------------- +# +# `openGlass` and `opensoft/openGlass` were the first two rows: each exited 0 +# through `execute` with no pin file, the second with the owner in the +# manifest. Each now records the pin at `plan`, writes the file at `execute` +# and leaves a root whose own gate passes, with no network: the stand-in the +# digest was read from is DELETED before `check`, so neither `check` nor +# `execute` can have read it, and `gh` is never run. + +#: `(the --pin name, the repository it pins)`. +SPELLINGS = [ + pytest.param(("openGlass", "opensoft/openGlass"), id="name"), + pytest.param(("opensoft/openGlass", "opensoft/openGlass"), id="owner"), + pytest.param(("ExplicitOwner/openGlass", "ExplicitOwner/openGlass"), + id="another-owner"), +] + + +@pytest.fixture(scope="module", params=SPELLINGS) +def adopted(request, tmp_path_factory, glass): + """One real adoption with one `--pin`: plan, answer, check, execute; + and, for the same pin read from the same stand-in, one real scaffold, + whose pin file the adoption's is compared with.""" + if shutil.which("git-filter-repo") is None: + pytest.skip("git filter-repo is not installed") + spelling, repository = request.param + base = tmp_path_factory.mktemp("adopt-pin") + env = stub_gh(base) + _, commit, _ = glass + # A copy the digest is read from, deleted before `check`. + readable = base / "openGlass-copy" + shutil.copytree(glass[0], readable, symlinks=True) + scaffolded = run_script( + SCAFFOLD, "--org", ORG, "--project", PROJECT, + "--elected-by", "Test Human", "--elected-on", "2026-09-02", + "--pin", f"{spelling}@{commit}", "--pin-source", str(readable), + "--local-remote-dir", str(base / "scaffold-remotes"), + "--work-dir", str(base / "scaffold-work"), env=env) + assert scaffolded.returncode == 0, scaffolded.stderr + scaffolded.stdout + source = make_source_repo(base / "Thing") + plan = base / "adoption-plan.yaml" + planned_run = plan_with(source, plan, f"{spelling}@{commit}", + extra=("--pin-source", str(readable)), env=env) + assert planned_run.returncode == 0, planned_run.stderr + planned_run.stdout + for path, leg in ANSWERS: + resolve(plan, path, leg) + shutil.rmtree(readable) + checked = run_script(ADOPT, "check", "--plan", str(plan), env=env) + executed = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(base / "remotes"), + "--work-dir", str(base / "work"), env=env) + clone = base / "clone" / PROJECT + if executed.returncode == 0: + proc = subprocess.run( + ["git", *FILE_PROTOCOL, "clone", "-q", "--recurse-submodules", + "-b", BRANCH, str(source), str(clone)], + capture_output=True, check=False) + assert proc.returncode == 0, proc.stderr.decode(UTF8, "replace") + return {"base": base, "source": source, "plan": plan, "env": env, + "repository": repository, "commit": commit, + "readable": readable, "planned": planned_run, + "checked": checked, "executed": executed, "clone": clone, + "scaffold_root": base / "scaffold-remotes" / f"{PROJECT}.git"} + + +@needs_filter_repo +def test_plan_records_the_pin_whole(adopted, glass): + """Product, repository, commit, digest and how it was read: everything + `execute` writes the file from, so that it reads no network.""" + pins = load_yaml(adopted["plan"])["pins"] + assert pins == [{ + "product": "openGlass", + "repository": adopted["repository"], + "commit": adopted["commit"], + "tree_sha256": tree_digest(glass[0], adopted["commit"]), + "digest_source": f"local-clone ({adopted['readable']})", + }] + assert (f"pin {adopted['repository']} @ {adopted['commit'][:12]}" + in adopted["planned"].stdout) + assert f"-> {PIN_FILE}" in adopted["planned"].stdout + + +@needs_filter_repo +def test_check_and_execute_read_neither_the_source_nor_gh(adopted): + """The stand-in was deleted after `plan`, and `gh` records any call: the + two commands ran on what the plan recorded, and nothing else.""" + checked, executed = adopted["checked"], adopted["executed"] + assert checked.returncode == 0, checked.stderr + checked.stdout + assert (f" write {PIN_FILE}: {adopted['repository']} @ " + f"{adopted['commit'][:12]}") in checked.stdout + assert executed.returncode == 0, executed.stderr + executed.stdout + assert "adoption verified" in executed.stdout + assert not adopted["readable"].exists() + assert gh_calls(adopted["env"]) == "", gh_calls(adopted["env"]) + + +@needs_filter_repo +def test_the_split_holds_the_pin_the_scaffold_writes(adopted): + """The file is the one `scaffold-project.py --pin` writes for the same + pin, byte for byte, both read from git and not from a working tree; and + the manifest lists the product with its owner stripped, so the entry and + the file agree (#273 decision 3).""" + split = git("show", f"{BRANCH}:{PIN_FILE}", cwd=adopted["source"]).stdout + scaffold = git("show", f"main:{PIN_FILE}", + cwd=adopted["scaffold_root"]).stdout + assert split == scaffold + pin = load_yaml(adopted["clone"] / PIN_FILE) + assert pin["pin_role"] == "neutral-product" + assert pin["product"] == "openGlass" + assert pin["source_repository"] == adopted["repository"] + assert pin["commit"] == adopted["commit"] + assert pin["revision_kind"] == "commit" + manifest = load_yaml(adopted["clone"] / "project.yaml") + assert manifest["neutral_product_pins"] == ["openGlass"] + assembly = next(leg for leg in manifest["legs"] + if leg["role"] == "assembly") + assert assembly["naming"]["form"] == "domain-descendant" + assert assembly["naming"]["referent_declared"] is True + + +@needs_filter_repo +def test_the_adopted_root_passes_its_own_gate(adopted, glass): + """The point of #273: the validators the root carries, and the bootstrap + `make bootstrap` runs, pass on the root `execute` called verified, with + the pin's digest recomputed from the stand-in (`SHAPE_PIN_SOURCE_*`).""" + clone = adopted["clone"] + env = {**adopted["env"], "SHAPE_PIN_SOURCE_OPENGLASS": str(glass[0])} + for validator, args in (("validate-repository-naming.py", + ["--project", "project.yaml"]), + ("validate-manifest.py", []), + ("validate-pins.py", [])): + result = run_script(clone / "scripts" / validator, *args, cwd=clone, + env=env) + assert result.returncode == 0, \ + f"{validator}: {result.stderr}{result.stdout}" + if validator == "validate-pins.py": + assert RECOMPUTES.format(commit=adopted["commit"][:12]) \ + in result.stdout, result.stdout + booted = run_script(clone / "scripts" / "bootstrap.py", cwd=clone, + env=env) + assert booted.returncode == 0, booted.stderr + booted.stdout + assert "bootstrap ok" in booted.stdout + assert RECOMPUTES.format(commit=adopted["commit"][:12]) in booted.stdout + assert gh_calls(adopted["env"]) == "" + + +# --- what `plan` refuses, in the scaffold's words --------------------------- + +@pytest.mark.parametrize("value", ["openGlass", "opensoft/openGlass"]) +def test_a_name_with_no_commit_is_refused_by_plan(source_repo, tmp_path, + gh_env, value): + """The old spelling. A name alone is a declaration with no referent, + and the refusal says the spelling that is one, as the scaffold's does.""" + out = tmp_path / "plan.yaml" + result = plan_with(source_repo, out, value, env=gh_env) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr.startswith( + f"REFUSED pin-malformed: --pin {value!r} is not " + "`[owner/]@<40 hex commit>`"), result.stderr + assert ("--pin openGlass@0123456789abcdef0123456789abcdef01234567" + in result.stderr) + assert not out.exists() + assert gh_calls(gh_env) == "" + + +#: `(the --pin values, what the refusal says)`, as +#: `tests/test_scaffold_pin_collisions.py` spells the scaffold's: `{A}` and +#: `{B}` are the stand-in's two commits. The first five name a file the shape +#: writes; the last three are two pins at one file. +SHAPE_WRITES = ("a file the shape writes into the assembly root itself: name " + "the product for the repository it pins.") +ONE_FILE = "which holds one pin: pass each product once." +REFUSED_CASES = [ + pytest.param(["spec@{A}"], + "--pin 'spec@{A}' writes the pin of the product `spec` at " + "`contracts/spec-pin.yaml`, " + SHAPE_WRITES, id="spec"), + pytest.param(["Spec@{A}"], + "--pin 'Spec@{A}' writes the pin of the product `Spec` at " + "`contracts/spec-pin.yaml`, " + SHAPE_WRITES, id="Spec"), + pytest.param(["code@{A}"], + "--pin 'code@{A}' writes the pin of the product `code` at " + "`contracts/code-pin.yaml`, " + SHAPE_WRITES, id="code"), + pytest.param([f"{ORG}/code@{{A}}"], + f"--pin '{ORG}/code@{{A}}' writes the pin of the product " + "`code` at `contracts/code-pin.yaml`, " + SHAPE_WRITES, + id="owner-code"), + pytest.param(["shape@{A}"], + "--pin 'shape@{A}' writes the pin of the product `shape` at " + "`contracts/shape-pin.yaml`, " + SHAPE_WRITES, id="shape"), + pytest.param(["openGlass@{A}", "OpenGlass@{A}"], + "--pin 'openGlass@{A}' and --pin 'OpenGlass@{A}' write the " + "pins of the products `openGlass` and `OpenGlass` at one " + "file, `contracts/openglass-pin.yaml`, " + ONE_FILE, + id="case-pair"), + pytest.param(["openGlass@{A}", "openGlass@{A}"], + "--pin 'openGlass@{A}' and --pin 'openGlass@{A}' write the " + "pins of the product `openGlass` at one file, " + "`contracts/openglass-pin.yaml`, " + ONE_FILE, + id="repeat-one-commit"), + pytest.param(["openGlass@{A}", "opensoft/openGlass@{B}"], + "--pin 'openGlass@{A}' and --pin 'opensoft/openGlass@{B}' " + "write the pins of the product `openGlass` at one file, " + "`contracts/openglass-pin.yaml`, " + ONE_FILE, + id="repeat-two-commits"), +] + + +@pytest.mark.parametrize("values, said", REFUSED_CASES) +def test_a_colliding_pin_is_refused_by_plan(source_repo, tmp_path, glass, + gh_env, values, said): + """#273 decisions 4 and 5, answered as the scaffold answers them (#241): + a product named for a file the shape writes, and one product twice, + refused before a plan is written and before any digest is read, in the + scaffold's sentence and remediation under adopt's own code.""" + path, first, second = glass + out = tmp_path / "plan.yaml" + result = plan_with(source_repo, out, + *(value.format(A=first, B=second) for value in values), + extra=("--pin-source", str(path)), env=gh_env) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr == ( + f"REFUSED adopt-pin-collides: {said.format(A=first, B=second)}\n" + f"{shape_materialize.PIN_COLLISION_REMEDIATION}\n") + assert not out.exists() + assert gh_calls(gh_env) == "" + + +def test_a_leg_mounted_over_a_pins_file_is_refused_by_plan(source_repo, + tmp_path, glass): + """`execute` mounts the legs and then writes each pin, so a leg at the + pin's file would take it. The control is the same leg path with no pin, + where that path is no file the shape writes.""" + path, commit, _ = glass + legs = ("--spec-path", PIN_FILE, "--pin-source", str(path)) + refused = plan_with(source_repo, tmp_path / "refused.yaml", + f"openGlass@{commit}", extra=legs) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refused.stderr.startswith( + f"REFUSED adopt-leg-path-collides: --spec-path '{PIN_FILE}' is " + f"`{PIN_FILE}`."), refused.stderr + assert f"`{PIN_FILE}`, `contracts/shape-pin.yaml`) or `shape`" \ + in refused.stderr + assert not (tmp_path / "refused.yaml").exists() + control = plan_with(source_repo, tmp_path / "control.yaml", extra=legs) + assert control.returncode == 0, control.stdout + control.stderr + + +@WINDOWS_SKIP # the stub `gh` is a `#!` script; Windows executes none +def test_with_no_pin_source_plan_reads_the_digest_from_gh(source_repo, + tmp_path, glass, + gh_env): + """The scaffold's other way to the digest: `gh api` over the forge's + tree listing, under `opensoft` for an unqualified name. The stub answers + with the stand-in's own tree, so the recorded digest is the one a clone + gives; and when the forge does not answer, the refusal is the + scaffold's, and no plan is written.""" + path, commit, _ = glass + listing = git("ls-tree", "-r", commit, cwd=path).stdout.splitlines() + rows = [] + for line in listing: + head, name = line.split("\t", 1) + mode, kind, sha = head.split() + rows.append({"mode": mode, "type": kind, "sha": sha, "path": name}) + tree = tmp_path / "tree.json" + tree.write_text(json.dumps({"tree": rows, "truncated": False}), + encoding=UTF8) + out = tmp_path / "plan.yaml" + result = plan_with(source_repo, out, f"openGlass@{commit}", + env={**gh_env, "GH_STUB_TREE": str(tree)}) + assert result.returncode == 0, result.stdout + result.stderr + assert gh_calls(gh_env).splitlines() == [ + f"api repos/opensoft/openGlass/git/trees/{commit}?recursive=1"] + pin = load_yaml(out)["pins"][0] + assert pin["tree_sha256"] == tree_digest(path, commit) + assert pin["digest_source"] == "gh-api-tree-recursive" + + unanswered = plan_with(source_repo, tmp_path / "unanswered.yaml", + f"openGlass@{commit}", env=gh_env) + assert unanswered.returncode == 2 + assert unanswered.stderr.startswith( + f"REFUSED pin-unreadable: `gh api` could not read opensoft/openGlass " + f"@ {commit}: HTTP 404: Not Found"), unanswered.stderr + assert not (tmp_path / "unanswered.yaml").exists() + + +def test_pin_owner_resolves_an_unqualified_name(source_repo, tmp_path, + glass, gh_env): + """`--pin-owner` means what it means for the scaffold: the owner an + unqualified name resolves under, never `--org`, and a value checked like + any other that reaches a command line. An owner named in `--pin` itself + wins, as the `another-owner` adoption above shows.""" + path, commit, _ = glass + out = tmp_path / "plan.yaml" + result = plan_with(source_repo, out, f"openGlass@{commit}", + extra=("--pin-owner", "MedxCorp", + "--pin-source", str(path)), env=gh_env) + assert result.returncode == 0, result.stdout + result.stderr + assert load_yaml(out)["pins"][0]["repository"] == "MedxCorp/openGlass" + + unsafe = plan_with(source_repo, tmp_path / "unsafe.yaml", + f"openGlass@{commit}", + extra=("--pin-owner=--upload-pack=touch pwned", + "--pin-source", str(path)), env=gh_env) + assert unsafe.returncode == 2, unsafe.stdout + unsafe.stderr + assert unsafe.stderr.startswith("REFUSED unsafe-value: --pin-owner"), \ + unsafe.stderr + assert not (tmp_path / "unsafe.yaml").exists() + assert gh_calls(gh_env) == "" + + +def test_a_pin_the_source_does_not_have_is_refused_by_plan(source_repo, + tmp_path, glass): + path, _, _ = glass + out = tmp_path / "plan.yaml" + result = plan_with(source_repo, out, "openGlass@" + "0" * 40, + extra=("--pin-source", str(path))) + assert result.returncode == 2 + assert "REFUSED pin-source-unreadable" in result.stderr + assert not out.exists() + + +# --- what `check` finds, and `execute` refuses before any leg exists --------- +# +# The plan is a file a person or an AI edits between `plan` and `execute`, and +# an old plan carries `pins: [openGlass]`. Each case below is written into a +# plan `plan` wrote with no pin: `check` names it, and `execute` refuses it +# with `check`'s own findings before the leg remotes' directory is made +# (`conftest.refused_before_any_leg`). + +def unrecorded(glass) -> dict: + """Each pin a plan holds with no commit or no digest: a name alone, as + an old plan holds it, or a mapping missing one of the two.""" + path, commit, _ = glass + digest = tree_digest(path, commit) + named = ("is a name with no commit and no digest. `execute` writes each " + f"declared pin's file, `{PIN_FILE}` for this one") + return { + "old-plan-name": ("pins: [openGlass]\n", + f"`pins:` entry 1, 'openGlass', {named}"), + "old-plan-owner": ("pins:\n - opensoft/openGlass\n", + f"`pins:` entry 1, 'opensoft/openGlass', {named}"), + "no-commit": ("pins:\n" + pin_entry(digest=digest, omit=("commit",)), + "`pins:` entry 1 ('openGlass') records no `commit:`."), + "no-digest": ("pins:\n" + pin_entry(commit=commit, + omit=("tree_sha256",)), + "`pins:` entry 1 ('openGlass') records no " + "`tree_sha256:`."), + "both-blank": ("pins:\n" + pin_entry(commit="", digest=""), + "`pins:` entry 1 ('openGlass') records no `commit:` " + "and no `tree_sha256:`."), + } + + +def malformed(glass) -> dict: + """Each `pins:` value `plan` never writes: not a list, not a pin, or a + pin whose values are not of the form `plan` records them in.""" + path, commit, _ = glass + digest = tree_digest(path, commit) + entry = "`pins:` entry 1 ('openGlass') records" + return { + "not-a-list": ("pins: openGlass\n", + "`pins:` is 'openGlass', not a list of pins"), + "a-number": ("pins:\n - 7\n", "`pins:` entry 1 is 7, not a pin"), + "short-commit": ("pins:\n" + pin_entry(commit=commit[:12], + digest=digest), + f"{entry} `commit: '{commit[:12]}'`, which is not " + "40 hex"), + "short-digest": ("pins:\n" + pin_entry(commit=commit, + digest=digest[:12]), + f"{entry} `tree_sha256: '{digest[:12]}'`, which is " + "not 64 hex"), + "another-product": ("pins:\n" + pin_entry( + repository="opensoft/openPane", + commit=commit, digest=digest), + f"{entry} `product: 'openGlass'` and " + "`repository: 'opensoft/openPane'`"), + "no-owner": ("pins:\n" + pin_entry(repository="openGlass", + commit=commit, digest=digest), + f"{entry} `product: 'openGlass'` and `repository: " + "'openGlass'`"), + "no-source": ("pins:\n" + pin_entry(commit=commit, digest=digest, + omit=("digest_source",)), + f"{entry} no `digest_source:`."), + } + + +def colliding(glass) -> dict: + """Two pins at one file, and one at a file the shape writes: what + `plan` refuses as `adopt-pin-collides`, written into a plan by hand.""" + path, commit, _ = glass + digest = tree_digest(path, commit) + return { + "case-pair": ("pins:\n" + pin_entry(commit=commit, digest=digest) + + pin_entry(product="OpenGlass", + repository="opensoft/OpenGlass", + commit=commit, digest=digest), + "`pins:` entry 1 ('openGlass') and `pins:` entry 2 " + "('OpenGlass') write the pins of the products " + "`openGlass` and `OpenGlass` at one file, " + f"`{PIN_FILE}`, {ONE_FILE}"), + "shape": ("pins:\n" + pin_entry(product="shape", + repository="opensoft/shape", + commit=commit, digest=digest), + "`pins:` entry 1 ('shape') writes the pin of the product " + "`shape` at `contracts/shape-pin.yaml`, " + SHAPE_WRITES), + } + + +@pytest.fixture(scope="module") +def cases(glass): + """Every hand-edited `pins:` block below and the code `check` names it + with, built once from the stand-in's commit and digest.""" + return { + "plan-pin-unrecorded": unrecorded(glass), + "plan-pin-malformed": malformed(glass), + "plan-pin-collides": colliding(glass), + } + + +CASE_IDS = [("plan-pin-unrecorded", name) for name in ( + "old-plan-name", "old-plan-owner", "no-commit", "no-digest", "both-blank")] +CASE_IDS += [("plan-pin-malformed", name) for name in ( + "not-a-list", "a-number", "short-commit", "short-digest", + "another-product", "no-owner", "no-source")] +CASE_IDS += [("plan-pin-collides", name) for name in ("case-pair", "shape")] + + +def test_every_case_is_asked(cases): + """The table below names every case the builders above make.""" + assert sorted(CASE_IDS) == sorted((code, name) for code, built + in cases.items() for name in built) + + +@pytest.mark.parametrize("code, name", CASE_IDS, + ids=[f"{code}-{name}" for code, name in CASE_IDS]) +def test_a_plan_whose_pin_execute_cannot_write_is_found_and_refused( + adopter, monkeypatch, source_repo, tmp_path, glass, gh_env, cases, + code, name): + """`check` finds the pin, and `execute` refuses it with `check`'s own + findings before any leg remote exists; the finding names the `plan` that + records the pin whole, and neither command runs `gh`.""" + block, said = cases[code][name] + plan = planned(source_repo, tmp_path, PLANNED) + with_pins(plan, block) + checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + assert checked.returncode == 1, checked.stdout + checked.stderr + lines = [line for line in checked.stderr.splitlines() + if line.startswith(f"FINDING {code}: ")] + assert len(lines) == 1, checked.stderr + assert lines[0].startswith(f"FINDING {code}: {said}"), lines[0] + assert ("re-run `plan`" in lines[0]), lines[0] + refusal = refused_before_any_leg(adopter, monkeypatch, plan, tmp_path) + assert refusal.code == code + assert gh_calls(gh_env) == "" + + +@pytest.mark.parametrize("kept, aside", [ + pytest.param(PIN_FILE, "", id="same-case"), + pytest.param("contracts/OpenGlass-pin.yaml", + shape_materialize.CASE_ASIDE, id="folded"), +]) +def test_a_kept_path_where_the_pin_is_written_is_found_and_refused( + adopter, monkeypatch, tmp_path, glass, kept, aside): + """The materializer writes a shape file the root holds beside it, under + `shape/`; for a pin that is the wrong file, because the root's own + `validate-pins.py` reads `contracts/-pin.yaml`, the source's. + The source's `contracts/` answered `root` keeps such a file there.""" + source = make_source_repo(tmp_path / "Thing", tree={ + **SYNTHETIC_TREE, kept: "schema_version: 1\n"}) + plan = planned(source, tmp_path, {**PLANNED, "contracts/": "root"}) + with_pins(plan, "pins:\n" + a_good_pin(glass)) + found = findings(plan) + assert [code for code, _ in found] == ["plan-root-holds-pin"], found + assert found[0][1] == ( + f"the plan keeps the file `{kept}` in the assembly root under the " + f"entry `contracts/`, at `{PIN_FILE}`{aside}, where `execute` writes " + "the pin the plan declares on `openGlass`. The shape's pin would be " + "written beside it, under `shape/`, and the root's own " + "`validate-pins.py` would read the source's file as that pin: send " + "`contracts/` to a leg, `drop` it, or rename it in the source and " + "re-run `plan`.") + refused_before_any_leg(adopter, monkeypatch, plan, tmp_path) + + +def test_a_leg_moved_over_a_pins_file_in_the_plan_is_found( + adopter, monkeypatch, source_repo, tmp_path, glass): + """The plan's mount path is edited after `plan` asked it: `check` asks + again, with each pin's file among the shape's paths.""" + plan = planned(source_repo, tmp_path, PLANNED) + with_pins(plan, "pins:\n" + a_good_pin(glass)) + text = plan.read_bytes().decode(UTF8) + plan.write_bytes(text.replace(" spec_path: spec\n", + f" spec_path: {PIN_FILE}\n", 1) + .encode(UTF8)) + found = findings(plan) + assert [code for code, _ in found] == ["plan-leg-path-collides"], found + assert found[0][1].startswith( + f"legs.spec_path '{PIN_FILE}' is `{PIN_FILE}`."), found + refused_before_any_leg(adopter, monkeypatch, plan, tmp_path) + + +def test_a_plan_with_a_whole_pin_checks_ok(source_repo, tmp_path, glass, + gh_env): + """The control for every case above: the same plan with the pin as + `plan` writes it checks ok, and says the file it will write.""" + plan = planned(source_repo, tmp_path, PLANNED) + with_pins(plan, "pins:\n" + a_good_pin(glass)) + checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + assert checked.returncode == 0, checked.stdout + checked.stderr + assert f" write {PIN_FILE}: opensoft/openGlass @ {glass[1][:12]}" \ + in checked.stdout + assert gh_calls(gh_env) == "" + + +@needs_filter_repo +def test_execute_refuses_an_old_plans_name_before_any_remote(source_repo, + tmp_path, + gh_env): + """The issue's own path, as a person runs it: an old plan's name-only + pin is refused by `execute` itself, and nothing is created.""" + plan = planned(source_repo, tmp_path, PLANNED) + with_pins(plan, "pins: [openGlass]\n") + remotes = tmp_path / "remotes" + result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(remotes), + "--work-dir", str(tmp_path / "work"), env=gh_env) + assert result.returncode == 2, result.stdout + result.stderr + assert "REFUSED plan-pin-unrecorded" in result.stderr + assert "no leg repository was created" in result.stderr + assert not remotes.exists() + assert gh_calls(gh_env) == "" + + +# --- one implementation ------------------------------------------------------ + +def test_both_tools_read_a_pin_with_one_implementation(adopter): + """The scaffold's pin functions moved beside the materializer and both + tools call them there (#273): no second copy to drift.""" + module_spec = importlib.util.spec_from_file_location("scaffold_pins", + SCAFFOLD) + scaffold = importlib.util.module_from_spec(module_spec) + module_spec.loader.exec_module(scaffold) + for name in ("declared_pin_values", "declared_pin_products"): + assert getattr(scaffold, name) is getattr(shape_materialize, name) + assert getattr(adopter, name) is getattr(shape_materialize, name) + assert scaffold._refuse_colliding_pins \ + is shape_materialize.refuse_colliding_pins + assert adopter.refuse_colliding_pins \ + is shape_materialize.refuse_colliding_pins + assert adopter.pin_collisions is shape_materialize.pin_collisions + assert not hasattr(adopter, "_checked_pin_name") + diff --git a/tests/test_adopt_plan.py b/tests/test_adopt_plan.py index e840bb90..15402876 100644 --- a/tests/test_adopt_plan.py +++ b/tests/test_adopt_plan.py @@ -339,36 +339,46 @@ def test_a_project_name_that_is_not_a_lowercase_id_is_refused(source_repo, # --- --pin, and --visibility internal --------------------------------------- -def test_a_bare_pin_name_is_declared(source_repo, tmp_path): - plan_path = tmp_path / "plan.yaml" - result = write_plan(source_repo, plan_path, extra=("--pin", "openGlass")) - assert result.returncode == 0, result.stderr - plan = load_yaml(plan_path) - assert plan["pins"] == ["openGlass"] +@pytest.mark.parametrize("value", ["openGlass", "opensoft/openGlass"]) +def test_a_pin_name_with_no_commit_is_refused(source_repo, tmp_path, value): + """Since #273 `plan --pin` takes the scaffold's `[owner/]product@<40 hex>` + and `execute` writes the pin file: a NAME alone, which this tool took + until then and wrote into `neutral_product_pins:` with no file beside + it, is refused in the scaffold's words, and no plan is written. + `tests/test_adopt_pins.py` carries the rest of the contract.""" + result = write_plan(source_repo, tmp_path / "plan.yaml", + extra=("--pin", value)) + assert result.returncode == 2 + assert f"REFUSED pin-malformed: --pin {value!r}" in result.stderr + assert "--pin openGlass@" in result.stderr + assert "opensoft" in result.stderr + assert not (tmp_path / "plan.yaml").exists() -def test_an_owner_qualified_pin_name_is_declared(source_repo, tmp_path): +def test_a_pin_carrying_a_commit_is_recorded(source_repo, tmp_path): + """The scaffold's syntax, which this tool refused until #273: the commit + and the digest of the product's tree there, read from `--pin-source`, + are recorded in the plan for `execute` to write the pin file from.""" + product = tmp_path / "openGlass" + product.mkdir() + subprocess.run(["git", "init", "-q", "-b", "main"], cwd=str(product), + check=True) + (product / "README.md").write_bytes(b"# openGlass\n") + subprocess.run(["git", "add", "-A"], cwd=str(product), check=True) + subprocess.run(["git", "-c", "user.name=t", "-c", "user.email=t@t", + "commit", "-qm", "seed"], cwd=str(product), check=True) + commit = subprocess.run(["git", "rev-parse", "HEAD"], cwd=str(product), + capture_output=True, text=True, + check=True).stdout.strip() plan_path = tmp_path / "plan.yaml" result = write_plan(source_repo, plan_path, - extra=("--pin", "opensoft/openGlass")) + extra=("--pin", f"openGlass@{commit}", + "--pin-source", str(product))) assert result.returncode == 0, result.stderr - plan = load_yaml(plan_path) - assert plan["pins"] == ["opensoft/openGlass"] - - -def test_a_pin_carrying_a_commit_is_refused(source_repo, tmp_path): - """`scaffold-project.py --pin openProduct@` syntax pasted here by - habit: adopting a project declares no commit for a neutral-product pin at - plan time, so this is refused rather than silently written into - `neutral_product_pins:` as a name nothing will ever match.""" - commit = "0" * 40 - result = write_plan(source_repo, tmp_path / "plan.yaml", - extra=("--pin", f"openGlass@{commit}")) - assert result.returncode == 2 - assert "adopt-pin-malformed" in result.stderr - assert "openGlass" in result.stderr - assert "opensoft" in result.stderr - assert not (tmp_path / "plan.yaml").exists() + pins = load_yaml(plan_path)["pins"] + assert [(pin["product"], pin["repository"], pin["commit"]) + for pin in pins] == [("openGlass", "opensoft/openGlass", commit)] + assert len(pins[0]["tree_sha256"]) == 64 def test_visibility_internal_is_accepted(source_repo, tmp_path): diff --git a/tests/test_git_output_decoding.py b/tests/test_git_output_decoding.py index 06bbe2ae..955612a8 100644 --- a/tests/test_git_output_decoding.py +++ b/tests/test_git_output_decoding.py @@ -116,8 +116,11 @@ def test_run_names_the_path_git_complained_of_under_a_c_locale( # two-pull-requests reason as above. ("adopt-project.py", "cross_leg_references"): frozenset(), # The `gh api` tree listing becomes a digest: UTF-8 yes, `replace` no, for - # a replaced byte would be a wrong digest with a confident tone. - ("scaffold-project.py", "pin_digest_from_gh"): frozenset({"encoding"}), + # a replaced byte would be a wrong digest with a confident tone. The + # first is `scaffold-project.py --pin`'s and `adopt-project.py plan + # --pin`'s, moved out of the scaffold for #273. + ("scripts/shape_materialize.py", "pin_digest_from_gh"): + frozenset({"encoding"}), ("scripts/repo_shape.py", "tree_digest_from_gh"): frozenset({"encoding"}), } From 2dc1f57c2b3c64053087fba0f46c57de51d9a461 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:35:03 +0000 Subject: [PATCH 3/9] adopt-project.py: say where the pin findings come from at their call site in `_leg_findings` only, not again in `_entry_findings`'s docstring (#273) The paragraph repeated the comment beside `findings += _pin_findings(plan, tree)`. Where it sat, it was the one line in the branch that conflicted with the open #303/#305 branch, which adds its own paragraph to that docstring at the same place. `_entry_findings` is now as it is on main. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index eed70657..9d2100af 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -3066,10 +3066,6 @@ def _entry_findings(plan: Plan, source: Source THE PATHS COME FIRST (#196), so that `_refuse_what_check_finds` wears `plan-unsafe-path` for a plan that has one: a path that is a `git` option is refused as that, and never as the uncovered file it also leaves. - - THE PINS COME AFTER THE LEGS (#273), from `_leg_findings`: each `pins:` - entry `execute` cannot write a pin file from, two pins at one file, and - a path the root keeps where a pin is written (`_pin_findings`). """ tree = source.tree() unsafe, unsafe_remediation = _unsafe_path_findings(plan) From d806e8fefc9d875d8e598526652c3abb230e8539 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 12 Oct 2026 14:46:07 +0000 Subject: [PATCH 4/9] tests/test_adopt_template_values.py: #270's `pins-surrogate` case expects `execute` to refuse a plan whose `pins:` holds the name `openX\ud800` under `plan-pin-unrecorded` first, where #273 reads a name-only pin as one nobody recorded, so the lone surrogate is still found as `plan-unwritable-value`, listed beneath it, and refused before any leg exists (#273, #255) #270 (f795cf7) wrote the case against a `pins:` list of names, the form every plan held before #273. On the merged tree `check` lists two findings for it: `plan-pin-unrecorded` (the entry is a name with no commit and no digest, Brett's ruling on #273) and `plan-unwritable-value` (the name holds a lone surrogate). The refusal's code is the first finding's, so only that one assertion changed. The finding assertions, `check` exit 1, `execute` exit 2, the finding listed under the refusal and "no leg repository was created" all still hold. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_adopt_template_values.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_adopt_template_values.py b/tests/test_adopt_template_values.py index 251bfff3..38dc1c60 100644 --- a/tests/test_adopt_template_values.py +++ b/tests/test_adopt_template_values.py @@ -422,7 +422,10 @@ def test_check_finds_and_execute_refuses_a_hand_edited_value( assert found.returncode == 1, said(found) assert finding in found.stderr, said(found) assert refused.returncode == 2, said(refused) - assert PLAN_REFUSED in refused.stderr, said(refused) + # A name-only `pins:` item is `plan-pin-unrecorded` first (#273). + headline = (b"REFUSED plan-pin-unrecorded: " if key == "pins" + else PLAN_REFUSED) + assert headline in refused.stderr, said(refused) assert b"\n " + finding in refused.stderr, said(refused) assert b"no leg repository was created" in refused.stderr From 7a94b6b4f6d579052f413019470d6ddebd38ee0b Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:07:13 +0000 Subject: [PATCH 5/9] shape_materialize.py: `pin_digest_from_source` turns any `OSError` out of `stat()` or git's `cwd=` into `pin-source-unreadable`, saying "no such directory" and "it is a file, not a directory" as `validate-pins.py` does, and its `Path.resolve()` no longer raises on a symlink loop, so a `--pin-source` that is missing, a file, under a parent this user cannot enter or a loop is exit 2 with its remediation from `scaffold-project.py` `--dry-run`, its real run and `adopt-project.py plan`, and no longer a traceback, exit 1 (#339) `pin_digest_from_source` caught only `Refusal`. A directory that is no clone is git's own failure, so it was always `pin-source-unreadable`, exit 2. A path `subprocess.run` cannot enter never reaches git: it is a `FileNotFoundError`, `NotADirectoryError` or `PermissionError` out of `cwd=`, and nothing caught it, so `--pin-source ` ended `scaffold-project.py`'s `--dry-run` and its real run in a traceback, exit 1, and since #273 `adopt-project.py plan --pin` ended the same way, both calling the one function through `declared_pin_values`. What it does now: - `stat()` judges the source inside the guard, as #312 does in `validate-pins.py`: a missing path or one beneath a file is "no such directory", a file is "it is a file, not a directory", and any other `OSError`, from `stat()` or from git's `cwd=`, is the system's own reason (`strerror`: "Permission denied", "File name too long"). - Each is the existing `pin-source-unreadable` refusal, naming the path and the reason, with the remediation it always had, exit 2, before anything exists and identically under `--dry-run`. The not-a-clone refusal is unchanged. - `Path.resolve()` is guarded too: a symlink loop is a `RuntimeError` out of it before 3.13 and an `OSError` since, raised before the digest was asked for. As typed, `stat()` meets the loop and says so. - A source that answers is read exactly as before: a relative path resolves against the working directory, a symlink to a clone is followed, and the digest is `tree_digest`'s. `tests/test_pin_source_unreadable.py` runs each case through `scaffold-project.py --dry-run`, its real run and `adopt-project.py plan`: missing, a file, beneath a file, relative and missing, a parent and a source with no search bit (skipped on Windows and as root), a 300-character name, a symlink loop (skipped on Windows) and a directory that is no clone. Each asserts exit 2, the refusal naming the path and the reason, its remediation, no traceback and nothing created. It also holds `--dry-run` and the real run to the same stdout and stderr, the path printed escaped under an ASCII stderr and as a surrogate when it is not UTF-8, and a relative path and a symlink to a clone read as before. 38 of its 46 rows fail on d806e8f; the other 8 are the unchanged behaviour. Closes #339 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- scripts/shape_materialize.py | 69 +++++- tests/test_pin_source_unreadable.py | 338 ++++++++++++++++++++++++++++ 2 files changed, 400 insertions(+), 7 deletions(-) create mode 100644 tests/test_pin_source_unreadable.py diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index e17d9637..1edf10a7 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -36,6 +36,7 @@ import os import re import shutil +import stat import subprocess import sys import unicodedata @@ -1702,20 +1703,74 @@ def parse_pin(raw: str, owner: str = NEUTRAL_PRODUCT_OWNER) -> tuple[str, str, s def pin_digest_from_source(source: Path, commit: str, repository: str) -> str: - """The `sorted-ls-tree-r-v1` digest, read from a clone on this machine.""" + """The `sorted-ls-tree-r-v1` digest, read from a clone on this machine. + + A SOURCE THAT CANNOT ANSWER IS `pin-source-unreadable`, HOWEVER IT FAILS + (#339): exit 2, with the remediation below, before anything is created, + `--dry-run` or not. `tree_digest` runs `git` IN the source, and a + directory that is not a clone is git's own failure, a `Refusal`. A path + `subprocess.run` cannot enter at all never reaches git: it is an + `OSError` raised out of `cwd=`, `FileNotFoundError` for a source that is + not there, `NotADirectoryError` for a file, `PermissionError` for one + under a parent this user cannot enter, and nothing caught it, so + `--pin-source ` ended both tools in a traceback, exit 1, where + a directory that is not a clone was this refusal. + + THE REASON IS `validate-pins.py`'S (#295, #312), and the sentence is the + same one that file prints for the same source: "no such directory", "it + is a file, not a directory", and for any other `OSError` the system's own + reason (`strerror`: "Permission denied", "File name too long"). `stat()` + INSIDE THE GUARD judges the path, and not `exists()`/`is_dir()`: before + 3.14 those swallow only a missing path and raise every other `OSError`, + so a source under a parent this user cannot enter was still a traceback, + and on 3.14 "no such directory" for a directory that is there. A source + `stat()` can read but `cwd=` cannot enter (a directory with no search + bit) fails in `tree_digest`, which the same guard answers. + """ try: - return tree_digest(source, commit) - except Refusal as exc: + return _digest_from_pin_source(source, commit) + except (Refusal, OSError) as exc: + why = (exc.detail if isinstance(exc, Refusal) + else exc.strerror or str(exc)) raise Refusal( "pin-source-unreadable", - f"{source} could not answer for {repository} @ {commit}: " - f"{exc.detail}", + f"{source} could not answer for {repository} @ {commit}: {why}", "Remediation: point --pin-source at a clone of that repository " "that HAS the pinned commit (`git fetch --all` first), or drop " "--pin-source and let `gh api` read the forge.", ) from exc +def _digest_from_pin_source(source: Path, commit: str) -> str: + """`tree_digest` over `source`, with a path that is no directory said + in `validate-pins.py`'s words (#339). Any other `OSError`, from `stat()` + or from git's `cwd=`, is left for `pin_digest_from_source` to name.""" + try: + mode = source.stat().st_mode + except (FileNotFoundError, NotADirectoryError): + raise Refusal("pin-source-missing", "no such directory") from None + if not stat.S_ISDIR(mode): + raise Refusal("pin-source-not-a-directory", + "it is a file, not a directory") + return tree_digest(source, commit) + + +def _resolved_pin_source(source: Path) -> Path: + """`source` made absolute, or AS TYPED when it cannot be (#339). + + A symlink loop is a `RuntimeError` out of `Path.resolve()` before 3.13 + and an `OSError` since, and either one ended the run in a traceback + before `pin_digest_from_source` could ask. As typed, `stat()` there meets + the loop and says "Too many levels of symbolic links". A path that is + merely absent or unreadable resolves, so a valid source is read exactly + as before. + """ + try: + return source.resolve() + except (OSError, RuntimeError): + return source + + def pin_digest_from_gh(repository: str, commit: str) -> str: """The same digest, computed from the forge's own recursive tree listing. @@ -1786,8 +1841,8 @@ def declared_pin_values(args) -> dict[str, dict[str, str]]: for raw in args.pin: product, repository, commit = parse_pin(raw, args.pin_owner) if args.pin_source is not None: - digest = pin_digest_from_source(args.pin_source.resolve(), commit, - repository) + digest = pin_digest_from_source( + _resolved_pin_source(args.pin_source), commit, repository) digest_source = f"local-clone ({args.pin_source})" else: digest = pin_digest_from_gh(repository, commit) diff --git a/tests/test_pin_source_unreadable.py b/tests/test_pin_source_unreadable.py new file mode 100644 index 00000000..b568ac72 --- /dev/null +++ b/tests/test_pin_source_unreadable.py @@ -0,0 +1,338 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A `--pin-source` that cannot answer is `pin-source-unreadable` (#339). + +`pin_digest_from_source` caught only `Refusal`. A source that is a directory +but no clone is git's own failure, a `Refusal`, and was always the refusal +`pin-source-unreadable` with its remediation, exit 2. A source +`subprocess.run` cannot enter at all never reaches git: `FileNotFoundError` +for one that is not there, `NotADirectoryError` for a file, +`PermissionError` for one under a parent this user cannot search, all raised +out of `cwd=`, and nothing caught them. `scaffold-project.py --pin ... +--pin-source ` ended `--dry-run` and the real run alike in a +traceback, exit 1, and since #273 `adopt-project.py plan --pin` reads its +digest through the same function and ended the same way. The symlink loop was +a fourth way in: `Path.resolve()` raised a `RuntimeError` before 3.13 (an +`OSError` since), before the digest was asked for at all. + +Both tools reach the one function through `declared_pin_values`, so every case +runs three ways: `scaffold-project.py --dry-run`, the same without it, and +`adopt-project.py plan`. Each must give exit 2 and the refusal NAMING the +source and the reason, with the remediation the not-a-clone refusal already +had, no traceback, and nothing created: no remote directory and no work +directory for the scaffold, no plan file for adopt. The reasons are the +sentences `validate-pins.py` gives the same sources (#295, #312): "no such +directory", "it is a file, not a directory", and the system's own for any +other `OSError`. + +NO NETWORK AND NO GITHUB. Every run names `--pin-source`, so `gh` is never +reached; the session's `gh` guard (`conftest.gh_guard`, #296) fails any test +whose run called it, which is the `gh` that fails if called. The sources are +never read except where a case says so: the commit asked for is any 40 hex. +""" + +from __future__ import annotations + +import errno +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import ADOPT, ORG, REPO, SCAFFOLD, git, make_source_repo, run_script + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import load_yaml, tree_digest # noqa: E402 + +PROJECT = "MedxGlass" +#: Any 40 hex: no source below is asked for it, but the clone that is not one. +COMMIT = "0123456789abcdef0123456789abcdef01234567" + +#: `validate-pins.py`'s two sentences for a source that is no directory. +NO_SUCH_DIRECTORY = "no such directory" +A_FILE = "it is a file, not a directory" + +#: What every refusal is told to do, as the not-a-clone refusal always said it. +REMEDIATION = ("Remediation: point --pin-source at a clone of that repository " + "that HAS the pinned commit (`git fetch --all` first), or drop " + "--pin-source and let `gh api` read the forge.") + +#: A directory mode denies nothing on Windows, where `chmod` has no search +#: bit to clear, and nothing to root. The skip is a mark, so it comes before +#: any fixture builds a directory or any tool runs. +SEARCH_DENIED = pytest.mark.skipif( + os.name == "nt" or getattr(os, "geteuid", lambda: 1)() == 0, + reason="needs a POSIX directory mode that denies search: Windows has no " + "search bit for chmod to clear, and root is not stopped by one") + +#: Windows grants a symlink only to an elevated user or a developer-mode one. +SYMLINKS = pytest.mark.skipif( + os.name == "nt", + reason="needs a symlink, which Windows grants only to an elevated user") + +#: A Windows command line is text, so it holds no byte that is not UTF-8. +POSIX_ARGV = pytest.mark.skipif( + os.name == "nt", + reason="needs a command-line argument that is not UTF-8, which a Windows " + "command line cannot hold") + +#: How a tool is asked, and what it must not have made when it refuses. +RUNS = ["scaffold-dry-run", "scaffold-run", "adopt-plan"] + + +def run(kind: str, base: Path, adoptable: Path, typed: str, + commit: str = COMMIT, cwd: Path | None = None, + env: dict | None = None) -> subprocess.CompletedProcess: + """`--pin openGlass@ --pin-source ` through `kind`, into + `base`.""" + if kind == "adopt-plan": + return run_script(ADOPT, "plan", "--source", str(adoptable), + "--project", PROJECT, "--org", ORG, + "--elected-by", "Test Human", + "--elected-on", "2026-09-02", + "--out", str(base / "plan.yaml"), + "--pin", f"openGlass@{commit}", + "--pin-source", typed, cwd=cwd, env=env) + return run_script(SCAFFOLD, "--org", ORG, "--project", PROJECT, + "--elected-by", "Test Human", + "--elected-on", "2026-09-02", + "--pin", f"openGlass@{commit}", "--pin-source", typed, + "--local-remote-dir", str(base / "remotes"), + "--work-dir", str(base / "work"), + *(("--dry-run",) if kind == "scaffold-dry-run" else ()), + cwd=cwd, env=env) + + +def made(kind: str, base: Path) -> list[str]: + """What the run created under `base`: nothing, when it refused.""" + names = ("plan.yaml",) if kind == "adopt-plan" else ("remotes", "work") + return [name for name in names if (base / name).exists()] + + +@pytest.fixture(scope="module") +def adoptable(tmp_path_factory) -> Path: + """The repository `adopt-project.py plan` is asked to plan.""" + return make_source_repo(tmp_path_factory.mktemp("adoptable") / "Thing") + + +@pytest.fixture(scope="module") +def glass(tmp_path_factory) -> tuple[Path, str]: + """A local stand-in for `openGlass`, which does answer: `(path, commit)`.""" + path = tmp_path_factory.mktemp("glass") / "openGlass" + path.mkdir() + git("init", "-q", "-b", "main", cwd=path) + (path / "README.md").write_bytes(b"# openGlass\n") + git("add", "-A", cwd=path) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", "seed", + cwd=path) + return path, git("rev-parse", "HEAD", cwd=path).stdout.strip() + + +class Unreadable: + """A `--pin-source` that cannot answer: what is typed, the path the + refusal names, the reason it gives (`None` where the system's own differs + by platform), and the directory a test unlocks again.""" + + def __init__(self, typed: str, named: str | None = None, + said: str | None = None, cwd: Path | None = None, + locked: Path | None = None, git_says: bool = False, + names_it: bool = True) -> None: + self.typed, self.named = typed, named or typed + self.said, self.cwd, self.locked = said, cwd, locked + #: The reason is git's own, which names the commit and the path. + self.git_says = git_says + #: The refusal names the path as it was built here. False where a + #: platform answers for the path itself (below), so that only the + #: refusal and its remediation are asserted there. + self.names_it = names_it + + +@pytest.fixture(params=[ + "missing", + "a-file", + "beneath-a-file", + "relative-missing", + pytest.param("parent-not-searchable", marks=SEARCH_DENIED), + pytest.param("source-not-searchable", marks=SEARCH_DENIED), + "name-too-long", + pytest.param("symlink-loop", marks=SYMLINKS), + "not-a-clone", +]) +def unreadable(request, tmp_path): + """Each way a source fails to answer, built under `tmp_path`. + + `missing`, `a-file` and `parent-not-searchable` are the issue's table; + the rest are its neighbours, the other ways `stat()` or git's `cwd=` + can raise; `not-a-clone` is the refusal that always held, unchanged. + `relative-missing` is typed relative to the working directory the tool + runs in and is named absolute, as the not-a-clone refusal always named + its source. + """ + case = request.param + locked = None + if case == "missing": + source = Unreadable(str(tmp_path / "missing"), said=NO_SUCH_DIRECTORY) + elif case == "a-file": + (tmp_path / "afile").write_bytes(b"not a checkout\n") + source = Unreadable(str(tmp_path / "afile"), said=A_FILE) + elif case == "beneath-a-file": + (tmp_path / "afile").write_bytes(b"not a checkout\n") + # POSIX answers ENOTDIR for a path beneath a file. Windows answers + # its own error, so the sentence is asserted only where it is known. + source = Unreadable(str(tmp_path / "afile" / "x"), + said=NO_SUCH_DIRECTORY if os.name != "nt" else None) + elif case == "relative-missing": + source = Unreadable("gone", named=str(tmp_path / "gone"), + said=NO_SUCH_DIRECTORY, cwd=tmp_path) + elif case == "parent-not-searchable": + locked = tmp_path / "locked" + (locked / "x").mkdir(parents=True) + source = Unreadable(str(locked / "x"), said=os.strerror(errno.EACCES), + locked=locked) + elif case == "source-not-searchable": + locked = tmp_path / "locked" + locked.mkdir() + source = Unreadable(str(locked), said=os.strerror(errno.EACCES), + locked=locked) + elif case == "name-too-long": + # One path component the filesystem has to refuse by name. What + # Windows calls it is its own business: it is an `OSError` there too, + # so the refusal and the absent traceback are asserted everywhere and + # the reason only where POSIX says it. + source = Unreadable(str(tmp_path / ("n" * 300)), + said=None if os.name == "nt" + else os.strerror(errno.ENAMETOOLONG), + names_it=os.name != "nt") + elif case == "symlink-loop": + os.symlink("loop", tmp_path / "loop") + source = Unreadable(str(tmp_path / "loop"), + said=os.strerror(errno.ELOOP)) + else: + (tmp_path / "plain").mkdir() + source = Unreadable(str(tmp_path / "plain"), git_says=True) + if locked is not None: + locked.chmod(0) + try: + yield source + finally: + if locked is not None: + locked.chmod(0o700) + + +def assert_refused(result: subprocess.CompletedProcess, kind: str, base: Path, + source: Unreadable) -> None: + """The refusal `pin-source-unreadable`, exit 2: naming the source and the + reason, with its remediation, and with nothing created.""" + assert result.returncode == 2, result.stdout + result.stderr + assert "Traceback" not in result.stderr, result.stderr + head = ("REFUSED pin-source-unreadable: " + f"{source.named} could not answer for opensoft/openGlass " + f"@ {COMMIT}: ") + if not source.names_it: + head = "REFUSED pin-source-unreadable: " + assert head in result.stderr, result.stderr + if source.said is not None: + assert f"{head}{source.said}\n{REMEDIATION}\n" in result.stderr, \ + result.stderr + else: + assert REMEDIATION in result.stderr, result.stderr + assert made(kind, base) == [], ( + "a refusal comes before anything exists, `--dry-run` or not") + + +@pytest.mark.parametrize("kind", RUNS) +def test_a_pin_source_that_cannot_answer_is_pin_source_unreadable( + kind, unreadable, tmp_path, adoptable): + """The issue's table, and its neighbours, through every way in. The one + case that held before is `not-a-clone`, git's own failure, whose + sentence is git's and is asserted as the part of it that does not depend + on where the repository is kept.""" + result = run(kind, tmp_path, adoptable, unreadable.typed, + cwd=unreadable.cwd) + if unreadable.git_says: + assert result.returncode == 2, result.stdout + result.stderr + assert (f"REFUSED pin-source-unreadable: {unreadable.named} could " + f"not answer for opensoft/openGlass @ {COMMIT}: `git ls-tree " + f"-r -z {COMMIT}` in {unreadable.named} exited" + in result.stderr), result.stderr + assert REMEDIATION in result.stderr + assert made(kind, tmp_path) == [] + else: + assert_refused(result, kind, tmp_path, unreadable) + + +def test_the_dry_run_and_the_real_run_refuse_in_the_same_words( + unreadable, tmp_path, adoptable): + """`--dry-run` is the real run's plan, so a source that cannot answer is + refused identically by both: the same exit, the same stderr, the same + stdout, and neither makes anything. Through `setup.sh`, which hands the + flags to the scaffold's `--dry-run`, it is the refusal an operator reads + in the middle of the plan block.""" + dry = run("scaffold-dry-run", tmp_path, adoptable, unreadable.typed, + cwd=unreadable.cwd) + real = run("scaffold-run", tmp_path, adoptable, unreadable.typed, + cwd=unreadable.cwd) + assert dry.returncode == real.returncode == 2, ( + dry.stdout + dry.stderr + real.stdout + real.stderr) + assert (dry.stderr, dry.stdout) == (real.stderr, real.stdout) + assert "Traceback" not in dry.stderr + assert made("scaffold-run", tmp_path) == [] + + +@pytest.mark.parametrize("kind", RUNS) +@pytest.mark.parametrize("name, spelled, env", [ + pytest.param("日本-missing", "\\u65e5\\u672c-missing", + {"PYTHONIOENCODING": "ascii"}, id="not-ascii"), + pytest.param("miss\udcffing", "miss\\udcffing", {}, + marks=POSIX_ARGV, id="not-utf-8"), +]) +def test_the_source_is_printed_escaped_and_never_raised_on( + kind, name, spelled, env, tmp_path, adoptable): + """The refusal names a path a person typed, and a stderr that cannot + encode it prints it escaped, as it does for every other path the tools + name: the refusal is never a `UnicodeEncodeError` in its place. The + second row is a name that is not UTF-8 (POSIX only: a Windows command + line has no such byte), read with `surrogateescape`, so it prints as + the surrogate it was read as.""" + typed = str(tmp_path / name) + result = run(kind, tmp_path, adoptable, typed, env=env) + assert result.returncode == 2, result.stdout + result.stderr + assert "Traceback" not in result.stderr, result.stderr + assert spelled in result.stderr, result.stderr + assert "REFUSED pin-source-unreadable: " in result.stderr + assert made(kind, tmp_path) == [] + + +# --- a source that answers is read exactly as before ------------------------- +# +# Nothing about a source that DOES answer moved: a relative path is resolved +# against the working directory the tool runs in, a symlink to a clone is +# followed, and the digest is the one `repo_shape.tree_digest` gives. + +@pytest.mark.parametrize("kind", ["scaffold-dry-run", "adopt-plan"]) +@pytest.mark.parametrize("spelling", [ + "relative", + pytest.param("symlinked", marks=SYMLINKS), +]) +def test_a_source_that_answers_is_read_as_before(kind, spelling, tmp_path, + adoptable, glass): + product, commit = glass + digest = tree_digest(product, commit) + if spelling == "relative": + typed, cwd = "openGlass", product.parent + else: + typed, cwd = str(tmp_path / "link"), None + os.symlink(product, tmp_path / "link", target_is_directory=True) + result = run(kind, tmp_path, adoptable, typed, commit=commit, cwd=cwd) + assert result.returncode == 0, result.stdout + result.stderr + assert "Traceback" not in result.stderr + if kind == "adopt-plan": + pin = load_yaml(tmp_path / "plan.yaml")["pins"][0] + assert pin["commit"] == commit + assert pin["tree_sha256"] == digest + assert pin["digest_source"] == f"local-clone ({typed})" + else: + assert f"tree {digest[:12]}" in result.stdout, result.stdout + assert f"(local-clone ({typed}))" in result.stdout, result.stdout + assert "--dry-run: nothing was created." in result.stdout From 6494714f6d0f3142dbfaa0b9a8a337c152d22ea7 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:47:52 +0000 Subject: [PATCH 6/9] =?UTF-8?q?validate-pins.py,=20repo=5Fshape.py:=20`neu?= =?UTF-8?q?tral-pin-source-collides`=20is=20a=20note=20and=20no=20finding,?= =?UTF-8?q?=20and=20two=20declarations=20are=20one=20product=20when=20they?= =?UTF-8?q?=20share=20a=20pin=20file=20and=20not=20a=20casefold,=20so=20a?= =?UTF-8?q?=20project=20that=20already=20declares=20`open-glass`=20and=20`?= =?UTF-8?q?open=5Fglass`=20takes=20`update-shape.py=20apply`=20again,=20an?= =?UTF-8?q?d=20`open=C3=9F`=20beside=20`openss`=20is=20found=20(#275)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix round 1 on #275, from the review of 34dbd86. The first round made two declared pins that share one `SHAPE_PIN_SOURCE_` a FINDING. Main through ab623dd scaffolds such a project (`--pin open-glass@A --pin open_glass@B` exits 0), and `update-shape.py apply` runs the NEW `validate-pins.py` over the rewritten root, so the finding ended every apply as `REFUSED update-validators-red`, rolled back, for this update and every later one, until a person deleted a true declaration of descent, which AGENTS.md tells an agent never to do. It is a NOTE now, the lane's choice and one Brett can reverse: the guard stays, so the variable is read for neither pin and each pin verifies from its own source, and the note names both pins, the variable and the keyed `--pin-source =`, and says which declaration stays is the human's decision. `scaffold-project.py` still refuses a NEW pair, so the situation is not created anew. Two declarations are now one product when they are one pin file (`pin_product_key`, `lower()`, the fold `contracts/-pin.yaml` is named by) and not when `casefold()` says so. `openß` and `openss`, `openſ` and `opens`, `openfi` and `openfi`, `openς` and `openσ` share a variable and have two pin files, and were never found; a hand-written `project.yaml` is the only way to declare them. The skip hint for a product two pins share offers the keyed `--pin-source =`, since the bare form answers for every pin with no more specific source. The guard now has a test that needs no `gh` (a sibling checkout beside the project for each pin, the variable naming one of them), so it runs on Windows, and the hint and the non-ASCII rows are tested in-process. A project that declares a pair takes `apply`, and the doctor's `pins` row stays `ok` before and after (`test_a_project_declaring_a_colliding_pair_takes_a_shape_update`). Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/repo_shape.py | 61 ++- .../assembly-root/scripts/validate-pins.py | 82 ++-- tests/test_pin_source_env_collisions.py | 461 ++++++++++++++++-- 3 files changed, 514 insertions(+), 90 deletions(-) diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 439b6cce..98335801 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -1097,9 +1097,11 @@ def recorded_gitlink(repo: Path, path: str) -> str | None: #: product's name and all become `_`, so `open-glass`, `open_glass` and #: `open.glass` share `SHAPE_PIN_SOURCE_OPEN_GLASS` while naming three pin #: files. Every existing name is kept, because an operator may have set it and -#: the docs name it; what is refused instead is DECLARING two products that -#: share one (`pin_source_env_collisions`), and a variable two declared -#: products share answers for neither. +#: the docs name it. What is done instead (`pin_source_env_collisions`): a +#: variable two declared products share answers for NEITHER, wherever it is +#: read; `scaffold-project.py --pin` refuses to create such a pair; and +#: `validate-pins.py` NOTES a pair a project already declares, and never fails +#: it, because the project did nothing wrong that an update could undo. PIN_SOURCE_ENV_PREFIX = "SHAPE_PIN_SOURCE_" @@ -1107,6 +1109,23 @@ def pin_source_env_name(product: str) -> str: return PIN_SOURCE_ENV_PREFIX + re.sub(r"[^A-Z0-9_]", "_", product.upper()) +def pin_product_key(product: str) -> str: + """What makes two declarations ONE product: the name its pin file carries, + `contracts/-pin.yaml` (`neutral_pin_path` in + `shape_materialize.py` writes it; `validate-pins.py` reads it, and spells + it again because it ships into projects). + + `lower()`, and not `casefold()`. The two agree over ASCII and part + company outside it: `casefold()` folds `ß` to `ss`, `ſ` to `s`, `fi` to + `fi` and `ς` to `σ`, which `lower()` leaves alone, so `openß` and `openss` + are two pin files that a casefold took for one product (#275's re-review, + N2). Whether two names are the same pin is whether they are the same + file, so that is what is asked here, and + `tests/test_pin_source_env_collisions.py` holds it to `neutral_pin_path`. + """ + return product.lower() + + def pin_source_env_collisions(products) -> dict[str, list[str]]: """The variables two or more DISTINCT products share, each with them: `{variable: [product, ...]}` (#275). @@ -1118,14 +1137,15 @@ def pin_source_env_collisions(products) -> dict[str, list[str]]: not a name and is left out, as `validate-pins.py` reports it as malformed on its own. - DISTINCT MEANS DISTINCT AFTER A CASEFOLD, the fold every other lookup of a - product makes (`contracts/-pin.yaml`, the key of - `--pin-source` and `--link-source`). `openGlass` and `OpenGlass` are one - product and name one variable rightly: whether the same product is - declared twice is a different question, which `scaffold-project.py` - answers as `scaffold-pin-collides` (#241). A product listed twice is - therefore never a collision here, and two that differ in more than case - are, when `pin_source_env_name` maps them to one name. + DISTINCT MEANS DISTINCT PIN FILES (`pin_product_key`): `openGlass` and + `OpenGlass` are one product and name one variable rightly, and whether the + same product is declared twice is a different question, which + `scaffold-project.py` answers as `scaffold-pin-collides` (#241). A product + listed twice is therefore never a collision here, and two that differ in + more than the case `lower()` folds are, when `pin_source_env_name` maps + them to one name. That includes names no ASCII `--pin` can spell, such as + `openß` and `openss`, which share `SHAPE_PIN_SOURCE_OPENSS` and have two + pin files. Variables are in the order their first product was given, and each group's products are in that order too, each in the spelling it was first @@ -1136,16 +1156,17 @@ def pin_source_env_collisions(products) -> dict[str, list[str]]: if not isinstance(product, str) or not product: continue group = by_variable.setdefault(pin_source_env_name(product), {}) - group.setdefault(product.casefold(), product) + group.setdefault(pin_product_key(product), product) return {variable: list(group.values()) for variable, group in by_variable.items() if len(group) > 1} def pin_source_env_shared(products) -> set[str]: - """The casefolded names, among `products`, whose variable another of them - shares (#275): the products a variable must not answer for, because it - does not say which.""" - return {product.casefold() + """The `pin_product_key` of each of `products` whose variable another of + them shares (#275): the products a variable must not answer for, because + it does not say which. A caller asks `pin_product_key(name) in` the + result.""" + return {pin_product_key(product) for group in pin_source_env_collisions(products).values() for product in group} @@ -1310,9 +1331,9 @@ def _chain_link_findings(chain, link_pins) -> tuple: declared = available.get(holder.casefold()) if declared is None: unverified.append(holder) - offered = ("a sibling checkout or" if holder.casefold() in shared - else f"a sibling checkout, {pin_source_env_name(holder)}, " - "or") + alone = pin_product_key(holder) not in shared + offered = (f"a sibling checkout, {pin_source_env_name(holder)}, or" + if alone else "a sibling checkout or") warnings.append( f"{CHAIN_UNVERIFIED}: {holder} declaring a pin on {held} " f"is a fact in {holder}'s own tree, which is not " @@ -2467,7 +2488,7 @@ def link_pins_from_trees(links, root: Path | None = None, name = repo_basename(str(link)) path, _how = resolve_link_source(name, root, keyed_sources, default_source, - name.casefold() in shared) + pin_product_key(name) in shared) if path is None: continue pins = declared_neutral_pins(path, key) diff --git a/templates/assembly-root/scripts/validate-pins.py b/templates/assembly-root/scripts/validate-pins.py index 54d8ac74..5604007b 100755 --- a/templates/assembly-root/scripts/validate-pins.py +++ b/templates/assembly-root/scripts/validate-pins.py @@ -61,9 +61,9 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) from repo_shape import ( # noqa: E402 COMMIT_RE, SHA256_RE, TREE_DIGEST_DEFINITION, Refusal, file_sha256, - find_repo_root, git_out, load_yaml, pin_source_env_collisions, - pin_source_env_name, pin_source_env_shared, recorded_gitlink, - repo_basename, tree_digest, tree_digest_from_gh, + find_repo_root, git_out, load_yaml, pin_product_key, + pin_source_env_collisions, pin_source_env_name, pin_source_env_shared, + recorded_gitlink, repo_basename, tree_digest, tree_digest_from_gh, ) # `PIN_SOURCE_ENV_PREFIX` / `pin_source_env_name` live in `repo_shape` because @@ -103,13 +103,18 @@ def skip(self, text: str) -> None: nonzero exit. A network absence is not evidence of anything.""" self.notes.append(f" skip {text}") - def unused(self, text: str) -> None: - """A `--pin-source` no declared pin read (#295) — printed beside the - skips and, like them, never a FINDING and never the reason for a - nonzero exit: the person named a source that did not answer, and - nothing about any pin is wrong for that.""" + def advise(self, text: str) -> None: + """A NOTE: something a person should know that is no fault of the + project's — printed beside the skips and, like them, never a FINDING + and never the reason for a nonzero exit.""" self.notes.append(f" note {text}") + def unused(self, text: str) -> None: + """A `--pin-source` no declared pin read (#295): the person named a + source that did not answer, and nothing about any pin is wrong for + that. A note, as every one of those is.""" + self.advise(text) + def _workflow_refs(root: Path) -> list[tuple[Path, str, str]]: """Every (file, owner/repo, sha) `@` reference in the workflows.""" @@ -451,7 +456,8 @@ def resolve_neutral_pin_source(root: Path, product: str, repository: str, Rule 2 is skipped when `env_shared` is true, which is when another declared pin names the same variable (#275): a variable that cannot say which product it holds answers for neither, and `neutral-pin-source- - collides` is reported for the pair. + collides` is noted for the pair. The pin still verifies through rule 1, + 3 or 4. Returns `(path, how, typed)`, `typed` being the `--pin-source` value as given when one of those answered, so the caller can mark it used (#295). @@ -617,7 +623,7 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, already reported in that case, exactly as the original loop's `continue` left it. """ - env_shared = product.casefold() in shared + env_shared = pin_product_key(product) in shared source, how, typed = resolve_neutral_pin_source(root, product, repository, sources, env_shared) if source is not None: @@ -641,8 +647,11 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, try: return tree_digest_from_gh(repository, commit), "gh-api-tree-recursive" except Refusal as exc: - # A variable another pin shares is not offered (#275). - offered = ("A --pin-source or" if env_shared else + # A variable another pin shares is not offered, and neither is the + # bare `--pin-source`, which answers for every pin that has no more + # specific answer: the keyed form is the one that says which pin it + # is for (#275). + offered = (f"A --pin-source {product}= or" if env_shared else f"A --pin-source, {pin_source_env_name(product)}, or") report.skip( f"{product}: pin commit {commit[:12]} digest NOT " @@ -694,26 +703,33 @@ def _check_one_neutral_product_pin(root: Path, product, report: Report, def _report_pin_source_collisions(products, report: Report) -> None: - """One `neutral-pin-source-collides` finding for each variable two or more + """One `neutral-pin-source-collides` NOTE for each variable two or more declared products share (#275), naming every one of them and the - variable. `pin_source_env_collisions` is `repo_shape`'s, the one answer - `scaffold-project.py` refuses on too: products are distinct by casefold, - so a product listed twice is not a collision here. The names are quoted - with `repr`, as `neutral-pin-entry-malformed` quotes an entry: they are - the manifest's own text and may hold a newline.""" + variable. It is a note and never a finding: such a project did nothing + the standard forbade when it was made (`scaffold-project.py` has refused + it only since #275), every pin in it still verifies through the keyed + `--pin-source =` or a checkout beside this assembly root, + and a finding would make `update-shape.py apply` refuse every shape + update (`update-validators-red`) until a person dropped a true + declaration of descent. Which declaration stays is that person's + decision, so the note says so and nothing here asks for one to go. + `pin_source_env_collisions` is `repo_shape`'s, the one answer + `scaffold-project.py` refuses on too: products are distinct by their pin + files, so a product listed twice is not a collision here. The names are + quoted with `repr`, as `neutral-pin-entry-malformed` quotes an entry: they + are the manifest's own text and may hold a newline, and a note is one + line.""" for variable, group in pin_source_env_collisions(products).items(): names = " and ".join(repr(product) for product in group) - report.finding( - "neutral-pin-source-collides", - f"project.yaml neutral_product_pins declares {names}, and one " - f"variable, {variable}, names the local checkout of each: it " - "cannot say which it holds, so it is read for none of them.\n" - " Remediation: `SHAPE_PIN_SOURCE_` is the product's " - "name upper-cased with every character outside `A-Z`, `0-9` and " - "`_` turned into `_`, so names that differ only in `-`, `.` or " - "`_` share one. Declare one product of each such group, or " - "products whose names differ by more.", - ) + report.advise( + "neutral-pin-source-collides: project.yaml neutral_product_pins " + f"declares {names}, and one variable, {variable}, names the local " + "checkout of each: it cannot say which it holds, so it is read " + "for none of them. Each pin still verifies from the keyed " + "`--pin-source =` or from a checkout beside this " + "assembly root; a bare `--pin-source` answers for every pin that " + "has no more specific source. Which declaration stays is the " + "human's decision, and nothing here fails on it.") def _check_neutral_product_pins(root: Path, manifest: dict, report: Report, @@ -728,13 +744,17 @@ def _check_neutral_product_pins(root: Path, manifest: dict, report: Report, available is a SKIP, never a finding and never a refusal: the absence of a way to answer is not evidence that the pin is wrong. - TWO PRODUCTS THAT SHARE ONE VARIABLE ARE A FINDING (#275), and neither + TWO PRODUCTS THAT SHARE ONE VARIABLE ARE A NOTE (#275), and neither reads it. `SHAPE_PIN_SOURCE_` is the product's name upper-cased with every character outside `[A-Z0-9_]` turned into `_`, so `open-glass`, `open_glass` and `open.glass` are three pin files and one variable. Whichever checkout it named answered for all of them, and the pin it was not a checkout of was reported as `neutral-pin-source-unreadable`: drift - in the project, said about a variable. + in the project, said about a variable. Now no pin reads it, each pin + verifies from its own source, and `neutral-pin-source-collides` says so + and goes no further: it is not a finding, because a project that already + declares such a pair could not otherwise take a shape update, and which + declaration to drop is a human's to decide. The `--pin-source` values are READ here rather than in `main`, because whether `a=b/openGlass` is keyed to a product or a bare path depends on diff --git a/tests/test_pin_source_env_collisions.py b/tests/test_pin_source_env_collisions.py index 26cf852d..52207870 100644 --- a/tests/test_pin_source_env_collisions.py +++ b/tests/test_pin_source_env_collisions.py @@ -10,16 +10,24 @@ with no source given the same variable was offered as the way to verify either. Every existing variable name is kept: an operator may have set one and the docs -name them. What is refused instead is DECLARING a set of products whose -variables collide, in each place that declares or reads pins: - -- `scaffold-project.py --pin`, beside #241's file collisions, dry run and real - run alike, before any process starts; -- the project's own `validate-pins.py`, as the finding - `neutral-pin-source-collides`, after which neither product reads the - variable; -- the chain readers in `repo_shape.py`, which never read a variable two links - of one chain share and never offer it. +name them. What is done instead, in each place that declares or reads pins: + +- `scaffold-project.py --pin` REFUSES a set of products whose variables + collide, beside #241's file collisions, dry run and real run alike, before + any process starts: no new project is made with a variable it cannot use; +- the project's own `validate-pins.py` NOTES a pair a project already declares + (`neutral-pin-source-collides`) and never fails it, after which neither + product reads the variable and each is verified from its own source. It was + a finding in the first round, and a project declaring such a pair could not + take `update-shape.py apply` (`update-validators-red`) until a person dropped + a true declaration; the second round made it a note, and + `test_a_project_declaring_a_colliding_pair_takes_a_shape_update` holds that; +- the chain readers in `repo_shape.py` never read a variable two links of one + chain share and never offer it. + +Two declarations are one product when they are one PIN FILE (`pin_product_key`, +`lower()`), and not when `casefold()` says so: `openß` and `openss` share +`SHAPE_PIN_SOURCE_OPENSS` and have two files. `adopt-project.py --pin` is not here: its half arrives with #273's change, which routes adopt through the one function this file pins. @@ -33,10 +41,15 @@ from __future__ import annotations +import itertools +import json import os +import shutil import stat +import subprocess import sys import textwrap +import types from pathlib import Path import pytest @@ -45,6 +58,9 @@ REPO, SCAFFOLD, WINDOWS_SKIP, clear_ambient_pin_sources, git, run_script, ) from test_scaffold_pin_and_reuse import product_repo +from test_update_shape import ( + PROJECT, commit_upstream_changes, scaffold_upstream_and_project, verdicts, +) # Fixtures and constants of #241's file, shared rather than re-made: the # stand-in `openGlass` with two commits, the module-level `scaffold`, the # refusal of every process start, and the remote and `gh` world. @@ -56,10 +72,13 @@ sys.path.insert(0, str(REPO / "scripts")) import repo_shape # noqa: E402 -from repo_shape import NamingPolicy, tree_digest # noqa: E402 +from repo_shape import NamingPolicy, Refusal, tree_digest # noqa: E402 +from shape_materialize import neutral_pin_path # noqa: E402 UTF8 = "utf-8" POLICY_PATH = REPO / "contracts" / "repository-naming.yaml" +VALIDATE_PINS = (REPO / "templates" / "assembly-root" / "scripts" + / "validate-pins.py") #: The two refusals' remediation for the variable, word for word. Alone it #: stands after `Remediation: nothing was created.`, and beside a file @@ -144,6 +163,95 @@ def test_it_reads_a_generator_once(): "SHAPE_PIN_SOURCE_A_B": ["a-b", "a_b"]} +# --- two declarations are one product when they are one PIN FILE (#275, N2) --- + +#: Pairs of names that share a variable and have TWO pin files, though +#: `casefold()` makes one of each. Hand-written `project.yaml` cases only: +#: scaffold's `--pin` and adopt's `pins:` take ASCII, as a GitHub repository +#: name is. +FOLDED_APART = [ + pytest.param("open\u00df", "openss", "SHAPE_PIN_SOURCE_OPENSS", + id="sharp-s"), + pytest.param("open\u017f", "opens", "SHAPE_PIN_SOURCE_OPENS", + id="long-s"), + pytest.param("open\ufb01", "openfi", "SHAPE_PIN_SOURCE_OPENFI", + id="fi-ligature"), + pytest.param("open\u03c2", "open\u03c3", "SHAPE_PIN_SOURCE_OPEN_", + id="final-sigma"), +] + + +@pytest.mark.parametrize("first, second, variable", FOLDED_APART) +def test_names_one_variable_cannot_tell_apart_are_found_beyond_ascii( + first, second, variable): + """The four pairs the first round missed. Each shares a variable and has + two pin files, so one checkout answered for both and the pin it was not a + checkout of was reported against the project; `casefold()`, which was the + rule, made each pair one product.""" + assert first.casefold() == second.casefold(), "the reason they were missed" + assert neutral_pin_path(first) != neutral_pin_path(second) + assert repo_shape.pin_source_env_name(first) == variable + assert repo_shape.pin_source_env_name(second) == variable + assert repo_shape.pin_source_env_collisions([first, second]) == { + variable: [first, second]} + assert repo_shape.pin_source_env_collisions([second, first]) == { + variable: [second, first]} + assert repo_shape.pin_source_env_shared([first, second]) == {first, second} + + +#: A pool of names covering every way two of them can relate: the same name in +#: another case, punctuation that folds to `_`, a non-ASCII letter whose +#: upper-case is ASCII (`\u0131` is `I`, `\u017f` is `S`), a character that +#: upper-cases to two (`\u00df`, `\ufb01`), one that upper-cases to a letter +#: that is no ASCII (`\u03c2`, `\u03c3`) and one that is only `_` once folded. +NAME_POOL = [ + "openGlass", "OpenGlass", "OPENGLASS", "open-glass", "Open-Glass", + "open_glass", "open.glass", "open-x", "open_x", "open.x", "openPane", + "open\u0131", "openi", "openI", "open\u00e9", "open-", "open_", + "open\uff47", "open\u00df", "openss", "openSS", "open\u017f", "opens", + "openS", "open\ufb01", "openfi", "openFI", "open\u03c2", "open\u03c3", + "open\u03a3", "open\u0130", +] + + +def test_every_pair_sharing_a_variable_with_two_pin_files_is_found(): + """Over every pair of the pool: found exactly when the variable is shared + AND the pin files are two. The oracle is `neutral_pin_path`, which names + the pin file, and `pin_source_env_name`, which names the variable; no + casefold, no case rule of this test's own.""" + for first, second in itertools.combinations(NAME_POOL, 2): + shares = (repo_shape.pin_source_env_name(first) + == repo_shape.pin_source_env_name(second)) + two_files = neutral_pin_path(first) != neutral_pin_path(second) + found = repo_shape.pin_source_env_collisions([first, second]) + assert bool(found) is (shares and two_files), (first, second, found) + if found: + (variable, group), = found.items() + assert group == [first, second] + assert variable == repo_shape.pin_source_env_name(first) + assert repo_shape.pin_source_env_shared([first, second]) == { + repo_shape.pin_product_key(first), + repo_shape.pin_product_key(second)} + + +@pytest.mark.parametrize("name", NAME_POOL) +def test_no_single_product_is_ever_a_collision(name): + """Nor is one product declared twice, in any spelling of its pin file.""" + assert repo_shape.pin_source_env_collisions([name]) == {} + assert repo_shape.pin_source_env_collisions([name, name]) == {} + assert repo_shape.pin_source_env_shared([name]) == set() + assert repo_shape.pin_source_env_collisions([name, name.lower()]) == {} + + +@pytest.mark.parametrize("name", NAME_POOL) +def test_the_key_is_the_name_the_pin_file_carries(name): + """`pin_product_key` is not a second rule: it is the stem of the file + `shape_materialize.neutral_pin_path` writes, and `validate-pins.py` reads + the same file under `contracts/-pin.yaml`.""" + assert neutral_pin_path(name) == ( + f"contracts/{repo_shape.pin_product_key(name)}-pin.yaml") + + # --- scaffold-project.py --pin: refused, dry and real, before any process ---- #: `(the --pin values, what the refusal says, its remediation)`, `{A}` and @@ -338,7 +446,7 @@ def test_pins_whose_variables_differ_still_scaffold(glass, world, values, assert not world["log"].exists() -# --- validate-pins.py: a finding naming each product and the variable --------- +# --- validate-pins.py: a note naming each product and the variable ------------- PIN_FILE = textwrap.dedent("""\ schema_version: 1 @@ -416,33 +524,51 @@ def gh_shim(tmp_path): "GH_STUB_LOG": str(log)}} -COLLIDES = "FINDING neutral-pin-source-collides: " +#: The note's line, and what it says after naming the products and the +#: variable, word for word. It is a NOTE (#275, fix round 1, S1): the lines the +#: validator prints that fail nothing are `ok`, `skip` and `note`. +NOTE = " note neutral-pin-source-collides: " + + +def collides(names: str, variable: str) -> str: + """The whole note, after its code, for the products `names` quotes.""" + return (f"project.yaml neutral_product_pins declares {names}, and one " + f"variable, {variable}, names the local checkout of each: it " + "cannot say which it holds, so it is read for none of them. Each " + "pin still verifies from the keyed `--pin-source " + "=` or from a checkout beside this assembly " + "root; a bare `--pin-source` answers for every pin that has no " + "more specific source. Which declaration stays is the human's " + "decision, and nothing here fails on it.") + +def collision_notes(result) -> list[str]: + return [line for line in result.stdout.splitlines() + if line.startswith(NOTE)] -def test_two_pins_sharing_a_variable_are_one_finding_naming_both( + +def test_two_pins_sharing_a_variable_are_one_note_naming_both( project, tmp_path): - """Every source named by product answers, so what is reported is the - declaration and nothing else: one finding, both products, the variable.""" + """Every source named by product answers, so what is said about the + declaration is a note and nothing else: one line, both products, the + variable, and a run that passes.""" stand_ins = declare_pins(project, tmp_path / "products", ["open-glass", "open_glass"]) result = validate(project, "--pin-source", f"open-glass={stand_ins['open-glass']}", "--pin-source", f"open_glass={stand_ins['open_glass']}") - assert result.returncode == 1, result.stdout + result.stderr - assert result.stderr.count("FINDING") == 1, result.stderr - assert (f"{COLLIDES}project.yaml neutral_product_pins declares " - "'open-glass' and 'open_glass', and one variable, " - "SHAPE_PIN_SOURCE_OPEN_GLASS, names the local checkout of each: " - "it cannot say which it holds, so it is read for none of them." - ) in result.stderr - assert "Remediation: `SHAPE_PIN_SOURCE_`" in result.stderr - assert "1 finding(s)" in result.stderr + assert result.returncode == 0, result.stdout + result.stderr + assert "FINDING" not in result.stderr and "finding(s)" not in result.stderr + assert collision_notes(result) == [ + NOTE + collides("'open-glass' and 'open_glass'", + "SHAPE_PIN_SOURCE_OPEN_GLASS")] + assert result.stdout.rstrip().endswith("pins ok") for name in ("open-glass", "open_glass"): assert f" ok {name}: pin commit " in result.stdout assert f"digest recomputes (--pin-source {name}=...)" in result.stdout -def test_three_products_one_variable_are_one_finding_naming_all_three( +def test_three_products_one_variable_are_one_note_naming_all_three( project, tmp_path): names = ["open-glass", "open.glass", "open_glass"] stand_ins = declare_pins(project, tmp_path / "products", @@ -451,12 +577,25 @@ def test_three_products_one_variable_are_one_finding_naming_all_three( for arg in ("--pin-source", f"{name}={stand_ins[name]}")), "--pin-source", f"openGlass={stand_ins['openGlass']}") - assert result.returncode == 1, result.stdout + result.stderr - assert result.stderr.count("FINDING") == 1, result.stderr - assert (f"{COLLIDES}project.yaml neutral_product_pins declares " - "'open-glass' and 'open.glass' and 'open_glass', and one " - "variable, SHAPE_PIN_SOURCE_OPEN_GLASS,") in result.stderr - assert "openGlass'" not in result.stderr.split("Remediation")[0] + assert result.returncode == 0, result.stdout + result.stderr + assert "FINDING" not in result.stderr + assert collision_notes(result) == [ + NOTE + collides("'open-glass' and 'open.glass' and 'open_glass'", + "SHAPE_PIN_SOURCE_OPEN_GLASS")] + assert "openGlass'" not in collision_notes(result)[0] + assert result.stdout.count("digest recomputes (--pin-source") == 4 + + +def test_a_quiet_run_prints_no_note_and_still_passes(project, tmp_path): + """`--quiet` prints the notes of no kind, as it always did: nothing else + is said about a pair that nothing fails on.""" + stand_ins = declare_pins(project, tmp_path / "products", + ["open-glass", "open_glass"]) + result = validate(project, "--quiet", "--pin-source", + f"open-glass={stand_ins['open-glass']}", + "--pin-source", f"open_glass={stand_ins['open_glass']}") + assert result.returncode == 0, result.stdout + result.stderr + assert result.stdout.strip() == "pins ok" and result.stderr == "" @WINDOWS_SKIP # the stub `gh` is a `#!` script; Windows executes none @@ -467,26 +606,54 @@ def test_a_shared_variable_answers_for_neither_pin(project, tmp_path, gh_shim, main through c7c02fa, answered for both: the other was reported `neutral-pin-source-unreadable`, a finding about the project made by a checkout of another. Now it answers for neither, so both fall through to - `gh` (stubbed, and asked about each) and the only finding is the - declaration's; the hint no longer offers the variable.""" + `gh` (stubbed, and asked about each); the declaration is a note, so the + run passes with the two skips; and the hint offers the keyed + `--pin-source =`, the one source that says which pin it is + for, and never the variable or the bare form.""" stand_ins = declare_pins(project, tmp_path / "products", ["open-glass", "open_glass"]) env = {**gh_shim["env"], "SHAPE_PIN_SOURCE_OPEN_GLASS": str(stand_ins[named])} result = validate(project, env=env) - assert result.returncode == 1, result.stdout + result.stderr + assert result.returncode == 0, result.stdout + result.stderr assert "neutral-pin-source-unreadable" not in result.stderr - assert result.stderr.count("FINDING") == 1, result.stderr - assert COLLIDES in result.stderr + assert "FINDING" not in result.stderr + assert len(collision_notes(result)) == 1 calls = gh_shim["log"].read_text(encoding=UTF8).splitlines() assert [call.split("/git/trees")[0] for call in calls] == [ "api repos/opensoft/open-glass", "api repos/opensoft/open_glass"] skips = [line for line in result.stdout.splitlines() if line.startswith(" skip ")] assert len(skips) == 2 - for line in skips: + for name, line in zip(("open-glass", "open_glass"), skips): assert "SHAPE_PIN_SOURCE" not in line - assert "A --pin-source or a checkout at ../open" in line + assert f"A --pin-source {name}= or a checkout at ../{name} " \ + "beside this assembly root would verify it offline." in line + + +@pytest.mark.parametrize("named", ["open-glass", "open_glass"]) +def test_a_shared_variable_answers_for_neither_pin_with_no_gh(project, named): + """THE SAME GUARD, WITH NO `gh` AT ALL, so it runs on Windows too (the + test above needs a `#!` stub). Each pin has a checkout BESIDE the project, + `../open-glass` and `../open_glass`, and the variable names ONE of them. + On ab623dd the variable answered for both pins, so the other was + `neutral-pin-source-unreadable` (a commit its checkout does not hold), exit + 1. Now neither reads it: each pin is recomputed from its own sibling, the + variable is named by no line, and only the note is said. No pin reaches + `gh`, which the session's guard would fail this test for.""" + stand_ins = declare_pins(project, project.parent, + ["open-glass", "open_glass"]) + result = validate(project, env={ + "SHAPE_PIN_SOURCE_OPEN_GLASS": str(stand_ins[named])}) + assert result.returncode == 0, result.stdout + result.stderr + assert "neutral-pin-source-unreadable" not in result.stderr + assert "FINDING" not in result.stderr + assert len(collision_notes(result)) == 1 + for name in ("open-glass", "open_glass"): + line = next(line for line in result.stdout.splitlines() + if line.startswith(f" ok {name}: pin commit ")) + assert "digest recomputes (sibling checkout (" in line, line + assert "($SHAPE_PIN_SOURCE" not in result.stdout @WINDOWS_SKIP # the stub `gh` is a `#!` script; Windows executes none @@ -537,6 +704,222 @@ def test_a_product_declared_twice_is_not_a_shared_variable(project, tmp_path): ) == 2 +# --- the skip hint, with no `gh` and no filesystem: runs on every OS --------- + +@pytest.fixture(scope="module") +def validate_pins(): + """The template's `validate-pins.py` as a module, so its functions are + called in-process. A hyphenated name is not importable, and + `importlib.util.spec_from_file_location` (which `tests/test_update_shape.py` + uses for `update-shape.py`) would write `__pycache__` BESIDE the template, + inside `templates/assembly-root`, where `tests/test_scaffold_leg_path_ + collisions.py` reads every file under that directory as text and dies on + the bytecode. So the source is compiled and run into a module here, and + nothing is written. `repo_shape` resolves to the module cached above, so + `Refusal` is the one the file raises.""" + module = types.ModuleType("validate_pins_under_test") + module.__file__ = str(VALIDATE_PINS) + exec(compile(VALIDATE_PINS.read_bytes(), str(VALIDATE_PINS), "exec", + dont_inherit=True), module.__dict__) + return module + + +def skip_hint(validate_pins, monkeypatch, tmp_path, products, product): + """The `skip` line `product` gets when nothing answers for it: no source, + no sibling, and a `gh` that is not there (`tree_digest_from_gh` refuses as + it does when `gh` is not on PATH, which is no process at all).""" + clear_ambient_pin_sources(monkeypatch) + + def no_gh(repository, commit): + raise Refusal("gh-not-found", "the `gh` CLI is not on PATH") + + monkeypatch.setattr(validate_pins, "tree_digest_from_gh", no_gh) + report = validate_pins.Report() + answered = validate_pins._recompute_neutral_pin_digest( + tmp_path / PROJECT, product, f"opensoft/{product}", "1" * 40, + f"contracts/{product}-pin.yaml", + validate_pins.PinSources([], products), report, + frozenset(repo_shape.pin_source_env_shared(products))) + assert answered is None + assert len(report.notes) == 1 and report.findings == [] + assert report.notes[0].startswith(f" skip {product}: pin commit ") + return report.notes[0] + + +def test_a_product_two_pins_share_is_offered_the_keyed_pin_source( + validate_pins, monkeypatch, tmp_path): + """A shared product's hint offers `--pin-source =` (N3): the + bare `--pin-source` answers for every pin with no more specific source, + so with two pins it gives the other the finding #275 was about, and the + variable answers for neither. The keyed form is the one source that says + which pin it is for.""" + for product in ("open-glass", "open_glass"): + line = skip_hint(validate_pins, monkeypatch, tmp_path, + ["open-glass", "open_glass"], product) + assert line.endswith( + f"A --pin-source {product}= or a checkout at ../{product} " + "beside this assembly root would verify it offline.") + assert "SHAPE_PIN_SOURCE" not in line + assert "A --pin-source or" not in line + + +@pytest.mark.parametrize("products", [ + pytest.param(["open-glass"], id="alone"), + pytest.param(["open-glass", "openGlass"], id="variables-differ"), + pytest.param(["open-glass", "Open-Glass"], id="one-product-twice"), +]) +def test_a_product_with_a_variable_to_itself_is_offered_it_as_before( + validate_pins, monkeypatch, tmp_path, products): + """The control: the hint's text for every product no other shares a + variable with is unchanged, the variable and the bare `--pin-source`.""" + line = skip_hint(validate_pins, monkeypatch, tmp_path, products, + "open-glass") + assert line.endswith( + "A --pin-source, SHAPE_PIN_SOURCE_OPEN_GLASS, or a checkout at " + "../open-glass beside this assembly root would verify it offline.") + + +@pytest.mark.parametrize("first, second, variable", FOLDED_APART) +def test_the_note_is_made_for_names_casefold_took_for_one_product( + validate_pins, tmp_path, first, second, variable): + """A hand-written `project.yaml` naming `openß` and `openss` (and the + three other pairs) declares two pins one variable names: the manifest is + READ, as the validator reads it, and `_report_pin_source_collisions` is + asked of what it declares. Every name is written as an escape in this + file, and the manifest holds it as UTF-8, so no part of the run depends + on the console's code page.""" + manifest = tmp_path / "project.yaml" + write(manifest, f"schema_version: 1\nneutral_product_pins:\n" + f" - {first}\n - {second}\n") + products = repo_shape.load_yaml(manifest)["neutral_product_pins"] + assert products == [first, second] + report = validate_pins.Report() + validate_pins._report_pin_source_collisions(products, report) + assert report.findings == [] + assert report.notes == [ + NOTE + collides(f"{first!r} and {second!r}", variable)] + + +# --- a project that already declares such a pair takes a shape update --------- + +#: The two copied files #275's change ships into every project, as upstream +#: commit B changes them: a line appended to each, which is all an update that +#: carries `validate-pins.py` and `repo_shape.py` has to be for `apply` to +#: rewrite both and then run the project's validators. +VALIDATE_PINS_SOURCE = "templates/assembly-root/scripts/validate-pins.py" +SHIPPED = {VALIDATE_PINS_SOURCE: "validate-pins", + "scripts/repo_shape.py": "repo_shape"} +SHIPPED_COPIES = ("scripts/validate-pins.py", "scripts/repo_shape.py") + + +@pytest.fixture(scope="module") +def behind(tmp_path_factory) -> dict: + """A clone of this repository at A, and B, which changes the two files a + project's pin validator is made of.""" + _base, upstream, commit_a, clone = scaffold_upstream_and_project( + tmp_path_factory, "pin-source-collides-update") + commit_b = commit_upstream_changes( + upstream, "Ship the pin validator's change", + {source: f"\n# {label}, as an update ships it.\n" + for source, label in SHIPPED.items()}) + return {"upstream": upstream, "clone": clone, "a": commit_a, "b": commit_b} + + +def declared_project(behind, base: Path, names) -> Path: + """A copy of the project as it was scaffolded at A, then declaring a pin + on each of `names` as an older scaffold's project does (main through + ab623dd scaffolded `--pin open-glass@A --pin open_glass@B`, exit 0, and + this is what that left), committed. A stand-in repository for each name + sits BESIDE the project, so each pin is recomputed offline and no `gh` is + asked.""" + root = base / PROJECT + shutil.copytree(behind["clone"], root, symlinks=True) + declare_pins(root, base, names) + git("add", "-A", cwd=root) + git("commit", "-q", "-m", "Declare the pins", cwd=root) + return root + + +def update(behind, root: Path, verb: str, *args: str): + return run_script(behind["upstream"] / "update-shape.py", verb, "--root", + str(root), "--upstream", str(behind["upstream"]), + "--at", behind["b"], *args) + + +def doctor_of(behind, root: Path) -> dict: + result = run_script(behind["upstream"] / "shape-doctor.py", "--root", + str(root), "--json") + payload = json.loads(result.stdout) + return {"exit": result.returncode, "verdict": payload["verdict"], + "rows": {row["id"]: row for row in payload["rows"]}} + + +@pytest.mark.parametrize("names", [ + pytest.param(["open-glass", "open_glass"], id="hyphen-underscore"), + pytest.param(["open-glass", "open.glass", "open_glass"], id="three"), +]) +def test_a_project_declaring_a_colliding_pair_takes_a_shape_update( + behind, tmp_path, names): + """THE FIRST ROUND'S FINDING LOCKED SUCH A PROJECT OUT. Main through + ab623dd scaffolds `--pin open-glass@A --pin open_glass@B`, and the first + round made `neutral-pin-source-collides` a finding, so the NEW + `validate-pins.py` that `update-shape.py apply` runs over the rewritten + root exited 1 and the whole apply ended `REFUSED update-validators-red`, + rolled back, for this update and every later one, until a person dropped a + true declaration of descent. Now it is a note: the update applies, the + project's validator passes and says the note, each pin still verifies from + its own checkout, and the doctor's `pins` row stays `ok` before and after + and moves no verdict a project declaring no pair does not have.""" + root = declared_project(behind, tmp_path, names) + control = declared_project(behind, tmp_path / "control", ["openGlass"]) + + before, before_control = doctor_of(behind, root), doctor_of(behind, + control) + assert before["rows"]["pins"]["status"] == "ok", before["rows"]["pins"] + assert before["rows"]["shape-currency"]["status"] == "FINDING" + assert (before["verdict"], before["exit"]) == ( + before_control["verdict"], before_control["exit"]), (before, + before_control) + + checked = update(behind, root, "check") + assert checked.returncode == 1, checked.stdout + checked.stderr + behind_by = verdicts(checked.stdout) + for copy in SHIPPED_COPIES: + assert behind_by[copy] == "upstream-changed", checked.stdout + + applied = update(behind, root, "apply", "--yes") + assert applied.returncode == 0, applied.stdout + applied.stderr + assert "update-validators-red" not in applied.stdout + applied.stderr + # The blob at B, never the upstream's working-tree file: a helper that + # appended to it with `write_text` wrote CRLF on Windows. + shipped = subprocess.run( + ["git", "show", f"{behind['b']}:{VALIDATE_PINS_SOURCE}"], + cwd=str(behind["upstream"]), capture_output=True, check=True).stdout + assert (root / "scripts" / "validate-pins.py").read_bytes() == shipped, ( + "the update was applied") + + result = validate(root) + assert result.returncode == 0, result.stdout + result.stderr + assert "FINDING" not in result.stderr + (note,) = collision_notes(result) + assert "SHAPE_PIN_SOURCE_OPEN_GLASS" in note + assert all(repr(name) in note for name in names) + for name in names: + assert f" ok {name}: pin commit " in result.stdout + assert result.stdout.count("digest recomputes (sibling checkout (") == len( + names) + + controlled = update(behind, control, "apply", "--yes") + assert controlled.returncode == 0, controlled.stdout + controlled.stderr + after, after_control = doctor_of(behind, root), doctor_of(behind, control) + assert after["rows"]["pins"]["status"] == "ok", after["rows"]["pins"] + assert after["rows"]["shape-currency"]["status"] == "ok" + assert (after["verdict"], after["exit"]) == ( + after_control["verdict"], after_control["exit"]), (after, + after_control) + assert after["exit"] == 0, after + + # --- the chain readers: a variable two links share is read for neither -------- def tree(path: Path, *pins: str) -> Path: From 74ce4d9bb0b16cf59151c6aa1f5b5b5dcadcaa0d Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 12 Oct 2026 17:52:42 +0000 Subject: [PATCH 7/9] shape_materialize.py, adopt-project.py: a `--pin-source` that is not printable is named as `repr` spells it, no `gh` on PATH is `gh-not-found`, `plan` refuses a `--pin-source` `check` would refuse, and two pins share a variable by their pin files, so a refusal no longer prints an ESC or forges a `REFUSED` line, a pin read with no `gh` is no traceback, and `check` no longer refuses the plan `plan` just wrote (#273, #339) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix round 1 of lane 5's review of this branch at 7a94b6b. - Q's `pin_product_key` (#275's fix round, merged in da2248f) is now how `pin_variable_collisions` keys each pin, where it casefolded. Q left `scaffold-project.py` untouched, and this branch had moved the function that used the casefold into `scripts/shape_materialize.py`, so the merge could not carry it. `openß` and `openss` are two pin files and one `SHAPE_PIN_SOURCE_OPENSS`: a casefold named each of the two pins twice. `refuse_colliding_pins`' docstring says, in Q's words, that the root's `validate-pins.py` now only NOTES such a pair. - S1: `pin_digest_from_source` names the path as typed unless it is not printable, and then as Python's `repr` spells it (`shown_source`), in the refusal's head and in git's own reason, which names the directory it ran in. Printed raw, an ESC reached the terminal, and a line break or a carriage return began a forged `REFUSED pin-source-unreadable` line, from the scaffold and from adopt; the traceback #339 replaced had shown a `repr`. A printable path is named byte for byte as before. - S2: `pin_digest_from_gh` with no `gh` on PATH is `gh-not-found`, exit 2, as `repo_shape.tree_digest_from_gh` names it, offering `--pin-source`. It was a `FileNotFoundError` traceback, exit 1, from the scaffold and, new with #273, from `adopt-project.py plan --pin`. - S3, the lane's decision (c): a commit or digest edited into the plan passes `check` and `execute`, by design: neither reads a network for a pin. `check` now says so once, after the pins it will write; the root's own `validate-pins.py` reports a digest that disagrees. No behaviour changed. - N2: `plan` asks `check`'s question (`one_printable_line`) of the `--pin-source` it records in each pin's `digest_source:`, before any digest is read, and refuses a path that fails it as `adopt-unwritable-value`, in #270's words, or "an invisible formatting character" where those name nothing. It was planned, exit 0, printed raw in the report, and `check` refused it as `plan-pin-malformed` with advice that wrote the same plan. The report's `pin` line and `check`'s `write` line spell the digest's source with `_shown`, as every path either prints. - N3: tests now kill each of the six hunks of this branch no test guarded: the two name guards (a `pins:` name holding an ESC and a forged line), the lower-casing of the commit and digest (an upper-cased hand edit), the `digest_source` printability check, the not-text check (`commit: 1234567` unquoted), and the gitlink filter (a submodule kept where the pin is written is `plan-submodule-root` alone). Tests: `tests/test_adopt_pins.py` and `tests/test_pin_source_unreadable.py` gain the rows above; one existing row of the latter now expects `adopt-unwritable-value` from `adopt-project.py plan` for a name that is not UTF-8, which `plan` cannot record. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 58 ++++- scripts/shape_materialize.py | 80 +++++-- tests/test_adopt_pins.py | 315 +++++++++++++++++++++++++++- tests/test_pin_source_unreadable.py | 110 +++++++++- 4 files changed, 533 insertions(+), 30 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 4a13469d..85b36781 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -2019,12 +2019,51 @@ def _checked_plan_inputs(args, source: Source, "named again, retyped; when `--elected-by` is not " "given, `plan` reads `git config user.name`, so pass " "the flag or correct that. " + WRITTEN_AS_TEXT) + if args.pin and args.pin_source is not None: + _refuse_an_unrecordable_pin_source(args.pin_source, products) names = _names(args.project) pins = declared_pin_values(args) _check_names(naming, names, set(pins)) return names, pins +def _refuse_an_unrecordable_pin_source(source: Path, products) -> None: + """`--pin-source` is a path `plan` can record, or this refuses it as + `adopt-unwritable-value`, before any digest is read and before a plan + is written. + + `plan` records the path in each pin's `digest_source:`, `execute` + writes that into the pin file as it stands, and `check` holds it to + `one_printable_line` (`_pin_row_problem`). A path holding a line break, + an ESC or a byte that is not UTF-8 was planned, exit 0, and printed raw + in `plan`'s report -- the line break split it -- and `check` then found + `plan-pin-malformed` and said to re-run the `plan` that wrote it. So + `plan` asks `check`'s question of the value it would record, and says + why in #270's words (`unwritable_reason`) where they name it; the one + thing `check` refuses that they do not is an invisible formatting + character, a zero-width joiner among them. The `local-clone (` and `)` + that `declared_pin_values` puts around the path are printable, so the + path alone decides. + """ + text = str(source) + if one_printable_line(text): + return + reason = (unwritable_reason(text) + or "it holds an invisible formatting character") + files = ", ".join(f"`{neutral_pin_path(product)}`" + for product in products) + raise Refusal( + "adopt-unwritable-value", + f"--pin-source is {text!r}, which `plan` records in each pin's " + f"`digest_source:` and `execute` writes into {files}, and {reason}.", + "Remediation: no plan was written. Point --pin-source at a clone " + "whose path is one line of printable UTF-8 text, or drop " + "--pin-source and let `gh api` read the forge. `check` refuses a " + "`digest_source:` that is not one printable line as " + "`plan-pin-malformed`, because `execute` writes it into the pin file " + "as it stands.") + + def _print_triad_advisory(path: Path, commit: str | None = None) -> None: """The Triad advisory, once, about the repository this run reads. @@ -2089,12 +2128,13 @@ def _print_plan_report(args, source: Source, tree: list, entries: list, print(f"project {args.project} ({args.id}) in {args.org}, mode in-place") # Each pin as the scaffold's plan prints it: what is pinned, at which # commit and digest, the file `execute` writes, and how the digest was - # read (#273). + # read (#273), that last `_shown`, as every path this report prints is. for product, values in (pins or {}).items(): print(f"pin {values['PIN_REPOSITORY']} @ " f"{values['PIN_COMMIT'][:12]} tree " f"{values['PIN_TREE_SHA256'][:12]}… -> " - f"{neutral_pin_path(product)} ({values['PIN_DIGEST_SOURCE']})") + f"{neutral_pin_path(product)} " + f"({_shown(values['PIN_DIGEST_SOURCE'])})") _print_triad_advisory(source.path, source.commit) _print_entries(entries) _print_summary(entries) @@ -3558,12 +3598,20 @@ def _print_what_will_happen(plan: Plan, source: Source, names: dict, print(f" {len(stays)} path(s) stay in the assembly root; " f"{len(drops)} dropped") # Each pin `execute` writes, as the plan records it (#273): a pin it - # cannot write is a finding, printed after this. - for product, values in _recorded_pins(plan).items(): + # cannot write is a finding, printed after this. NOT READ AGAIN, by + # design: `check` and `execute` read no network for a pin, so a commit + # or a digest edited into the plan is written as it stands, and the + # root's own gate is what recomputes it (#273's review, S3). + recorded = _recorded_pins(plan) + for product, values in recorded.items(): print(f" write {neutral_pin_path(product)}: " f"{values['PIN_REPOSITORY']} @ {values['PIN_COMMIT'][:12]} tree " f"{values['PIN_TREE_SHA256'][:12]}… " - f"({values['PIN_DIGEST_SOURCE']})") + f"({_shown(values['PIN_DIGEST_SOURCE'])})") + if recorded: + print(" each pin as `plan` read it, not read again here: the root's " + "own `validate-pins.py` recomputes its digest wherever it can " + "read the product") print(" the source is never deleted, never renamed, never force-pushed") print(_topics_line(topic, local)) for item in plan.get("follow_ups", []): diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 6268c86e..e8c6047e 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -55,10 +55,11 @@ # What a declared neutral-product pin is read with (#273): the forge's tree # listing, the owner an unqualified name resolves under, and the digest; and # which products share a `SHAPE_PIN_SOURCE_` variable, the one answer -# the project's own `validate-pins.py` reports on (#275). +# the project's own `validate-pins.py` reports on (#275), with two +# declarations ONE product when they share a pin file (`pin_product_key`). from repo_shape import ( # noqa: E402 GH_TREE_API, NEUTRAL_PRODUCT_OWNER, PIN_SOURCE_ENV_PREFIX, - pin_source_env_collisions, tree_digest, + pin_product_key, pin_source_env_collisions, tree_digest, ) SHAPE_REPOSITORY = "opensoft/openRepoShape" @@ -1840,21 +1841,42 @@ def pin_digest_from_source(source: Path, commit: str, repository: str) -> str: and on 3.14 "no such directory" for a directory that is there. A source `stat()` can read but `cwd=` cannot enter (a directory with no search bit) fails in `tree_digest`, which the same guard answers. + + THE PATH IS NAMED AS TYPED, UNLESS IT IS NOT PRINTABLE (`shown_source`), + and then as Python's `repr` spells it, wherever the refusal names it -- + git's own reason names the directory it ran in. Printed raw, an ESC in + it reached the terminal, and a line break or a carriage return began a + line of the person's choosing, a forged `REFUSED` among them; the + traceback it replaced had shown the path as a `repr`. A printable path + is named byte for byte as it always was. """ try: return _digest_from_pin_source(source, commit) except (Refusal, OSError) as exc: why = (exc.detail if isinstance(exc, Refusal) else exc.strerror or str(exc)) + named = shown_source(source) raise Refusal( "pin-source-unreadable", - f"{source} could not answer for {repository} @ {commit}: {why}", + f"{named} could not answer for {repository} @ {commit}: " + f"{why.replace(str(source), named)}", "Remediation: point --pin-source at a clone of that repository " "that HAS the pinned commit (`git fetch --all` first), or drop " "--pin-source and let `gh api` read the forge.", ) from exc +def shown_source(source) -> str: + """A `--pin-source` path as a refusal names it: as it is, unless a + character in it is not printable (`str.isprintable`), and then as + Python's `repr` spells it -- `\\x1b`, `\\n`, `\\r`, and `\\udcff` for a + byte that is not UTF-8, as `sys.argv` keeps one -- which is how every + other value a refusal here names is spelled (`--pin {raw!r}`). The + question `adopt-project.py`'s `_shown` asks of a path it prints.""" + text = str(source) + return text if text.isprintable() else repr(text) + + def _digest_from_pin_source(source: Path, commit: str) -> str: """`tree_digest` over `source`, with a path that is no directory said in `validate-pins.py`'s words (#339). Any other `OSError`, from `stat()` @@ -1894,13 +1916,32 @@ def pin_digest_from_gh(repository: str, commit: str) -> str: the same number. Tree rows are dropped because `-r` emits none; a submodule arrives as `type: commit` with mode `160000` and is kept, as `ls-tree` keeps it. + + NO `gh` ON PATH IS `gh-not-found`, as `repo_shape.tree_digest_from_gh`, + the validator's twin of this reader, names it: exit 2, before anything + is created, with `--pin-source` offered. It was a `FileNotFoundError` + traceback, exit 1, out of `subprocess.run`, from the scaffold and, since + #273 reads adopt's pins here, from `adopt-project.py plan --pin`, which + needed no `gh` before. """ - proc = subprocess.run( - ["gh", "api", GH_TREE_API.format(repo=repository, commit=commit)], - # UTF-8 and STRICT, not `errors="replace"`: this JSON becomes a - # digest, and a replaced byte would be a wrong digest with a - # confident tone (#217). - capture_output=True, text=True, encoding="utf-8", check=False) + try: + proc = subprocess.run( + ["gh", "api", GH_TREE_API.format(repo=repository, commit=commit)], + # UTF-8 and STRICT, not `errors="replace"`: this JSON becomes a + # digest, and a replaced byte would be a wrong digest with a + # confident tone (#217). + capture_output=True, text=True, encoding="utf-8", check=False) + except FileNotFoundError as exc: + raise Refusal( + "gh-not-found", + f"the `gh` CLI is not on PATH, so `gh api` cannot read " + f"{repository} @ {commit}: {exc}", + "Remediation: pass --pin-source to compute the digest with no network at all, or " + "put `gh` on PATH and run this again (`openRepoShape " + "--preflight` names each missing prerequisite and offers to " + "install it, on a typed yes).", + ) from exc if proc.returncode != 0: raise Refusal( "pin-unreadable", @@ -2076,18 +2117,21 @@ def pin_variable_collisions(typed) -> list[str]: for all of them, and the project's own `validate-pins.py` reported the pin it was not a checkout of as `neutral-pin-source-unreadable`. The question is `repo_shape.pin_source_env_collisions`'s, so it is the one - `validate-pins.py` answers, and it takes products as their pin files do: - the same product typed twice is #241's `one file`, said by - `pin_collisions`, and is never a collision here. + `validate-pins.py` answers, and it takes products as their pin files do + (`pin_product_key`): the same product typed twice is #241's `one file`, + said by `pin_collisions`, and is never a collision here. Each pin is + named under its own product by that key too, and not by a casefold, + which took `openß` and `openss`, two pin files, for one product and + named each of the two pins twice. """ said_of = {} for said, product in typed: - said_of.setdefault(product.casefold(), []).append(said) + said_of.setdefault(pin_product_key(product), []).append(said) out = [] for variable, products in pin_source_env_collisions( product for _, product in typed).items(): named = [said for product in products - for said in said_of[product.casefold()]] + for said in said_of[pin_product_key(product)]] out.append( " and ".join(named) + " name the products " + " and ".join(f"`{product}`" for product in products) @@ -2149,8 +2193,12 @@ def refuse_colliding_pins(pins, owner: str, TWO PINS THAT SHARE A SOURCE VARIABLE ARE REFUSED HERE TOO (#275), in the same refusal and after the file collisions, by `pin_variable_collisions`: `--pin open-glass@ --pin open_glass@` are two files and one - `SHAPE_PIN_SOURCE_OPEN_GLASS`. The refusal is the one the project's own - `validate-pins.py` would make as a finding, said before anything exists. + `SHAPE_PIN_SOURCE_OPEN_GLASS`. This is refused so that no new project is + made, and no plan written, with a variable it cannot use: the project's + own `validate-pins.py` only NOTES such a pair in one that already + declares it (`neutral-pin-source-collides`), since nothing it could have + done then is wrong, but a pair made now is said no to before anything + exists. """ typed = [(f"--pin {raw!r}", parse_pin(raw, owner)[0]) for raw in pins] collisions = pin_collisions(typed) diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index 5fb8af20..6f7bae60 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -44,12 +44,13 @@ import pytest from conftest import (ADOPT, FILE_PROTOCOL, REPO, SCAFFOLD, SYNTHETIC_TREE, - WINDOWS_SKIP, findings, git, make_source_repo, planned, - refused_before_any_leg, resolve, run_script) + WINDOWS_SKIP, as_human, findings, git, make_source_repo, + planned, refused_before_any_leg, resolve, run_script) sys.path.insert(0, str(REPO / "scripts")) import shape_materialize # noqa: E402 -from repo_shape import load_yaml, tree_digest # noqa: E402 +from repo_shape import (Refusal, load_yaml, pin_source_env_name, # noqa: E402 + tree_digest) UTF8 = "utf-8" PROJECT = "MedxGlass" @@ -520,6 +521,187 @@ def test_a_pin_the_source_does_not_have_is_refused_by_plan(source_repo, assert not out.exists() +# --- no `gh` on PATH (#273's review, S2) ------------------------------------- +# +# `gh api` is the DEFAULT way to a pin's digest: `--pin-source` is the test +# path. With no `gh` installed, `subprocess.run(["gh", ...])` raised +# `FileNotFoundError` out of the shared reader, a traceback, exit 1, from the +# scaffold and, since #273, from `adopt-project.py plan --pin`, which needed +# no `gh` before. It is `gh-not-found`, as the validator's twin +# (`repo_shape.tree_digest_from_gh`) names it. + +GH_NOT_FOUND = ("REFUSED gh-not-found: the `gh` CLI is not on PATH, so `gh " + "api` cannot read opensoft/openGlass @ {commit}: ") +GH_NOT_FOUND_REMEDIATION = ( + "Remediation: pass --pin-source to " + "compute the digest with no network at all, or put `gh` on PATH and run " + "this again (`openRepoShape --preflight` names each missing prerequisite " + "and offers to install it, on a typed yes).") + + +def test_with_no_gh_on_path_the_forge_reader_is_gh_not_found(monkeypatch, + tmp_path): + """The shared reader itself, with a PATH that holds nothing at all: on + every platform, and with no `gh` of anybody's reachable.""" + nothing = tmp_path / "nothing" + nothing.mkdir() + monkeypatch.setenv("PATH", str(nothing)) + commit = "0" * 40 + with pytest.raises(Refusal) as refused: + shape_materialize.pin_digest_from_gh("opensoft/openGlass", commit) + assert refused.value.code == "gh-not-found" + assert str(refused.value).startswith(GH_NOT_FOUND.format(commit=commit)) + assert refused.value.remediation == GH_NOT_FOUND_REMEDIATION + + +#: A PATH that finds `git` and no `gh` is a directory holding a link to git +#: alone, and Windows grants a symlink only to an elevated user. +GIT_ALONE = pytest.mark.skipif( + os.name == "nt", + reason="needs a PATH that finds git and no gh, made with a symlink to " + "git, which Windows grants only to an elevated user") + + +@GIT_ALONE +@pytest.mark.parametrize("kind", ["scaffold-dry-run", "adopt-plan"]) +def test_with_no_gh_on_path_a_pin_is_gh_not_found(kind, source_repo, + tmp_path, glass): + """Both tools, end to end, with no `--pin-source`: exit 2, the refusal + and its remediation, no traceback, and nothing made -- no plan, no + remote and no work directory.""" + _, commit, _ = glass + alone = tmp_path / "git-alone" + alone.mkdir() + os.symlink(shutil.which("git"), alone / "git") + assert shutil.which("gh", path=str(alone)) is None + env = {"PATH": str(alone)} + if kind == "adopt-plan": + made = [tmp_path / "plan.yaml"] + result = plan_with(source_repo, made[0], f"openGlass@{commit}", + env=env) + else: + made = [tmp_path / "remotes", tmp_path / "work"] + result = run_script( + SCAFFOLD, "--org", ORG, "--project", PROJECT, + "--elected-by", "Test Human", "--elected-on", "2026-09-02", + "--pin", f"openGlass@{commit}", + "--local-remote-dir", str(made[0]), "--work-dir", str(made[1]), + "--dry-run", env=env) + assert result.returncode == 2, result.stdout + result.stderr + assert "Traceback" not in result.stderr, result.stderr + assert result.stderr.startswith(GH_NOT_FOUND.format(commit=commit)), \ + result.stderr + assert result.stderr.endswith(f"\n{GH_NOT_FOUND_REMEDIATION}\n"), \ + result.stderr + assert not [path for path in made if path.exists()] + + +# --- a `--pin-source` `plan` cannot record (#273's review, N2) --------------- +# +# `plan` records the path in each pin's `digest_source:`, and `check` holds +# that to one printable line. A path with a line break, an ESC or a byte +# that is not UTF-8 was planned, exit 0, and printed raw in `plan`'s report, +# and `check` then found `plan-pin-malformed` with advice that wrote the +# same plan again. `plan` now asks `check`'s question first, before any +# digest is read: nothing is made and nothing is read. + +UNRECORDABLE_REMEDIATION = ( + "Remediation: no plan was written. Point --pin-source at a clone whose " + "path is one line of printable UTF-8 text, or drop --pin-source and let " + "`gh api` read the forge. `check` refuses a `digest_source:` that is not " + "one printable line as `plan-pin-malformed`, because `execute` writes it " + "into the pin file as it stands.") + +#: A command line and a file name on Windows hold none of these. +POSIX_NAMES = pytest.mark.skipif( + os.name == "nt", + reason="needs a command-line argument holding a control character or a " + "byte that is not UTF-8, which a Windows file name cannot hold") + +LINE_BREAK = "it holds a line break, and it is written as one line" + + +@pytest.mark.parametrize("name, reason", [ + pytest.param("gl\x1bass", "it holds a control character", id="esc", + marks=POSIX_NAMES), + pytest.param("gl\nass", LINE_BREAK, id="lf", marks=POSIX_NAMES), + pytest.param("gl\rass", LINE_BREAK, id="cr", marks=POSIX_NAMES), + pytest.param("gl\x1b[31mRED\nREFUSED forged-line: not the tool's\r", + LINE_BREAK, id="forged-line", marks=POSIX_NAMES), + pytest.param("gl\udcffass", "it holds a byte that is not UTF-8", + id="not-utf-8", marks=POSIX_NAMES), +]) +def test_a_pin_source_check_would_refuse_is_refused_by_plan( + source_repo, tmp_path, gh_env, name, reason): + """Before any digest is read, so the path need not exist: in `check`'s + terms and #270's words, with the value `repr`'d, so the refusal is one + line of the tool's and nothing of the path's reaches the terminal raw.""" + typed = str(tmp_path / name) + out = tmp_path / "plan.yaml" + result = plan_with(source_repo, out, "openGlass@" + "0" * 40, + extra=("--pin-source", typed), env=gh_env) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr == ( + f"REFUSED adopt-unwritable-value: --pin-source is {typed!r}, which " + "`plan` records in each pin's `digest_source:` and `execute` writes " + f"into `{PIN_FILE}`, and {reason}.\n{UNRECORDABLE_REMEDIATION}\n") + assert not out.exists() + assert gh_calls(gh_env) == "" + + +def test_a_clone_at_a_path_check_would_refuse_is_refused_by_plan( + source_repo, tmp_path, glass, gh_env): + """A clone that DOES answer, at a path `check` refuses: a zero-width + joiner is text a file holds (`unwritable_reason` passes it) and is not + one printable line (`check` does not), so `plan` says so in its own + words. It is the case #270's words do not name.""" + path, commit, _ = glass + clone = tmp_path / "open‍Glass" + shutil.copytree(path, clone, symlinks=True) + out = tmp_path / "plan.yaml" + result = plan_with(source_repo, out, f"openGlass@{commit}", + extra=("--pin-source", str(clone)), env=gh_env) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr.startswith( + f"REFUSED adopt-unwritable-value: --pin-source is {str(clone)!r}, " + f"which `plan` records"), result.stderr + assert (f"into `{PIN_FILE}`, and it holds an invisible formatting " + "character.\n") in result.stderr + assert not out.exists() + + +def test_a_pin_source_printed_in_a_report_is_spelled(source_repo, tmp_path, + glass, gh_env): + """A path that IS one printable line for `check` and still not + `str.isprintable` -- a no-break space -- is planned, and `plan`'s report + and `check`'s `write` line spell it, as every other path either prints + (`_shown`); the plan records it exactly.""" + path, commit, _ = glass + clone = tmp_path / "open Glass" + shutil.copytree(path, clone, symlinks=True) + out = tmp_path / "plan.yaml" + result = plan_with(source_repo, out, f"openGlass@{commit}", + extra=("--pin-source", str(clone)), env=gh_env) + assert result.returncode == 0, result.stdout + result.stderr + assert load_yaml(out)["pins"][0]["digest_source"] \ + == f"local-clone ({clone})" + line = next(line for line in result.stdout.splitlines() + if line.startswith("pin ")) + assert line.endswith("open\\xa0Glass)`)"), line + assert " " not in result.stdout + + plan = planned(source_repo, tmp_path, PLANNED) + with_pins(plan, "pins:\n" + pin_entry( + commit=commit, digest=tree_digest(path, commit), + source="local-clone (open\\u00a0Glass)")) + checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + assert checked.returncode == 0, checked.stdout + checked.stderr + assert f"tree {tree_digest(path, commit)[:12]}… (`local-clone " \ + "(open\\xa0Glass)`)\n" in checked.stdout, checked.stdout + assert " " not in checked.stdout + assert gh_calls(gh_env) == "" + + # --- what `check` finds, and `execute` refuses before any leg exists --------- # # The plan is a file a person or an AI edits between `plan` and `execute`, and @@ -549,6 +731,19 @@ def unrecorded(glass) -> dict: "both-blank": ("pins:\n" + pin_entry(commit="", digest=""), "`pins:` entry 1 ('openGlass') records no `commit:` " "and no `tree_sha256:`."), + # A name no `--pin` takes, holding an ESC and a line break that + # would begin a forged line: the entry is `repr`'d, and neither the + # file it names nor the `plan` it offers is spelled from it (#273's + # review, N3: the two name guards). + "old-plan-forged": ('pins:\n - "open\\x1bGlass\\nREFUSED forged"\n', + "`pins:` entry 1, 'open\\x1bGlass\\nREFUSED " + "forged', is a name with no commit and no " + "digest. `execute` writes each declared pin's " + "file, one, from the commit and the digest " + "`plan` records, and without it the root's own " + "`validate-pins.py` refuses the declaration " + "(`neutral-pin-missing`): re-run `plan` with " + "`--pin @<40 hex commit>`"), } @@ -582,6 +777,22 @@ def malformed(glass) -> dict: "no-source": ("pins:\n" + pin_entry(commit=commit, digest=digest, omit=("digest_source",)), f"{entry} no `digest_source:`."), + # `execute` writes `digest_source:` into the pin file bare, so a + # line break there would be a line of the file (N3). + "source-line-break": ("pins:\n" + pin_entry( + commit=commit, digest=digest, + source="local-clone (/x\\nforged: y)"), + f"{entry} `digest_source: 'local-clone " + "(/x\\nforged: y)'`, which is not one " + "printable line"), + # Unquoted, the reader reads a number: `not text`, and never a + # `TypeError` out of `COMMIT_RE` (N3). + "a-number-commit": ("pins:\n - product: openGlass\n" + " repository: opensoft/openGlass\n" + " commit: 1234567\n" + f' tree_sha256: "{digest}"\n' + ' digest_source: "local-clone (openGlass)"\n', + f"{entry} `commit: 1234567`, which is not text"), } @@ -632,10 +843,12 @@ def cases(glass): CASE_IDS = [("plan-pin-unrecorded", name) for name in ( - "old-plan-name", "old-plan-owner", "no-commit", "no-digest", "both-blank")] + "old-plan-name", "old-plan-owner", "no-commit", "no-digest", "both-blank", + "old-plan-forged")] CASE_IDS += [("plan-pin-malformed", name) for name in ( "not-a-list", "a-number", "short-commit", "short-digest", - "another-product", "no-owner", "no-source")] + "another-product", "no-owner", "no-source", "source-line-break", + "a-number-commit")] CASE_IDS += [("plan-pin-collides", name) for name in ("case-pair", "shape", "variable-pair")] @@ -653,7 +866,9 @@ def test_a_plan_whose_pin_execute_cannot_write_is_found_and_refused( code, name): """`check` finds the pin, and `execute` refuses it with `check`'s own findings before any leg remote exists; the finding names the `plan` that - records the pin whole, and neither command runs `gh`.""" + records the pin whole, and neither command runs `gh`. Whatever the plan + holds, nothing of it reaches the terminal raw: no ESC, and no line the + plan's text began.""" block, said = cases[code][name] plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, block) @@ -664,6 +879,10 @@ def test_a_plan_whose_pin_execute_cannot_write_is_found_and_refused( assert len(lines) == 1, checked.stderr assert lines[0].startswith(f"FINDING {code}: {said}"), lines[0] assert ("re-run `plan`" in lines[0]), lines[0] + assert "\x1b" not in checked.stderr + checked.stdout, checked.stderr + assert not [line for line in checked.stderr.splitlines() + if line.startswith(("REFUSED forged", "forged"))], \ + checked.stderr refusal = refused_before_any_leg(adopter, monkeypatch, plan, tmp_path) assert refusal.code == code assert gh_calls(gh_env) == "" @@ -717,16 +936,73 @@ def test_a_leg_moved_over_a_pins_file_in_the_plan_is_found( def test_a_plan_with_a_whole_pin_checks_ok(source_repo, tmp_path, glass, gh_env): """The control for every case above: the same plan with the pin as - `plan` writes it checks ok, and says the file it will write.""" + `plan` writes it checks ok, and says the file it will write, and that + it writes the pin as `plan` read it (#273's review, S3: `check` and + `execute` read no network for a pin, by design, and the root's own gate + is what recomputes it).""" plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, "pins:\n" + a_good_pin(glass)) checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr assert f" write {PIN_FILE}: opensoft/openGlass @ {glass[1][:12]}" \ in checked.stdout + assert ("\n each pin as `plan` read it, not read again here: the root's " + "own `validate-pins.py` recomputes its digest wherever it can " + "read the product\n") in checked.stdout, checked.stdout assert gh_calls(gh_env) == "" +def test_a_plan_with_no_pin_says_nothing_of_pins(source_repo, tmp_path, + gh_env): + """The S3 sentence is said of pins, and only where there is one.""" + plan = planned(source_repo, tmp_path, PLANNED) + checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + assert checked.returncode == 0, checked.stdout + checked.stderr + assert " write contracts/" not in checked.stdout + assert "each pin as `plan` read it" not in checked.stdout + + +def test_an_upper_cased_hand_edit_is_written_lower_cased(adopter, source_repo, + tmp_path, glass, + gh_env): + """`plan` records a commit and a digest lower-cased, as `parse_pin` + reads the commit and `tree_digest` spells the digest. One edited into + the plan upper-cased is the same pin and passes `check`; `execute` + writes it as `plan` would have, lower-cased, so the pin file the + scaffold would write for it is the one written (#273's review, N3).""" + path, commit, _ = glass + digest = tree_digest(path, commit) + plan = planned(source_repo, tmp_path, PLANNED) + with_pins(plan, "pins:\n" + pin_entry(commit=commit.upper(), + digest=digest.upper())) + checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + assert checked.returncode == 0, checked.stdout + checked.stderr + assert (f" write {PIN_FILE}: opensoft/openGlass @ {commit[:12]} tree " + f"{digest[:12]}… (local-clone (openGlass))") in checked.stdout + recorded = adopter._recorded_pins(adopter.Plan.load(plan)) + assert recorded["openGlass"]["PIN_COMMIT"] == commit + assert recorded["openGlass"]["PIN_TREE_SHA256"] == digest + + +def test_a_submodule_kept_where_the_pin_is_written_is_said_once( + adopter, monkeypatch, tmp_path, glass): + """A submodule the root keeps is `plan-submodule-root` (#166), wherever + it is, and the pin finding about a path kept where the pin is written + does not say it a second time, as the shape's own files do not (#273's + review, N3: the gitlink filter).""" + source = make_source_repo(tmp_path / "Thing") + head = git("rev-parse", "HEAD", cwd=source).stdout.strip() + git("update-index", "--add", "--cacheinfo", f"160000,{head},{PIN_FILE}", + cwd=source) + as_human("commit", "-qm", "a submodule where the pin is written", + cwd=source) + plan = planned(source, tmp_path, {**PLANNED, "contracts/": "root"}) + with_pins(plan, "pins:\n" + a_good_pin(glass)) + found = findings(plan) + assert [code for code, _ in found] == ["plan-submodule-root"], found + refused_before_any_leg(adopter, monkeypatch, plan, tmp_path) + + @needs_filter_repo def test_execute_refuses_an_old_plans_name_before_any_remote(source_repo, tmp_path, @@ -765,3 +1041,28 @@ def test_both_tools_read_a_pin_with_one_implementation(adopter): assert adopter.pin_collisions is shape_materialize.pin_collisions assert not hasattr(adopter, "_checked_pin_name") + +@pytest.mark.parametrize("first, second", [ + pytest.param("openß", "openss", id="sharp-s"), + pytest.param("openſ", "opens", id="long-s"), +]) +def test_two_pins_are_one_product_by_their_pin_file_not_a_casefold(first, + second): + """#275's fix round decides "the same product" by the pin file's name, + `repo_shape.pin_product_key` (`lower()`), as `validate-pins.py` does; + both tools ask it here. `casefold()` folds `ß` to `ss` and `ſ` to `s`, + which `lower()` does not, so the two names below are two pin files and + one `SHAPE_PIN_SOURCE_`: one collision, each pin named once. + Keyed by a casefold, each was named twice. No `--pin` or recorded + `pins:` entry `PIN_ARG_RE` admits can spell either, so the function is + asked directly.""" + variable = pin_source_env_name(first) + assert variable == pin_source_env_name(second) + assert shape_materialize.neutral_pin_path(first) \ + != shape_materialize.neutral_pin_path(second) + typed = [("`pins:` entry 1", first), ("`pins:` entry 2", second)] + assert shape_materialize.pin_variable_collisions(typed) == [ + f"`pins:` entry 1 and `pins:` entry 2 name the products `{first}` " + f"and `{second}`, and one variable, `{variable}`, names the local " + "checkout of each: it cannot say which it holds."] + diff --git a/tests/test_pin_source_unreadable.py b/tests/test_pin_source_unreadable.py index b568ac72..9e12e8ef 100644 --- a/tests/test_pin_source_unreadable.py +++ b/tests/test_pin_source_unreadable.py @@ -294,13 +294,119 @@ def test_the_source_is_printed_escaped_and_never_raised_on( name: the refusal is never a `UnicodeEncodeError` in its place. The second row is a name that is not UTF-8 (POSIX only: a Windows command line has no such byte), read with `surrogateescape`, so it prints as - the surrogate it was read as.""" + the surrogate it was read as. `adopt-project.py plan` records the path + in the plan, which cannot hold that byte, so it refuses the name as a + value it cannot record before it reads any digest (#273's review, N2), + and spells it the same way.""" typed = str(tmp_path / name) result = run(kind, tmp_path, adoptable, typed, env=env) assert result.returncode == 2, result.stdout + result.stderr assert "Traceback" not in result.stderr, result.stderr assert spelled in result.stderr, result.stderr - assert "REFUSED pin-source-unreadable: " in result.stderr + code = ("adopt-unwritable-value" + if kind == "adopt-plan" and "\udcff" in name + else "pin-source-unreadable") + assert f"REFUSED {code}: " in result.stderr + assert made(kind, tmp_path) == [] + + +# --- a source that is not printable is named spelled (#339's review, S1) ----- +# +# The refusal named the path raw, so an ESC in it reached the terminal, and a +# line break or a carriage return began a line of the path's choosing, a +# forged `REFUSED` among them; the traceback it replaced had shown it as a +# `repr`. The path is now spelled as Python's `repr` spells it wherever the +# refusal names it -- git's own reason names it too -- when a character in it +# is not printable, and named as typed otherwise, byte for byte as above. + +#: Names that print as something other than themselves: an ESC, a carriage +#: return, a line break, and all three beginning a forged line. +CONTROL_NAMES = [ + pytest.param("miss\x1b[31mred", id="esc"), + pytest.param("miss\ring", id="cr"), + pytest.param("miss\ning", id="lf"), + pytest.param("miss\x1b[31mRED\nREFUSED forged-line: this line was not " + "printed by the tool\r", id="forged-line"), +] + +#: A Windows file name holds no control character, so neither does a path +#: one could mean there. +CONTROL_ARGV = pytest.mark.skipif( + os.name == "nt", + reason="needs a path holding a control character, which a Windows file " + "name cannot hold") + + +def assert_one_refusal_of_the_tools(result: subprocess.CompletedProcess, + typed: str) -> None: + """Exit 2 and one refusal, with the path `repr`'d in it: no raw ESC, and + no line but the tool's own. (`run_script` reads in text mode, so a raw + carriage return would arrive as a line break.)""" + assert result.returncode == 2, result.stdout + result.stderr + assert "Traceback" not in result.stderr, result.stderr + assert "\x1b" not in result.stderr, result.stderr + lines = result.stderr.splitlines() + assert [line for line in lines if line.startswith("REFUSED")] \ + == lines[:1], result.stderr + assert repr(typed) in result.stderr, result.stderr + + +@CONTROL_ARGV +@pytest.mark.parametrize("kind", RUNS) +@pytest.mark.parametrize("name", CONTROL_NAMES) +def test_a_source_holding_a_control_character_is_named_spelled( + kind, name, tmp_path, adoptable): + """Through every way in. The scaffold refuses it as + `pin-source-unreadable`; `adopt-project.py plan` refuses it before any + digest is read, as a value it cannot record (#273's review, N2), and + names it the same way.""" + typed = str(tmp_path / name) + result = run(kind, tmp_path, adoptable, typed) + assert_one_refusal_of_the_tools(result, typed) + if kind == "adopt-plan": + assert result.stderr.startswith( + f"REFUSED adopt-unwritable-value: --pin-source is {typed!r}, "), \ + result.stderr + else: + assert result.stderr == ( + f"REFUSED pin-source-unreadable: {typed!r} could not answer for " + f"opensoft/openGlass @ {COMMIT}: {NO_SUCH_DIRECTORY}\n" + f"{REMEDIATION}\n") + assert made(kind, tmp_path) == [] + + +@CONTROL_ARGV +@pytest.mark.parametrize("kind", ["scaffold-dry-run", "scaffold-run"]) +def test_git_s_own_reason_names_the_source_spelled_too(kind, tmp_path, + adoptable): + """A directory that is no clone is git's own failure, and git's reason + names the directory it ran in: spelled there as in the refusal's head.""" + typed = str(tmp_path / ("plain\x1b[31mRED\nREFUSED forged-line: this " + "line was not printed by the tool\r")) + Path(typed).mkdir() + result = run(kind, tmp_path, adoptable, typed) + assert_one_refusal_of_the_tools(result, typed) + assert result.stderr.startswith( + f"REFUSED pin-source-unreadable: {typed!r} could not answer for " + f"opensoft/openGlass @ {COMMIT}: `git ls-tree -r -z {COMMIT}` in " + f"{typed!r} exited "), result.stderr + assert made(kind, tmp_path) == [] + + +@pytest.mark.parametrize("kind", RUNS) +def test_a_source_that_is_one_line_and_not_printable_is_named_spelled( + kind, tmp_path, adoptable): + """A no-break space is one printable line to `check` (`plan` records + such a path) and is not `str.isprintable`, so every tool names it + spelled: the escape is the printable question, asked of the path alone, + on every platform.""" + typed = str(tmp_path / "miss ing") + result = run(kind, tmp_path, adoptable, typed) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr == ( + f"REFUSED pin-source-unreadable: {typed!r} could not answer for " + f"opensoft/openGlass @ {COMMIT}: {NO_SUCH_DIRECTORY}\n" + f"{REMEDIATION}\n") assert made(kind, tmp_path) == [] From f63d1edd035a17629230974ca114f4b1bd3cdec7 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 12 Oct 2026 20:56:38 +0000 Subject: [PATCH 8/9] tests/test_adopt_pins.py: `plan` with a `--pin-source` and no `--pin` asks nothing of the path, and a printable `--pin-source` is printed as it is in `plan`'s `pin` line and `check`'s `write` line, so dropping the `args.pin and` guard, or spelling every digest source, no longer passes every test (#273, #339) Lane 5's second review of this branch (at 421e4e5) found two hunks of fix round 1 that no test failed without, and asked for a test for each. No code changes. - n1. `plan` asks N2's question (`one_printable_line`) of `--pin-source` only when a `--pin` records it (`adopt-project.py`, `if args.pin and args.pin_source is not None`). With the `args.pin and` dropped, an unused `--pin-source` holding an ESC or a zero-width joiner was refused in a sentence naming no file ("`execute` writes into , and ..."), and all 118 tests still passed. `test_a_pin_source_no_pin_records_is_not_asked_about` plans with `--pin-source` and no `--pin`: exit 0, `pins: []`, the path named nowhere in the plan or the report, in any spelling. The ESC row is POSIX-only, by the file's `POSIX_NAMES` mark; the zero-width joiner row runs everywhere. Whether an unused `--pin-source` should be refused or noted instead is #339's third decision, which this branch leaves open; the test holds today's answer, silence, so a change to it is deliberate. - n2. `plan`'s `pin` line prints `digest_source` through `_shown`. With `_spelled` in its place, which backticks and escapes every path, all 118 tests still passed, because only a no-break-space path was asserted. `test_a_printable_pin_source_is_printed_as_it_is` plans from a printable clone path and holds the `pin` line, and `check`'s `write` line for the plan `plan` wrote, to their exact text, ending `(local-clone ())` with no backtick. Mutation proof, in a throwaway worktree at 421e4e5 with these tests, over `tests/test_adopt_pins.py` and `tests/test_pin_source_unreadable.py` less the five end-to-end adoptions: - `args.pin and` dropped: 2 failed (both new n1 rows), 118 passed; - `plan`'s `pin` line `_spelled`: 1 failed (the new n2 test), 119 passed; - `check`'s `write` line `_spelled`: 2 failed (the new n2 test and `test_an_upper_cased_hand_edit_is_written_lower_cased`); - `plan`'s `pin` line raw (fix round 1's R1): 1 failed, the existing no-break-space test, which is that hunk's killer; the new n2 test passes there, as it must. The review's third nit, a plan with two unwritable values refused in two runs (`--elected-by` first, then `--pin-source`), needs a code change and is not made here. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_pins.py | 68 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index 6f7bae60..c176bf12 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -670,6 +670,44 @@ def test_a_clone_at_a_path_check_would_refuse_is_refused_by_plan( assert not out.exists() +#: The leaf of a `--pin-source` no `--pin` records, spelled so that the plan +#: and the report can be searched for it: neither holds the word otherwise. +UNREAD = "unreadclone" + + +@pytest.mark.parametrize("name", [ + pytest.param(f"{UNREAD}\x1bx", id="esc", marks=POSIX_NAMES), + pytest.param(f"{UNREAD}‍x", id="zwj"), +]) +def test_a_pin_source_no_pin_records_is_not_asked_about(source_repo, + tmp_path, gh_env, + name): + """`plan` asks the question above only of a path a `--pin` will record + (#273's review, n1). With no `--pin`, `--pin-source` names nothing `plan` + records, reads or prints: the plan is written with `pins: []`, and + neither it nor the report names the path, in any spelling. Asked + anyway, `plan` refused a plan that writes no pin file, in a sentence + naming none ("`execute` writes into , and ..."). Whether an unused + `--pin-source` is refused or noted instead is #339's third decision, + left open by this change; this test holds its answer today, silence, so + that whoever decides it changes this test on purpose. The path need not + exist, because nothing reads it.""" + typed = str(tmp_path / name) + out = tmp_path / "plan.yaml" + result = plan_with(source_repo, out, extra=("--pin-source", typed), + env=gh_env) + assert result.returncode == 0, result.stdout + result.stderr + text = out.read_bytes().decode(UTF8) + assert "\npins: []\n" in text, text + assert load_yaml(out)["pins"] == [] + said = result.stdout + result.stderr + assert UNREAD not in text + said, said + assert "\x1b" not in said and "‍" not in said, said + assert not [line for line in result.stdout.splitlines() + if line.startswith("pin ")], result.stdout + assert gh_calls(gh_env) == "" + + def test_a_pin_source_printed_in_a_report_is_spelled(source_repo, tmp_path, glass, gh_env): """A path that IS one printable line for `check` and still not @@ -702,6 +740,36 @@ def test_a_pin_source_printed_in_a_report_is_spelled(source_repo, tmp_path, assert gh_calls(gh_env) == "" +def test_a_printable_pin_source_is_printed_as_it_is(source_repo, tmp_path, + glass, gh_env): + """The other half of `_shown` (#273's review, n2): a printable + `--pin-source` is printed byte for byte, with no backtick and no escape, + in `plan`'s `pin` line and in `check`'s `write` line for the plan `plan` + wrote, as the report prints every other printable path, a Windows + drive's backslashes among them. Spelled whatever it holds, the line + read `(`local-clone ()`)`.""" + path, commit, _ = glass + digest = tree_digest(path, commit) + out = tmp_path / "plan.yaml" + result = plan_with(source_repo, out, f"openGlass@{commit}", + extra=("--pin-source", str(path)), env=gh_env) + assert result.returncode == 0, result.stdout + result.stderr + pin = (f"opensoft/openGlass @ {commit[:12]} tree {digest[:12]}… -> " + f"{PIN_FILE} (local-clone ({path}))") + assert [line for line in result.stdout.splitlines() + if line.startswith("pin ")] == [f"pin {pin}"], \ + result.stdout + for answered, leg in ANSWERS: + resolve(out, answered, leg) + checked = run_script(ADOPT, "check", "--plan", str(out), env=gh_env) + assert checked.returncode == 0, checked.stdout + checked.stderr + assert [line for line in checked.stdout.splitlines() + if line.startswith(" write ")] == [ + f" write {PIN_FILE}: opensoft/openGlass @ {commit[:12]} tree " + f"{digest[:12]}… (local-clone ({path}))"], checked.stdout + assert gh_calls(gh_env) == "" + + # --- what `check` finds, and `execute` refuses before any leg exists --------- # # The plan is a file a person or an AI edits between `plan` and `execute`, and From 73d65c63d8c324227ef017ab30e141e72bc50448 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Tue, 13 Oct 2026 01:57:36 +0000 Subject: [PATCH 9/9] adopt-project.py: `_read_pins` reads `pins:` with `plan.get`, so `_commit_the_split` takes the `SplitPlan` mapping #445's tests hand it as the plan and no longer stops at "committing the split" with an AttributeError, and a loaded plan reads exactly as before (#273) #445 (75052cb) tests `_commit_the_split` directly. In tests/test_adopt_late_failure.py, `split_arguments` passes `plan=SplitPlan()`, a `dict` subclass that adds `entries` (#394). Those are all the function read before this branch: `plan.get("follow_ups", [])` and `plan.entries`. Since #273 it also writes each declared pin, `neutral_pins=_recorded_pins(plan)`, and `_read_pins` read `plan.data.get("pins")`. On the merge commit before this one, three of #445's tests failed (3 failed, 83 passed in that file), each with "STOPPED committing the split by AttributeError: 'SplitPlan' object has no attribute 'data'": - test_a_split_that_fails_names_its_step_and_both_pushed_legs[commit] - test_a_split_that_fails_names_its_step_and_both_pushed_legs[push] - test_a_split_the_materializer_cannot_write_is_refused_with_the_notice `Plan.get(key, default=None)` returns `default` when the value is None, and the value otherwise. With no default it returns `plan.data.get("pins")` exactly, so a loaded plan's `pins:` reads as before, absent and `null` alike, and the `rows is None` branch below is unchanged. A mapping has the same `get`, so the test's plan reads too. This was the only `plan.data` in the file. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adopt-project.py b/adopt-project.py index 80e0107b..4734fc1c 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -330,7 +330,7 @@ def _read_pins(plan: "Plan") -> tuple[list, list[str]]: plan's, or an edit's) or a mapping missing its commit or digest is `plan-pin-unrecorded`, and anything else `plan` never writes there is `plan-pin-malformed`.""" - rows = plan.data.get("pins") + rows = plan.get("pins") if rows is None: return [], [] if not isinstance(rows, list):