From c28ebe77bc6747a33b117d098640f17869f1658d Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 22:45:14 +0000 Subject: [PATCH 01/10] scaffold-project.py and adopt-project.py: a pin read from `--pin-source` records `digest_source: local-clone` and not the path, so no committed pin file holds the directory a clone sat in on the machine that ran the tool (#540, #450); a `--pin-source` that nothing reads is refused as `pin-source-unused` before anything exists (#451) #540: `declared_pin_values` (scripts/shape_materialize.py), which both tools call, wrote `digest_source: local-clone ()` into the committed pin file. An absolute `--pin-source` therefore committed this machine's own directory, which tells no other reader anything. The field exists to say which of the two ways the digest was read, so it now says `local-clone` or `gh-api-tree-recursive` and nothing more. The refusals for a source that cannot answer still name the path the run was given. #450 rides along. The path was written bare. One holding a line break wrote a line of its own into the pin file, exit 0. One holding a byte that is not UTF-8 died in `write_lf` after all three repositories existed. With no path recorded, a clone at either path scaffolds and adopts like any other. The same reason removes adopt's `_refuse_an_unrecordable_pin_source` (#273's review, N2). It refused such a path as `adopt-unwritable-value` because `plan` recorded it. `plan` records no path now, so it reads the path and, where the path cannot answer, refuses it as `pin-source-unreadable`, as the scaffold does. #451: with no `--pin`, and in the scaffold no `--referent-chain` (whose links read the bare `--pin-source` through `link_pins_from_trees`), nothing read `--pin-source`. A path that did not exist was accepted, and `--dry-run`, the real scaffold and `plan` each exited 0 with no word about the flag. Someone who typed it meaning to pin, and forgot the `--pin`, got a project that declares no descent. `refuse_an_unread_pin_source`, beside `declared_pin_values`, refuses it in one sentence for both tools: exit 2, before any process and before anything exists, the scaffold's `--dry-run` alike, with two remedies (add the `--pin`, or drop the flag). It refuses rather than notes, as `validate-pins.py` does (#295), because these two tools make the project. `--help` for both tools' `--pin-source` says this, and `docs/cli.md` is regenerated. The template's comment on `digest_source:` says no path is recorded. Its header now names `adopt-project.py execute` as a renderer too (L2-16 on #459). Main: bfed143 (origin/main when this branch was taken). Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 52 +---- docs/cli.md | 10 +- scaffold-project.py | 17 +- scripts/shape_materialize.py | 64 +++++- .../contracts/neutral-product-pin.yaml | 7 +- tests/test_adopt_pins.py | 200 +++++++----------- tests/test_pin_source_unreadable.py | 170 ++++++++++++--- 7 files changed, 322 insertions(+), 198 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index e4fa0451..c3c7a72e 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -246,7 +246,7 @@ from shape_materialize import ( # noqa: E402 PIN_ARG_RE, declared_pin_products, declared_pin_values, neutral_pin_path, one_printable_line, pin_collisions, pin_variable_collisions, - refuse_colliding_pins, + refuse_an_unread_pin_source, refuse_colliding_pins, ) from repo_shape import SHA256_RE, repo_basename # noqa: E402 @@ -2857,6 +2857,11 @@ def _checked_plan_inputs(args, source: Source, # 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) + # A `--pin-source` no `--pin` will read is refused, in the scaffold's + # words (#451): it names the clone a pin's digest is read from, and with + # no `--pin` it changed nothing and was never even asked about. + refuse_an_unread_pin_source(args.pin_source, args.pin, + nothing_made="no plan was written.") 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 @@ -2918,8 +2923,6 @@ 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), source) @@ -2952,43 +2955,6 @@ def _refuse_misread_values(args, given: set, pins: dict, source: Source, f"{MANIFEST_MISREAD_REMEDIATION} No plan was written.") -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.") - - # --------------------------------------------------------------------------- # The project must be able to hold what `execute` writes into it (#261) # --------------------------------------------------------------------------- @@ -9640,7 +9606,11 @@ def main(argv: list[str] | None = None) -> int: 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)") + "TEST path; no network). The plan and the pin " + "file say only that the digest was read from " + "`local-clone`, never the path. Given with no " + "--pin to read it, the flag is refused as " + "`pin-source-unused`.") 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 c793a3e0..4fc19f81 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -339,7 +339,10 @@ options: --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) + network). The pin file says only that the digest was + read from `local-clone`, never the path. Given with no + --pin and no --referent-chain to read it, it is + refused as `pin-source-unused`. --work-dir WORK_DIR where the working trees are built (default: a temporary directory) ``` @@ -466,7 +469,10 @@ options: --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) + network). The plan and the pin file say only that the + digest was read from `local-clone`, never the path. + Given with no --pin to read it, the flag is refused as + `pin-source-unused`. --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 98bb95c5..21c5f4c2 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -126,6 +126,7 @@ #: by, so what it refuses, and in which words, is unchanged. from shape_materialize import ( # noqa: E402 declared_pin_products, declared_pin_values, + refuse_an_unread_pin_source, refuse_colliding_pins as _refuse_colliding_pins, ) @@ -304,7 +305,11 @@ def main(argv: list[str] | None = None) -> int: parser.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)") + "TEST path; no network). The pin file says " + "only that the digest was read from " + "`local-clone`, never the path. Given with no " + "--pin and no --referent-chain to read it, it " + "is refused as `pin-source-unused`.") parser.add_argument("--work-dir", type=Path, default=None, help="where the working trees are built " "(default: a temporary directory)") @@ -648,6 +653,10 @@ def _identity(args, project: str) -> tuple[str, str, str, str, str]: "the assembly root's `project.yaml` and `README.md`, each repository's " "`AGENTS.md` and each first commit's message") +#: How a `pin-source-unused` refusal's remediation begins (#451). +UNREAD_PIN_SOURCE_MADE = ("nothing was created, and `--dry-run` refuses the " + "same.") + #: What every `scaffold-unwritable-value` is told to do (#282). UNWRITABLE_REMEDIATION = ( "Remediation: nothing was created, and `--dry-run` refuses the same. Pass " @@ -1362,6 +1371,12 @@ def _scaffold(args) -> int: args.code_path = checked_value(*code_leg) args.org = checked_value("--org", args.org) args.pin_owner = checked_value("--pin-owner", args.pin_owner) + # A `--pin-source` NOTHING WILL READ (#451): no `--pin` reads a digest + # there and no `--referent-chain` link a declaration. Asked of the flags + # as typed, with no process started, so a dry run refuses it too. + refuse_an_unread_pin_source(args.pin_source, args.pin, + referent_chain_links(args.referent_chain), + nothing_made=UNREAD_PIN_SOURCE_MADE) # EACH `--pin`'S FILE AGAINST THE SHAPE'S OWN AND AGAINST EACH OTHER, from # the arguments as typed (#241), and first: the leg-path question below # asks about every pin's file, and would miscount one written over the diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index fe2c40e6..f19e05a0 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -2606,6 +2606,12 @@ def pin_digest_from_gh(repository: str, commit: str) -> str: return digest.hexdigest() +#: How a pin file says its digest was read (`digest_source:`): from a clone +#: on this machine, or from the forge's own recursive tree listing. +DIGEST_FROM_CLONE = "local-clone" +DIGEST_FROM_GH = "gh-api-tree-recursive" + + def declared_pin_values(args) -> dict[str, dict[str, str]]: """The template values for every `--pin`, with its digest resolved. @@ -2617,6 +2623,17 @@ def declared_pin_values(args) -> dict[str, dict[str, str]]: 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). + + A CLONE IS RECORDED AS `local-clone`, AND ITS PATH IS NOT (#540). The + path went into the committed pin file's `digest_source:` as it was + typed, `local-clone ()`, so an absolute `--pin-source` committed + the directory a clone sat in on the machine that ran the tool, which is + of no use to any other reader of the file. The path was also written + bare (#450): one holding a line break wrote a line of its own choosing + into the pin file, and the scaffold died in `write_lf` on one holding a + byte that is not UTF-8, after all three repositories existed. What the + field is for is which of the two ways the digest was read, so that is + what it says; the refusals above still name the path the run was given. """ pins: dict[str, dict[str, str]] = {} for raw in args.pin: @@ -2624,10 +2641,10 @@ def declared_pin_values(args) -> dict[str, dict[str, str]]: if args.pin_source is not None: digest = pin_digest_from_source( _resolved_pin_source(args.pin_source), commit, repository) - digest_source = f"local-clone ({args.pin_source})" + digest_source = DIGEST_FROM_CLONE else: digest = pin_digest_from_gh(repository, commit) - digest_source = "gh-api-tree-recursive" + digest_source = DIGEST_FROM_GH pins[product] = { "PIN_PRODUCT": product, "PIN_REPOSITORY": repository, @@ -2654,6 +2671,49 @@ def declared_pin_products(args) -> dict[str, None]: return dict.fromkeys(parse_pin(raw, args.pin_owner)[0] for raw in args.pin) +def refuse_an_unread_pin_source(pin_source, pins, chain=None, *, + nothing_made: str) -> None: + """`--pin-source` is read by this run, or this refuses it as + `pin-source-unused` (#451): before anything exists and before any digest + is read, so the scaffold's `--dry-run` refuses it too and `plan` writes + no plan. + + ITS READERS ARE `declared_pin_values`, which reads each `--pin`'s digest + there, and, in the scaffold, `link_pins_from_trees`, which reads a + `--referent-chain` link's declaration there when nothing keyed names that + link's tree. With neither, nothing read it, so nothing asked about it + either: a path that does not exist was accepted, `--dry-run` said + `nothing was created.` and `plan` wrote its plan, each exit 0 with no + word that the flag had changed nothing. Typed meaning to pin, with the + `--pin` forgotten, it made a project that declares no descent. The + root's own `validate-pins.py` NOTES an unused `--pin-source` of its own + (#295), a check that is run again and again; the two tools that make a + project refuse one, before it is made, as they refuse each other value + they cannot use. + + `pins` is the `--pin` list and `chain` the scaffold's `--referent-chain` + links, or `None` for a tool that takes no chain (`adopt-project.py + plan`), so the sentence names only flags the tool has. `nothing_made` + begins the remediation in the tool's own words. The path is never read + here, and is named as `shown_source` names it. + """ + if pin_source is None or pins or chain: + return + if chain is None: + given, reads = "no --pin", "each --pin's digest is read from" + else: + given = "no --pin and no --referent-chain" + reads = ("each --pin's digest is read from, and each " + "--referent-chain link's declaration") + raise Refusal( + "pin-source-unused", + f"--pin-source {shown_source(pin_source)} was given with {given}, so " + f"nothing reads it: it names the clone {reads}.", + f"Remediation: {nothing_made} Add the --pin " + "@<40 hex commit> that clone was given for, or drop " + "--pin-source.") + + #: 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. It #: names `spec`, `code` and `shape` outright (#288): for a repository truly diff --git a/templates/assembly-root/contracts/neutral-product-pin.yaml b/templates/assembly-root/contracts/neutral-product-pin.yaml index 4233973b..57cb042b 100644 --- a/templates/assembly-root/contracts/neutral-product-pin.yaml +++ b/templates/assembly-root/contracts/neutral-product-pin.yaml @@ -7,7 +7,8 @@ kind: pinned_contract_manifest # This file is not copied into a project under this name. It is RENDERED once # per declared pin as `contracts/-pin.yaml` — so a project # declaring `openGlass` carries `contracts/openglass-pin.yaml` — by -# `scaffold-project.py --pin @`. +# `scaffold-project.py --pin @`, and by +# `adopt-project.py execute` for each pin its `plan --pin` recorded. # # WHAT IT IS FOR. `` is a CLAIM of descent and a claim needs # a REFERENT (Brett Heap, 2026-09-02). The referent is a pin the project @@ -40,7 +41,9 @@ digests: tree_sha256: "{{PIN_TREE_SHA256}}" # How the digest above was read, so a later reader knows whether it came from -# a clone on this machine or from the forge's own tree listing. +# a clone on the machine that wrote this file (`local-clone`) or from the +# forge's own tree listing (`gh-api-tree-recursive`). The clone's path is that +# machine's own, so it is not recorded. digest_source: {{PIN_DIGEST_SOURCE}} declared_in: project.yaml diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index 77897adc..74d6d999 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -253,7 +253,7 @@ def test_plan_records_the_pin_whole(adopted, glass): "repository": adopted["repository"], "commit": adopted["commit"], "tree_sha256": tree_digest(glass[0], adopted["commit"]), - "digest_source": f"local-clone ({adopted['readable']})", + "digest_source": "local-clone", }] assert (f"pin {adopted['repository']} @ {adopted['commit'][:12]}" in adopted["planned"].stdout) @@ -598,11 +598,13 @@ 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.""" + where that path is no file the shape writes, and no `--pin-source`, + which with no pin to read it is refused (#451).""" path, commit, _ = glass - legs = ("--spec-path", PIN_FILE, "--pin-source", str(path)) + legs = ("--spec-path", PIN_FILE) refused = plan_with(source_repo, tmp_path / "refused.yaml", - f"openGlass@{commit}", extra=legs) + f"openGlass@{commit}", + extra=(*legs, "--pin-source", str(path))) assert refused.returncode == 2, refused.stdout + refused.stderr assert refused.stderr.startswith( f"REFUSED adopt-leg-path-collides: --spec-path '{PIN_FILE}' is " @@ -763,21 +765,17 @@ def test_with_no_gh_on_path_a_pin_is_gh_not_found(kind, source_repo, assert not [path for path in made if path.exists()] -# --- a `--pin-source` `plan` cannot record (#273's review, N2) --------------- +# --- a `--pin-source` is read, and not recorded (#540, #450) ----------------- # -# `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.") +# `plan` recorded the path in each pin's `digest_source:` as it was typed, +# `local-clone ()`, and `execute` committed it into the pin file, so an +# absolute `--pin-source` put the directory a clone sat in on this machine +# into the adopted root. Since #273's review (N2), `plan` refused, before any +# digest was read, a path `check` could not hold on that line: a line break, +# an ESC, a byte that is not UTF-8, a zero-width joiner. The plan now records +# `local-clone` alone, so a clone is read wherever it sits, and a path that +# cannot answer is `pin-source-unreadable`, named spelled, as the scaffold +# names it (`tests/test_pin_source_unreadable.py`). #: A command line and a file name on Windows hold none of these. POSIX_NAMES = pytest.mark.skipif( @@ -785,116 +783,85 @@ def test_with_no_gh_on_path_a_pin_is_gh_not_found(kind, source_repo, 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" +#: The leaf of each `--pin-source` below, spelled so that a plan, a report +#: and a refusal can be searched for it: none holds the word otherwise. +UNREAD = "unreadclone" -@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), +@pytest.mark.parametrize("name", [ + pytest.param(f"{UNREAD}\x1bx", id="esc", marks=POSIX_NAMES), + pytest.param(f"{UNREAD}\nx", id="lf", marks=POSIX_NAMES), + pytest.param(f"{UNREAD}\rx", id="cr", marks=POSIX_NAMES), + pytest.param(f"{UNREAD}\x1b[31mRED\nREFUSED forged-line: not the " + "tool's\r", id="forged-line", marks=POSIX_NAMES), + pytest.param(f"{UNREAD}\udcffx", id="not-utf-8", marks=POSIX_NAMES), + pytest.param(f"{UNREAD}‍x", id="zwj"), + pytest.param(f"{UNREAD} x", id="no-break-space"), ]) -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.""" +def test_a_clone_is_read_wherever_it_is_and_its_path_is_not_recorded( + source_repo, tmp_path, glass, gh_env, name): + """A clone that answers, at each path #273's review refused: planned, + exit 0, with the digest it gives, and `digest_source: local-clone`. No + spelling of the path is in the plan, which `execute` writes the pin + file from, or in the report, whose `pin` line says how the digest was + read and nothing more; so nothing of it reaches the terminal raw.""" path, commit, _ = glass - clone = tmp_path / "open‍Glass" + clone = tmp_path / name 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() - - -#: 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" + assert result.returncode == 0, result.stdout + result.stderr + digest = tree_digest(path, commit) + pin = load_yaml(out)["pins"][0] + assert (pin["tree_sha256"], pin["digest_source"]) \ + == (digest, "local-clone") + said = result.stdout + result.stderr + assert UNREAD not in out.read_bytes().decode(UTF8) + said, said + assert [line for line in result.stdout.splitlines() + if line.startswith("pin ")] == [ + f"pin opensoft/openGlass @ {commit[:12]} tree {digest[:12]}… " + f"-> {PIN_FILE} (local-clone)"], result.stdout + assert gh_calls(gh_env) == "" @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.""" +def test_a_pin_source_no_pin_reads_is_refused(source_repo, tmp_path, gh_env, + name): + """With no `--pin`, nothing reads `--pin-source` (#451). It was planned + in silence, exit 0, `pins: []`, the path never asked about -- which this + test held on purpose, as #339's third decision left open, so that + whoever decided it changed this test. Decided: `pin-source-unused`, + exit 2, before a plan exists and before anything is read, in the + scaffold's words; the path need not exist, and is named spelled, as + every path a refusal here names is.""" 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 result.returncode == 2, result.stdout + result.stderr + assert result.stderr == ( + f"REFUSED pin-source-unused: --pin-source {typed!r} was given with " + "no --pin, so nothing reads it: it names the clone each --pin's " + "digest is read from.\nRemediation: no plan was written. Add the " + "--pin @<40 hex commit> that clone was given for, or " + "drop --pin-source.\n") + assert "\x1b" not in result.stderr and "‍" not in result.stderr + assert not out.exists() 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 - `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.""" +def test_check_spells_a_recorded_source_that_is_not_printable( + source_repo, tmp_path, glass, gh_env): + """A plan `plan` wrote before #540, or one edited since, can still hold + a path in `digest_source:`. One that is one printable line for `check` + and still not `str.isprintable` -- a no-break space -- passes, and + `check`'s `write` line spells it, as every other path either report + prints (`_shown`).""" 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), @@ -907,25 +874,17 @@ 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 ()`)`.""" +def test_neither_report_names_the_clone(source_repo, tmp_path, glass, gh_env): + """`plan`'s `pin` line and `check`'s `write` line for the plan `plan` + wrote say how the digest was read, `local-clone`, and not where: the + path is this machine's own, and `execute` writes into the pin file + what `check` prints there (#540).""" 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) @@ -933,7 +892,8 @@ def test_a_printable_pin_source_is_printed_as_it_is(source_repo, tmp_path, 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 + f"{digest[:12]}… (local-clone)"], checked.stdout + assert str(path) not in result.stdout + checked.stdout assert gh_calls(gh_env) == "" diff --git a/tests/test_pin_source_unreadable.py b/tests/test_pin_source_unreadable.py index 9e12e8ef..a550842c 100644 --- a/tests/test_pin_source_unreadable.py +++ b/tests/test_pin_source_unreadable.py @@ -34,6 +34,7 @@ import errno import os +import shutil import subprocess import sys from pathlib import Path @@ -43,7 +44,7 @@ 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 +from repo_shape import load_yaml, parse_yaml, tree_digest # noqa: E402 PROJECT = "MedxGlass" #: Any 40 hex: no source below is asked for it, but the clone that is not one. @@ -82,22 +83,24 @@ def run(kind: str, base: Path, adoptable: Path, typed: str, - commit: str = COMMIT, cwd: Path | None = None, - env: dict | None = None) -> subprocess.CompletedProcess: + commit: str | None = COMMIT, cwd: Path | None = None, + env: dict | None = None, + extra: tuple = ()) -> subprocess.CompletedProcess: """`--pin openGlass@ --pin-source ` through `kind`, into - `base`.""" + `base`: with no `--pin` when `commit` is None, and `extra` after.""" + pin = ("--pin", f"openGlass@{commit}") if commit is not None else () 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) + *pin, "--pin-source", typed, *extra, + 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, + *pin, "--pin-source", typed, *extra, "--local-remote-dir", str(base / "remotes"), "--work-dir", str(base / "work"), *(("--dry-run",) if kind == "scaffold-dry-run" else ()), @@ -294,19 +297,16 @@ 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. `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.""" + the surrogate it was read as. `adopt-project.py plan` refused that name + as a value it could not record (#273's review, N2) while it recorded + the path; it records none since #540, so it is read, and refused, as + the scaffold refuses it.""" 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 - code = ("adopt-unwritable-value" - if kind == "adopt-plan" and "\udcff" in name - else "pin-source-unreadable") - assert f"REFUSED {code}: " in result.stderr + assert "REFUSED pin-source-unreadable: " in result.stderr assert made(kind, tmp_path) == [] @@ -356,22 +356,17 @@ def assert_one_refusal_of_the_tools(result: subprocess.CompletedProcess, @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.""" + """Through every way in, in one refusal, `pin-source-unreadable`. + `adopt-project.py plan` refused such a path before it read a digest, as + a value it could not record (#273's review, N2); it records no path + since #540, so it reads this one and says what the scaffold says.""" 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 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) == [] @@ -437,8 +432,123 @@ def test_a_source_that_answers_is_read_as_before(kind, spelling, tmp_path, 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})" + assert pin["digest_source"] == "local-clone" else: assert f"tree {digest[:12]}" in result.stdout, result.stdout - assert f"(local-clone ({typed}))" in result.stdout, result.stdout + assert " (local-clone)\n" in result.stdout, result.stdout assert "--dry-run: nothing was created." in result.stdout + + +# --- a `--pin-source` nothing reads is `pin-source-unused` (#451) ------------ +# +# `--pin-source` is read by each `--pin` (its digest) and, in the scaffold, by +# each `--referent-chain` link (its declaration). With neither, nothing read +# it, so nothing asked about it: `--dry-run` said `nothing was created.`, +# the real run made a project that declares no descent, and `plan` wrote a +# plan, each exit 0, with no word of the flag, even for a path that does not +# exist. It is refused, before anything exists, as `pin-source-unused`. + +#: What each tool says of a `--pin-source ` no flag of its reads. +UNUSED = { + "scaffold": ( + "REFUSED pin-source-unused: --pin-source {typed} was given with no " + "--pin and no --referent-chain, so nothing reads it: it names the " + "clone each --pin's digest is read from, and each --referent-chain " + "link's declaration.\nRemediation: nothing was created, and " + "`--dry-run` refuses the same. Add the --pin @<40 hex " + "commit> that clone was given for, or drop --pin-source.\n"), + "adopt": ( + "REFUSED pin-source-unused: --pin-source {typed} was given with no " + "--pin, so nothing reads it: it names the clone each --pin's digest " + "is read from.\nRemediation: no plan was written. Add the --pin " + "@<40 hex commit> that clone was given for, or drop " + "--pin-source.\n"), +} + + +@pytest.mark.parametrize("kind", RUNS) +def test_a_pin_source_nothing_reads_is_pin_source_unused(kind, tmp_path, + adoptable): + """#451's table, through every way in: no `--pin`, and a `--pin-source` + that does not exist. Exit 2, the one refusal in each tool's words, the + path as typed, and nothing made; the dry run and the real run say the + same.""" + typed = str(tmp_path / "no-such-clone") + result = run(kind, tmp_path, adoptable, typed, commit=None) + assert result.returncode == 2, result.stdout + result.stderr + tool = kind.split("-", 1)[0] + assert result.stderr == UNUSED[tool].format(typed=typed), result.stderr + assert result.stdout == "" + assert made(kind, tmp_path) == [] + + +@pytest.mark.parametrize("kind", ["scaffold-dry-run", "adopt-plan"]) +def test_a_pin_source_a_pin_reads_is_not_unused(kind, tmp_path, adoptable, + glass): + """The control: the same command line with a `--pin` the clone answers + for is planned. (The real scaffold is the last test's.)""" + product, commit = glass + result = run(kind, tmp_path, adoptable, str(product), commit=commit) + assert result.returncode == 0, result.stdout + result.stderr + assert "pin-source-unused" not in result.stdout + result.stderr + + +@pytest.mark.parametrize("kind", ["scaffold-dry-run", "scaffold-run"]) +def test_a_referent_chain_reads_the_pin_source(kind, tmp_path, adoptable): + """A `--referent-chain` link with no keyed source reads the bare + `--pin-source` (`link_pins_from_trees`), so a chain is a reader: with + no `--pin`, the refusal is the chain's own, which names the `--pin` the + chain begins at, and not `pin-source-unused`.""" + result = run(kind, tmp_path, adoptable, str(tmp_path / "no-such-clone"), + commit=None, + extra=("--referent-chain", "openXglass,openGlass")) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr.startswith("REFUSED chain-not-declared: "), \ + result.stderr + assert "pin-source-unused" not in result.stderr + assert made(kind, tmp_path) == [] + + +# --- a source that answers is not recorded (#540, #450) ---------------------- +# +# The pin file's `digest_source:` was `local-clone ()`: an +# absolute `--pin-source` committed the directory a clone sat in on the +# machine that scaffolded (#540). Written bare, a path holding a line break +# wrote a line of its own into the pin file, exit 0, and one holding a byte +# that is not UTF-8 died in `write_lf` once all three repositories existed, +# exit 1 (#450). The field says how the digest was read, `local-clone`, and +# the path is in no file. + +#: Where each clone below sits: in a directory of its own, so that the pin +#: file can be searched for it. +CLONE_NAMES = [ + pytest.param("openGlass", id="absolute"), + pytest.param("gl\nass", id="lf", marks=CONTROL_ARGV), + pytest.param("gl\udcffass", id="not-utf-8", marks=POSIX_ARGV), +] + + +@pytest.mark.parametrize("name", CLONE_NAMES) +def test_the_pin_file_says_how_the_digest_was_read_and_not_where( + name, tmp_path, adoptable, glass): + """A real scaffold from a clone at each path: exit 0, the plan's `pin` + line ending `(local-clone)`, and the pin file the root commits, read + from git, holding `digest_source: local-clone` and no byte of the + clone's directory.""" + product, commit = glass + clones = tmp_path / "clones" + shutil.copytree(product, clones / name, symlinks=True) + result = run("scaffold-run", tmp_path, adoptable, str(clones / name), + commit=commit) + assert result.returncode == 0, result.stdout + result.stderr + assert "Traceback" not in result.stderr, result.stderr + assert [line for line in result.stdout.splitlines() + if line.startswith("pin ")][0].endswith( + "-> contracts/openglass-pin.yaml (local-clone)"), result.stdout + blob = subprocess.run( + ["git", "show", "main:contracts/openglass-pin.yaml"], + cwd=tmp_path / "remotes" / f"{PROJECT}.git", capture_output=True, + check=True).stdout + assert os.fsencode(str(clones)) not in blob, blob + assert parse_yaml(blob.decode("utf-8"))["digest_source"] == "local-clone" + assert b"\ndigest_source: local-clone\n" in blob, blob From 79dfd336dc4254ee059b4e7b6491431c1a508288 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 22:59:26 +0000 Subject: [PATCH 02/10] tests/test_scaffold_leg_path_collisions.py: the pinned control reads the plan's `pin` line as `(local-clone)`, so it holds what the scaffold prints since the pin file stopped recording the `--pin-source` path (#540) `test_a_pinned_run_refuses_before_anything_exists` asserted the plan line `-> contracts/openglass-pin.yaml (local-clone (`, the path that #540 takes out of `digest_source:`. The targeted run at c28ebe77 found it, the only failure in its 1,518 tests. It now asserts the whole value, `(local-clone)`. Main: bfed143. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_scaffold_leg_path_collisions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_scaffold_leg_path_collisions.py b/tests/test_scaffold_leg_path_collisions.py index 68ca4d36..a2d6708f 100644 --- a/tests/test_scaffold_leg_path_collisions.py +++ b/tests/test_scaffold_leg_path_collisions.py @@ -536,7 +536,7 @@ def test_a_pinned_run_refuses_before_anything_exists(tmp_path, glass): "--local-remote-dir", str(remotes), "--work-dir", str(work)) control = run_script(SCAFFOLD, *args, "--dry-run") assert control.returncode == 0, control.stdout + control.stderr - assert f"-> {GLASS_PIN} (local-clone (" in control.stdout + assert f"-> {GLASS_PIN} (local-clone)\n" in control.stdout dry = run_script(SCAFFOLD, *args, "--spec-path", GLASS_PIN, "--dry-run") real = run_script(SCAFFOLD, *args, "--spec-path", GLASS_PIN) assert (dry.returncode, real.returncode) == (2, 2), ( From 08764e1c1963917296f64fb06a4cf9671dd4b002 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:09:44 +0000 Subject: [PATCH 03/10] adopt-project.py: `check` holds a plan's `digest_source:` to `local-clone` or `gh-api-tree-recursive`, so a plan that still records a `--pin-source` path is `plan-pin-malformed` and `execute` no longer commits that path (#540) rv540 F1. `_pin_row_problem` held `digest_source:` to one printable line. A plan from a `plan` before #540, which recorded `local-clone ()`, passed the head's `check`, whose `write` line printed the path. `execute` then wrote the path into `contracts/-pin.yaml`, the directory a clone sat in on the machine that planned. A hand edit did the same. `_read_pins` already says that anything else `plan` never writes there is `plan-pin-malformed`. The field is now held to the two words `plan` writes, `DIGEST_FROM_CLONE` and `DIGEST_FROM_GH`. Any other value is `plan-pin-malformed`, spelled as every finding spells a value, with the existing remedy: re-run `plan` with the `--pin`. `execute` refuses it with `check`'s findings before any leg exists. So no committed object holds a local path, whatever plan `execute` is given. The refusal was chosen over having `execute` write `local-clone` for any `local-clone (...)` value, because a canonical rewrite would still pass any other printable value into the committed file, a path among them. Tests. `test_check_spells_a_recorded_source_that_is_not_printable` becomes `test_check_finds_a_recorded_source_that_names_a_path`: the no-break-space path is now a finding, spelled. The table of plans `execute` cannot write gains `source-old-plan`, the value main's `plan` wrote, which `check` finds and `execute` refuses before any leg. The pin fixtures that wrote `local-clone ()` in `tests/test_adopt_pins.py`, `tests/test_adopt_execute_refusals.py` and `tests/test_adopt_root_symlinks.py` now write `local-clone`, the value `plan` records since #540. Main: 7d9bec92. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 26 +++++++++---- tests/test_adopt_execute_refusals.py | 2 +- tests/test_adopt_pins.py | 56 ++++++++++++++++++++-------- tests/test_adopt_root_symlinks.py | 2 +- 4 files changed, 62 insertions(+), 24 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index c3c7a72e..d5e688f5 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -244,9 +244,10 @@ #: 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, pin_variable_collisions, - refuse_an_unread_pin_source, refuse_colliding_pins, + DIGEST_FROM_CLONE, DIGEST_FROM_GH, PIN_ARG_RE, declared_pin_products, + declared_pin_values, neutral_pin_path, pin_collisions, + pin_variable_collisions, refuse_an_unread_pin_source, + refuse_colliding_pins, ) from repo_shape import SHA256_RE, repo_basename # noqa: E402 @@ -317,7 +318,16 @@ def _pin_row_problem(row: dict) -> tuple[str, str] | None: 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.""" + `SHA256_RE`, and the digest's source to `DIGEST_FROM_CLONE` or + `DIGEST_FROM_GH`. + + THE DIGEST'S SOURCE IS ONE OF THE TWO WORDS `plan` WRITES (#540). It was + held to one printable line, so a plan from a `plan` before #540, which + recorded `local-clone ()`, or a hand edit, passed + `check`, and `execute` committed that path into the root's pin file, the + directory a clone sat in on the machine that planned. `execute` writes + the field as the plan holds it, so a value `plan` never writes is + `plan-pin-malformed`, with the `plan` that records the pin whole.""" 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: @@ -345,9 +355,11 @@ def _pin_row_problem(row: dict) -> tuple[str, str] | None: 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") + if how not in (DIGEST_FROM_CLONE, DIGEST_FROM_GH): + return PIN_MALFORMED, ( + f"records `digest_source: {how!r}`, and `plan` records only how " + f"it read the digest, `{DIGEST_FROM_CLONE}` or " + f"`{DIGEST_FROM_GH}`, and never where") return None diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index 7c767af6..c6599625 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -1009,7 +1009,7 @@ def a_whole_pin(product: str) -> str: f" repository: opensoft/{product}\n" f' commit: "{"a" * 40}"\n' f' tree_sha256: "{"b" * 64}"\n' - f' digest_source: "local-clone ({product})"\n') + ' digest_source: "local-clone"\n') #: What `check` and `execute` still take, by test id: the key, the block diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index 6b2bc49c..eb4b1047 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -153,7 +153,7 @@ def plan_with(source: Path, out: Path, *pins: str, extra: tuple = (), def pin_entry(product: str = "openGlass", repository: str = "opensoft/openGlass", commit: str = "", - digest: str = "", source: str = "local-clone (openGlass)", + digest: str = "", source: str = "local-clone", omit: tuple = ()) -> str: """One `pins:` mapping as `plan` writes it, its keys in `plan`'s order, with each key in `omit` left out.""" @@ -846,23 +846,34 @@ def test_a_pin_source_no_pin_reads_is_refused(source_repo, tmp_path, gh_env, assert gh_calls(gh_env) == "" -def test_check_spells_a_recorded_source_that_is_not_printable( +def test_check_finds_a_recorded_source_that_names_a_path( source_repo, tmp_path, glass, gh_env): """A plan `plan` wrote before #540, or one edited since, can still hold - a path in `digest_source:`. One that is one printable line for `check` - and still not `str.isprintable` -- a no-break space -- passes, and - `check`'s `write` line spells it, as every other path either report - prints (`_shown`).""" + a path in `digest_source:`. `check` held the field to one printable + line, so it passed such a plan and printed the path in its `write` + line, and `execute` committed the path into the pin file (rv540 F1). + The field is held to the two words `plan` writes, so `check` finds the + path `plan-pin-malformed`, spelled where it is not `str.isprintable` (a + no-break space), as every value a finding names is, with the `plan` + that records the pin whole; `execute` refuses it before any leg exists + (`source-old-plan` in the table below).""" path, commit, _ = glass 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 checked.returncode == 1, checked.stdout + checked.stderr + found = [line for line in checked.stderr.splitlines() + if line.startswith("FINDING ")] + assert len(found) == 1, checked.stderr + assert found[0].startswith( + "FINDING plan-pin-malformed: `pins:` entry 1 ('openGlass') records " + "`digest_source: 'local-clone (open\\xa0Glass)'`, " + f"{HOW_NOT_WHERE}. `execute` writes the pin file from the commit and " + "the digest `plan` records, and nothing else: re-run `plan` with " + "`--pin opensoft/openGlass@<40 hex commit>`"), found[0] + assert "\u00a0" not in checked.stdout + checked.stderr assert gh_calls(gh_env) == "" @@ -934,12 +945,21 @@ def unrecorded(glass) -> dict: } +#: How `check` ends its finding on a `digest_source:` that `plan` never +#: writes (#540). +HOW_NOT_WHERE = ("and `plan` records only how it read the digest, " + "`local-clone` or `gh-api-tree-recursive`, and never where") + + 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" + # `as_posix`, so that a Windows path's backslashes are no escapes in the + # plan's double-quoted value. + old = f"local-clone ({path.as_posix()})" return { "not-a-list": ("pins: openGlass\n", "`pins:` is 'openGlass', not a list of pins"), @@ -970,15 +990,21 @@ def malformed(glass) -> dict: 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"), + f"(/x\\nforged: y)'`, {HOW_NOT_WHERE}"), + # What `plan` recorded before #540: the `--pin-source` path, which + # `execute` committed into the pin file (rv540 F1). One printable + # line, so `check` passed it. + "source-old-plan": ("pins:\n" + pin_entry( + commit=commit, digest=digest, source=old), + f"{entry} `digest_source: {old!r}`, " + f"{HOW_NOT_WHERE}"), # 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', + ' digest_source: "local-clone"\n', f"{entry} `commit: 1234567`, which is not text"), } @@ -1035,7 +1061,7 @@ def cases(glass): CASE_IDS += [("plan-pin-malformed", name) for name in ( "not-a-list", "a-number", "short-commit", "short-digest", "another-product", "no-owner", "no-source", "source-line-break", - "a-number-commit")] + "source-old-plan", "a-number-commit")] CASE_IDS += [("plan-pin-collides", name) for name in ("case-pair", "shape", "variable-pair")] @@ -1165,7 +1191,7 @@ def test_an_upper_cased_hand_edit_is_written_lower_cased(adopter, source_repo, 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 + f"{digest[:12]}… (local-clone)") in checked.stdout recorded = adopter._recorded_pins(adopter.Plan.load(plan)) assert recorded["openGlass"]["PIN_COMMIT"] == commit assert recorded["openGlass"]["PIN_TREE_SHA256"] == digest diff --git a/tests/test_adopt_root_symlinks.py b/tests/test_adopt_root_symlinks.py index 5a098072..6695d6e8 100644 --- a/tests/test_adopt_root_symlinks.py +++ b/tests/test_adopt_root_symlinks.py @@ -811,7 +811,7 @@ def pin_row(product: str, commit: bool = True) -> str: f" repository: opensoft/{product}", f" commit: \"{'0123456789abcdef' * 2}01234567\"", f" tree_sha256: \"{'0123456789abcdef' * 4}\"", - f" digest_source: \"local-clone ({product})\""] + " digest_source: \"local-clone\""] return "\n".join(lines if commit else lines[:2] + lines[3:]) + "\n" From 6a241a6106c5a4cab3f8492599f5d1e503535b71 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:10:43 +0000 Subject: [PATCH 04/10] adopt-project.py: `plan` asks `pin-source-unused` first, from the flags alone, so an unread `--pin-source` is refused before a work directory is made, before git runs in the source and before an `org/repo` source is `gh repo clone`d (#451) rv540 F2. #451 asks for the refusal "before anything exists". The scaffold asks it before any process starts, but adopt asked it in `_checked_plan_inputs`, after `cmd_plan` had run `_work_root` and `Source.open`. So the refusal came after these: - an `openreposhape-adopt-*` temporary directory, or the `--work-dir`, was made and left behind; - git ran in a local source; - an `org/repo` source was `gh repo clone`d, and with a `gh` that could not clone it, that refusal came instead. `refuse_an_unread_pin_source` needs only the flags, so `cmd_plan` now asks it first, in the same words. Test: `test_an_unread_pin_source_is_refused_before_anything_exists` (`tests/test_adopt_pins.py`) runs `plan` with no `--pin` in three ways: - an `org/repo` source; - a local source; - a local source with `--work-dir`. Each run has a fresh `TMPDIR`, a `GIT_TRACE` file and the logging `gh` stub. Each must refuse `pin-source-unused` with nothing on stdout, an empty `TMPDIR`, no work directory, no trace file, no plan and no `gh` call. Main: 7d9bec92. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 15 ++++++++++----- tests/test_adopt_pins.py | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index d5e688f5..3ed58399 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -2869,11 +2869,6 @@ def _checked_plan_inputs(args, source: Source, # 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) - # A `--pin-source` no `--pin` will read is refused, in the scaffold's - # words (#451): it names the clone a pin's digest is read from, and with - # no `--pin` it changed nothing and was never even asked about. - refuse_an_unread_pin_source(args.pin_source, args.pin, - nothing_made="no plan was written.") 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 @@ -3280,6 +3275,16 @@ def _print_plan_report(args, source: Source, tree: list, entries: list, def cmd_plan(args) -> int: + # A `--pin-source` no `--pin` will read is refused FIRST, in the + # scaffold's words (#451): it names the clone a pin's digest is read + # from, and with no `--pin` it changed nothing and was never even asked + # about. It needs the flags alone, so it is asked before anything exists + # and before any process starts. Asked in `_checked_plan_inputs`, it + # came after `_work_root` had made the work directory, which stayed + # behind, and after `Source.open` had run git in the source or `gh repo + # clone` for an `org/repo` one (rv540 F2). + refuse_an_unread_pin_source(args.pin_source, args.pin, + nothing_made="no plan was written.") work_root = _work_root(args) source = Source.open(args.source, work_root) policy = PathPolicy.load(args.path_policy or PATH_POLICY) diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index eb4b1047..003e7e7f 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -846,6 +846,46 @@ def test_a_pin_source_no_pin_reads_is_refused(source_repo, tmp_path, gh_env, assert gh_calls(gh_env) == "" +@pytest.mark.parametrize("source, work_dir", [ + pytest.param("org/repo", False, id="org-repo"), + pytest.param("local", False, id="local"), + pytest.param("local", True, id="local-work-dir"), +]) +def test_an_unread_pin_source_is_refused_before_anything_exists( + source_repo, tmp_path, gh_env, source, work_dir): + """`pin-source-unused` needs the flags alone, so `plan` asks it first, + before anything exists and before any process starts, as the scaffold + does (#451). It was asked after `_work_root` and `Source.open` (rv540 + F2): an `openreposhape-adopt-*` directory was made and left behind, or + the `--work-dir`; git ran in a local source; and an `org/repo` source + was `gh repo clone`d, whose own refusal came instead. Here the + temporary directory is a fresh one, `GIT_TRACE` names a file any git + process would write, and the `gh` stub logs every call.""" + temp, trace, work = (tmp_path / "tmp", tmp_path / "git-trace.log", + tmp_path / "work") + temp.mkdir() + out = tmp_path / "plan.yaml" + result = run_script( + ADOPT, "plan", + "--source", "testorg/Thing" if source == "org/repo" else str( + source_repo), + "--project", PROJECT, "--org", ORG, "--elected-by", "Test Human", + "--elected-on", "2026-09-02", "--out", str(out), + "--pin-source", str(tmp_path / "no-such-clone"), + *(("--work-dir", str(work)) if work_dir else ()), + cwd=tmp_path, + env={**gh_env, "TMPDIR": str(temp), "GIT_TRACE": str(trace)}) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr.startswith("REFUSED pin-source-unused: "), \ + result.stderr + assert result.stdout == "" + assert list(temp.iterdir()) == [] + assert not work.exists() + assert not trace.exists() + assert not out.exists() + assert gh_calls(gh_env) == "" + + def test_check_finds_a_recorded_source_that_names_a_path( source_repo, tmp_path, glass, gh_env): """A plan `plan` wrote before #540, or one edited since, can still hold From a3117d812e1c628358a723fc17b363bb2f7d8d23 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:11:02 +0000 Subject: [PATCH 05/10] tests/test_adopt_pins.py: the no-raw-character check of `test_a_pin_source_no_pin_reads_is_refused` is two asserts, one per character, so a failure names the character and SonarCloud's S9073 has nothing to report (#451) rv540 N1. `assert "\x1b" not in ... and "" not in ...` was SonarCloud's only new issue on #570 (python:S9073, a composite assertion). It is split in two, each with the stderr as its message, and the zero-width joiner is written as its escape rather than as the invisible character. The exact-stderr assert above it is unchanged. Main: 7d9bec92. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_pins.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index 003e7e7f..db91cf23 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -841,7 +841,8 @@ def test_a_pin_source_no_pin_reads_is_refused(source_repo, tmp_path, gh_env, "digest is read from.\nRemediation: no plan was written. Add the " "--pin @<40 hex commit> that clone was given for, or " "drop --pin-source.\n") - assert "\x1b" not in result.stderr and "‍" not in result.stderr + assert "\x1b" not in result.stderr, result.stderr + assert "\u200d" not in result.stderr, result.stderr assert not out.exists() assert gh_calls(gh_env) == "" From 4e0e21a8096a742204cb5899390fd75de43f4285 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:11:59 +0000 Subject: [PATCH 06/10] tests: the two `not-utf-8` clone rows skip where the filesystem refuses a file name that is not UTF-8, so macOS's EILSEQ is a skip and not a failure (#540, #450) CI on macOS failed at 79dfd33 (run 38652642202, job 116154875090: 2 failed, 8403 passed). Each failure raised `OSError: [Errno 92] Illegal byte sequence` from `shutil.copytree`, because APFS refuses a file name that is not UTF-8, so no clone can sit at such a path there: - `tests/test_adopt_pins.py::test_a_clone_is_read_wherever_it_is_and_its_path_is_not_recorded[not-utf-8]`; - `tests/test_pin_source_unreadable.py::test_the_pin_file_says_how_the_digest_was_read_and_not_where[not-utf-8]`. Each of those rows now makes a probe directory with such a name under `tmp_path` first. On `OSError` EILSEQ the row skips, as the Windows mark skips a command line that cannot hold the byte. Any other error is raised. Every other row is unchanged, and on Linux both rows run as before. Main: 7d9bec92. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_pins.py | 19 +++++++++++++++++++ tests/test_pin_source_unreadable.py | 18 ++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index db91cf23..636f0c91 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -31,6 +31,7 @@ from __future__ import annotations +import errno import importlib.util import json import os @@ -780,6 +781,22 @@ def test_with_no_gh_on_path_a_pin_is_gh_not_found(kind, source_repo, UNREAD = "unreadclone" +def skip_where_a_name_cannot_be_not_utf_8(directory: Path) -> None: + """Skip, as `POSIX_NAMES` skips on Windows, on a filesystem that refuses + a file name that is not UTF-8: macOS answers EILSEQ ("Illegal byte + sequence"), so no clone can sit at such a path there. Asked of a + directory made under `directory`; any other error is the test's own.""" + probe = directory / "probe\udcff" + try: + probe.mkdir() + except OSError as exc: + if exc.errno != errno.EILSEQ: + raise + pytest.skip("needs a file name that is not UTF-8, which this " + "filesystem refuses (EILSEQ)") + probe.rmdir() + + @pytest.mark.parametrize("name", [ pytest.param(f"{UNREAD}\x1bx", id="esc", marks=POSIX_NAMES), pytest.param(f"{UNREAD}\nx", id="lf", marks=POSIX_NAMES), @@ -798,6 +815,8 @@ def test_a_clone_is_read_wherever_it_is_and_its_path_is_not_recorded( file from, or in the report, whose `pin` line says how the digest was read and nothing more; so nothing of it reaches the terminal raw.""" path, commit, _ = glass + if "\udcff" in name: + skip_where_a_name_cannot_be_not_utf_8(tmp_path) clone = tmp_path / name shutil.copytree(path, clone, symlinks=True) out = tmp_path / "plan.yaml" diff --git a/tests/test_pin_source_unreadable.py b/tests/test_pin_source_unreadable.py index a550842c..fafe61ed 100644 --- a/tests/test_pin_source_unreadable.py +++ b/tests/test_pin_source_unreadable.py @@ -528,6 +528,22 @@ def test_a_referent_chain_reads_the_pin_source(kind, tmp_path, adoptable): ] +def skip_where_no_file_name_is_not_utf_8(directory: Path) -> None: + """Skip, as a Windows command line skips, where the filesystem refuses + a file name that is not UTF-8, so that no clone can sit at one: macOS + answers EILSEQ ("Illegal byte sequence"). Asked by making one under + `directory`; any other error is the test's to report.""" + probe = directory / "probe\udcff" + try: + probe.mkdir() + except OSError as exc: + if exc.errno != errno.EILSEQ: + raise + pytest.skip("needs a file name that is not UTF-8, which this " + "filesystem refuses (EILSEQ)") + probe.rmdir() + + @pytest.mark.parametrize("name", CLONE_NAMES) def test_the_pin_file_says_how_the_digest_was_read_and_not_where( name, tmp_path, adoptable, glass): @@ -536,6 +552,8 @@ def test_the_pin_file_says_how_the_digest_was_read_and_not_where( from git, holding `digest_source: local-clone` and no byte of the clone's directory.""" product, commit = glass + if "\udcff" in name: + skip_where_no_file_name_is_not_utf_8(tmp_path) clones = tmp_path / "clones" shutil.copytree(product, clones / name, symlinks=True) result = run("scaffold-run", tmp_path, adoptable, str(clones / name), From b1db67c9df4cc70c582edd02a29968b228016ec3 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:14:48 +0000 Subject: [PATCH 07/10] scaffold-project.py and adopt-project.py: `--pin-source ''` is refused as `pin-source-no-path` before anything exists, so an empty value is no longer read as the directory the tool runs in (#540) Lane 5 found this beside #442 (#570 issuecomment-6180853841), and lane 2 took it into this PR (issuecomment-6180905220). Both tools took `--pin-source` as `type=Path`, and `Path("")` is `Path(".")`. So with `--pin-source ''`: - run from inside a clone with a `--pin`, the dry run, the real scaffold and `plan` pinned that clone's digest, exit 0; - run from anywhere else, they read whatever directory they stood in; - with no `--pin`, `pin-source-unused` named `.`, which nobody typed. #576 (#442) refuses an empty value in the naming gate (`link-source-no-path`) and in the root's `validate-pins.py` (`pin-source-no-path`). The two tools that make a project now refuse it under the same code and in the same sentence, "`--pin-source ''` names no path. An empty path would be read as the directory this runs in." Each tool begins the remedy in its own words, as `pin-source-unused` does: give the clone's path, or `.` to mean this directory. `pin_source_path` (`scripts/shape_materialize.py`, beside `refuse_an_unread_pin_source`) asks this for both tools and hands back the `Path` they read. Both parsers therefore take the flag as text, since once parsed as a `Path` an empty value and a typed `.` are one. It is asked from the flags alone, before `pin-source-unused` and before any process starts. In the scaffold that is beside `pin-source-unused`, so `--dry-run` refuses it too. In `plan` it is the first thing `cmd_plan` asks. A typed `.` is read as it always was. Both `--pin-source` helps say so, and `docs/cli.md` is regenerated (`render-cli-reference.py --check` passes). Tests (`tests/test_pin_source_unreadable.py`): - `test_a_pin_source_that_names_no_path_is_pin_source_no_path` runs the dry run, the real scaffold and `plan`, with and without a `--pin`, each from inside a clone that answers: exit 2, the exact refusal, nothing on stdout and nothing made; - `test_a_typed_dot_is_still_the_directory_the_tool_runs_in` is the control. Main: 7d9bec92. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 34 +++++++++++------- docs/cli.md | 7 ++-- scaffold-project.py | 23 ++++++++---- scripts/shape_materialize.py | 31 +++++++++++++++++ tests/test_pin_source_unreadable.py | 54 +++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 21 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 3ed58399..f0807fd3 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -246,9 +246,11 @@ from shape_materialize import ( # noqa: E402 DIGEST_FROM_CLONE, DIGEST_FROM_GH, PIN_ARG_RE, declared_pin_products, declared_pin_values, neutral_pin_path, pin_collisions, - pin_variable_collisions, refuse_an_unread_pin_source, + pin_source_path, pin_variable_collisions, refuse_an_unread_pin_source, refuse_colliding_pins, ) +#: How `plan`'s refusals of a `--pin-source` begin their remediation. +PLAN_NOT_WRITTEN = "no plan was written." 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 @@ -3275,16 +3277,20 @@ def _print_plan_report(args, source: Source, tree: list, entries: list, def cmd_plan(args) -> int: - # A `--pin-source` no `--pin` will read is refused FIRST, in the - # scaffold's words (#451): it names the clone a pin's digest is read - # from, and with no `--pin` it changed nothing and was never even asked - # about. It needs the flags alone, so it is asked before anything exists - # and before any process starts. Asked in `_checked_plan_inputs`, it - # came after `_work_root` had made the work directory, which stayed - # behind, and after `Source.open` had run git in the source or `gh repo - # clone` for an `org/repo` one (rv540 F2). + # A `--pin-source` that names no path, `''`, which as a `Path` is the + # directory this runs in, and then one no `--pin` will read, are refused + # FIRST, in the scaffold's words (#451): it names the clone a pin's + # digest is read from, and with no `--pin` it changed nothing and was + # never even asked about. They need the flags alone, so they are asked + # before anything exists and before any process starts. Asked in + # `_checked_plan_inputs`, the second came after `_work_root` had made + # the work directory, which stayed behind, and after `Source.open` had + # run git in the source or `gh repo clone` for an `org/repo` one (rv540 + # F2). + args.pin_source = pin_source_path(args.pin_source, + nothing_made=PLAN_NOT_WRITTEN) refuse_an_unread_pin_source(args.pin_source, args.pin, - nothing_made="no plan was written.") + nothing_made=PLAN_NOT_WRITTEN) work_root = _work_root(args) source = Source.open(args.source, work_root) policy = PathPolicy.load(args.path_policy or PATH_POLICY) @@ -9620,14 +9626,18 @@ def main(argv: list[str] | None = None) -> int: "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, + # TEXT, not `type=Path`: `Path("")` is `.`, so an empty value could not + # be told from a typed `.` once parsed (`pin_source_path`). + plan.add_argument("--pin-source", default=None, help="a local clone to compute --pin digests from, " "instead of reading the forge with `gh api` (the " "TEST path; no network). The plan and the pin " "file say only that the digest was read from " "`local-clone`, never the path. Given with no " "--pin to read it, the flag is refused as " - "`pin-source-unused`.") + "`pin-source-unused`; an empty value is refused " + "as `pin-source-no-path`, and `.` means this " + "directory.") 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 929232af..42042eb1 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -342,7 +342,9 @@ options: network). The pin file says only that the digest was read from `local-clone`, never the path. Given with no --pin and no --referent-chain to read it, it is - refused as `pin-source-unused`. + refused as `pin-source-unused`; an empty value is + refused as `pin-source-no-path`, and `.` means this + directory. --work-dir WORK_DIR where the working trees are built (default: a temporary directory) ``` @@ -472,7 +474,8 @@ options: network). The plan and the pin file say only that the digest was read from `local-clone`, never the path. Given with no --pin to read it, the flag is refused as - `pin-source-unused`. + `pin-source-unused`; an empty value is refused as + `pin-source-no-path`, and `.` means this directory. --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 21c5f4c2..a72680a2 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -125,7 +125,7 @@ #: 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, + declared_pin_products, declared_pin_values, pin_source_path, refuse_an_unread_pin_source, refuse_colliding_pins as _refuse_colliding_pins, ) @@ -302,14 +302,18 @@ def main(argv: list[str] | None = None) -> int: "openXdox); the manifest RECORDS it, which is " "what makes a descendant that pins a LAYER a " "descendant (2026-09-05).") - parser.add_argument("--pin-source", type=Path, default=None, + # TEXT, not `type=Path`: `Path("")` is `.`, so an empty value could not + # be told from a typed `.` once parsed (`pin_source_path`). + parser.add_argument("--pin-source", default=None, help="a local clone to compute --pin digests from, " "instead of reading the forge with `gh api` (the " "TEST path; no network). The pin file says " "only that the digest was read from " "`local-clone`, never the path. Given with no " "--pin and no --referent-chain to read it, it " - "is refused as `pin-source-unused`.") + "is refused as `pin-source-unused`; an empty " + "value is refused as `pin-source-no-path`, and " + "`.` means this directory.") parser.add_argument("--work-dir", type=Path, default=None, help="where the working trees are built " "(default: a temporary directory)") @@ -653,7 +657,8 @@ def _identity(args, project: str) -> tuple[str, str, str, str, str]: "the assembly root's `project.yaml` and `README.md`, each repository's " "`AGENTS.md` and each first commit's message") -#: How a `pin-source-unused` refusal's remediation begins (#451). +#: How a `pin-source-unused` refusal's remediation begins (#451), and a +#: `pin-source-no-path` one's. UNREAD_PIN_SOURCE_MADE = ("nothing was created, and `--dry-run` refuses the " "same.") @@ -1371,9 +1376,13 @@ def _scaffold(args) -> int: args.code_path = checked_value(*code_leg) args.org = checked_value("--org", args.org) args.pin_owner = checked_value("--pin-owner", args.pin_owner) - # A `--pin-source` NOTHING WILL READ (#451): no `--pin` reads a digest - # there and no `--referent-chain` link a declaration. Asked of the flags - # as typed, with no process started, so a dry run refuses it too. + # A `--pin-source` THAT NAMES NO PATH, `''`, which as a `Path` is the + # directory this runs in, and then one NOTHING WILL READ (#451): no + # `--pin` reads a digest there and no `--referent-chain` link a + # declaration. Asked of the flags as typed, with no process started, so + # a dry run refuses them too. + args.pin_source = pin_source_path(args.pin_source, + nothing_made=UNREAD_PIN_SOURCE_MADE) refuse_an_unread_pin_source(args.pin_source, args.pin, referent_chain_links(args.referent_chain), nothing_made=UNREAD_PIN_SOURCE_MADE) diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index f19e05a0..94c08fad 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -2671,6 +2671,37 @@ def declared_pin_products(args) -> dict[str, None]: return dict.fromkeys(parse_pin(raw, args.pin_owner)[0] for raw in args.pin) +def pin_source_path(typed: str | None, *, nothing_made: str) -> Path | None: + """`--pin-source` as the `Path` both tools read, or None when it was not + given; a value that names no path is `pin-source-no-path`. + + AN EMPTY VALUE WAS THE DIRECTORY THE TOOL RAN IN. `Path("")` is + `Path(".")`, so `--pin-source ''` read the current directory as the + clone: the digest came from wherever the command was run, or + `pin-source-unreadable` blamed `.`, a path nobody typed. #442 refuses + an empty value in the naming gate (`link-source-no-path`) and in the + root's `validate-pins.py` (`pin-source-no-path`); the two tools that + make a project refuse it in the same words. A typed `.` still means + the current directory. + + So both tools take the flag as TEXT and ask here, before anything + exists and before any process starts, the scaffold's `--dry-run` alike: + once `argparse` has made a `Path` of it, an empty value and a typed `.` + are one. `nothing_made` begins the remediation in the tool's own words, + as it does for `refuse_an_unread_pin_source`. + """ + if typed is None: + return None + if not str(typed): + raise Refusal( + "pin-source-no-path", + "`--pin-source ''` names no path. An empty path would be read as " + "the directory this runs in.", + f"Remediation: {nothing_made} Give the clone's path " + "(`--pin-source `), or `.` to mean this directory.") + return Path(typed) + + def refuse_an_unread_pin_source(pin_source, pins, chain=None, *, nothing_made: str) -> None: """`--pin-source` is read by this run, or this refuses it as diff --git a/tests/test_pin_source_unreadable.py b/tests/test_pin_source_unreadable.py index fafe61ed..7c9f1849 100644 --- a/tests/test_pin_source_unreadable.py +++ b/tests/test_pin_source_unreadable.py @@ -509,6 +509,60 @@ def test_a_referent_chain_reads_the_pin_source(kind, tmp_path, adoptable): assert made(kind, tmp_path) == [] +# --- a `--pin-source` that names no path is `pin-source-no-path` ------------- +# +# `--pin-source ''` was `Path("")`, which is `.`: the directory the tool ran +# in was read as the clone, so a run from inside a clone pinned that clone's +# digest, exit 0, and with no `--pin` the refusal named `.`, a path nobody +# typed. #442 refuses an empty value in the naming gate and in the root's +# `validate-pins.py`; the two tools that make a project refuse it in the +# same words, before anything exists. A typed `.` still means this directory. + +#: What each tool says of `--pin-source ''`, its remediation begun in the +#: tool's own words. +NO_PATH = ("REFUSED pin-source-no-path: `--pin-source ''` names no path. An " + "empty path would be read as the directory this runs in.\n" + "Remediation: {made} Give the clone's path (`--pin-source " + "`), or `.` to mean this directory.\n") +MADE = {"scaffold": "nothing was created, and `--dry-run` refuses the same.", + "adopt": "no plan was written."} + + +@pytest.mark.parametrize("kind", RUNS) +@pytest.mark.parametrize("pinned", [True, False], ids=["a-pin", "no-pin"]) +def test_a_pin_source_that_names_no_path_is_pin_source_no_path( + kind, pinned, tmp_path, adoptable, glass): + """Each run is made from inside a clone that answers for the pin, so an + empty value read as `.` would be read: exit 2, the one refusal, nothing + on stdout and nothing made, `--dry-run` alike. With no `--pin` it is + this refusal too, and not `pin-source-unused` naming `.`.""" + product, commit = glass + result = run(kind, tmp_path, adoptable, "", + commit=commit if pinned else None, cwd=product) + assert result.returncode == 2, result.stdout + result.stderr + tool = kind.split("-", 1)[0] + assert result.stderr == NO_PATH.format(made=MADE[tool]), result.stderr + assert result.stdout == "" + assert made(kind, tmp_path) == [] + + +@pytest.mark.parametrize("kind", ["scaffold-dry-run", "adopt-plan"]) +def test_a_typed_dot_is_still_the_directory_the_tool_runs_in( + kind, tmp_path, adoptable, glass): + """The control: `.`, typed, is read as the directory the tool runs in, + as it always was, so the digest is that clone's.""" + product, commit = glass + digest = tree_digest(product, commit) + result = run(kind, tmp_path, adoptable, ".", commit=commit, cwd=product) + assert result.returncode == 0, result.stdout + result.stderr + assert "pin-source-no-path" not in result.stdout + result.stderr + if kind == "adopt-plan": + pin = load_yaml(tmp_path / "plan.yaml")["pins"][0] + assert pin["tree_sha256"] == digest + else: + assert f"tree {digest[:12]}" in result.stdout, result.stdout + + # --- a source that answers is not recorded (#540, #450) ---------------------- # # The pin file's `digest_source:` was `local-clone ()`: an From e0a2ddf25500de150d39c7883023b43a53c97ed7 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 16:54:38 +0000 Subject: [PATCH 08/10] scripts/shape_materialize.py: the `pin-source-unused` refusal says `--pin-source` is read by each `--pin` and by the FIRST `--referent-chain` link, so the sentence agrees with #576's rule that a bare path is the first link's tree alone (#451) rv540b N3. The refusal said `--pin-source` names the clone "each --pin's digest is read from, and each --referent-chain link's declaration". Lane 5's #576 (#441) makes the scaffold hand a bare `--pin-source` to a chain's first link and no other, because a later link read from the same tree answered with the wrong product's declaration. The wording follows its rule ("the FIRST link alone", docs/cli.md of #576), in the refusal's `reads` clause and in `refuse_an_unread_pin_source`'s docstring, where it names #441. The same claim is reworded in `tests/test_pin_source_unreadable.py`: the section comment, the `UNUSED` scaffold text the test compares the refusal with, and the docstring of `test_a_referent_chain_reads_the_pin_source`, whose chain `openXglass,openGlass` is read through its first link. The scaffold's own comment at the call (`no --referent-chain link a declaration`) stays: it says only that a run with no chain has no reader. The adopt wording is unchanged: `plan` takes no chain. Main: 2bf8fd4c. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- scripts/shape_materialize.py | 25 +++++++++++++------------ tests/test_pin_source_unreadable.py | 22 ++++++++++++---------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 6545ba4e..654e6811 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -2715,17 +2715,18 @@ def refuse_an_unread_pin_source(pin_source, pins, chain=None, *, no plan. ITS READERS ARE `declared_pin_values`, which reads each `--pin`'s digest - there, and, in the scaffold, `link_pins_from_trees`, which reads a - `--referent-chain` link's declaration there when nothing keyed names that - link's tree. With neither, nothing read it, so nothing asked about it - either: a path that does not exist was accepted, `--dry-run` said - `nothing was created.` and `plan` wrote its plan, each exit 0 with no - word that the flag had changed nothing. Typed meaning to pin, with the - `--pin` forgotten, it made a project that declares no descent. The - root's own `validate-pins.py` NOTES an unused `--pin-source` of its own - (#295), a check that is run again and again; the two tools that make a - project refuse one, before it is made, as they refuse each other value - they cannot use. + there, and, in the scaffold, `link_pins_from_trees`, which reads the + first `--referent-chain` link's declaration there when nothing keyed + names that link's tree: a bare path is the first link's tree alone + (#441), never a later link's. With neither, nothing read it, so nothing + asked about it either: a path that does not exist was accepted, + `--dry-run` said `nothing was created.` and `plan` wrote its plan, each + exit 0 with no word that the flag had changed nothing. Typed meaning to + pin, with the `--pin` forgotten, it made a project that declares no + descent. The root's own `validate-pins.py` NOTES an unused + `--pin-source` of its own (#295), a check that is run again and again; + the two tools that make a project refuse one, before it is made, as + they refuse each other value they cannot use. `pins` is the `--pin` list and `chain` the scaffold's `--referent-chain` links, or `None` for a tool that takes no chain (`adopt-project.py @@ -2739,7 +2740,7 @@ def refuse_an_unread_pin_source(pin_source, pins, chain=None, *, given, reads = "no --pin", "each --pin's digest is read from" else: given = "no --pin and no --referent-chain" - reads = ("each --pin's digest is read from, and each " + reads = ("each --pin's digest is read from, and the first " "--referent-chain link's declaration") raise Refusal( "pin-source-unused", diff --git a/tests/test_pin_source_unreadable.py b/tests/test_pin_source_unreadable.py index 7c9f1849..a71751d6 100644 --- a/tests/test_pin_source_unreadable.py +++ b/tests/test_pin_source_unreadable.py @@ -442,21 +442,23 @@ def test_a_source_that_answers_is_read_as_before(kind, spelling, tmp_path, # --- a `--pin-source` nothing reads is `pin-source-unused` (#451) ------------ # # `--pin-source` is read by each `--pin` (its digest) and, in the scaffold, by -# each `--referent-chain` link (its declaration). With neither, nothing read -# it, so nothing asked about it: `--dry-run` said `nothing was created.`, -# the real run made a project that declares no descent, and `plan` wrote a -# plan, each exit 0, with no word of the flag, even for a path that does not -# exist. It is refused, before anything exists, as `pin-source-unused`. +# the first `--referent-chain` link (its declaration: a bare path is that +# link's tree alone, #441). With neither, nothing read it, so nothing asked +# about it: `--dry-run` said `nothing was created.`, the real run made a +# project that declares no descent, and `plan` wrote a plan, each exit 0, +# with no word of the flag, even for a path that does not exist. It is +# refused, before anything exists, as `pin-source-unused`. #: What each tool says of a `--pin-source ` no flag of its reads. UNUSED = { "scaffold": ( "REFUSED pin-source-unused: --pin-source {typed} was given with no " "--pin and no --referent-chain, so nothing reads it: it names the " - "clone each --pin's digest is read from, and each --referent-chain " - "link's declaration.\nRemediation: nothing was created, and " - "`--dry-run` refuses the same. Add the --pin @<40 hex " - "commit> that clone was given for, or drop --pin-source.\n"), + "clone each --pin's digest is read from, and the first " + "--referent-chain link's declaration.\nRemediation: nothing was " + "created, and `--dry-run` refuses the same. Add the --pin " + "@<40 hex commit> that clone was given for, or drop " + "--pin-source.\n"), "adopt": ( "REFUSED pin-source-unused: --pin-source {typed} was given with no " "--pin, so nothing reads it: it names the clone each --pin's digest " @@ -495,7 +497,7 @@ def test_a_pin_source_a_pin_reads_is_not_unused(kind, tmp_path, adoptable, @pytest.mark.parametrize("kind", ["scaffold-dry-run", "scaffold-run"]) def test_a_referent_chain_reads_the_pin_source(kind, tmp_path, adoptable): - """A `--referent-chain` link with no keyed source reads the bare + """A `--referent-chain`'s FIRST link with no keyed source reads the bare `--pin-source` (`link_pins_from_trees`), so a chain is a reader: with no `--pin`, the refusal is the chain's own, which names the `--pin` the chain begins at, and not `pin-source-unused`.""" From 3c0da258eccb51c36982b39dabb6d3f74a7b1b66 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 20:08:08 +0000 Subject: [PATCH 09/10] adopt-project.py: `plan` records `local-clone ()` in the plan again and `execute` writes `local-clone`, so #449's `check` recomputes each pin in its clone and still no committed pin file holds the path (#540) #589 (#449, on main since a61cf47) makes `check` recompute each pin's digest in the clone `plan` read it from, which it finds in the plan's `digest_source: local-clone ()`. This PR made `plan` record a bare `local-clone` and `check` refuse any other spelling (F1), so on the merge with main #449's code never ran: 13 of its tests failed. Both are kept: - The plan records the path again, `local-clone ()`, as main does (`_as_planned`). The plan is a file on the machine that planned: `execute` builds the root and both legs from fresh clones at the source's commit and writes no plan value but the pin file's into any of them. - Every committed or printed value is bare. `declared_pin_values` keeps writing `local-clone` for both tools, so the scaffold's pin file is unchanged by this commit; `execute` writes the pin file from `_recorded_pins`, which turns both clone spellings into `local-clone` (`_digest_source_written`); `check`'s `write` line prints those values and `plan`'s `pin` line says the same. - F1 takes a closed set of three spellings (`_a_digest_source`): `gh-api-tree-recursive`, `local-clone`, and `local-clone ()` as `LOCAL_CLONE_RE` fullmatches it, on one printable line. Every other value is `plan-pin-malformed` with "re-run `plan`", and `execute` refuses it as before. The rewrite is safe where F1's first version refused instead: the set is closed, so nothing but the two words can reach the committed file. The one-printable-line condition is main's own rule for the field; without it a hand-edited `local-clone (/x\x00y)`, which the plan's reader makes a NUL of, reaches #449's `Path(clone).stat()` and raises `ValueError`, a traceback. - `_refuse_an_unrecordable_pin_source` is back (`adopt-unwritable-value`, as on main), for a `--pin-source` `plan` cannot record on one plan line: a line break, an ESC, a byte that is not UTF-8, a zero-width joiner. Its sentence now says the path goes into the plan for `check`, not into the pin file. The scaffold records no path, so it still scaffolds from such a clone (#450). - `--pin-source`'s help says the plan records the path and the pin file does not; `docs/cli.md` is regenerated. #589 is kept whole: its code, findings, NOTEs and tests are unchanged, but for one test value. `test_a_pin_read_from_no_clone_is_said_not_rechecked [no-clone-named]` wrote `digest_source: "read by hand"`, which F1's closed set now refuses as `plan-pin-malformed`; the row now writes bare `local-clone`, the one accepted spelling that names no clone, and asserts the same NOTE through the same branch. `read by hand` joins the malformed table. Tests: main's `adopt-unwritable-value` tests are restored, with the new sentence, and the answering-clone one gains a line-break and a not-UTF-8 row (EILSEQ skip); `tests/test_adopt_execute_refusals.py` and `tests/test_adopt_root_symlinks.py` are main's again. The plan-records assertions expect `local-clone ()`; the `pin` and `write` lines expect `(local-clone)`; a new end-to-end test greps `git log -p --all` of the root, both legs and the scaffold's three repositories for the path; the scaffold's test greps its three. F1's table gains a bare path, `local-clone()` with no space, `read by hand` and a NUL clone, and loses `source-old-plan`, which passes now. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 165 ++++++++++++---- docs/cli.md | 12 +- scripts/shape_materialize.py | 4 + tests/test_adopt_execute_refusals.py | 2 +- tests/test_adopt_pins.py | 280 ++++++++++++++++++++------- tests/test_adopt_root_symlinks.py | 2 +- tests/test_pin_source_unreadable.py | 50 +++-- 7 files changed, 383 insertions(+), 132 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index e0d209d6..56ebad87 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -256,12 +256,17 @@ #: machine, and is no partial clone, `check` and `execute` recompute the #: digest there with `plan`'s own reader (`pin_digest_from_source`), which #: in such a clone reads no network, and find a commit or a digest edited -#: after `plan` read them (#449). +#: after `plan` read them (#449). So the plan, a file on this machine that +#: nothing commits, records where that clone is, `local-clone ()` +#: (`_as_planned`); the pin file `execute` commits says only how the digest +#: was read, `local-clone` or `gh-api-tree-recursive`, and never where +#: (`_recorded_pins`, #540). from shape_materialize import ( # noqa: E402 DIGEST_FROM_CLONE, DIGEST_FROM_GH, PIN_ARG_RE, declared_pin_products, - declared_pin_values, neutral_pin_path, pin_collisions, - pin_digest_from_source, pin_source_path, pin_variable_collisions, - refuse_an_unread_pin_source, refuse_colliding_pins, + declared_pin_values, neutral_pin_path, one_printable_line, + pin_collisions, pin_digest_from_source, pin_source_path, + pin_variable_collisions, refuse_an_unread_pin_source, + refuse_colliding_pins, ) #: How `plan`'s refusals of a `--pin-source` begin their remediation. PLAN_NOT_WRITTEN = "no plan was written." @@ -286,7 +291,7 @@ #: otherwise, or cannot answer for at all, at the plan's commit (#449). PIN_DIGEST_MISMATCH = "plan-pin-digest-mismatch" PIN_SOURCE_UNREADABLE = "plan-pin-source-unreadable" -#: How `declared_pin_values` records where it read a digest: from a clone on +#: How `plan` records where it read a digest (`_as_planned`): from a clone on #: this machine, `--pin-source` as typed, or from the forge. LOCAL_CLONE_RE = re.compile(r"local-clone \((?P.+)\)") GH_DIGEST_SOURCE = "gh-api-tree-recursive" @@ -344,16 +349,20 @@ def _pin_row_problem(row: dict) -> tuple[str, str] | None: 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 `DIGEST_FROM_CLONE` or - `DIGEST_FROM_GH`. - - THE DIGEST'S SOURCE IS ONE OF THE TWO WORDS `plan` WRITES (#540). It was - held to one printable line, so a plan from a `plan` before #540, which - recorded `local-clone ()`, or a hand edit, passed - `check`, and `execute` committed that path into the root's pin file, the - directory a clone sat in on the machine that planned. `execute` writes - the field as the plan holds it, so a value `plan` never writes is - `plan-pin-malformed`, with the `plan` that records the pin whole.""" + `SHA256_RE`, and the digest's source to `_a_digest_source`. + + THE DIGEST'S SOURCE IS ONE OF THREE SPELLINGS (#540). It was held to one + printable line, and `execute` wrote it into the root's pin file as it + stood, so `plan`'s `local-clone ()` committed the + directory a clone sat in on the machine that planned, and a hand edit + could commit any line at all. The plan still records the path, because + `check` recomputes the digest in that clone (#449), but the field is + now held to a closed set, and `execute` writes the pin file's from it + (`_digest_source_written`): `gh-api-tree-recursive` as it is, and + `local-clone` for both `local-clone` and `local-clone ()`. So no + value of the plan's reaches the committed file but those two words, and + any other is `plan-pin-malformed`, with the `plan` that records the pin + whole.""" 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: @@ -381,14 +390,40 @@ def _pin_row_problem(row: dict) -> tuple[str, str] | None: if not SHA256_RE.match(digest): return PIN_MALFORMED, (f"records `tree_sha256: {digest!r}`, which is " "not 64 hex") - if how not in (DIGEST_FROM_CLONE, DIGEST_FROM_GH): + if not _a_digest_source(how): return PIN_MALFORMED, ( - f"records `digest_source: {how!r}`, and `plan` records only how " - f"it read the digest, `{DIGEST_FROM_CLONE}` or " - f"`{DIGEST_FROM_GH}`, and never where") + f"records `digest_source: {how!r}`, and `plan` records " + f"`{DIGEST_FROM_GH}`, or `{DIGEST_FROM_CLONE} ()` on one printable line, and nothing else") return None +def _a_digest_source(how: str) -> bool: + """Whether a pin's `digest_source:` is one `check` passes (#540): one + of the two words the pin file says, `gh-api-tree-recursive` and + `local-clone`, or `local-clone ()` as `LOCAL_CLONE_RE` reads it, + the clone `check` recomputes the digest in (#449). + + THE PATH IS HELD TO ONE PRINTABLE LINE, as the whole field was before + #540, and as `plan` holds `--pin-source` before it records it + (`_refuse_an_unrecordable_pin_source`). `LOCAL_CLONE_RE` takes any + character but a line break, a NUL among them, and the plan's reader + makes one of the escape `\\x00`; `Path.stat` raises `ValueError` on + such a path, where `_why_not_rechecked` waits for an `OSError`.""" + if how in (DIGEST_FROM_CLONE, DIGEST_FROM_GH): + return True + return (LOCAL_CLONE_RE.fullmatch(how) is not None + and one_printable_line(how)) + + +def _digest_source_written(how: str) -> str: + """What the pin file `execute` commits says for a `digest_source:` + `check` passed (`_a_digest_source`): how the digest was read, and never + where (#540). `check`'s `write` line and `plan`'s `pin` line say the + same, so neither prints a clone's path as what is written.""" + return DIGEST_FROM_CLONE if LOCAL_CLONE_RE.fullmatch(how) else how + + 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.""" @@ -476,9 +511,14 @@ def _declared_pins(plan: "Plan") -> list[str]: 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]} + `declared_pin_values` hands it the scaffold's: the digest's source as + the pin file says it, `local-clone` and no clone's path + (`_digest_source_written`, #540). Read only after `execute` has refused + every finding `_read_pins` makes. `check`'s `write` line prints these + values, so it says what is written.""" + return {product: {**values, "PIN_DIGEST_SOURCE": _digest_source_written( + values["PIN_DIGEST_SOURCE"])} + for _, product, values in _read_pins(plan)[0]} def _pin_files(plan: "Plan") -> list[tuple[str, str | None]]: @@ -3258,8 +3298,10 @@ 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) names = _names(args.project) - pins = declared_pin_values(args) + pins = _as_planned(declared_pin_values(args), args.pin_source) _check_names(naming, names, set(pins), source) # LAST here, over every value as the plan will record it: `execute` # writes them into the project's own YAML. The project's own validator @@ -3291,6 +3333,59 @@ def _refuse_misread_values(args, given: set, pins: dict, source: Source, f"{MANIFEST_MISREAD_REMEDIATION} No plan was written.") +def _refuse_an_unrecordable_pin_source(source: Path) -> 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:` in the plan, so + that `check` can recompute the digest in that clone (#449), and `check` + holds it to `one_printable_line` (`_a_digest_source`). 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 `_as_planned` puts around the path are printable, so the path alone + decides. + + THE PIN FILE NEVER HOLDS THE PATH (#540): `execute` writes `local-clone` + there, so this is about the plan alone, and the scaffold, which records + no path, scaffolds from such a clone (#450). + """ + text = str(source) + if one_printable_line(text): + return + reason = (unwritable_reason(text) + or "it holds an invisible formatting character") + raise Refusal( + "adopt-unwritable-value", + f"--pin-source is {text!r}, which `plan` records in each pin's " + "`digest_source:` in the plan, for `check` to recompute the digest " + f"in that clone, 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`.") + + +def _as_planned(pins: dict, pin_source: Path | None) -> dict: + """`declared_pin_values`'s pins as `plan` records them: a digest read + from a `--pin-source` clone records where, `local-clone ()`, so that `check` and `execute` recompute it in that clone + (#449). `declared_pin_values` says `local-clone` alone, as the pin file + does (#540): the plan is a file on this machine, which nothing commits, + and the pin file is the one written from it (`_recorded_pins`).""" + if pin_source is None: + return pins + where = f"{DIGEST_FROM_CLONE} ({pin_source})" + return {product: {**values, "PIN_DIGEST_SOURCE": where} + for product, values in pins.items()} + + # --------------------------------------------------------------------------- # The project must be able to hold what `execute` writes into it (#261) # --------------------------------------------------------------------------- @@ -3586,13 +3681,14 @@ 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), that last `_shown`, as every path this report prints is. + # read (#273), as that file will say it: `local-clone`, and not the + # clone's path, which the plan records beside it for `check` (#540). 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)} " - f"({_shown(values['PIN_DIGEST_SOURCE'])})") + f"({_digest_source_written(values['PIN_DIGEST_SOURCE'])})") _print_triad_advisory(source.path, source.commit) _print_entries(entries) _print_summary(entries) @@ -6107,7 +6203,9 @@ def _print_what_will_happen(plan: Plan, source: Source, names: dict, f"legs{mounted}") 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 + # Each pin `execute` writes, as the plan records it (#273) and as the + # pin file will say it: `local-clone`, and not the clone's path the + # plan records beside it (`_recorded_pins`, #540). A pin it # cannot write is a finding, printed after this. Whether its digest was # recomputed is said once per pin with the NOTEs `_entry_findings` # returns: it recomputes, or it was not rechecked here, where the clone @@ -10129,13 +10227,14 @@ def main(argv: list[str] | None = None) -> int: plan.add_argument("--pin-source", default=None, help="a local clone to compute --pin digests from, " "instead of reading the forge with `gh api` (the " - "TEST path; no network). The plan and the pin " - "file say only that the digest was read from " - "`local-clone`, never the path. Given with no " - "--pin to read it, the flag is refused as " - "`pin-source-unused`; an empty value is refused " - "as `pin-source-no-path`, and `.` means this " - "directory.") + "TEST path; no network). The plan records the " + "path, which must be one printable line, so that " + "`check` recomputes each digest there; the pin " + "file says only `local-clone`, never the path. " + "Given with no --pin to read it, the flag is " + "refused as `pin-source-unused`; an empty value " + "is refused as `pin-source-no-path`, and `.` " + "means this directory.") 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 25e1a1df..c93eeaa5 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -483,11 +483,13 @@ options: --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). The plan and the pin file say only that the - digest was read from `local-clone`, never the path. - Given with no --pin to read it, the flag is refused as - `pin-source-unused`; an empty value is refused as - `pin-source-no-path`, and `.` means this directory. + network). The plan records the path, which must be one + printable line, so that `check` recomputes each digest + there; the pin file says only `local-clone`, never the + path. Given with no --pin to read it, the flag is + refused as `pin-source-unused`; an empty value is + refused as `pin-source-no-path`, and `.` means this + directory. --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/scripts/shape_materialize.py b/scripts/shape_materialize.py index 654e6811..55ac97dd 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -2639,6 +2639,10 @@ def declared_pin_values(args) -> dict[str, dict[str, str]]: byte that is not UTF-8, after all three repositories existed. What the field is for is which of the two ways the digest was read, so that is what it says; the refusals above still name the path the run was given. + `adopt-project.py plan` records the path in its plan beside these + values, `local-clone ()`, so that `check` recomputes the digest in + that clone (#449): the plan is a file on that machine, which nothing + commits, and `execute` writes the pin file's field as this says it. """ pins: dict[str, dict[str, str]] = {} for raw in args.pin: diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index c6599625..7c767af6 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -1009,7 +1009,7 @@ def a_whole_pin(product: str) -> str: f" repository: opensoft/{product}\n" f' commit: "{"a" * 40}"\n' f' tree_sha256: "{"b" * 64}"\n' - ' digest_source: "local-clone"\n') + f' digest_source: "local-clone ({product})"\n') #: What `check` and `execute` still take, by test id: the key, the block diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index d446cc59..7c741e7d 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -159,7 +159,7 @@ def plan_with(source: Path, out: Path, *pins: str, extra: tuple = (), def pin_entry(product: str = "openGlass", repository: str = "opensoft/openGlass", commit: str = "", - digest: str = "", source: str = "local-clone", + 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.""" @@ -255,11 +255,11 @@ def test_plan_records_the_pin_whole(adopted, glass): "repository": adopted["repository"], "commit": adopted["commit"], "tree_sha256": tree_digest(glass[0], adopted["commit"]), - "digest_source": "local-clone", + "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 + assert f"-> {PIN_FILE} (local-clone)\n" in adopted["planned"].stdout @needs_filter_repo @@ -300,6 +300,33 @@ def test_the_split_holds_the_pin_the_scaffold_writes(adopted): assert assembly["naming"]["referent_declared"] is True +@needs_filter_repo +def test_no_committed_object_holds_the_clone_s_path(adopted): + """#540: the plan records where the clone `plan` read the digest from, + `local-clone ()`, so that `check` can recompute it there (#449), + and it is the only file that does. The pin file `execute` commits says + `digest_source: local-clone`, and no commit of any repository the + adoption made or pushed to -- the root, which holds the split, and each + leg -- nor of the scaffold's three, holds the path or its leaf, by + `git log -p --all`, read as bytes.""" + readable = adopted["readable"] + assert f"local-clone ({readable})" in \ + adopted["plan"].read_bytes().decode(UTF8) + split = git("show", f"{BRANCH}:{PIN_FILE}", + cwd=adopted["source"]).stdout + assert "\ndigest_source: local-clone\n" in split, split + legs = sorted((adopted["base"] / "remotes").glob("*.git")) + scaffolded = sorted((adopted["base"] / "scaffold-remotes").glob("*.git")) + assert (len(legs), len(scaffolded)) == (2, 3), (legs, scaffolded) + for repository in (adopted["source"], *legs, *scaffolded): + history = subprocess.run(["git", "log", "-p", "--all"], + cwd=repository, capture_output=True, + check=True).stdout + for spelled in (os.fsencode(str(readable)), + os.fsencode(readable.name)): + assert spelled not in history, (repository, spelled) + + @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 @@ -763,17 +790,29 @@ def test_with_no_gh_on_path_a_pin_is_gh_not_found(kind, source_repo, assert not [path for path in made if path.exists()] -# --- a `--pin-source` is read, and not recorded (#540, #450) ----------------- +# --- a `--pin-source` the plan records, and no committed file does (#540) --- # -# `plan` recorded the path in each pin's `digest_source:` as it was typed, -# `local-clone ()`, and `execute` committed it into the pin file, so an -# absolute `--pin-source` put the directory a clone sat in on this machine -# into the adopted root. Since #273's review (N2), `plan` refused, before any -# digest was read, a path `check` could not hold on that line: a line break, -# an ESC, a byte that is not UTF-8, a zero-width joiner. The plan now records -# `local-clone` alone, so a clone is read wherever it sits, and a path that -# cannot answer is `pin-source-unreadable`, named spelled, as the scaffold -# names it (`tests/test_pin_source_unreadable.py`). +# `plan` records the path in each pin's `digest_source:` as it was typed, +# `local-clone ()`, so that `check` can recompute the digest in that +# clone (#449), and `check` holds that to one printable line. `execute` +# committed it into the pin file as it stood, so an absolute `--pin-source` +# put the directory a clone sat in on this machine into the adopted root; it +# now writes `local-clone` there, and `plan`'s `pin` line and `check`'s +# `write` line say what it writes. 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 (#273's review, N2). `plan` 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`.") +#: What `adopt-unwritable-value` says `plan` does with the path. +UNRECORDABLE_USE = ("which `plan` records in each pin's `digest_source:` in " + "the plan, for `check` to recompute the digest in that " + "clone") #: A command line and a file name on Windows hold none of these. POSIX_NAMES = pytest.mark.skipif( @@ -781,6 +820,36 @@ def test_with_no_gh_on_path_a_pin_is_gh_not_found(kind, source_repo, 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" +NOT_UTF_8 = "it holds a byte that is not UTF-8" + + +@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", 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}, " + f"{UNRECORDABLE_USE}, and {reason}.\n{UNRECORDABLE_REMEDIATION}\n") + assert not out.exists() + assert gh_calls(gh_env) == "" + + #: The leaf of each `--pin-source` below, spelled so that a plan, a report #: and a refusal can be searched for it: none holds the word otherwise. UNREAD = "unreadclone" @@ -802,26 +871,51 @@ def skip_where_a_name_cannot_be_not_utf_8(directory: Path) -> None: probe.rmdir() +@pytest.mark.parametrize("name, reason", [ + pytest.param(f"{UNREAD}‍x", + "it holds an invisible formatting character", id="zwj"), + pytest.param(f"{UNREAD}\nx", LINE_BREAK, id="lf", marks=POSIX_NAMES), + pytest.param(f"{UNREAD}\udcffx", NOT_UTF_8, id="not-utf-8", + marks=POSIX_NAMES), +]) +def test_a_clone_at_a_path_check_would_refuse_is_refused_by_plan( + source_repo, tmp_path, glass, gh_env, name, reason): + """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. A line break and a byte + that is not UTF-8 are #450's two, which the scaffold, recording no path, + scaffolds from (`tests/test_pin_source_unreadable.py`).""" + path, commit, _ = glass + if "\udcff" in name: + skip_where_a_name_cannot_be_not_utf_8(tmp_path) + clone = tmp_path / name + 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 == ( + f"REFUSED adopt-unwritable-value: --pin-source is {str(clone)!r}, " + f"{UNRECORDABLE_USE}, and {reason}.\n{UNRECORDABLE_REMEDIATION}\n") + assert not out.exists() + assert gh_calls(gh_env) == "" + + @pytest.mark.parametrize("name", [ - pytest.param(f"{UNREAD}\x1bx", id="esc", marks=POSIX_NAMES), - pytest.param(f"{UNREAD}\nx", id="lf", marks=POSIX_NAMES), - pytest.param(f"{UNREAD}\rx", id="cr", marks=POSIX_NAMES), - pytest.param(f"{UNREAD}\x1b[31mRED\nREFUSED forged-line: not the " - "tool's\r", id="forged-line", marks=POSIX_NAMES), - pytest.param(f"{UNREAD}\udcffx", id="not-utf-8", marks=POSIX_NAMES), - pytest.param(f"{UNREAD}‍x", id="zwj"), + pytest.param(UNREAD, id="printable"), pytest.param(f"{UNREAD} x", id="no-break-space"), ]) -def test_a_clone_is_read_wherever_it_is_and_its_path_is_not_recorded( +def test_a_clone_the_plan_records_is_printed_as_how_it_was_read( source_repo, tmp_path, glass, gh_env, name): - """A clone that answers, at each path #273's review refused: planned, - exit 0, with the digest it gives, and `digest_source: local-clone`. No - spelling of the path is in the plan, which `execute` writes the pin - file from, or in the report, whose `pin` line says how the digest was - read and nothing more; so nothing of it reaches the terminal raw.""" + """A clone that answers, at a path that is one printable line for + `check` -- a no-break space is one, and is not `str.isprintable` -- + is planned, exit 0, with the digest it gives. The plan records the path + as typed, `local-clone ()`, for `check` to recompute the digest + there (#449); the report's `pin` line says what the pin file will, how + the digest was read and not where (#540), so no spelling of the path + reaches the terminal.""" path, commit, _ = glass - if "\udcff" in name: - skip_where_a_name_cannot_be_not_utf_8(tmp_path) clone = tmp_path / name shutil.copytree(path, clone, symlinks=True) out = tmp_path / "plan.yaml" @@ -831,9 +925,9 @@ def test_a_clone_is_read_wherever_it_is_and_its_path_is_not_recorded( digest = tree_digest(path, commit) pin = load_yaml(out)["pins"][0] assert (pin["tree_sha256"], pin["digest_source"]) \ - == (digest, "local-clone") + == (digest, f"local-clone ({clone})") said = result.stdout + result.stderr - assert UNREAD not in out.read_bytes().decode(UTF8) + said, said + assert UNREAD not in said, said assert [line for line in result.stdout.splitlines() if line.startswith("pin ")] == [ f"pin opensoft/openGlass @ {commit[:12]} tree {digest[:12]}… " @@ -911,48 +1005,57 @@ def test_an_unread_pin_source_is_refused_before_anything_exists( assert gh_calls(gh_env) == "" -def test_check_finds_a_recorded_source_that_names_a_path( +def test_check_passes_the_clone_a_plan_records_and_writes_no_path( source_repo, tmp_path, glass, gh_env): - """A plan `plan` wrote before #540, or one edited since, can still hold - a path in `digest_source:`. `check` held the field to one printable - line, so it passed such a plan and printed the path in its `write` - line, and `execute` committed the path into the pin file (rv540 F1). - The field is held to the two words `plan` writes, so `check` finds the - path `plan-pin-malformed`, spelled where it is not `str.isprintable` (a - no-break space), as every value a finding names is, with the `plan` - that records the pin whole; `execute` refuses it before any leg exists - (`source-old-plan` in the table below).""" + """`local-clone ()` is what `plan` records for a clone (#449), so + `check` passes it, whatever the path, on one printable line: here a + no-break space, which is not `str.isprintable`. Its `write` line says + what `execute` writes into the pin file, `local-clone` (#540); the one + line that names the clone is #449's NOTE, which spells it, as every + path `check` prints is (`source-old-plan` was a finding while the + field was held to the two words; the table below holds the values it + still finds).""" path, commit, _ = glass + digest = tree_digest(path, commit) plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, "pins:\n" + pin_entry( - commit=commit, digest=tree_digest(path, commit), + commit=commit, digest=digest, source="local-clone (open\\u00a0Glass)")) checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) - assert checked.returncode == 1, checked.stdout + checked.stderr - found = [line for line in checked.stderr.splitlines() - if line.startswith("FINDING ")] - assert len(found) == 1, checked.stderr - assert found[0].startswith( - "FINDING plan-pin-malformed: `pins:` entry 1 ('openGlass') records " - "`digest_source: 'local-clone (open\\xa0Glass)'`, " - f"{HOW_NOT_WHERE}. `execute` writes the pin file from the commit and " - "the digest `plan` records, and nothing else: re-run `plan` with " - "`--pin opensoft/openGlass@<40 hex commit>`"), found[0] - assert "\u00a0" not in checked.stdout + checked.stderr + 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)"], checked.stdout + notes = pin_notes(checked) + assert len(notes) == 1, checked.stdout + assert notes[0].startswith(NOT_RECHECKED), notes + assert "open\\xa0Glass" in notes[0], notes + assert " " not in checked.stdout + checked.stderr assert gh_calls(gh_env) == "" -def test_neither_report_names_the_clone(source_repo, tmp_path, glass, gh_env): - """`plan`'s `pin` line and `check`'s `write` line for the plan `plan` - wrote say how the digest was read, `local-clone`, and not where: the - path is this machine's own, and `execute` writes into the pin file - what `check` prints there (#540).""" +def test_the_pin_and_write_lines_say_how_and_not_where(source_repo, tmp_path, + glass, gh_env): + """For the plan a real `plan --pin-source ` wrote, which records + the clone: `plan`'s `pin` line and `check`'s `write` line say how the + digest was read, `local-clone`, and not where, because the path is this + machine's own and `execute` writes into the pin file what `check` + prints there (#540). The one line of either that names the clone is + #449's NOTE that the digest recomputes there.""" 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 + assert load_yaml(out)["pins"][0]["digest_source"] \ + == f"local-clone ({path})" + assert [line for line in result.stdout.splitlines() + if line.startswith("pin ")] == [ + f"pin opensoft/openGlass @ {commit[:12]} tree {digest[:12]}… " + f"-> {PIN_FILE} (local-clone)"], result.stdout + assert str(path) not in result.stdout + result.stderr for answered, leg in ANSWERS: resolve(out, answered, leg) checked = run_script(ADOPT, "check", "--plan", str(out), env=gh_env) @@ -961,7 +1064,11 @@ def test_neither_report_names_the_clone(source_repo, tmp_path, glass, gh_env): if line.startswith(" write ")] == [ f" write {PIN_FILE}: opensoft/openGlass @ {commit[:12]} tree " f"{digest[:12]}… (local-clone)"], checked.stdout - assert str(path) not in result.stdout + checked.stdout + assert [line for line in checked.stdout.splitlines() + if str(path) in line] == [ + f"NOTE {ENTRY}: digest recomputes at {commit[:12]} in the clone " + f"`plan` read it from, {path}."], checked.stdout + assert str(path) not in checked.stderr assert gh_calls(gh_env) == "" @@ -1010,10 +1117,12 @@ def unrecorded(glass) -> dict: } -#: How `check` ends its finding on a `digest_source:` that `plan` never -#: writes (#540). -HOW_NOT_WHERE = ("and `plan` records only how it read the digest, " - "`local-clone` or `gh-api-tree-recursive`, and never where") +#: How `check` ends its finding on a `digest_source:` outside the three +#: spellings it passes (#540): `local-clone`, `gh-api-tree-recursive`, and +#: `local-clone ()`, the clone `plan` read the digest from (#449). +HOW_NOT_WHERE = ("and `plan` records `gh-api-tree-recursive`, or " + "`local-clone ()` on one printable line, " + "and nothing else") def malformed(glass) -> dict: @@ -1024,7 +1133,7 @@ def malformed(glass) -> dict: entry = "`pins:` entry 1 ('openGlass') records" # `as_posix`, so that a Windows path's backslashes are no escapes in the # plan's double-quoted value. - old = f"local-clone ({path.as_posix()})" + bare, glued = path.as_posix(), f"local-clone({path.as_posix()})" return { "not-a-list": ("pins: openGlass\n", "`pins:` is 'openGlass', not a list of pins"), @@ -1049,27 +1158,47 @@ 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). + # `execute` wrote `digest_source:` into the pin file bare, so a + # line break there was a line of the file (N3); `LOCAL_CLONE_RE` + # reads no clone across one. "source-line-break": ("pins:\n" + pin_entry( commit=commit, digest=digest, source="local-clone (/x\\nforged: y)"), f"{entry} `digest_source: 'local-clone " f"(/x\\nforged: y)'`, {HOW_NOT_WHERE}"), - # What `plan` recorded before #540: the `--pin-source` path, which - # `execute` committed into the pin file (rv540 F1). One printable - # line, so `check` passed it. - "source-old-plan": ("pins:\n" + pin_entry( - commit=commit, digest=digest, source=old), - f"{entry} `digest_source: {old!r}`, " + # The clone's path alone, and the clone with no space before its + # path: neither is a spelling `plan` writes, and `LOCAL_CLONE_RE` + # reads no clone in either, so #449's recompute would not run. + "source-bare-path": ("pins:\n" + pin_entry( + commit=commit, digest=digest, source=bare), + f"{entry} `digest_source: {bare!r}`, " + f"{HOW_NOT_WHERE}"), + "source-no-space": ("pins:\n" + pin_entry( + commit=commit, digest=digest, source=glued), + f"{entry} `digest_source: {glued!r}`, " f"{HOW_NOT_WHERE}"), + # Any other text, which #449's `check` NOTEd as naming no clone + # while the field was held to one printable line alone. + "source-by-hand": ("pins:\n" + pin_entry( + commit=commit, digest=digest, + source="read by hand"), + f"{entry} `digest_source: 'read by hand'`, " + f"{HOW_NOT_WHERE}"), + # A clone's path the plan's reader makes a NUL of: `LOCAL_CLONE_RE` + # reads it, and `Path.stat` raises `ValueError` on it, so it is held + # to one printable line, as `plan` holds `--pin-source`. + "source-clone-nul": ("pins:\n" + pin_entry( + commit=commit, digest=digest, + source="local-clone (/x\\x00y)"), + f"{entry} `digest_source: 'local-clone " + f"(/x\\x00y)'`, {HOW_NOT_WHERE}"), # 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"\n', + ' digest_source: "local-clone (openGlass)"\n', f"{entry} `commit: 1234567`, which is not text"), } @@ -1126,7 +1255,8 @@ def cases(glass): CASE_IDS += [("plan-pin-malformed", name) for name in ( "not-a-list", "a-number", "short-commit", "short-digest", "another-product", "no-owner", "no-source", "source-line-break", - "source-old-plan", "a-number-commit")] + "source-bare-path", "source-no-space", "source-by-hand", + "source-clone-nul", "a-number-commit")] CASE_IDS += [("plan-pin-collides", name) for name in ("case-pair", "shape", "variable-pair")] @@ -1331,8 +1461,8 @@ def test_a_pin_whose_clone_is_gone_is_said_not_rechecked(source_repo, pytest.param("gh-api-tree-recursive", "`plan` read it with `gh api`, and `check` reads no network", id="gh-api"), - pytest.param("read by hand", - "`digest_source: read by hand` names no clone `plan` read it " + pytest.param("local-clone", + "`digest_source: local-clone` names no clone `plan` read it " "from", id="no-clone-named"), ]) def test_a_pin_read_from_no_clone_is_said_not_rechecked(source_repo, diff --git a/tests/test_adopt_root_symlinks.py b/tests/test_adopt_root_symlinks.py index 71cfcfe3..2e566ee5 100644 --- a/tests/test_adopt_root_symlinks.py +++ b/tests/test_adopt_root_symlinks.py @@ -811,7 +811,7 @@ def pin_row(product: str, commit: bool = True) -> str: f" repository: opensoft/{product}", f" commit: \"{'0123456789abcdef' * 2}01234567\"", f" tree_sha256: \"{'0123456789abcdef' * 4}\"", - " digest_source: \"local-clone\""] + f" digest_source: \"local-clone ({product})\""] return "\n".join(lines if commit else lines[:2] + lines[3:]) + "\n" diff --git a/tests/test_pin_source_unreadable.py b/tests/test_pin_source_unreadable.py index a71751d6..177e8c94 100644 --- a/tests/test_pin_source_unreadable.py +++ b/tests/test_pin_source_unreadable.py @@ -297,16 +297,19 @@ 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. `adopt-project.py plan` refused that name - as a value it could not record (#273's review, N2) while it recorded - the path; it records none since #540, so it is read, and refused, as - the scaffold refuses it.""" + 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) == [] @@ -356,17 +359,22 @@ def assert_one_refusal_of_the_tools(result: subprocess.CompletedProcess, @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, in one refusal, `pin-source-unreadable`. - `adopt-project.py plan` refused such a path before it read a digest, as - a value it could not record (#273's review, N2); it records no path - since #540, so it reads this one and says what the scaffold says.""" + """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) - 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") + 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) == [] @@ -432,7 +440,7 @@ def test_a_source_that_answers_is_read_as_before(kind, spelling, tmp_path, pin = load_yaml(tmp_path / "plan.yaml")["pins"][0] assert pin["commit"] == commit assert pin["tree_sha256"] == digest - assert pin["digest_source"] == "local-clone" + assert pin["digest_source"] == f"local-clone ({typed})" else: assert f"tree {digest[:12]}" in result.stdout, result.stdout assert " (local-clone)\n" in result.stdout, result.stdout @@ -604,9 +612,10 @@ def skip_where_no_file_name_is_not_utf_8(directory: Path) -> None: def test_the_pin_file_says_how_the_digest_was_read_and_not_where( name, tmp_path, adoptable, glass): """A real scaffold from a clone at each path: exit 0, the plan's `pin` - line ending `(local-clone)`, and the pin file the root commits, read - from git, holding `digest_source: local-clone` and no byte of the - clone's directory.""" + line ending `(local-clone)`, the pin file the root commits, read from + git, holding `digest_source: local-clone`, and no commit of the three + repositories holding a byte of the clone's directory (`git log -p + --all`, read as bytes).""" product, commit = glass if "\udcff" in name: skip_where_no_file_name_is_not_utf_8(tmp_path) @@ -626,3 +635,10 @@ def test_the_pin_file_says_how_the_digest_was_read_and_not_where( assert os.fsencode(str(clones)) not in blob, blob assert parse_yaml(blob.decode("utf-8"))["digest_source"] == "local-clone" assert b"\ndigest_source: local-clone\n" in blob, blob + repositories = sorted((tmp_path / "remotes").glob("*.git")) + assert len(repositories) == 3, repositories + for repository in repositories: + history = subprocess.run(["git", "log", "-p", "--all"], + cwd=repository, capture_output=True, + check=True).stdout + assert os.fsencode(str(clones)) not in history, repository From 8af10e32914219ce458436b58a72ae477675188f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 21:01:56 +0000 Subject: [PATCH 10/10] adopt-project.py: `plan` refuses a `--pin-source` it cannot record as `adopt-unwritable-value` straight after the two flag-only refusals, so a path holding a line break, an ESC, a zero-width joiner or a byte that is not UTF-8 no longer leaves an `openreposhape-adopt-*` directory behind, runs git in the source or `gh repo clone`s an `org/repo` one first, and `--help` names the refusal (#450) rv540c's fix list on PR #570, items 1, 2 and 4. The restored refusal (`_refuse_an_unrecordable_pin_source`) is asked from `cmd_plan` right after `refuse_an_unread_pin_source`, guarded by `args.pin and args.pin_source is not None`, and no longer inside `_checked_plan_inputs`: it needs the flags alone, and rv540's F2 moved `pin-source-unused` ahead of `_work_root` and `Source.open` for exactly that reason. By the time `_checked_plan_inputs` asked it, the work directory had been made, git had run in a local source (4 `GIT_TRACE` lines), and an `org/repo` source had been `gh repo clone`d, so a `gh` that fails answered `source-unclonable` in its place. The order changes on one point: a malformed `--pin` (or any refusal `_checked_plan_inputs` asks before the old call site: `--reference` or `--elected-by` unwritable, `adopt-pin-collides`, a source that cannot be opened) given with such a path now gets `adopt-unwritable-value` first. No test held the old order; the new rows hold the new one. `test_a_pin_source_refused_by_its_flags_is_refused_before_anything_exists` (was `test_an_unread_pin_source_is_refused_before_anything_exists`) gains the rows `lf`, `not-utf-8`, `zwj` and `lf-and-a-malformed-pin` beside `unused`, over the `org/repo`, `local` and `local-work-dir` sources: a fresh `TMPDIR` that stays empty, a `GIT_TRACE` file that is never written, no work directory, no plan and no `gh` call. The comment above the `--pin-source` tests ("nothing is made and nothing is read") is true as it stands. adopt's `--pin-source` help says a path that is not one printable line is refused as `adopt-unwritable-value`; docs/cli.md is regenerated (`scripts/render-cli-reference.py --check` ok). Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 33 ++++++++++++++++++--------------- docs/cli.md | 14 +++++++------- tests/test_adopt_pins.py | 40 +++++++++++++++++++++++++++------------- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 8e9fc01d..dbcaa909 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -3302,8 +3302,6 @@ 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) names = _names(args.project) pins = _as_planned(declared_pin_values(args), args.pin_source) _check_names(naming, names, set(pins), source) @@ -3393,8 +3391,10 @@ def _refuse_misread_values(args, given: set, pins: dict, source: Source, def _refuse_an_unrecordable_pin_source(source: Path) -> 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. + `adopt-unwritable-value`, before anything is made, before any digest is + read and before a plan is written: `cmd_plan` asks it straight after + `refuse_an_unread_pin_source`, for it needs the flags alone, so no work + directory, git process or `gh repo clone` has come first. `plan` records the path in each pin's `digest_source:` in the plan, so that `check` can recompute the digest in that clone (#449), and `check` @@ -3783,19 +3783,21 @@ def _print_plan_report(args, source: Source, tree: list, entries: list, def cmd_plan(args) -> int: # A `--pin-source` that names no path, `''`, which as a `Path` is the - # directory this runs in, and then one no `--pin` will read, are refused - # FIRST, in the scaffold's words (#451): it names the clone a pin's - # digest is read from, and with no `--pin` it changed nothing and was - # never even asked about. They need the flags alone, so they are asked - # before anything exists and before any process starts. Asked in - # `_checked_plan_inputs`, the second came after `_work_root` had made - # the work directory, which stayed behind, and after `Source.open` had - # run git in the source or `gh repo clone` for an `org/repo` one (rv540 - # F2). + # directory this runs in, one no `--pin` will read, and one `plan` cannot + # record, are refused FIRST, in this order (#451, #450): it names the + # clone a pin's digest is read from, and with no `--pin` it changed + # nothing and was never even asked about. They need the flags alone, so + # they are asked before anything exists and before any process starts. + # Asked in `_checked_plan_inputs`, the second came after `_work_root` had + # made the work directory, which stayed behind, and after `Source.open` + # had run git in the source or `gh repo clone` for an `org/repo` one + # (rv540 F2), and so did the third (rv540c's item 1). args.pin_source = pin_source_path(args.pin_source, nothing_made=PLAN_NOT_WRITTEN) refuse_an_unread_pin_source(args.pin_source, args.pin, nothing_made=PLAN_NOT_WRITTEN) + if args.pin and args.pin_source is not None: + _refuse_an_unrecordable_pin_source(args.pin_source) work_root = _work_root(args) source = Source.open(args.source, work_root) policy = PathPolicy.load(args.path_policy or PATH_POLICY) @@ -10342,8 +10344,9 @@ def main(argv: list[str] | None = None) -> int: help="a local clone to compute --pin digests from, " "instead of reading the forge with `gh api` (the " "TEST path; no network). The plan records the " - "path, which must be one printable line, so that " - "`check` recomputes each digest there; the pin " + "path, so that `check` recomputes each digest " + "there; a path that is not one printable line is " + "refused as `adopt-unwritable-value`. The pin " "file says only `local-clone`, never the path. " "Given with no --pin to read it, the flag is " "refused as `pin-source-unused`; an empty value " diff --git a/docs/cli.md b/docs/cli.md index 5d425bdc..3edf01c4 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -495,13 +495,13 @@ options: --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). The plan records the path, which must be one - printable line, so that `check` recomputes each digest - there; the pin file says only `local-clone`, never the - path. Given with no --pin to read it, the flag is - refused as `pin-source-unused`; an empty value is - refused as `pin-source-no-path`, and `.` means this - directory. + network). The plan records the path, so that `check` + recomputes each digest there; a path that is not one + printable line is refused as `adopt-unwritable-value`. + The pin file says only `local-clone`, never the path. + Given with no --pin to read it, the flag is refused as + `pin-source-unused`; an empty value is refused as + `pin-source-no-path`, and `.` means this directory. --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/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index 7c741e7d..719af0f7 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -970,16 +970,30 @@ def test_a_pin_source_no_pin_reads_is_refused(source_repo, tmp_path, gh_env, pytest.param("local", False, id="local"), pytest.param("local", True, id="local-work-dir"), ]) -def test_an_unread_pin_source_is_refused_before_anything_exists( - source_repo, tmp_path, gh_env, source, work_dir): - """`pin-source-unused` needs the flags alone, so `plan` asks it first, - before anything exists and before any process starts, as the scaffold - does (#451). It was asked after `_work_root` and `Source.open` (rv540 - F2): an `openreposhape-adopt-*` directory was made and left behind, or - the `--work-dir`; git ran in a local source; and an `org/repo` source - was `gh repo clone`d, whose own refusal came instead. Here the - temporary directory is a fresh one, `GIT_TRACE` names a file any git - process would write, and the `gh` stub logs every call.""" +@pytest.mark.parametrize("name, pin, refusal", [ + pytest.param("no-such-clone", "", "pin-source-unused", id="unused"), + pytest.param("no-such\nclone", f"openGlass@{'0' * 40}", + "adopt-unwritable-value", id="lf", marks=POSIX_NAMES), + pytest.param("no-such\udcffclone", f"openGlass@{'0' * 40}", + "adopt-unwritable-value", id="not-utf-8", marks=POSIX_NAMES), + pytest.param("no-such\u200dclone", f"openGlass@{'0' * 40}", + "adopt-unwritable-value", id="zwj"), + # A malformed `--pin` with a path `plan` cannot record is refused for + # the path: it is asked before the `--pin` is (rv540c's item 1). + pytest.param("no-such\nclone", "openGlass", "adopt-unwritable-value", + id="lf-and-a-malformed-pin", marks=POSIX_NAMES), +]) +def test_a_pin_source_refused_by_its_flags_is_refused_before_anything_exists( + source_repo, tmp_path, gh_env, source, work_dir, name, pin, refusal): + """`pin-source-unused` (#451) and `adopt-unwritable-value` (#450) need + the flags alone, so `plan` asks them first, before anything exists and + before any process starts, as the scaffold does. Both were asked after + `_work_root` and `Source.open` (rv540 F2, rv540c's item 1): an + `openreposhape-adopt-*` directory was made and left behind, or the + `--work-dir`; git ran in a local source; and an `org/repo` source was + `gh repo clone`d, whose own refusal came instead. Here the temporary + directory is a fresh one, `GIT_TRACE` names a file any git process would + write, and the `gh` stub logs every call.""" temp, trace, work = (tmp_path / "tmp", tmp_path / "git-trace.log", tmp_path / "work") temp.mkdir() @@ -990,13 +1004,13 @@ def test_an_unread_pin_source_is_refused_before_anything_exists( source_repo), "--project", PROJECT, "--org", ORG, "--elected-by", "Test Human", "--elected-on", "2026-09-02", "--out", str(out), - "--pin-source", str(tmp_path / "no-such-clone"), + "--pin-source", str(tmp_path / name), + *(("--pin", pin) if pin else ()), *(("--work-dir", str(work)) if work_dir else ()), cwd=tmp_path, env={**gh_env, "TMPDIR": str(temp), "GIT_TRACE": str(trace)}) assert result.returncode == 2, result.stdout + result.stderr - assert result.stderr.startswith("REFUSED pin-source-unused: "), \ - result.stderr + assert result.stderr.startswith(f"REFUSED {refusal}: "), result.stderr assert result.stdout == "" assert list(temp.iterdir()) == [] assert not work.exists()