From aed9190bce93dd3242606615038ee65b561e7756 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sun, 11 Oct 2026 22:38:57 +0000 Subject: [PATCH 01/12] adopt-project.py: a value `execute` writes into a file is asked whether a file can hold it, in `plan`, in `check` and in `execute` before any leg exists, so a byte that is not UTF-8 in `--elected-by`, `--reference` or a local `--source` directory no longer pushes both legs before `write_lf` raises (#255) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since #247 the plan writer keeps a lone surrogate (a byte that is not UTF-8, as Python decodes argv and paths) as `\udce9`, and the reader gives it back. `plan --elected-by b'Jos\xe9'`, `--reference b'ref-\xe9'` and a source under a directory `caf\xe9` with local remotes therefore passed `plan` and `check`, and `execute` created and pushed both leg remotes before `write_lf` raised `UnicodeEncodeError` (exit 1). Before #247 `plan` raised and nothing was created. A hand-edited `\ud800` in `elected_by`, `elected_on`, `reference`, `visibility`, a pin or a follow-up ended the same way, and a NUL or a line break there went into the manifest. `shape_materialize.unwritable_reason` asks the question, beside `one_printable_line`: one line, no lone surrogate, no control character. It is `one_printable_line` less `Cf`, and it takes the empty value: a Persian name joins with U+200C, which a file holds and `execute` adopted before #247 and since, and an empty `elected_by` is what `plan` records when `git config user.name` names nobody. - `plan` refuses `--elected-by` and `--reference` as `adopt-unwritable-value` and writes no plan. - `check` reports `elected_by`, `elected_on`, `reference`, `visibility`, each pin and each follow-up as `plan-unwritable-value`, through `_entry_findings`, so `execute` refuses the same findings in `check`'s words through `_refuse_what_check_finds`, before `_create_leg_remotes`. - `execute` refuses a local clone URL (the source's own path under `--local-remote-dir`) as `adopt-unwritable-value`, right after `_repository_urls`. `check` cannot know it will be written, and against GitHub it is an https URL, so `plan` and `check` still pass such a directory, as #247 made them. - `Source.open` refuses a `local_path` that does not exist and is not text as `source-unresolvable`, where a NUL or a surrogate reached `print` or `subprocess` as a traceback. - `check`'s WHAT WILL HAPPEN shows the plan's visibility, adopt branch, leg paths and follow-ups through `_shown`, so a surrogate there no longer raises on a strict stdout before the finding is printed. tests/test_adopt_template_values.py covers each argv route, the local source directory, each hand-edited value, `local_path`, the printed values, and controls (`José`, U+200C, a `café` source directory) that still adopt. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 171 +++++++++- scripts/shape_materialize.py | 39 +++ tests/test_adopt_template_values.py | 507 ++++++++++++++++++++++++++++ 3 files changed, 709 insertions(+), 8 deletions(-) create mode 100644 tests/test_adopt_template_values.py diff --git a/adopt-project.py b/adopt-project.py index 6958d153..d505b2c2 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -120,6 +120,9 @@ MOVE_THE_LEG, first_and_more as _first_and_more, path_overlap as _overlap, shape_path_collisions, ) +# Whether a value this tool writes into a file is text a file can hold +# (#255), asked beside `one_printable_line`, the trailer's question. +from shape_materialize import unwritable_reason # noqa: E402 #: The naming policy this tool classifies leg names against. One constant, #: because three spellings of the same path is how the second one goes stale. @@ -395,13 +398,20 @@ def open(cls, spec: str, work_root: Path) -> "Source": path = candidate.resolve() repository = _remote_repository(path) else: - if "/" not in spec or WINDOWS_ABSOLUTE_RE.match(spec): + if ("/" not in spec or WINDOWS_ABSOLUTE_RE.match(spec) + or unwritable_reason(spec)): # A DRIVE-LETTER PATH IS A PATH THAT IS NOT THERE, never an # `org/repo`. `D:/work/Thing` carries a `/` and would # otherwise be handed to `gh repo clone` as an owner named # `D:`; `os.path.isabs` cannot be asked instead, because it # answers False for that string on Linux and macOS and this # refusal must read the same on every platform. + # NOR IS A NAME THAT IS NOT TEXT (#255). Since #247 a plan's + # `local_path:` can spell a NUL, a control character or a + # lone surrogate, and the reader gives it back: a path that + # cannot exist, which reached `print` (a surrogate raised on + # a strict stdout) or `subprocess` (a NUL raised there) as a + # traceback, where main had refused it. raise Refusal( "source-unresolvable", f"--source {spec!r} is neither a path that exists nor an " @@ -1235,6 +1245,18 @@ def _checked_plan_inputs(args, source: Source, election_date(args.elected_on) args.reference = args.reference or default_reference(args.elected_on) args.elected_by = args.elected_by or _elector() + # Before the plan is written, in the words `check` would use (#255): a + # value `execute` writes into a file must be text one can hold. The + # date has had its answer above, `election_date`. + unwritable = unwritable_values( + (("--elected-by", args.elected_by, ROOT_MANIFEST_AND_README), + ("--reference", args.reference, ROOT_MANIFEST_AND_README))) + if unwritable: + raise Refusal("adopt-unwritable-value", + " ".join(f"{problem}." for problem in unwritable), + "Remediation: no plan was written. Pass each value " + "named again, retyped (`--elected-by` defaults to " + "`git config user.name`). " + WRITTEN_AS_TEXT) names = _names(args.project) pins = [_checked_pin_name(p) for p in (args.pin or []) if p.strip()] _check_names(naming, names, set(pins)) @@ -1903,7 +1925,8 @@ def _entry_findings(plan: Plan, source: Source ) -> tuple[list[str], list[str]]: """Every FINDING `check` prints about the plan's ENTRIES, in its order, and what it prints once after them: coverage, then each entry's `leg:` - and the two mount paths, then the source's own submodules. + and the two mount paths, then the source's own submodules, then each + value `execute` writes into a file that no file can hold (#255). ONE LIST, read by `check` and by `execute`'s `_refuse_what_check_finds`. #168 and #166 each added findings to `check` and a refusal to `execute` @@ -1923,7 +1946,9 @@ def _entry_findings(plan: Plan, source: Source [path for path, _, _, _ in tree]) + _leg_findings(plan, tree)) submodule_findings, remediation = _submodule_findings(plan, source, tree) - return findings + submodule_findings, remediation + value_findings, value_remediation = _value_findings(plan) + return (findings + submodule_findings + value_findings, + remediation + value_remediation) def _topics_line(topic: str, local: bool) -> str: @@ -1945,23 +1970,27 @@ def _print_what_will_happen(plan: Plan, source: Source, names: dict, seeded = seeded_legs(assigned_paths(plan.entries)) where = source.repository or _shown(str(source.path)) print(f"\nWHAT WILL HAPPEN, against {where} @ {source.commit[:12]}:") + # Every value below that is the plan's own is `_shown`, as a name is: + # since #247 the reader gives back a lone surrogate, which raised on a + # strict stdout before the FINDING about it was printed (#255). print(f" create {names['spec']} and {names['code']} " - f"({plan.get('visibility', 'private')})") + f"({_shown(str(plan.get('visibility', 'private')))})") print(f" extract {len(moved)} path(s) with `git filter-repo`, history " "preserved") for role in seeded: print(f" SEED the {role} leg from {SEED_TEMPLATE[role]}/ — no path is " "assigned to it") - print(f" ONE split commit on branch {plan.get('adopt_branch', ADOPT_BRANCH)} " + print(f" ONE split commit on branch " + f"{_shown(str(plan.get('adopt_branch', ADOPT_BRANCH)))} " f"of {names['assembly']}: `git rm -r` those paths, mount the legs at " - f"{_plan_leg_path(plan, 'spec')}/ and " - f"{_plan_leg_path(plan, 'code')}/") + f"{_shown(str(_plan_leg_path(plan, 'spec')))}/ and " + f"{_shown(str(_plan_leg_path(plan, 'code')))}/") print(f" {len(stays)} path(s) stay in the assembly root; " f"{len(drops)} dropped") print(" the source is never deleted, never renamed, never force-pushed") print(_topics_line(topic, local)) for item in plan.get("follow_ups", []): - print(f" follow-up: {item}") + print(f" follow-up: {_shown(str(item))}") def cmd_check(args) -> int: @@ -2779,6 +2808,7 @@ def cmd_execute(args) -> int: # validated before it is used rather than after something has gone wrong. spec_path, code_path, branch, tracking = _checked_plan_values(plan) local, repositories, urls = _repository_urls(args, plan, names, source) + _refuse_an_unwritable_clone_url(urls["assembly"]) paths_for = _leg_paths(plan) _refuse_what_check_finds(plan, source) @@ -2839,6 +2869,131 @@ def cmd_execute(args) -> int: return 2 if topics_failed else 0 +# --------------------------------------------------------------------------- +# What `execute` writes into a file is text a file can hold (#255) +# --------------------------------------------------------------------------- +# +# `_template_values` puts `elected_by`, `elected_on`, `reference`, +# `visibility`, each pin and the assembly root's clone URL into the templates +# as they stand, and `write_lf` writes the result as strict UTF-8; +# `_commit_the_split` puts each follow-up into the split commit's message. +# Since #247 a plan can carry a lone surrogate -- a byte that is not UTF-8, +# as `sys.argv` and `os.fsdecode` keep one -- and the reader gives it back. +# So `plan` and `check` passed `--elected-by b'Jos\xe9'`, and `execute` +# created and pushed both legs before `write_lf` raised `UnicodeEncodeError`. +# Before #247, `plan` raised on it and nothing was made. A line break or a +# NUL in the plan went on into the manifest. +# +# So `unwritable_reason` is asked of each such value before anything exists. +# `plan` refuses `--elected-by` and `--reference` as +# `adopt-unwritable-value`, and writes no plan (`--elected-on` is a date, and +# `election_date` refuses anything else). `check` reports each plan value as +# `plan-unwritable-value`, and `execute` refuses those findings through +# `_refuse_what_check_finds`, before `_create_leg_remotes`. Every other value +# the templates take is a `checked_value`, the project id, or one of this +# tool's own. +# +# THE CLONE URL IS `execute`'S OWN. Under `--local-remote-dir` it is the +# source's path, and `check` has no such flag to know it will be written. +# Against GitHub it is `https://github.com//.git`, and the path is +# written nowhere. So `execute` refuses an unwritable one itself, right +# after `_repository_urls`, and `plan` and `check` still pass a source +# directory whose name is not UTF-8, as #247 made them. + +#: `check`'s finding, and the code `execute` refuses it under. +UNWRITABLE_VALUE = "plan-unwritable-value" + +#: Where `execute` writes the values it is asked about. +ROOT_MANIFEST = "the assembly root's `project.yaml`" +ROOT_MANIFEST_AND_README = "the assembly root's `project.yaml` and `README.md`" +SPLIT_MESSAGE = "the split commit's message" + +#: Each plan value `_template_values` substitutes as it stands, by its key, +#: and where it lands. `pins` and `follow_ups` are lists, asked item by item. +WRITTEN_PLAN_VALUES = {"elected_by": ROOT_MANIFEST_AND_README, + "elected_on": ROOT_MANIFEST_AND_README, + "reference": ROOT_MANIFEST_AND_README, + "visibility": ROOT_MANIFEST} + +#: Why such a value must be text, said after every refusal of one. +WRITTEN_AS_TEXT = ( + "`execute` writes each one as it stands, into the assembly root's files " + "or the split commit's message, so it must be one line of UTF-8 text. A " + "byte that is not UTF-8, or a lone surrogate, cannot be written at all, " + "and a line break or a control character would be written raw into a " + "value that is read back as one line. A byte that is not UTF-8 reaches a " + "command line from a terminal or a script whose encoding is not the " + "locale's.") + +PLAN_VALUE_REMEDIATION = ("Remediation: correct each value named in the plan, " + "or re-run `plan` with it retyped. " + + WRITTEN_AS_TEXT) + + +def unwritable_values(fields) -> list[str]: + """A sentence for each `(what, value, where)` whose `value` is not text + a file can hold; `[]` when every one is. + + `what` names the value as the person has it in front of them -- + `--elected-by` to `plan`, `elected_by` read out of a plan -- and `where` + is the file `execute` writes it into. The value is spelled as Python + spells it, as `unsafe-value` spells one, so the sentence stays one line + of text whatever the value holds. + """ + out = [] + for what, value, where in fields: + reason = unwritable_reason(value) + if reason: + out.append(f"{what} is {value!r}, which `execute` writes into " + f"{where}, and {reason}") + return out + + +def _plan_values(plan: Plan) -> list[tuple[str, str, str]]: + """`(what, value, where)` for each value of the plan's that `execute` + writes into a file, as `str` makes it, which is how `_template_values` + and `_commit_the_split` take it. A key the plan does not carry is filled + by this tool (`private`, today's date) and is left out.""" + fields = [(key, str(plan.get(key)), where) + for key, where in WRITTEN_PLAN_VALUES.items() + if plan.get(key) is not None] + fields += [("a `pins:` item", pin, ROOT_MANIFEST) for pin in plan.pins] + return fields + [("a `follow_ups:` item", str(item), SPLIT_MESSAGE) + for item in plan.get("follow_ups", [])] + + +def _value_findings(plan: Plan) -> tuple[list[str], list[str]]: + """`unwritable_values` of the plan as `check` prints them, one FINDING + each, and what to print once after every finding: how to correct them + when there is one, nothing when there is none.""" + found = [f"FINDING {UNWRITABLE_VALUE}: {problem}" + for problem in unwritable_values(_plan_values(plan))] + return found, [PLAN_VALUE_REMEDIATION] if found else [] + + +def _refuse_an_unwritable_clone_url(url: str) -> None: + """Refuse the assembly root's clone URL when no file can hold it, before + any leg exists. + + Only a local one can be refused: under `--local-remote-dir` the URL is + the source's own path, which the assembly root's `README.md` names (and + a seeded leg's), so a source directory whose name is not UTF-8 pushed + both legs and then raised in `write_lf` (#255). + """ + reason = unwritable_reason(url) + if reason: + raise Refusal( + "adopt-unwritable-value", + "under --local-remote-dir the assembly root's clone URL is the " + f"source's own path, {url!r}, which `execute` writes into its " + f"`README.md`, and {reason}", + "Remediation: nothing was created. Clone the source to a path " + "that is one line of UTF-8 text, and pass that path to `execute` " + "with --source; or adopt against GitHub, without " + "--local-remote-dir, where the clone URL is " + "`https://github.com//.git`.") + + def _template_values(plan: Plan, names, repositories, urls, spec_path, code_path, tracking, leg_commits, leg_digests, shape_commit, pins, naming) -> dict[str, str]: diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 81780a70..8c4905b2 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -777,6 +777,45 @@ def is_trailer(value: str) -> bool: return bool(TRAILER_RE.fullmatch(value)) and one_printable_line(value) +def unwritable_reason(value: str) -> str | None: + r"""Why `value` is not one line of text a file can hold, in words; None + when it is (#255). + + `adopt-project.py` writes a value a person gave it -- `--elected-by`, + `--reference`, the source's own path as a local clone URL -- into the + assembly root's `project.yaml` and `README.md` through `write_lf`, which + is strict UTF-8. Since #247 the plan keeps a lone surrogate (a byte that + is not UTF-8, as `sys.argv` and `os.fsdecode` keep it) as `\udce9`, and + the reader gives it back, so `plan` and `check` passed such a value and + `write_lf` raised `UnicodeEncodeError` after both legs were pushed. This + is the question asked of every such value before anything exists. + + `one_printable_line` LESS `Cf`, AND THE EMPTY VALUE, because both are + text a file holds and both were adopted before #247 and since. A `Cf` + character is invisible, not unwritable: UTF-8 encodes it, the reader + reads it back, and a name may need one -- a Persian name joins its parts + with U+200C ZERO WIDTH NON-JOINER, which `one_printable_line` refuses. + An empty `elected_by` is what `plan` records when `git config user.name` + names nobody, and the manifest holds it as `""`. What is left is + refused: a lone surrogate, which UTF-8 cannot encode; a line break, + which ends the line the manifest's reader reads the value from (CR, + U+0085 and U+2028 among them, as `str.splitlines` ends lines); and any + other control character, NUL among them, which YAML does not allow raw + (TAB aside, and no name, date or reference holds one either). + """ + if value.splitlines() not in ([], [value]): + return "it holds a line break, and it is written as one line" + for char in value: + category = unicodedata.category(char) + if category == "Cs": + return ("it holds a byte that is not UTF-8" + if "\udc80" <= char <= "\udcff" else + "it holds a lone surrogate, which is no character") + if category in UNPRINTABLE_CATEGORIES and category != "Cf": + return "it holds a control character" + return None + + #: The git that learned `commit --trailer` (2.32, 2021-06-06). An older one #: still has `git interpret-trailers`, but not the commit option — so on that #: host the trailer is appended to the message TEXT instead, which produces diff --git a/tests/test_adopt_template_values.py b/tests/test_adopt_template_values.py new file mode 100644 index 00000000..1fd177e3 --- /dev/null +++ b/tests/test_adopt_template_values.py @@ -0,0 +1,507 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A value `execute` writes into a file is asked, before anything exists, +whether a file can hold it (#255). + +Since #247 the plan writer keeps a lone surrogate -- a byte that is not +UTF-8, as Python decodes argv and paths -- as `\\udce9`, and the reader gives +it back. So `plan --elected-by b'Jos\\xe9'`, `--reference b'ref-\\xe9'`, or a +`--source` under a directory `caf\\xe9` with local remotes (the clone URL +carries it) passed `plan` and `check`, and `execute` created and pushed both +leg remotes before `write_lf` raised `UnicodeEncodeError`, exit 1. Before +#247, `plan` raised on the same input and nothing was created. A hand-edited +`\\ud800` in `elected_by`, `elected_on`, `reference`, `visibility`, a pin or a +follow-up ended the same way; a NUL or a line break there went on into the +manifest; and a hand-edited `\\x00` or surrogate in `local_path` ended +`check` and `execute` in a traceback where main had refused it. + +Now `plan` refuses such a flag as `adopt-unwritable-value` and writes no +plan; `check` reports such a plan value as `plan-unwritable-value`, and +`execute` refuses the same finding before any leg exists; `execute` refuses +a local clone URL no file can hold as `adopt-unwritable-value`; and a +`local_path` that is not text is `source-unresolvable`. Every run is read +for a traceback, and asked afterwards whether a leg remote exists. + +THE QUESTION IS `one_printable_line` LESS `Cf`. A Persian name joins its +parts with U+200C ZERO WIDTH NON-JOINER, which UTF-8 holds and the reader +reads back, and `execute` adopted it before #247 and since: it still adopts, +as `José` and a source directory `café` do. + +Every subprocess runs under `PYTHONIOENCODING=utf-8:strict`, so that a value +printed raw raises on every platform as it does on a strict terminal, and is +answered in BYTES: a value that is not UTF-8 is not text to decode. +""" + +from __future__ import annotations + +import os +import re +import shutil +import subprocess +import sys +import unicodedata +from pathlib import Path + +import pytest + +from conftest import (ADOPT, REPO, blank_unnamed_pin_sources, + make_source_repo, resolve) + +sys.path.insert(0, str(REPO / "scripts")) +import shape_materialize # noqa: E402 +from repo_shape import double_quoted, load_yaml # noqa: E402 + +PLAN_FILE = "adoption-plan.yaml" +REMOTES = "remotes" +WORK = "work" +PROJECT = "Northwind" +#: Where `execute` pushes the split commit: back into the source, under +#: `--local-remote-dir`. +SPLIT_BRANCH = "adopt/three-repo-shape" +#: The entries of `SYNTHETIC_TREE` that `plan` asks about, answered. +ANSWERS = (("examples/", "spec"), (".claude/", "root"), + ("release.yaml", "root")) +#: A stdout that raises on what it cannot encode, as a strict terminal does. +STRICT = {"PYTHONIOENCODING": "utf-8:strict"} +#: What the git that `execute` runs needs, as `conftest.run_script` sets it. +IDENTITY = {"GIT_AUTHOR_NAME": "openRepoShape tests", + "GIT_AUTHOR_EMAIL": "tests@openreposhape.invalid", + "GIT_COMMITTER_NAME": "openRepoShape tests", + "GIT_COMMITTER_EMAIL": "tests@openreposhape.invalid"} + +TRACEBACK = b"Traceback" +VERIFIED = b"adoption verified" +PLAN_OK = b"\nplan ok\n" +ADOPT_REFUSED = b"REFUSED adopt-unwritable-value: " +PLAN_REFUSED = b"REFUSED plan-unwritable-value: " +FINDING = b"FINDING plan-unwritable-value: " +SOURCE_REFUSED = b"REFUSED source-unresolvable: " + +needs_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git filter-repo is not installed: `pip install git-filter-repo`") + + +def adopted(*args: str) -> subprocess.CompletedProcess: + """`adopt-project.py ` on a strict stdout, answered in bytes, each + stream with LF line ends whatever the platform printed.""" + env = blank_unnamed_pin_sources( + {**IDENTITY, **os.environ, "LANES_LANE": "", **STRICT}, STRICT) + done = subprocess.run([sys.executable, str(ADOPT), *args], + capture_output=True, env=env, + stdin=subprocess.DEVNULL, check=False) + done.stdout = done.stdout.replace(b"\r\n", b"\n") + done.stderr = done.stderr.replace(b"\r\n", b"\n") + return done + + +def planned(base: Path, source: Path, + *flags: str) -> subprocess.CompletedProcess: + """`plan`, as `conftest.write_plan` runs it, with `flags` after it: a + flag given twice is read as its last value.""" + return adopted("plan", "--source", str(source), "--project", PROJECT, + "--org", "testorg", "--elected-by", "Test Human", + "--elected-on", "2026-09-02", "--out", + str(base / PLAN_FILE), "--work-dir", str(base / WORK), + *flags) + + +def checked(base: Path) -> subprocess.CompletedProcess: + return adopted("check", "--plan", str(base / PLAN_FILE), "--work-dir", + str(base / WORK)) + + +def executed(base: Path) -> subprocess.CompletedProcess: + return adopted("execute", "--plan", str(base / PLAN_FILE), "--yes", + "--local-remote-dir", str(base / REMOTES), "--work-dir", + str(base / WORK)) + + +def answer(base: Path) -> None: + for path, leg in ANSWERS: + resolve(base / PLAN_FILE, path, leg) + + +def plan_check_execute(base: Path, source: Path, + *flags: str) -> dict[str, subprocess.CompletedProcess]: + """`plan` with `flags`, then, if it wrote a plan, that plan answered, + `check`, and `execute` with local remotes: each run there was.""" + runs = {"plan": planned(base, source, *flags)} + if runs["plan"].returncode == 0: + answer(base) + runs["check"] = checked(base) + runs["execute"] = executed(base) + return runs + + +def said(run: subprocess.CompletedProcess) -> str: + """A run's output, readable in an assertion message.""" + return (run.stdout + run.stderr).decode("utf-8", "backslashreplace") + + +def assert_no_traceback(runs: dict) -> None: + for name, run in runs.items(): + assert TRACEBACK not in run.stderr, f"{name}:\n{said(run)}" + + +def assert_no_traceback_and_no_leg(base: Path, runs: dict) -> None: + """The issue's two symptoms, asked first so that a failure names them: + a traceback in any run, and a leg remote left behind.""" + assert_no_traceback(runs) + remotes = base / REMOTES + left = (sorted(path.name for path in remotes.iterdir()) + if remotes.exists() else []) + assert not left, f"leg remotes exist: {left}" + + +def in_the_split(source: Path, path: str) -> str: + """`path` as the split commit `execute` pushed into `source` holds it.""" + return subprocess.run(["git", "show", f"{SPLIT_BRANCH}:{path}"], + cwd=str(source), capture_output=True, + check=True).stdout.decode("utf-8") + + +# --- `plan`: a flag no file can hold ----------------------------------------- + +#: `(flag, value)`, as `plan`'s argv carries it. `\udce9` is how Python keeps +#: the byte 0xE9 of a command line that is not UTF-8, which is what a +#: terminal or a script whose encoding is not the locale's sends; the other +#: two are text a file holds, but on no one line. +FLAGS = [ + ("--elected-by", "Jos\udce9"), + ("--reference", "ref-\udce9"), + ("--elected-by", "Jos
e"), + ("--reference", "openxFactory\tdocs/project-repo-schema.md"), +] + + +@pytest.mark.parametrize("flag, value", FLAGS, + ids=[f"{flag}={ascii(value)}" + for flag, value in FLAGS]) +def test_plan_refuses_a_flag_no_file_can_hold(tmp_path, flag, value): + """The issue's first two rows. On main `plan` exited 0, `check` said + `plan ok`, and `execute` raised in `write_lf` after both legs were + pushed (a surrogate), or adopted with the line break or the tab in the + manifest. Now `plan` refuses, and writes no plan.""" + source = make_source_repo(tmp_path / "Thing") + runs = plan_check_execute(tmp_path, source, flag, value) + assert_no_traceback_and_no_leg(tmp_path, runs) + refused = runs["plan"] + assert refused.returncode == 2, said(refused) + assert ADOPT_REFUSED + f"{flag} is {value!r}, which ".encode() \ + in refused.stderr, said(refused) + assert b"no plan was written" in refused.stderr + assert not (tmp_path / PLAN_FILE).exists() + + +# --- `execute`: a local clone URL no file can hold --------------------------- + +@needs_filter_repo +def test_execute_refuses_a_local_source_directory_that_is_not_utf8(tmp_path): + """The issue's third row. Under `--local-remote-dir` the assembly root's + clone URL is the source's own path, which its `README.md` names: `plan` + and `check` pass such a directory, as #247 made them, and `execute` + refuses it before any leg exists, where it raised after both were + pushed. Skipped where no directory can be named with the byte 0xE9 + (Windows, macOS's APFS) or git will not work in one.""" + try: + parent = Path(os.fsdecode(os.path.join(os.fsencode(tmp_path), + b"caf\xe9"))) + parent.mkdir() + except (OSError, UnicodeError) as exc: + pytest.skip(f"no directory here is named b'caf\\xe9': {exc!r}") + try: + source = make_source_repo(parent / "Thing") + except AssertionError as exc: + pytest.skip(f"git will not work under b'caf\\xe9' here: {exc}") + runs = plan_check_execute(tmp_path, source) + assert_no_traceback_and_no_leg(tmp_path, runs) + assert runs["plan"].returncode == 0, said(runs["plan"]) + assert runs["check"].returncode == 0, said(runs["check"]) + assert PLAN_OK in runs["check"].stdout, said(runs["check"]) + refused = runs["execute"] + assert refused.returncode == 2, said(refused) + assert ADOPT_REFUSED + b"under --local-remote-dir the assembly root's " \ + b"clone URL is the source's own path, " \ + + repr(str(source.resolve())).encode() in refused.stderr, said(refused) + assert b"nothing was created" in refused.stderr + + +# --- `check` and `execute`: a plan value edited by hand ---------------------- + +@pytest.fixture(scope="module") +def answered(tmp_path_factory) -> tuple[Path, str]: + """A source and the plan `plan` wrote for it, answered and `plan ok`: + the text each test edits a line of, in a copy of its own.""" + base = tmp_path_factory.mktemp("answered") + source = make_source_repo(base / "Thing") + written = planned(base, source) + assert written.returncode == 0, said(written) + answer(base) + ok = checked(base) + assert ok.returncode == 0, said(ok) + return source, (base / PLAN_FILE).read_text(encoding="utf-8") + + +def with_line(text: str, line: str) -> str: + """`text` with the one line holding `line`'s key -- `elected_by`, or an + indented ` spec_path` -- replaced by `line`.""" + key = line.split(": ", 1)[0] + text, done = re.subn(rf"^{re.escape(key)}: .*$", lambda _: line, text, + count=1, flags=re.M) + assert done == 1, line + return text + + +def edited_plan(base: Path, answered: tuple[Path, str], line: str, + follow_up: bool = False) -> None: + """The answered plan, its `local_path:` a copy of the source made for + this test alone (on main `execute` pushes its split branch into the + source), with `line` in place of the line of its key -- or, as + `follow_up`, added as the first `follow_ups:` item -- written LF as + `plan` writes it.""" + source, text = answered + copy = base / "Thing" + shutil.copytree(source, copy, symlinks=True) + text = with_line(text, f" local_path: {double_quoted(str(copy))}") + if follow_up: + text, done = re.subn(r"^follow_ups:$", + lambda _: f"follow_ups:\n{line}", text, count=1, + flags=re.M) + assert done == 1 + else: + text = with_line(text, line) + with (base / PLAN_FILE).open("w", encoding="utf-8", + newline="\n") as handle: + handle.write(text) + + +#: `(id, key, value, what the FINDING calls it)`. Each value is spelled in +#: the plan by `double_quoted`, as the plan writer escapes it and as a person +#: editing the plan would. +EDITS = [ + ("elected_by-surrogate", "elected_by", "T\ud800", "elected_by"), + ("elected_by-byte", "elected_by", "T\udce9", "elected_by"), + ("elected_by-nul", "elected_by", "T\x00", "elected_by"), + ("elected_by-cr", "elected_by", "T\rX", "elected_by"), + ("elected_by-nel", "elected_by", "T\x85X", "elected_by"), + ("elected_by-u2028", "elected_by", "T
X", "elected_by"), + ("elected_by-tab", "elected_by", "T\tX", "elected_by"), + ("elected_on-surrogate", "elected_on", "2026-09-02\ud800", "elected_on"), + ("reference-surrogate", "reference", "x\ud800", "reference"), + ("reference-nul", "reference", "x\x00", "reference"), + ("visibility-surrogate", "visibility", "priv\ud800ate", "visibility"), + ("pins-surrogate", "pins", "openX\ud800", "a `pins:` item"), + ("follow_ups-surrogate", "follow_ups", "fix x\ud800", + "a `follow_ups:` item"), +] + + +def edit(base: Path, answered: tuple[Path, str], key: str, + value: str) -> None: + """The answered plan, with `key` given `value`.""" + if key == "follow_ups": + edited_plan(base, answered, f" - {double_quoted(value)}", + follow_up=True) + elif key == "pins": + edited_plan(base, answered, f"pins: [{double_quoted(value)}]") + else: + edited_plan(base, answered, f"{key}: {double_quoted(value)}") + + +@needs_filter_repo +@pytest.mark.parametrize("key, value, what", [row[1:] for row in EDITS], + ids=[row[0] for row in EDITS]) +def test_check_finds_and_execute_refuses_a_hand_edited_value( + tmp_path, answered, key, value, what): + """On main `check` said `plan ok` (or raised printing the value), and + `execute` raised in `write_lf` after both legs were pushed, or adopted + with the NUL or the line break in the manifest. Now `check` has a + FINDING, and `execute` refuses it through `_refuse_what_check_finds`, in + `check`'s words, before any leg exists.""" + edit(tmp_path, answered, key, value) + runs = {"check": checked(tmp_path), "execute": executed(tmp_path)} + assert_no_traceback_and_no_leg(tmp_path, runs) + found, refused = runs["check"], runs["execute"] + finding = FINDING + f"{what} is {value!r}, which `execute` writes " \ + "into ".encode() + assert found.returncode == 1, said(found) + assert finding in found.stderr, said(found) + assert refused.returncode == 2, said(refused) + assert PLAN_REFUSED in refused.stderr, said(refused) + assert b"\n " + finding in refused.stderr, said(refused) + assert b"no leg repository was created" in refused.stderr + + +def test_plan_and_check_say_the_same_sentence(tmp_path, answered): + """`plan`'s refusal of `--elected-by` and `check`'s finding about + `elected_by` are one sentence about one value, each naming it as the + person has it in front of them.""" + value = "Jos\udce9" + flagged, by_hand = tmp_path / "flag", tmp_path / "edit" + flagged.mkdir() + by_hand.mkdir() + refused = planned(flagged, answered[0], "--elected-by", value) + edit(by_hand, answered, "elected_by", value) + found = checked(by_hand) + assert ADOPT_REFUSED in refused.stderr, said(refused) + assert FINDING in found.stderr, said(found) + detail = refused.stderr.split(ADOPT_REFUSED, 1)[1].split(b"\n", 1)[0] + finding = found.stderr.split(FINDING, 1)[1].split(b"\n", 1)[0] + assert detail.startswith(b"--elected-by is "), said(refused) + assert finding.startswith(b"elected_by is "), said(found) + assert detail[len(b"--elected-by"):] == finding[len(b"elected_by"):] + b"." + + +# --- a value `check` prints, edited by hand ---------------------------------- + +#: `(id, line key, value, how `check` shows it, check's exit, what it says, +#: execute's refusal)`: a value `check` printed raw under `WHAT WILL +#: HAPPEN`, which raised on a strict stdout before any finding about it was +#: printed. It is shown as a name is (`_shown`), a byte that is not UTF-8 as +#: `\xe9`. A leg path is `plan-bad-leg-path`; the adopt branch passes `check` +#: and is `unsafe-value` to `execute`, as each was on main before #247. +PRINTED = [ + ("spec_path-surrogate", " spec_path", "sp\ud800ec", rb"`sp\ud800ec`/", + 1, b"FINDING plan-bad-leg-path: legs.spec_path is ", + b"plan-bad-leg-path"), + ("code_path-byte", " code_path", "co\udce9de", rb"`co\xe9de`/", 1, + b"FINDING plan-bad-leg-path: legs.code_path is ", b"plan-bad-leg-path"), + ("adopt_branch-surrogate", "adopt_branch", "adopt/x\ud800", + rb"`adopt/x\ud800` of", 0, PLAN_OK, b"unsafe-value"), +] + + +@needs_filter_repo +@pytest.mark.parametrize("key, value, shown, exit_code, says, code", + [row[1:] for row in PRINTED], + ids=[row[0] for row in PRINTED]) +def test_check_prints_a_hand_edited_value_spelled(tmp_path, answered, key, + value, shown, exit_code, + says, code): + edited_plan(tmp_path, answered, f"{key}: {double_quoted(value)}") + runs = {"check": checked(tmp_path), "execute": executed(tmp_path)} + assert_no_traceback_and_no_leg(tmp_path, runs) + found, refused = runs["check"], runs["execute"] + assert found.returncode == exit_code, said(found) + assert says in found.stdout + found.stderr, said(found) + assert shown in found.stdout, said(found) + assert refused.returncode == 2, said(refused) + assert b"REFUSED " + code + b": " in refused.stderr, said(refused) + + +# --- `local_path`, edited by hand -------------------------------------------- + +#: How each `local_path` is made from the source's own path: the path that +#: exists with a NUL or a surrogate after it, and paths that do not exist. +LOCAL_PATHS = { + "nul": lambda source: f"{source}\x00", + "surrogate": lambda source: f"{source}\ud800", + "missing-nul": lambda _: "/nonexistent\x00/Thing", + "missing-byte": lambda _: "/nonexistent/caf\udce9", +} + + +@pytest.mark.parametrize("make", list(LOCAL_PATHS.values()), + ids=list(LOCAL_PATHS)) +def test_a_local_path_that_is_not_text_is_unresolvable(tmp_path, answered, + make): + """On main `check` and `execute` ended in a traceback: `print` raised on + the surrogate and `subprocess` on the NUL, as the value was about to be + handed to `gh repo clone` as an `org/repo`. It is no path that exists + and no `org/repo` name, which is what `source-unresolvable` says, and + what Windows, where such a path holds no `/`, said already.""" + value = make(tmp_path / "Thing") + edited_plan(tmp_path, answered, f" local_path: {double_quoted(value)}") + assert load_yaml(tmp_path / PLAN_FILE)["source"]["local_path"] == value + runs = {"check": checked(tmp_path), "execute": executed(tmp_path)} + assert_no_traceback_and_no_leg(tmp_path, runs) + for name, run in runs.items(): + assert run.returncode == 2, f"{name}:\n{said(run)}" + assert SOURCE_REFUSED + f"--source {value!r} is neither".encode() \ + in run.stderr, f"{name}:\n{said(run)}" + + +# --- controls: valid text still adopts, as on main --------------------------- + +@needs_filter_repo +@pytest.mark.parametrize("elected_by", ["José", "Ali‌Reza"], + ids=["accented", "zero-width-non-joiner"]) +def test_a_valid_non_ascii_elector_still_adopts(tmp_path, elected_by): + """`José`, and a name joined with U+200C, which `one_printable_line` + refuses and a file holds: adopted, and written into the manifest.""" + source = make_source_repo(tmp_path / "Thing") + runs = plan_check_execute(tmp_path, source, "--elected-by", elected_by) + assert_no_traceback(runs) + assert runs["check"].returncode == 0, said(runs["check"]) + done = runs["execute"] + assert done.returncode == 0, said(done) + assert VERIFIED in done.stdout, said(done) + assert f'\nelected_by: "{elected_by}"\n' in in_the_split(source, + "project.yaml") + + +@needs_filter_repo +def test_a_utf8_source_directory_still_adopts_with_local_remotes(tmp_path): + """A source under a directory `café`: its path is the clone URL, which a + file holds, so it adopts, and the shape's README (beside the source's + own) names it.""" + source = make_source_repo(tmp_path / "café" / "Thing") + runs = plan_check_execute(tmp_path, source) + assert_no_traceback(runs) + assert runs["check"].returncode == 0, said(runs["check"]) + done = runs["execute"] + assert done.returncode == 0, said(done) + assert VERIFIED in done.stdout, said(done) + readme = in_the_split(source, "shape/README.md") + assert f"git clone --recurse-submodules {source.resolve()}\n" in readme + + +# --- the question itself ----------------------------------------------------- + +#: Text a file holds: the empty value `plan` records when `git config +#: user.name` names nobody, a name, a date, the default reference, a literal +#: backslash, invisible `Cf` characters, and a space that is not ASCII's. +WRITABLE = ["", "José", "Ali‌Reza", "2026-09-02", + "openxFactory docs/project-repo-schema.md", "CORP\\rsmith", + "日本", "a b", "‮evil", "bom", "a­b"] + +#: `(value, the words its reason holds)`. +UNWRITABLE = [ + ("Jos\udce9", "a byte that is not UTF-8"), + ("x\ud800", "a lone surrogate"), + ("x\udfff", "a lone surrogate"), + ("a\nb", "a line break"), ("a\rb", "a line break"), + ("a\r\nb", "a line break"), ("end\n", "a line break"), + ("a\x0bb", "a line break"), ("a\x0cb", "a line break"), + ("a\x1cb", "a line break"), ("a\x85b", "a line break"), + ("a
b", "a line break"), ("a
b", "a line break"), + ("a\x00b", "a control character"), ("a\tb", "a control character"), + ("a\x1bb", "a control character"), ("a\x7fb", "a control character"), + ("a\x9fb", "a control character"), +] + + +@pytest.mark.parametrize("value", WRITABLE, ids=[ascii(v) for v in WRITABLE]) +def test_text_a_file_holds_has_no_reason(value): + assert shape_materialize.unwritable_reason(value) is None + assert value.encode("utf-8").decode("utf-8") == value + + +@pytest.mark.parametrize("value, words", UNWRITABLE, + ids=[ascii(v) for v, _ in UNWRITABLE]) +def test_what_no_file_holds_is_named(value, words): + reason = shape_materialize.unwritable_reason(value) + assert reason is not None and words in reason, reason + assert reason.isprintable() and reason.isascii() + + +def test_the_question_is_one_printable_line_less_cf(): + """Every code point, alone: refused exactly when `one_printable_line` + refuses it and it is not `Cf`.""" + reason = shape_materialize.unwritable_reason + printable = shape_materialize.one_printable_line + wrong = [hex(code) for code in range(sys.maxunicode + 1) + if (reason(chr(code)) is None) + != (printable(chr(code)) + or unicodedata.category(chr(code)) == "Cf")] + assert not wrong, wrong[:20] From 7c3c0205283f3b24f974eb8703ff2f69da4e5f99 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:16:37 +0000 Subject: [PATCH 02/12] adopt-project.py: a bidi embedding, override or isolate control in a value `execute` writes is refused before anything exists, and the clone-URL refusal's remediation moves the source and plans again, so no README line is shown reversed and following the remediation adopts instead of raising (#255) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix round 1 on PR #270, answering lane openRepoShape-2's review at aed9190 (F1, F2, F3, and N1's test point). F1. `unwritable_reason` was `one_printable_line` less all of `Cf`, so U+202A..U+202E and U+2066..U+2069 passed `plan`, `check` and `execute` and went raw into `project.yaml`, the README's `Elected by` line and the split commit's message; a viewer showed the rest of that line reversed. Issue #255's question, `one_printable_line`, refuses them. The question is now `one_printable_line` less `Cf` other than those nine (`BIDI_CONTROLS`, by code point), plus the empty value. The joiners U+200C and U+200D, U+00AD, U+FEFF and the marks U+200E, U+200F and U+061C stay writable: a Persian name needs U+200C, and a mark sets no span's direction. `plan` refuses such a flag as `adopt-unwritable-value`, `check` finds such a plan value as `plan-unwritable-value`, `execute` refuses it before any leg exists, and a local clone URL holding one is `adopt-unwritable-value`. F2. The test file held a raw U+202E, which SonarCloud's text:S6389 flags, and other invisible literals a reader cannot see. Each is now an escape; only `é` and `日本` stay literal. The two composite assertions SonarCloud's python:S9073 flagged are four, each with a message. F3. The clone-URL refusal said to clone the source to a UTF-8 path and pass it to `execute` with --source. Followed, that raised: the clone's `origin` is the old path, and `_remote_repository` reads it as UTF-8. The remediation now says to rename or move the source and run `plan --source `, `check` and `execute` again, and the test follows it to `adoption verified`, for a byte that is not UTF-8 and for a bidi override in the directory's name. N1. The same-sentence test now asserts both exit codes and that no run raised. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 28 +++-- scripts/shape_materialize.py | 43 +++++-- tests/test_adopt_template_values.py | 169 +++++++++++++++++++++------- 3 files changed, 176 insertions(+), 64 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index d505b2c2..2bb8e161 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -2882,7 +2882,8 @@ def cmd_execute(args) -> int: # So `plan` and `check` passed `--elected-by b'Jos\xe9'`, and `execute` # created and pushed both legs before `write_lf` raised `UnicodeEncodeError`. # Before #247, `plan` raised on it and nothing was made. A line break or a -# NUL in the plan went on into the manifest. +# NUL in the plan went on into the manifest, and a bidi override or isolate +# went into the README, which then showed the rest of its line reversed. # # So `unwritable_reason` is asked of each such value before anything exists. # `plan` refuses `--elected-by` and `--reference` as @@ -2919,9 +2920,11 @@ def cmd_execute(args) -> int: WRITTEN_AS_TEXT = ( "`execute` writes each one as it stands, into the assembly root's files " "or the split commit's message, so it must be one line of UTF-8 text. A " - "byte that is not UTF-8, or a lone surrogate, cannot be written at all, " - "and a line break or a control character would be written raw into a " - "value that is read back as one line. A byte that is not UTF-8 reaches a " + "byte that is not UTF-8, or a lone surrogate, cannot be written at all; " + "a line break or a control character would be written raw into a value " + "that is read back as one line; and a bidirectional embedding, override " + "or isolate control would show the rest of the line it is written into " + "in another order than it is written. A byte that is not UTF-8 reaches a " "command line from a terminal or a script whose encoding is not the " "locale's.") @@ -2978,7 +2981,13 @@ def _refuse_an_unwritable_clone_url(url: str) -> None: Only a local one can be refused: under `--local-remote-dir` the URL is the source's own path, which the assembly root's `README.md` names (and a seeded leg's), so a source directory whose name is not UTF-8 pushed - both legs and then raised in `write_lf` (#255). + both legs and then raised in `write_lf` (#255), and one whose name holds + a bidi override was written into the README raw. + + THE REMEDIATION MOVES THE SOURCE AND PLANS AGAIN. A clone of it at a + path that is text, passed to `execute` with --source, still has the old + path as its `origin`, and `_remote_repository` reads that as UTF-8 and + raises; renaming the directory and running `plan` again adopts. """ reason = unwritable_reason(url) if reason: @@ -2987,10 +2996,11 @@ def _refuse_an_unwritable_clone_url(url: str) -> None: "under --local-remote-dir the assembly root's clone URL is the " f"source's own path, {url!r}, which `execute` writes into its " f"`README.md`, and {reason}", - "Remediation: nothing was created. Clone the source to a path " - "that is one line of UTF-8 text, and pass that path to `execute` " - "with --source; or adopt against GitHub, without " - "--local-remote-dir, where the clone URL is " + "Remediation: nothing was created. Rename or move the source " + "repository so that its path is one line of UTF-8 text with no " + "control character in it, then run `plan --source `, `check` and `execute` again; or adopt against GitHub, " + "without --local-remote-dir, where the clone URL is " "`https://github.com//.git`.") diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 8c4905b2..c00fd4c7 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -777,6 +777,15 @@ def is_trailer(value: str) -> bool: return bool(TRAILER_RE.fullmatch(value)) and one_printable_line(value) +#: The nine bidirectional embedding, override and isolate controls, by code +#: point: U+202A..U+202E (LRE, RLE, PDF, LRO, RLO) and U+2066..U+2069 (LRI, +#: RLI, FSI, PDI). Each is `Cf`, and each sets the direction of the text +#: after it, so a line that holds one is shown in another order than it is +#: written. The marks U+200E, U+200F and U+061C are not among them: a mark +#: is one character of one direction, and sets no span's. +BIDI_CONTROLS = frozenset((*range(0x202A, 0x202F), *range(0x2066, 0x206A))) + + def unwritable_reason(value: str) -> str | None: r"""Why `value` is not one line of text a file can hold, in words; None when it is (#255). @@ -790,18 +799,24 @@ def unwritable_reason(value: str) -> str | None: `write_lf` raised `UnicodeEncodeError` after both legs were pushed. This is the question asked of every such value before anything exists. - `one_printable_line` LESS `Cf`, AND THE EMPTY VALUE, because both are - text a file holds and both were adopted before #247 and since. A `Cf` - character is invisible, not unwritable: UTF-8 encodes it, the reader - reads it back, and a name may need one -- a Persian name joins its parts - with U+200C ZERO WIDTH NON-JOINER, which `one_printable_line` refuses. - An empty `elected_by` is what `plan` records when `git config user.name` - names nobody, and the manifest holds it as `""`. What is left is - refused: a lone surrogate, which UTF-8 cannot encode; a line break, - which ends the line the manifest's reader reads the value from (CR, - U+0085 and U+2028 among them, as `str.splitlines` ends lines); and any - other control character, NUL among them, which YAML does not allow raw - (TAB aside, and no name, date or reference holds one either). + `one_printable_line` LESS `Cf` OTHER THAN THE NINE `BIDI_CONTROLS`, AND + THE EMPTY VALUE, because both are text a file holds and both were + adopted before #247 and since. Such a `Cf` character is invisible, not + unwritable: UTF-8 encodes it, the reader reads it back, and a name may + need one -- a Persian name joins its parts with U+200C ZERO WIDTH + NON-JOINER, which `one_printable_line` refuses. An empty `elected_by` is + what `plan` records when `git config user.name` names nobody, and the + manifest holds it as `""`. What is left is refused: a lone surrogate, + which UTF-8 cannot encode; a line break, which ends the line the + manifest's reader reads the value from (CR, U+0085 and U+2028 among + them, as `str.splitlines` ends lines); a bidi embedding, override or + isolate control, which a file holds but which shows the rest of the line + it is written into -- the README's `Elected by` line, its date and + reference among it -- in another order than it is written, as + `double_quoted` in `scripts/repo_shape.py` spells one in the plan for + the same reason; and any other control character, NUL among them, which + YAML does not allow raw (TAB aside, and no name, date or reference holds + one either). """ if value.splitlines() not in ([], [value]): return "it holds a line break, and it is written as one line" @@ -811,6 +826,10 @@ def unwritable_reason(value: str) -> str | None: return ("it holds a byte that is not UTF-8" if "\udc80" <= char <= "\udcff" else "it holds a lone surrogate, which is no character") + if ord(char) in BIDI_CONTROLS: + return ("it holds a bidirectional control (an embedding, " + "override or isolate), which shows the rest of the line " + "in another order than it is written") if category in UNPRINTABLE_CATEGORIES and category != "Cf": return "it holds a control character" return None diff --git a/tests/test_adopt_template_values.py b/tests/test_adopt_template_values.py index 1fd177e3..3c5f6d32 100644 --- a/tests/test_adopt_template_values.py +++ b/tests/test_adopt_template_values.py @@ -21,14 +21,19 @@ `local_path` that is not text is `source-unresolvable`. Every run is read for a traceback, and asked afterwards whether a leg remote exists. -THE QUESTION IS `one_printable_line` LESS `Cf`. A Persian name joins its -parts with U+200C ZERO WIDTH NON-JOINER, which UTF-8 holds and the reader -reads back, and `execute` adopted it before #247 and since: it still adopts, -as `José` and a source directory `café` do. +THE QUESTION IS `one_printable_line` LESS `Cf`, EXCEPT THE NINE BIDI +CONTROLS. A Persian name joins its parts with U+200C ZERO WIDTH NON-JOINER, +which UTF-8 holds and the reader reads back, and `execute` adopted it before +#247 and since: it still adopts, as `José` and a source directory `café` do. +The embedding, override and isolate controls U+202A..U+202E and +U+2066..U+2069 are refused: written raw into `README.md`, one shows the rest +of its line in another order than it is written. Every subprocess runs under `PYTHONIOENCODING=utf-8:strict`, so that a value printed raw raises on every platform as it does on a strict terminal, and is -answered in BYTES: a value that is not UTF-8 is not text to decode. +answered in BYTES: a value that is not UTF-8 is not text to decode. Every +invisible character in this file is spelled as an escape: a reader cannot +see one typed raw, and a raw bidi control reorders the line it is on. """ from __future__ import annotations @@ -164,13 +169,16 @@ def in_the_split(source: Path, path: str) -> str: #: `(flag, value)`, as `plan`'s argv carries it. `\udce9` is how Python keeps #: the byte 0xE9 of a command line that is not UTF-8, which is what a -#: terminal or a script whose encoding is not the locale's sends; the other -#: two are text a file holds, but on no one line. +#: terminal or a script whose encoding is not the locale's sends; the next +#: two are text a file holds, but on no one line; the last two hold a bidi +#: override or isolate, which a file holds and a reader is shown reordered. FLAGS = [ ("--elected-by", "Jos\udce9"), ("--reference", "ref-\udce9"), - ("--elected-by", "Jos
e"), + ("--elected-by", "Jos\u2028e"), ("--reference", "openxFactory\tdocs/project-repo-schema.md"), + ("--elected-by", "\u202eevil"), + ("--reference", "ref \u2066x\u2069 y"), ] @@ -180,8 +188,9 @@ def in_the_split(source: Path, path: str) -> str: def test_plan_refuses_a_flag_no_file_can_hold(tmp_path, flag, value): """The issue's first two rows. On main `plan` exited 0, `check` said `plan ok`, and `execute` raised in `write_lf` after both legs were - pushed (a surrogate), or adopted with the line break or the tab in the - manifest. Now `plan` refuses, and writes no plan.""" + pushed (a surrogate), or adopted with the line break, the tab or the + bidi control in the manifest and the README. Now `plan` refuses, and + writes no plan.""" source = make_source_repo(tmp_path / "Thing") runs = plan_check_execute(tmp_path, source, flag, value) assert_no_traceback_and_no_leg(tmp_path, runs) @@ -195,35 +204,84 @@ def test_plan_refuses_a_flag_no_file_can_hold(tmp_path, flag, value): # --- `execute`: a local clone URL no file can hold --------------------------- +#: What the clone-URL refusal tells the person to do. `refused_then_moved` +#: does it, as it is worded. +MOVE_REMEDIATION = ( + b"Remediation: nothing was created. Rename or move the source repository " + b"so that its path is one line of UTF-8 text with no control character " + b"in it, then run `plan --source `, `check` and `execute` " + b"again") + + +def refused_then_moved(base: Path, parent: Path, reason: bytes) -> None: + """A source in the directory `parent`: `plan` and `check` pass it, as + #247 made them, and `execute` refuses its path as the assembly root's + clone URL, giving `reason`, before any leg exists. Then the remediation + the refusal gives, followed as it is worded -- `parent` renamed, then + `plan --source `, `check` and `execute` run again -- ends + `adoption verified`.""" + try: + source = make_source_repo(parent / "Thing") + except AssertionError as exc: + pytest.skip(f"git will not work under {parent.name!r} here: {exc}") + runs = plan_check_execute(base, source) + assert_no_traceback_and_no_leg(base, runs) + assert runs["plan"].returncode == 0, said(runs["plan"]) + assert runs["check"].returncode == 0, said(runs["check"]) + assert PLAN_OK in runs["check"].stdout, said(runs["check"]) + refused = runs["execute"] + assert refused.returncode == 2, said(refused) + assert ADOPT_REFUSED + b"under --local-remote-dir the assembly root's " \ + b"clone URL is the source's own path, " \ + + repr(str(source.resolve())).encode() + b", which " \ + in refused.stderr, said(refused) + assert reason in refused.stderr, said(refused) + assert MOVE_REMEDIATION in refused.stderr, said(refused) + + moved = parent.rename(parent.with_name("cafe")) / "Thing" + again = base / "again" + again.mkdir() + runs = plan_check_execute(again, moved) + assert_no_traceback(runs) + assert runs["check"].returncode == 0, said(runs["check"]) + done = runs["execute"] + assert done.returncode == 0, said(done) + assert VERIFIED in done.stdout, said(done) + + @needs_filter_repo def test_execute_refuses_a_local_source_directory_that_is_not_utf8(tmp_path): """The issue's third row. Under `--local-remote-dir` the assembly root's clone URL is the source's own path, which its `README.md` names: `plan` - and `check` pass such a directory, as #247 made them, and `execute` - refuses it before any leg exists, where it raised after both were - pushed. Skipped where no directory can be named with the byte 0xE9 - (Windows, macOS's APFS) or git will not work in one.""" + and `check` pass such a directory, and `execute` refuses it before any + leg exists, where it raised after both were pushed. Its remediation + once said to clone the source to a UTF-8 path and pass that with + --source, which ended in a traceback: the clone's `origin` is the old + path, which `_remote_repository` reads as UTF-8. Skipped where no + directory can be named with the byte 0xE9 (Windows, macOS's APFS) or + git will not work in one.""" try: parent = Path(os.fsdecode(os.path.join(os.fsencode(tmp_path), b"caf\xe9"))) parent.mkdir() except (OSError, UnicodeError) as exc: pytest.skip(f"no directory here is named b'caf\\xe9': {exc!r}") + refused_then_moved(tmp_path, parent, b"a byte that is not UTF-8") + + +@needs_filter_repo +def test_execute_refuses_a_local_source_directory_holding_a_bidi_control( + tmp_path): + """A source under a directory whose name begins with U+202E RIGHT-TO-LEFT + OVERRIDE: on main `execute` adopted it, and the shape's README showed + the rest of its `git clone` line reversed. Now it is refused as the + byte is, and moving the source adopts.""" + parent = tmp_path / "\u202ecaf" try: - source = make_source_repo(parent / "Thing") - except AssertionError as exc: - pytest.skip(f"git will not work under b'caf\\xe9' here: {exc}") - runs = plan_check_execute(tmp_path, source) - assert_no_traceback_and_no_leg(tmp_path, runs) - assert runs["plan"].returncode == 0, said(runs["plan"]) - assert runs["check"].returncode == 0, said(runs["check"]) - assert PLAN_OK in runs["check"].stdout, said(runs["check"]) - refused = runs["execute"] - assert refused.returncode == 2, said(refused) - assert ADOPT_REFUSED + b"under --local-remote-dir the assembly root's " \ - b"clone URL is the source's own path, " \ - + repr(str(source.resolve())).encode() in refused.stderr, said(refused) - assert b"nothing was created" in refused.stderr + parent.mkdir() + except OSError as exc: + pytest.skip(f"no directory here is named '\\u202ecaf': {exc!r}") + refused_then_moved(tmp_path, parent, b"a bidirectional control") # --- `check` and `execute`: a plan value edited by hand ---------------------- @@ -284,7 +342,7 @@ def edited_plan(base: Path, answered: tuple[Path, str], line: str, ("elected_by-nul", "elected_by", "T\x00", "elected_by"), ("elected_by-cr", "elected_by", "T\rX", "elected_by"), ("elected_by-nel", "elected_by", "T\x85X", "elected_by"), - ("elected_by-u2028", "elected_by", "T
X", "elected_by"), + ("elected_by-u2028", "elected_by", "T\u2028X", "elected_by"), ("elected_by-tab", "elected_by", "T\tX", "elected_by"), ("elected_on-surrogate", "elected_on", "2026-09-02\ud800", "elected_on"), ("reference-surrogate", "reference", "x\ud800", "reference"), @@ -293,6 +351,10 @@ def edited_plan(base: Path, answered: tuple[Path, str], line: str, ("pins-surrogate", "pins", "openX\ud800", "a `pins:` item"), ("follow_ups-surrogate", "follow_ups", "fix x\ud800", "a `follow_ups:` item"), + ("elected_by-override", "elected_by", "\u202eevil", "elected_by"), + ("visibility-override", "visibility", "private\u202e", "visibility"), + ("follow_ups-isolate", "follow_ups", "fix \u2066x", + "a `follow_ups:` item"), ] @@ -315,9 +377,10 @@ def test_check_finds_and_execute_refuses_a_hand_edited_value( tmp_path, answered, key, value, what): """On main `check` said `plan ok` (or raised printing the value), and `execute` raised in `write_lf` after both legs were pushed, or adopted - with the NUL or the line break in the manifest. Now `check` has a - FINDING, and `execute` refuses it through `_refuse_what_check_finds`, in - `check`'s words, before any leg exists.""" + with the NUL, the line break or the bidi control in the manifest or the + split commit's message. Now `check` has a FINDING, and `execute` refuses + it through `_refuse_what_check_finds`, in `check`'s words, before any + leg exists.""" edit(tmp_path, answered, key, value) runs = {"check": checked(tmp_path), "execute": executed(tmp_path)} assert_no_traceback_and_no_leg(tmp_path, runs) @@ -343,6 +406,9 @@ def test_plan_and_check_say_the_same_sentence(tmp_path, answered): refused = planned(flagged, answered[0], "--elected-by", value) edit(by_hand, answered, "elected_by", value) found = checked(by_hand) + assert_no_traceback({"plan": refused, "check": found}) + assert refused.returncode == 2, said(refused) + assert found.returncode == 1, said(found) assert ADOPT_REFUSED in refused.stderr, said(refused) assert FINDING in found.stderr, said(found) detail = refused.stderr.split(ADOPT_REFUSED, 1)[1].split(b"\n", 1)[0] @@ -424,7 +490,7 @@ def test_a_local_path_that_is_not_text_is_unresolvable(tmp_path, answered, # --- controls: valid text still adopts, as on main --------------------------- @needs_filter_repo -@pytest.mark.parametrize("elected_by", ["José", "Ali‌Reza"], +@pytest.mark.parametrize("elected_by", ["José", "Ali\u200cReza"], ids=["accented", "zero-width-non-joiner"]) def test_a_valid_non_ascii_elector_still_adopts(tmp_path, elected_by): """`José`, and a name joined with U+200C, which `one_printable_line` @@ -460,10 +526,17 @@ def test_a_utf8_source_directory_still_adopts_with_local_remotes(tmp_path): #: Text a file holds: the empty value `plan` records when `git config #: user.name` names nobody, a name, a date, the default reference, a literal -#: backslash, invisible `Cf` characters, and a space that is not ASCII's. -WRITABLE = ["", "José", "Ali‌Reza", "2026-09-02", +#: backslash, a space that is not ASCII's, and the invisible `Cf` characters +#: that are not bidi controls -- the joiners U+200C and U+200D, a byte-order +#: mark, a soft hyphen, and the marks U+200E, U+200F and U+061C, which set +#: no span's direction. +WRITABLE = ["", "José", "Ali\u200cReza", "2026-09-02", "openxFactory docs/project-repo-schema.md", "CORP\\rsmith", - "日本", "a b", "‮evil", "bom", "a­b"] + "日本", "a\u00a0b", "\ufeffbom", "a\u00adb", "a\u200db", + "a\u200eb", "a\u200fb", "a\u061cb"] + +#: The nine bidi embedding, override and isolate controls, by code point. +BIDI_CONTROLS = [*range(0x202A, 0x202F), *range(0x2066, 0x206A)] #: `(value, the words its reason holds)`. UNWRITABLE = [ @@ -474,10 +547,14 @@ def test_a_utf8_source_directory_still_adopts_with_local_remotes(tmp_path): ("a\r\nb", "a line break"), ("end\n", "a line break"), ("a\x0bb", "a line break"), ("a\x0cb", "a line break"), ("a\x1cb", "a line break"), ("a\x85b", "a line break"), - ("a
b", "a line break"), ("a
b", "a line break"), + ("a\u2028b", "a line break"), ("a\u2029b", "a line break"), ("a\x00b", "a control character"), ("a\tb", "a control character"), ("a\x1bb", "a control character"), ("a\x7fb", "a control character"), ("a\x9fb", "a control character"), + ("\u202eevil", "a bidirectional control"), + ("\u202a", "a bidirectional control"), + ("\u2066x\u2069", "a bidirectional control"), + ("\u2069", "a bidirectional control"), ] @@ -491,17 +568,23 @@ def test_text_a_file_holds_has_no_reason(value): ids=[ascii(v) for v, _ in UNWRITABLE]) def test_what_no_file_holds_is_named(value, words): reason = shape_materialize.unwritable_reason(value) - assert reason is not None and words in reason, reason - assert reason.isprintable() and reason.isascii() + assert reason is not None, ascii(value) + assert words in reason, reason + assert reason.isprintable(), reason + assert reason.isascii(), reason -def test_the_question_is_one_printable_line_less_cf(): +def test_the_question_is_one_printable_line_less_cf_but_the_bidi_controls(): """Every code point, alone: refused exactly when `one_printable_line` - refuses it and it is not `Cf`.""" + refuses it and it is not `Cf`, or when it is one of the nine bidi + controls, which are `Cf` and still refused.""" reason = shape_materialize.unwritable_reason printable = shape_materialize.one_printable_line + assert {unicodedata.category(chr(code)) + for code in BIDI_CONTROLS} == {"Cf"} wrong = [hex(code) for code in range(sys.maxunicode + 1) if (reason(chr(code)) is None) - != (printable(chr(code)) - or unicodedata.category(chr(code)) == "Cf")] + != (code not in BIDI_CONTROLS + and (printable(chr(code)) + or unicodedata.category(chr(code)) == "Cf"))] assert not wrong, wrong[:20] From 3aa0e5d5dcce2493637e7953013fff1056256d84 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:16:53 +0000 Subject: [PATCH 03/12] adopt-project.py: `plan` names an elector it read from `git config user.name` as `--elected-by (not given, so `git config user.name`)` when it refuses it, so the refusal no longer names a flag the person never passed (#255) Fix round 1 on PR #270, N2 of lane openRepoShape-2's review at aed9190. With no --elected-by, `plan` records `git config user.name`. A TAB there was refused as `--elected-by is 'Jo\tSmith'`, naming a flag the operator never passed. It is now named as #181's `FLAG_DEFAULTS` names a value `plan` filled in, and the remediation says to pass the flag or correct `git config user.name`. A test runs `plan` under `GIT_CONFIG_GLOBAL` holding a TAB name, and skips where a repository's own `user.name` answers first. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 11 ++++++--- tests/test_adopt_template_values.py | 37 +++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 2bb8e161..4d733007 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1244,19 +1244,24 @@ def _checked_plan_inputs(args, source: Source, # manifest field instead of as a question. election_date(args.elected_on) args.reference = args.reference or default_reference(args.elected_on) + # A value `plan` filled in by itself is named by where it came from, as + # #181 names one, and not as a flag the person never passed. + elector = ("--elected-by" if args.elected_by else + "--elected-by (not given, so `git config user.name`)") args.elected_by = args.elected_by or _elector() # Before the plan is written, in the words `check` would use (#255): a # value `execute` writes into a file must be text one can hold. The # date has had its answer above, `election_date`. unwritable = unwritable_values( - (("--elected-by", args.elected_by, ROOT_MANIFEST_AND_README), + ((elector, args.elected_by, ROOT_MANIFEST_AND_README), ("--reference", args.reference, ROOT_MANIFEST_AND_README))) if unwritable: raise Refusal("adopt-unwritable-value", " ".join(f"{problem}." for problem in unwritable), "Remediation: no plan was written. Pass each value " - "named again, retyped (`--elected-by` defaults to " - "`git config user.name`). " + WRITTEN_AS_TEXT) + "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) names = _names(args.project) pins = [_checked_pin_name(p) for p in (args.pin or []) if p.strip()] _check_names(naming, names, set(pins)) diff --git a/tests/test_adopt_template_values.py b/tests/test_adopt_template_values.py index 3c5f6d32..6dc4442b 100644 --- a/tests/test_adopt_template_values.py +++ b/tests/test_adopt_template_values.py @@ -86,11 +86,14 @@ reason="git filter-repo is not installed: `pip install git-filter-repo`") -def adopted(*args: str) -> subprocess.CompletedProcess: - """`adopt-project.py ` on a strict stdout, answered in bytes, each - stream with LF line ends whatever the platform printed.""" +def adopted(*args: str, + extra: dict | None = None) -> subprocess.CompletedProcess: + """`adopt-project.py ` on a strict stdout, with `extra` in its + environment, answered in bytes, each stream with LF line ends whatever + the platform printed.""" env = blank_unnamed_pin_sources( - {**IDENTITY, **os.environ, "LANES_LANE": "", **STRICT}, STRICT) + {**IDENTITY, **os.environ, "LANES_LANE": "", **(extra or {}), + **STRICT}, STRICT) done = subprocess.run([sys.executable, str(ADOPT), *args], capture_output=True, env=env, stdin=subprocess.DEVNULL, check=False) @@ -202,6 +205,32 @@ def test_plan_refuses_a_flag_no_file_can_hold(tmp_path, flag, value): assert not (tmp_path / PLAN_FILE).exists() +def test_plan_names_the_elector_it_took_from_git_config(tmp_path): + """With no --elected-by, `plan` records `git config user.name`. A TAB + there is refused like a TAB passed with the flag, and named by where + it came from, as #181 names a value `plan` filled in: not as a flag + the person never passed.""" + config = tmp_path / "gitconfig" + config.write_bytes(b'[user]\n\tname = "Jo\\tSmith"\n') + extra = {"GIT_CONFIG_GLOBAL": str(config)} + read = subprocess.run(["git", "config", "user.name"], cwd=str(REPO), + capture_output=True, env={**os.environ, **extra}, + check=False) + if read.stdout.rstrip(b"\r\n") != b"Jo\tSmith": + pytest.skip("`git config user.name` here is not read from " + f"GIT_CONFIG_GLOBAL: {read.stdout!r}") + source = make_source_repo(tmp_path / "Thing") + refused = adopted("plan", "--source", str(source), "--project", PROJECT, + "--org", "testorg", "--elected-on", "2026-09-02", + "--out", str(tmp_path / PLAN_FILE), "--work-dir", + str(tmp_path / WORK), extra=extra) + assert_no_traceback({"plan": refused}) + assert refused.returncode == 2, said(refused) + assert ADOPT_REFUSED + b"--elected-by (not given, so `git config " \ + b"user.name`) is 'Jo\\tSmith', which " in refused.stderr, said(refused) + assert not (tmp_path / PLAN_FILE).exists() + + # --- `execute`: a local clone URL no file can hold --------------------------- #: What the clone-URL refusal tells the person to do. `refused_then_moved` From 7e9cdc3a698a1e0ffba2d051daa5e3bf03b7755c Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:34:44 +0000 Subject: [PATCH 04/12] tests/test_adopt_template_values.py: the valid-elector control reads the manifest back through the reader instead of matching its line, so it holds on main, which since #250 writes a YAML template's value as `double_quoted` spells it (#255) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix round 1 on PR #270. Built on the merge with main be2790e, the U+200C control failed: `execute` adopted, but main b30595c (#250) renders a value between a YAML template's double quotes through `double_quoted`, so `project.yaml` holds `"Ali‌Reza"` with the character spelled as an escape, and the test looked for it raw. The reader gives back U+200C either way, so the control now asks the reader, and also asks for the name in the shape's README, which is not YAML and holds it as it stands on this branch and on main. GitHub's checks run on the pull request's merge with main, so they would have failed here. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_template_values.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/test_adopt_template_values.py b/tests/test_adopt_template_values.py index 6dc4442b..0150c0c7 100644 --- a/tests/test_adopt_template_values.py +++ b/tests/test_adopt_template_values.py @@ -523,7 +523,12 @@ def test_a_local_path_that_is_not_text_is_unresolvable(tmp_path, answered, ids=["accented", "zero-width-non-joiner"]) def test_a_valid_non_ascii_elector_still_adopts(tmp_path, elected_by): """`José`, and a name joined with U+200C, which `one_printable_line` - refuses and a file holds: adopted, and written into the manifest.""" + refuses and a file holds: adopted, read back from the manifest as + itself, and named in the shape's README. + + The manifest is asked through the reader, not for a line: since #250 a + YAML template's value is written as `double_quoted` spells it, so the + line holds U+200C as an escape there, and as it stands before.""" source = make_source_repo(tmp_path / "Thing") runs = plan_check_execute(tmp_path, source, "--elected-by", elected_by) assert_no_traceback(runs) @@ -531,8 +536,11 @@ def test_a_valid_non_ascii_elector_still_adopts(tmp_path, elected_by): done = runs["execute"] assert done.returncode == 0, said(done) assert VERIFIED in done.stdout, said(done) - assert f'\nelected_by: "{elected_by}"\n' in in_the_split(source, - "project.yaml") + manifest = tmp_path / "project.yaml" + manifest.write_bytes(in_the_split(source, "project.yaml").encode("utf-8")) + assert load_yaml(manifest)["elected_by"] == elected_by + assert f"Elected by {elected_by} on 2026-09-02" in in_the_split( + source, "shape/README.md") @needs_filter_repo From 8e09208fd86ee866cdac37c3df62093032e6459b Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:13:28 +0000 Subject: [PATCH 05/12] adopt-project.py: a `--source` that begins with `-` is `source-unresolvable` before anything runs, so `gh repo clone` is no longer handed it as its first argument, where gh's own parser read it as one of its options (#255) SonarCloud's S8705 on #270 asked whether a value reaches a command as an option. The lane's re-verify found one that does: `plan --source=-u/x` is no path that exists and holds a `/`, so `Source.open` took it for an `org/repo` name and ran `gh repo clone -u/x -- --quiet`. gh read `-u` as `--upstream-remote-name` with the value `/x`, and `--source=--upload-pack=x/y` as a flag it does not know. A hand-edited `local_path:` or `repository:` reaches the same call from `check` and `execute`. It cannot reach git as an option (gh hands git only what follows its `--`), and main does the same. The guard sits in the condition this PR already edits. No GitHub owner name begins with `-`, so nothing gh could clone is refused, and an existing path that begins with `-` is still taken as a path. The tests put a stub `gh` first on PATH that records each call: `plan` with either value, and `check` and `execute` on a plan whose `local_path:` holds it, each exit 2 `source-unresolvable` with no traceback and no leg, and the stub records nothing. Against 7e9cdc3's code all four fail with the stub's record of `repo clone -u/x ...` and `repo clone --upload-pack=x/y ...`. A control shows the stub records the clone of `testorg/Thing`. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 7 +- tests/test_adopt_template_values.py | 103 ++++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 6 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 4d733007..653b9bc9 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -399,7 +399,7 @@ def open(cls, spec: str, work_root: Path) -> "Source": repository = _remote_repository(path) else: if ("/" not in spec or WINDOWS_ABSOLUTE_RE.match(spec) - or unwritable_reason(spec)): + or unwritable_reason(spec) or spec.startswith("-")): # A DRIVE-LETTER PATH IS A PATH THAT IS NOT THERE, never an # `org/repo`. `D:/work/Thing` carries a `/` and would # otherwise be handed to `gh repo clone` as an owner named @@ -412,6 +412,11 @@ def open(cls, spec: str, work_root: Path) -> "Source": # cannot exist, which reached `print` (a surrogate raised on # a strict stdout) or `subprocess` (a NUL raised there) as a # traceback, where main had refused it. + # NOR IS A NAME THAT BEGINS WITH `-`. It is the first + # argument `gh repo clone` is given, and gh's own parser + # reads it as an option: `-u/x` set `--upstream-remote-name` + # to `/x`. No GitHub owner name begins with `-`, so nothing + # gh could clone is refused here. raise Refusal( "source-unresolvable", f"--source {spec!r} is neither a path that exists nor an " diff --git a/tests/test_adopt_template_values.py b/tests/test_adopt_template_values.py index 0150c0c7..251bfff3 100644 --- a/tests/test_adopt_template_values.py +++ b/tests/test_adopt_template_values.py @@ -41,6 +41,7 @@ import os import re import shutil +import stat import subprocess import sys import unicodedata @@ -48,7 +49,7 @@ import pytest -from conftest import (ADOPT, REPO, blank_unnamed_pin_sources, +from conftest import (ADOPT, REPO, WINDOWS_SKIP, blank_unnamed_pin_sources, make_source_repo, resolve) sys.path.insert(0, str(REPO / "scripts")) @@ -113,15 +114,17 @@ def planned(base: Path, source: Path, *flags) -def checked(base: Path) -> subprocess.CompletedProcess: +def checked(base: Path, + extra: dict | None = None) -> subprocess.CompletedProcess: return adopted("check", "--plan", str(base / PLAN_FILE), "--work-dir", - str(base / WORK)) + str(base / WORK), extra=extra) -def executed(base: Path) -> subprocess.CompletedProcess: +def executed(base: Path, + extra: dict | None = None) -> subprocess.CompletedProcess: return adopted("execute", "--plan", str(base / PLAN_FILE), "--yes", "--local-remote-dir", str(base / REMOTES), "--work-dir", - str(base / WORK)) + str(base / WORK), extra=extra) def answer(base: Path) -> None: @@ -516,6 +519,96 @@ def test_a_local_path_that_is_not_text_is_unresolvable(tmp_path, answered, in run.stderr, f"{name}:\n{said(run)}" +# --- `--source`: a name `gh` would read as one of its options ---------------- + +#: `--source` values that begin with `-`. Neither is a path that exists, and +#: each holds a `/`, so `Source.open` took it for an `org/repo` name and ran +#: `gh repo clone -- --quiet`, where gh's own parser read it +#: as an option: `-u` is `--upstream-remote-name`, and `--upload-pack=x/y` +#: is a flag gh does not know. No GitHub owner name begins with `-`. +OPTION_SOURCES = ["-u/x", "--upload-pack=x/y"] + + +def gh_stub(base: Path) -> tuple[dict, Path]: + """The environment that puts a `gh` first on PATH, and the file it + records each call's arguments in, one to a line. It refuses every call. + It is a `#!` script, which Windows does not execute: there nothing is + recorded, and the refusal's id alone says that `gh` did not run.""" + binary = base / "bin" + binary.mkdir() + script = binary / "gh" + script.write_bytes(b"#!/bin/sh\n" + b"printf '%s\\n' \"$@\" >> \"$GH_STUB_LOG\"\n" + b"echo 'stub gh: refusing' >&2\n" + b"exit 1\n") + script.chmod(script.stat().st_mode | stat.S_IEXEC) + log = base / "gh.log" + return ({"PATH": f"{binary}{os.pathsep}{os.environ.get('PATH', '')}", + "GH_STUB_LOG": str(log)}, log) + + +def gh_calls(log: Path) -> str: + """What the stub was given, readable in an assertion message.""" + return log.read_text(encoding="utf-8") if log.exists() else "" + + +@pytest.mark.parametrize("spec", OPTION_SOURCES, ids=OPTION_SOURCES) +def test_plan_refuses_a_source_gh_would_read_as_an_option(tmp_path, spec): + """On main `plan` handed `spec` to `gh repo clone` as its first + argument (SonarCloud's S8705 asked the question). Now it is + `source-unresolvable` before anything runs: no path that exists, and no + `org/repo` name.""" + stub, log = gh_stub(tmp_path) + refused = adopted("plan", f"--source={spec}", "--project", PROJECT, + "--org", "testorg", "--elected-by", "Test Human", + "--elected-on", "2026-09-02", "--out", + str(tmp_path / PLAN_FILE), "--work-dir", + str(tmp_path / WORK), extra=stub) + assert not log.exists(), f"gh was run with:\n{gh_calls(log)}" + assert_no_traceback({"plan": refused}) + assert refused.returncode == 2, said(refused) + assert SOURCE_REFUSED + f"--source {spec!r} is neither".encode() \ + in refused.stderr, said(refused) + assert not (tmp_path / PLAN_FILE).exists() + + +@pytest.mark.parametrize("spec", OPTION_SOURCES, ids=OPTION_SOURCES) +def test_check_and_execute_refuse_a_hand_edited_source_gh_would_read( + tmp_path, answered, spec): + """The plan file is edited between `plan` and `execute`, on purpose, and + its `local_path:` reaches the same `Source.open`. On main `check` and + `execute` each ran `gh repo clone` with it first.""" + edited_plan(tmp_path, answered, f" local_path: {double_quoted(spec)}") + assert load_yaml(tmp_path / PLAN_FILE)["source"]["local_path"] == spec + stub, log = gh_stub(tmp_path) + runs = {"check": checked(tmp_path, stub), + "execute": executed(tmp_path, stub)} + assert not log.exists(), f"gh was run with:\n{gh_calls(log)}" + assert_no_traceback_and_no_leg(tmp_path, runs) + for name, run in runs.items(): + assert run.returncode == 2, f"{name}:\n{said(run)}" + assert SOURCE_REFUSED + f"--source {spec!r} is neither".encode() \ + in run.stderr, f"{name}:\n{said(run)}" + + +@WINDOWS_SKIP # the stub `gh` is a `#!` script; Windows executes none +def test_the_gh_stub_records_an_org_repo_source(tmp_path): + """The control for the two tests above: an `org/repo` name is handed to + `gh repo clone`, and the stub records the call it refuses. A stub that + recorded nothing would let them pass without asking anything.""" + stub, log = gh_stub(tmp_path) + refused = adopted("plan", "--source=testorg/Thing", "--project", PROJECT, + "--org", "testorg", "--elected-by", "Test Human", + "--elected-on", "2026-09-02", "--out", + str(tmp_path / PLAN_FILE), "--work-dir", + str(tmp_path / WORK), extra=stub) + assert_no_traceback({"plan": refused}) + assert refused.returncode == 2, said(refused) + assert b"REFUSED source-unclonable: " in refused.stderr, said(refused) + assert gh_calls(log).splitlines()[:3] == ["repo", "clone", + "testorg/Thing"], gh_calls(log) + + # --- controls: valid text still adopts, as on main --------------------------- @needs_filter_repo From 2533515cfa303fb7dfc7249ba0be0c8bc2bb915f Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:55:25 +0000 Subject: [PATCH 06/12] adopt-project.py: `check` finds, and `execute` refuses before any leg exists, a plan `follow_ups:` that is not a list of text, so `follow_ups: 5` is no longer a `TypeError` traceback and `abc` is no longer written as three follow-ups (#293) `plan` writes `follow_ups:` as a block list, and its three readers (`_plan_values`, `_print_what_will_happen`, `_commit_the_split`) iterated the value with no test that it was one. A hand-edited `follow_ups: 5`, `true`, `false` or `1.5` ended `check` in `TypeError: ... is not iterable`, exit 1, with no FINDING naming the key. On main `execute` died the same way in `_commit_the_split`, after both legs were pushed; since #270 it dies in `_plan_values`, before any leg, but still as a traceback. `abc` went into the split commit's message as `- a`, `- b`, `- c`, and an item `- true` or `- null` as `True` or `None`. The plan is untrusted input (`cmd_execute`'s own comment), so the value is asked before it is used: - `_plan_follow_ups` is the one read for the three readers: the items of text of a `follow_ups:` list, and nothing for anything else. - `_follow_ups_findings` is a `plan-bad-follow-ups` FINDING for a `follow_ups:` that is present and not a list, naming what it holds (`the number 5`, `the text 'abc'`, `a mapping of 1 key(s)`), and one for each item that is not text. An empty list and a bare key stay valid. - They join `_entry_findings` through `_plan_field_findings`, last, so `check` reports them and `execute` refuses them through `_refuse_what_check_finds`, before `_create_leg_remotes`, under the finding's own code, with a remediation that says how `plan` writes the list. Tests in tests/test_adopt_execute_refusals.py: `check` (a finding, exit 1, no traceback) and `execute` (the refusal, no leg, no split branch) for nine edits, three controls, and one run through `main` with `git filter-repo` that is refused with exit 2 and then adopts from the corrected plan. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 119 +++++++++++++++++++-- tests/test_adopt_execute_refusals.py | 149 ++++++++++++++++++++++++++- 2 files changed, 257 insertions(+), 11 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index eb972407..d489e987 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -2724,6 +2724,99 @@ def _refused_by_policy(path: str) -> str | None: return None +# --------------------------------------------------------------------------- +# Plan values `execute` read late, asked by `check` before any leg exists +# --------------------------------------------------------------------------- +# +# Each value below was read by `execute` after `check` had said `plan ok`, and +# nothing in between asked what it held. `check` now asks, and `execute` +# refuses what it finds through `_refuse_what_check_finds`, before any leg +# exists. +# +# `follow_ups:` (#293). `plan` writes it as a block list, and every reader +# iterated the value with no test that it was one. A hand-edited +# `follow_ups: 5` (or `true`, `1.5`, `false`) ended `check` in +# `TypeError: 'int' object is not iterable`, a traceback with no FINDING +# naming the key, and on main ended `execute` the same way in +# `_commit_the_split`, after both legs were pushed; `abc` was written into the +# split commit's message as three follow-ups, `a`, `b` and `c`, and an item +# `- true` as `True`. + +#: `check`'s finding for a `follow_ups:` that is not a list of text, and the +#: code `execute` refuses it under (#293). +BAD_FOLLOW_UPS = "plan-bad-follow-ups" + +#: Said once after the `follow_ups:` findings. +FOLLOW_UPS_REMEDIATION = ( + "Remediation: write `follow_ups:` in the plan file as `plan` writes it, " + "a list with one item of text per follow-up, each on its own line as " + "` - \"\"`, or `follow_ups: []` for none. `execute` " + "writes each item as one line of the split commit's message, so it takes " + "nothing else: a number or a boolean is not a list, text on the key's own " + "line would be written one character per follow-up, and an item that is " + "not text would be written as Python spells it (`True`, `None`).") + + +def _read_as(value) -> str: + """What the plan's reader handed back for a value, in words, spelled as a + YAML plan spells it where that differs from Python (`true`, `null`).""" + if isinstance(value, bool): + return f"the boolean {str(value).lower()}" + if isinstance(value, (int, float)): + return f"the number {value!r}" + if isinstance(value, str): + return f"the text {value!r}" + if value is None: + return "null" + if isinstance(value, dict): + return f"a mapping of {len(value)} key(s)" + if isinstance(value, list): + return f"a list of {len(value)} item(s)" + return f"a value of type {type(value).__name__}" + + +def _plan_follow_ups(plan: Plan) -> list[str]: + """The plan's follow-ups as `execute` writes them into the split commit's + message: each item of a `follow_ups:` LIST that is text, and nothing for a + `follow_ups:` that is absent, bare or anything but a list. + + THE ONE READ, for the three readers (`_plan_values`, + `_print_what_will_happen`, `_commit_the_split`), so that none of them + iterates a number or spells a string out one character at a time. What it + leaves out is never silently dropped from a run: `_follow_ups_findings` + reports it in `check`, and `execute` has refused it through + `_refuse_what_check_finds` before the split commit is written. + """ + value = plan.get("follow_ups", []) + if not isinstance(value, list): + return [] + return [item for item in value if isinstance(item, str)] + + +def _follow_ups_findings(plan: Plan) -> list[str]: + """A FINDING for a `follow_ups:` that is present and not a list, or for + each item of one that is not text; `[]` for a list of text, an empty + list and a bare key (#293).""" + value = plan.get("follow_ups") + if value is None: + return [] + if not isinstance(value, list): + return [f"FINDING {BAD_FOLLOW_UPS}: `follow_ups:` is " + f"{_read_as(value)}, not a list: `execute` writes each item " + "of a list into the split commit's message as one follow-up"] + return [f"FINDING {BAD_FOLLOW_UPS}: item {number} of `follow_ups:` is " + f"{_read_as(item)}, not text: `execute` writes each item into the " + "split commit's message as one line of text" + for number, item in enumerate(value, 1) + if not isinstance(item, str)] + + +def _plan_field_findings(plan: Plan) -> tuple[list[str], list[str]]: + """`_follow_ups_findings`, and what `check` prints once after them.""" + followed = _follow_ups_findings(plan) + return followed, [FOLLOW_UPS_REMEDIATION] if followed else [] + + #: What a plan whose entry `entry_path_problem` refuses is told to do. Said by #: `check`, once after its findings, and by `execute`'s refusal. UNSAFE_PATH_REMEDIATION = ( @@ -2763,7 +2856,9 @@ def _entry_findings(plan: Plan, source: Source and what it prints once after them: each entry's path, then coverage, then each entry's `leg:`, the two mount paths and what the root keeps where the shape writes, then the source's own submodules, then each - value `execute` writes into a file that no file can hold (#255). + value `execute` writes into a file that no file can hold (#255). Last, + each plan value `execute` reads late and `_plan_field_findings` asks + about (#293). ONE LIST, read by `check` and by `execute`'s `_refuse_what_check_finds`. #168 and #166 each added findings to `check` and a refusal to `execute` @@ -2791,8 +2886,11 @@ def _entry_findings(plan: Plan, source: Source + _leg_findings(plan, tree)) submodule_findings, remediation = _submodule_findings(plan, source, tree) unwritable_findings, unwritable_remediation = _value_findings(plan) - return (findings + submodule_findings + unwritable_findings, - unsafe_remediation + remediation + unwritable_remediation) + field_findings, field_remediation = _plan_field_findings(plan) + return (findings + submodule_findings + unwritable_findings + + field_findings, + unsafe_remediation + remediation + unwritable_remediation + + field_remediation) def _topics_line(topic: str, local: bool) -> str: @@ -2833,8 +2931,8 @@ def _print_what_will_happen(plan: Plan, source: Source, names: dict, f"{len(drops)} dropped") print(" the source is never deleted, never renamed, never force-pushed") print(_topics_line(topic, local)) - for item in plan.get("follow_ups", []): - print(f" follow-up: {_shown(str(item))}") + for item in _plan_follow_ups(plan): + print(f" follow-up: {_shown(item)}") def cmd_check(args) -> int: @@ -3645,7 +3743,7 @@ def _commit_the_split(plan: Plan, source: Source, assembly: Path, print(f" beside {actual} (the source already has {intended}; nothing " "was overwritten)") - follow_ups = [str(f) for f in plan.get("follow_ups", [])] + follow_ups = _plan_follow_ups(plan) message = _split_message(names, paths_for, leg_commits, spec_path, code_path, follow_ups, materialized.collisions, seeded) @@ -3827,13 +3925,16 @@ def _plan_values(plan: Plan) -> list[tuple[str, str, str]]: """`(what, value, where)` for each value of the plan's that `execute` writes into a file, as `str` makes it, which is how `_template_values` and `_commit_the_split` take it. A key the plan does not carry is filled - by this tool (`private`, today's date) and is left out.""" + by this tool (`private`, today's date) and is left out. The follow-ups + are `_plan_follow_ups`', the items of text `_commit_the_split` writes; a + `follow_ups:` that is not a list of text is `plan-bad-follow-ups` (#293), + and is never iterated here.""" fields = [(key, str(plan.get(key)), where) for key, where in WRITTEN_PLAN_VALUES.items() if plan.get(key) is not None] fields += [("a `pins:` item", pin, ROOT_MANIFEST) for pin in plan.pins] - return fields + [("a `follow_ups:` item", str(item), SPLIT_MESSAGE) - for item in plan.get("follow_ups", [])] + return fields + [("a `follow_ups:` item", item, SPLIT_MESSAGE) + for item in _plan_follow_ups(plan)] def _value_findings(plan: Plan) -> tuple[list[str], list[str]]: diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index 4c4e5b79..86356dc1 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -21,8 +21,8 @@ themselves; #168's submodule cases are refused by the same function under the same two codes, `plan-uncovered` and `plan-bad-leg`. -NO NETWORK, NO GITHUB, AND NO `git filter-repo` except in the one end-to-end -test at the bottom. The rest load `adopt-project.py` through importlib and call +NO NETWORK, NO GITHUB, AND NO `git filter-repo` except in the end-to-end tests +at the bottom. The rest load `adopt-project.py` through importlib and call `cmd_execute` in this process. Its probe for `git filter-repo` is stubbed, because every refusal under test is raised BEFORE the tool would be used. A `cmd_execute` that stopped refusing would carry on and make the legs, then @@ -35,6 +35,7 @@ import importlib.util import io +import re import shutil import sys from argparse import Namespace @@ -432,6 +433,125 @@ def test_a_path_that_is_a_git_option_is_still_unsafe_not_uncovered(adopter, no_leg_was_created(world.tmp) +# --- plan values `execute` read late --------------------------------------- + +#: `follow_ups:` and every item under it, as `plan` writes the block; the +#: edits below replace the whole of it. +FOLLOW_UPS_BLOCK = re.compile(r"^follow_ups:\n(?: - .*\n)*", re.M) +BAD_FOLLOW_UPS_CODE = "plan-bad-follow-ups" + +#: What a hand edit can leave `follow_ups:` holding that is not a list of +#: text, by test id: the block it is written as, and what the finding must say +#: of it (#293). The scalars ended `check` in `TypeError: ... is not iterable` +#: with no FINDING, and on main ended `execute` the same way after both legs +#: were pushed; `abc` went into the split commit's message as three +#: follow-ups, `a`, `b` and `c`, and an item `true` as `True`. +BAD_FOLLOW_UPS = { + "int": ("follow_ups: 5\n", "`follow_ups:` is the number 5, not a list"), + "true": ("follow_ups: true\n", + "`follow_ups:` is the boolean true, not a list"), + "false": ("follow_ups: false\n", + "`follow_ups:` is the boolean false, not a list"), + "float": ("follow_ups: 1.5\n", + "`follow_ups:` is the number 1.5, not a list"), + "text": ("follow_ups: abc\n", + "`follow_ups:` is the text 'abc', not a list"), + "mapping": ("follow_ups: {a: 1}\n", + "`follow_ups:` is a mapping of 1 key(s), not a list"), + "item-int": ("follow_ups:\n - 5\n", + "item 1 of `follow_ups:` is the number 5, not text"), + "item-true": ('follow_ups:\n - "keep"\n - true\n', + "item 2 of `follow_ups:` is the boolean true, not text"), + "item-null": ("follow_ups:\n - null\n", + "item 1 of `follow_ups:` is null, not text"), +} + +#: What `check` and `execute` still take, by test id, and the follow-ups each +#: reads: an empty list, a bare key, and a list of text as `plan` writes it. +GOOD_FOLLOW_UPS = { + "empty-list": ("follow_ups: []\n", []), + "bare-key": ("follow_ups:\n", []), + "text-items": ('follow_ups:\n - "Re-point CI at code/"\n' + ' - "merge shape/README.md"\n', + ["Re-point CI at code/", "merge shape/README.md"]), +} + + +def with_follow_ups(text: str, block: str) -> str: + """`text` with its `follow_ups:` block replaced by `block`.""" + edited, done = FOLLOW_UPS_BLOCK.subn(lambda _: block, text, count=1) + assert done == 1, "the plan has no `follow_ups:` block to edit" + return edited + + +def assert_no_traceback(result) -> None: + for output in (result.stdout, result.stderr): + assert "Traceback" not in output, output + + +def findings_of(result) -> list[str]: + return [line for line in result.stderr.splitlines() + if line.startswith("FINDING ")] + + +@pytest.mark.parametrize("edit", sorted(BAD_FOLLOW_UPS)) +def test_check_finds_a_follow_ups_that_is_not_a_list_of_text(world, edit): + """#293: a FINDING that names the key and what it holds, exit 1, and no + traceback. At #270's head `check` read the value in `_plan_values` and + ended in `TypeError: 'int' object is not iterable`; on main it did so in + `_print_what_will_happen`. `abc` printed three follow-ups, `a`, `b` and + `c`, which the summary no longer reads.""" + block, said = BAD_FOLLOW_UPS[edit] + result = run_check(write_edited(world, + with_follow_ups(world.answered, block))) + assert result.returncode == 1, result.stderr + result.stdout + assert_no_traceback(result) + found = findings_of(result) + assert len(found) == 1, found + assert found[0].startswith(f"FINDING {BAD_FOLLOW_UPS_CODE}: {said}: "), \ + found + assert "Remediation: write `follow_ups:` in the plan file" in \ + result.stderr + shown = [line for line in result.stdout.splitlines() + if line.startswith(" follow-up: ")] + assert shown == ([" follow-up: keep"] if '"keep"' in block else []), \ + shown + + +@pytest.mark.parametrize("edit", sorted(BAD_FOLLOW_UPS)) +def test_execute_refuses_a_follow_ups_that_is_not_a_list_of_text_and_creates_no_leg( # noqa: E501 + adopter, world, edit): + """Through `_refuse_what_check_finds`, before any leg exists, under the + finding's own code. At #270's head this was the same `TypeError` raised + from the same list; on main it was raised in `_commit_the_split`, after + both legs were pushed.""" + block, said = BAD_FOLLOW_UPS[edit] + world.plan.write_text(with_follow_ups(world.answered, block), + encoding=UTF8) + refusal = refused_by(adopter, execute_args(world.tmp, world.plan)) + assert refusal.code == BAD_FOLLOW_UPS_CODE + assert f"FINDING {BAD_FOLLOW_UPS_CODE}: {said}" in refusal.detail + assert "write `follow_ups:` in the plan file" in refusal.remediation + no_leg_was_created(world.tmp) + assert source_branches(world.source) == ["main"] + + +@pytest.mark.parametrize("edit", sorted(GOOD_FOLLOW_UPS)) +def test_a_list_of_text_an_empty_list_and_a_bare_key_still_pass(adopter, + world, edit): + """The controls: what `plan` writes, and the two ways of writing none, + pass `check` and `execute`'s refusal, and the follow-ups read are the + items as written.""" + block, items = GOOD_FOLLOW_UPS[edit] + text = with_follow_ups(world.answered, block) + checked = run_check(write_edited(world, text)) + assert checked.returncode == 0, checked.stderr + checked.stdout + for item in items: + assert f" follow-up: {item}" in checked.stdout + plan, source = open_plan(adopter, world, text) + assert adopter._refuse_what_check_finds(plan, source) is None + + # --- a plan that is not UTF-8 (#192) ---------------------------------------- #: The byte `0xff` is in no UTF-8 text, so wherever it lands the file cannot be @@ -541,3 +661,28 @@ def test_an_uncovered_plan_creates_no_leg_and_the_corrected_plan_then_runs( assert "adoption verified" in corrected.stdout for role in ROLES: assert (remotes / f"{PROJECT}-{role}.git").is_dir() + + +@needs_filter_repo +def test_a_follow_ups_that_is_not_a_list_creates_no_leg_and_the_corrected_plan_runs( # noqa: E501 + world): + """#293 as a person met it, through `main`: exit 2, a REFUSED line, no + traceback and no leg, so the corrected plan does not meet + `leg-remote-exists`.""" + remotes, work = world.tmp / REMOTES, world.tmp / WORK + flags = ("--plan", str(world.plan), "--yes", "--local-remote-dir", + str(remotes), "--work-dir", str(work)) + world.plan.write_text(with_follow_ups(world.answered, "follow_ups: 5\n"), + encoding=UTF8) + refused = run_script(ADOPT, "execute", *flags) + assert refused.returncode == 2, refused.stderr + refused.stdout + assert f"REFUSED {BAD_FOLLOW_UPS_CODE}: " in refused.stderr + assert_no_traceback(refused) + assert "creating the leg repositories" not in refused.stdout + assert not remotes.exists(), "not even the directory the legs go in" + assert source_branches(world.source) == ["main"] + + world.plan.write_text(world.answered, encoding=UTF8) + corrected = run_script(ADOPT, "execute", *flags) + assert corrected.returncode == 0, corrected.stderr + corrected.stdout + assert "adoption verified" in corrected.stdout From 84f380677f2ee55ff3badd49b956a07f4a8ab6cc Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:55:26 +0000 Subject: [PATCH 07/12] adopt-project.py: `check` asks `election_date` about the plan's `elected_on:` whether or not the plan has a `reference:`, and `execute` refuses what it finds before any leg exists, so `elected_on: someday` is no longer refused after both legs are created, under `--elected-on`, a flag neither command takes (#294) `check` never read `elected_on:`. `execute` read it in `_template_values` only for a plan with no `reference:`, where `default_reference` asks `election_date`. That runs after `_create_leg_remotes`, so a hand-edited `elected_on: someday` passed `check` (`plan ok`). `execute` then created both leg repositories and only afterwards refused `election-date-malformed`. The refusal named `--elected-on` and said to re-run with it, but only `plan` takes that flag. A second `execute` met `leg-remote-exists`. With `reference:` kept, the date was never read, and `execute` wrote `someday` into the manifest. `_elected_on_findings` asks `election_date` about `str` of what the plan holds. That is the value `_template_values` takes, and the rule `plan` asks of `--elected-on`. It asks whether or not the plan has a `reference:`. A date it refuses is a FINDING `election-date-malformed` that names the plan's `elected_on:` and no flag, with a remediation that says to edit the key in the plan file. It joins `_plan_field_findings` ahead of the follow-ups, in the order the plan file holds the two keys. `execute` refuses it through `_refuse_what_check_finds`, before any leg exists. The rule stays `election_date`'s, unchanged. It reads `2026-W41-6` and `20261011` from Python 3.11, and the project's validator refuses both. That is #261's to rule on (PR #314 holds `election_date` to the validator's `DATE_RE`). Asking the same function keeps `plan`, `check` and `execute` in agreement now, and asks the tighter rule with no edit once it lands. Only the words are this caller's: `election_date`'s refusal names `--elected-on`, which is right for `plan` and wrong here. Only that refusal is a finding about the date. Anything else `election_date` raises is raised as it is, so a date it never judged is not reported as malformed. An example is #314's `manifest-validator-unusable`, when the validator it consults is missing. Tests in tests/test_adopt_execute_refusals.py: - `check` and `execute` for five dates (four with no `reference:`, one with it kept), and four controls; - `check`'s verdict equals `election_date`'s on the running interpreter, for `2026-W41-6`, `20261011`, a date and a word; - a stricter `election_date`, stood in by monkeypatch, refuses `2026-W41-6` before any leg; - another refusal from `election_date` is raised as it is; - both findings in one plan; - one run through `main` that is refused with exit 2 and no leg, then adopts from a corrected plan with no `reference:` and records the reference its date chooses. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 71 +++++++++- tests/test_adopt_execute_refusals.py | 203 +++++++++++++++++++++++++++ 2 files changed, 270 insertions(+), 4 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index d489e987..b0349c7a 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -2741,6 +2741,19 @@ def _refused_by_policy(path: str) -> str | None: # `_commit_the_split`, after both legs were pushed; `abc` was written into the # split commit's message as three follow-ups, `a`, `b` and `c`, and an item # `- true` as `True`. +# +# `elected_on:` (#294). Nothing asked whether it was a date but +# `_template_values`, and only for a plan with no `reference:`: +# `default_reference` asked `election_date` after `_create_leg_remotes` had +# made both legs, and the refusal named `--elected-on`, a flag neither `check` +# nor `execute` takes. With `reference:` kept the date was never asked, and +# `execute` wrote `someday` into the manifest. THE DATE IS ASKED OF +# `election_date`, the rule `plan` asks of `--elected-on` and +# `_template_values` of the plan's date, and of nothing else: what that rule +# admits is #261's to decide, and a tighter rule there is asked here with no +# edit. Only the words are this caller's, because the date came from the plan +# file and not from a flag: the refusal `election_date` raises names +# `--elected-on`, which is right for `plan` and wrong here. #: `check`'s finding for a `follow_ups:` that is not a list of text, and the #: code `execute` refuses it under (#293). @@ -2811,10 +2824,60 @@ def _follow_ups_findings(plan: Plan) -> list[str]: if not isinstance(item, str)] +#: `check`'s finding for an `elected_on:` that `election_date` does not read, +#: and the code `execute` refuses it under: the code `plan` refuses a bad +#: `--elected-on` under, said here of the plan's key (#294). +ELECTION_DATE_MALFORMED = "election-date-malformed" + +#: Said once after the `elected_on:` finding. It names the plan file's key and +#: no flag: `check` and `execute` take none for the date. +ELECTED_ON_REMEDIATION = ( + "Remediation: edit `elected_on:` in the plan file to the day the shape " + "was elected, written as a zero-padded ISO date in quotes, e.g. " + "`elected_on: \"2026-09-02\"`: not 2026-9-2, and not a word like " + "'yesterday'. `execute` writes it into the project's `project.yaml`, and " + "a plan with no `reference:` has its reference chosen BY that date, so " + "it is read before anything is created.") + + +def _elected_on_findings(plan: Plan) -> list[str]: + """A FINDING for an `elected_on:` that `election_date` does not read, + whether or not the plan has a `reference:` (#294); `[]` for a date it + reads and for no `elected_on:` at all, which `execute` fills with today's. + + The value asked is the one `_template_values` takes, `str` of what the + plan holds, so the two cannot answer differently. `election_date`'s own + refusal is not passed on, because its words are `plan`'s: they name + `--elected-on`, which neither `check` nor `execute` takes. Only THAT + refusal is a finding about the date: any other `election_date` raises + is about something else (a rule it could not ask), and is raised as it + is. + """ + elected_on = plan.get("elected_on") + if elected_on is None: + return [] + try: + election_date(str(elected_on)) + except Refusal as refused: + if refused.code != ELECTION_DATE_MALFORMED: + raise + chosen = ("" if plan.get("reference") else + ", and, the plan naming no `reference:`, chooses the " + "reference by it") + return [f"FINDING {ELECTION_DATE_MALFORMED}: the plan's `elected_on:` " + f"is {_read_as(elected_on)}, which is not a day written " + "YYYY-MM-DD, the one form `execute` reads: it writes the date " + f"into the project's `project.yaml`{chosen}"] + return [] + + def _plan_field_findings(plan: Plan) -> tuple[list[str], list[str]]: - """`_follow_ups_findings`, and what `check` prints once after them.""" - followed = _follow_ups_findings(plan) - return followed, [FOLLOW_UPS_REMEDIATION] if followed else [] + """`_elected_on_findings` and `_follow_ups_findings`, in the order the + plan file holds the two keys, and what `check` prints once after them.""" + dated, followed = _elected_on_findings(plan), _follow_ups_findings(plan) + return (dated + followed, + ([ELECTED_ON_REMEDIATION] if dated else []) + + ([FOLLOW_UPS_REMEDIATION] if followed else [])) #: What a plan whose entry `entry_path_problem` refuses is told to do. Said by @@ -2858,7 +2921,7 @@ def _entry_findings(plan: Plan, source: Source where the shape writes, then the source's own submodules, then each value `execute` writes into a file that no file can hold (#255). Last, each plan value `execute` reads late and `_plan_field_findings` asks - about (#293). + about (#293, #294). ONE LIST, read by `check` and by `execute`'s `_refuse_what_check_finds`. #168 and #166 each added findings to `check` and a refusal to `execute` diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index 86356dc1..bde46eb9 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -552,6 +552,178 @@ def test_a_list_of_text_an_empty_list_and_a_bare_key_still_pass(adopter, assert adopter._refuse_what_check_finds(plan, source) is None +DATE_CODE = "election-date-malformed" +DATE_FLAG = "--elected-on" + +#: An `elected_on:` that `election_date` refuses on every Python, by test id: +#: as the plan holds it, as the reader reads it, and whether the plan keeps +#: its `reference:` (#294). Without one, `execute` made both legs and only +#: then refused, naming `--elected-on`; with one it never read the date and +#: wrote the word into the manifest. +BAD_DATES = { + "word": ("someday", "someday", False), + "notadate": ("notadate", "notadate", False), + "unpadded": ('"2026-9-2"', "2026-9-2", False), + "not-a-day": ('"2026-02-30"', "2026-02-30", False), + "word-reference-kept": ("someday", "someday", True), +} +ONE_PER_BAD_DATE = pytest.mark.parametrize("edit", sorted(BAD_DATES)) + + +def with_elected_on(text: str, value: str, reference: bool) -> str: + """`text` with `elected_on:` set to `value` by hand, and its `reference:` + line deleted unless `reference`.""" + if not reference: + text, done = re.subn(r"^reference:.*\n", "", text, count=1, + flags=re.M) + assert done == 1, "the plan has no `reference:` line to delete" + edited, done = re.subn(r"^elected_on: .*$", + lambda _: f"elected_on: {value}", text, count=1, + flags=re.M) + assert done == 1, "the plan has no `elected_on:` line to edit" + return edited + + +@ONE_PER_BAD_DATE +def test_check_finds_an_elected_on_it_cannot_read_under_the_plans_key(world, + edit): + """#294: a FINDING `election-date-malformed` that names the plan's + `elected_on:` and no flag, whether or not the plan has a `reference:`. + `check` used to say `plan ok`: it never read the date.""" + value, read, reference = BAD_DATES[edit] + result = run_check(write_edited( + world, with_elected_on(world.answered, value, reference))) + assert result.returncode == 1, result.stderr + result.stdout + assert_no_traceback(result) + found = findings_of(result) + assert len(found) == 1, found + assert found[0].startswith( + f"FINDING {DATE_CODE}: the plan's `elected_on:` is the text " + f"{read!r}, which is not a day written YYYY-MM-DD"), found + assert ("the plan naming no `reference:`" in found[0]) is not reference + assert "Remediation: edit `elected_on:` in the plan file" in \ + result.stderr + assert DATE_FLAG not in result.stderr + result.stdout + + +@ONE_PER_BAD_DATE +def test_execute_refuses_an_elected_on_it_cannot_read_before_any_leg( + adopter, world, edit): + """The same finding, refused through `_refuse_what_check_finds` before + `_create_leg_remotes`. With no `reference:`, `execute` used to create + both legs and then refuse from `default_reference`, in words that named + `--elected-on`; with one, it went on and wrote the word into the + manifest.""" + value, read, reference = BAD_DATES[edit] + world.plan.write_text(with_elected_on(world.answered, value, reference), + encoding=UTF8) + refusal = refused_by(adopter, execute_args(world.tmp, world.plan)) + assert refusal.code == DATE_CODE + no_leg_was_created(world.tmp) + assert source_branches(world.source) == ["main"] + assert f"the plan's `elected_on:` is the text {read!r}" in refusal.detail + assert "edit `elected_on:` in the plan file" in refusal.remediation + assert DATE_FLAG not in str(refusal) + + +@pytest.mark.parametrize("value,reference", [ + ('"2026-09-02"', False), ('"2026-06-01"', False), ('"2026-09-02"', True), + ("null", False)]) +def test_an_elected_on_election_date_reads_still_passes(adopter, world, value, + reference): + """The controls: a date, with or without the `reference:` it chooses, and + no date at all, which `execute` fills with today's.""" + text = with_elected_on(world.answered, value, reference) + checked = run_check(write_edited(world, text)) + assert checked.returncode == 0, checked.stderr + checked.stdout + plan, source = open_plan(adopter, world, text) + assert adopter._refuse_what_check_finds(plan, source) is None + + +@pytest.mark.parametrize("value", ["2026-W41-6", "20261011", "2026-09-02", + "someday"]) +def test_check_asks_election_date_itself_of_the_plans_date(adopter, world, + value): + """ONE QUESTION, `plan`'s. `check` asks the plan's `elected_on:` what + `plan` asks `--elected-on`, `election_date` on this interpreter, and + nothing of its own: `2026-W41-6` and `20261011` are dates to + `date.fromisoformat` from Python 3.11 and not to the project's own + validator, which is #261's to rule on. Whatever `election_date` answers, + `check` answers the same, so `plan`, `check` and `execute` cannot + disagree about one value.""" + try: + adopter.election_date(value) + refused = False + except adopter.Refusal: + refused = True + result = run_check(write_edited( + world, with_elected_on(world.answered, f'"{value}"', False))) + assert result.returncode == (1 if refused else 0), \ + result.stderr + result.stdout + assert (f"FINDING {DATE_CODE}:" in result.stderr) is refused + + +def test_a_tighter_election_date_is_asked_with_no_edit(adopter, world, + monkeypatch): + """`election_date` held to `YYYY-MM-DD` on every Python, as #261 asks of + it, stood in for here: `check`'s list follows it, and `execute` refuses + `2026-W41-6` before any leg. The stand-in's own words name + `--elected-on`; the refusal says the plan's key and not them.""" + real = adopter.election_date + + def held_to_the_form(elected_on): + if not re.fullmatch(r"\d{4}-\d{2}-\d{2}", str(elected_on)): + raise adopter.Refusal( + DATE_CODE, f"{DATE_FLAG} {elected_on!r} is not YYYY-MM-DD") + return real(elected_on) + + monkeypatch.setattr(adopter, "election_date", held_to_the_form) + world.plan.write_text( + with_elected_on(world.answered, '"2026-W41-6"', False), encoding=UTF8) + refusal = refused_by(adopter, execute_args(world.tmp, world.plan)) + assert refusal.code == DATE_CODE + assert "the plan's `elected_on:` is the text '2026-W41-6'" in \ + refusal.detail + assert DATE_FLAG not in str(refusal) + no_leg_was_created(world.tmp) + + +def test_another_refusal_from_election_date_is_not_a_date_finding( + adopter, world, monkeypatch): + """Only `election-date-malformed` is a finding about the date. Anything + else `election_date` raises -- a rule it could not ask, as when the + validator it may consult is missing -- is raised as it is, and a date it + never judged is not reported as malformed.""" + def cannot_ask(elected_on): + raise adopter.Refusal("rule-unusable", "the rule could not be asked") + + monkeypatch.setattr(adopter, "election_date", cannot_ask) + plan, source = open_plan(adopter, world, world.answered) + with pytest.raises(adopter.Refusal) as caught: + adopter._refuse_what_check_finds(plan, source) + assert caught.value.code == "rule-unusable" + + +def test_both_findings_are_listed_and_the_code_is_the_first(adopter, world): + """Both keys wrong at once: one list, the date first, as the plan file + holds the two keys, and each remediation once.""" + text = with_follow_ups(with_elected_on(world.answered, "someday", False), + "follow_ups: 5\n") + printed = check_findings(write_edited(world, text)) + assert [line.split(":", 1)[0] for line in printed] == [ + f"FINDING {DATE_CODE}", f"FINDING {BAD_FOLLOW_UPS_CODE}"] + plan, source = open_plan(adopter, world, text) + with pytest.raises(adopter.Refusal) as caught: + adopter._refuse_what_check_finds(plan, source) + assert caught.value.code == DATE_CODE + for finding in printed: + assert finding in caught.value.detail + assert caught.value.remediation.count("Remediation: edit `elected_on:`") \ + == 1 + assert caught.value.remediation.count( + "Remediation: write `follow_ups:`") == 1 + + # --- a plan that is not UTF-8 (#192) ---------------------------------------- #: The byte `0xff` is in no UTF-8 text, so wherever it lands the file cannot be @@ -686,3 +858,34 @@ def test_a_follow_ups_that_is_not_a_list_creates_no_leg_and_the_corrected_plan_r corrected = run_script(ADOPT, "execute", *flags) assert corrected.returncode == 0, corrected.stderr + corrected.stdout assert "adoption verified" in corrected.stdout + + +@needs_filter_repo +def test_an_elected_on_it_cannot_read_creates_no_leg_and_the_corrected_plan_runs( # noqa: E501 + adopter, world): + """#294 as a person met it, through `main`: exit 2, a REFUSED line that + names no flag, no traceback and no leg, so the corrected plan does not + meet `leg-remote-exists`. The corrected plan has no `reference:`, and the + manifest records the one its date chooses.""" + remotes, work = world.tmp / REMOTES, world.tmp / WORK + flags = ("--plan", str(world.plan), "--yes", "--local-remote-dir", + str(remotes), "--work-dir", str(work)) + world.plan.write_text(with_elected_on(world.answered, "someday", False), + encoding=UTF8) + refused = run_script(ADOPT, "execute", *flags) + assert refused.returncode == 2, refused.stderr + refused.stdout + assert f"REFUSED {DATE_CODE}: " in refused.stderr + assert_no_traceback(refused) + assert DATE_FLAG not in refused.stderr + assert "creating the leg repositories" not in refused.stdout + assert not remotes.exists(), "not even the directory the legs go in" + assert source_branches(world.source) == ["main"] + + world.plan.write_text( + with_elected_on(world.answered, '"2026-09-02"', False), encoding=UTF8) + corrected = run_script(ADOPT, "execute", *flags) + assert corrected.returncode == 0, corrected.stderr + corrected.stdout + assert "adoption verified" in corrected.stdout + manifest = adopter.load_yaml(work / PROJECT / "project.yaml") + assert manifest["reference"] == adopter.default_reference("2026-09-02") + assert str(manifest["elected_on"]) == "2026-09-02" From d53602b37d2f10c81770135336631c67632637b2 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:00:20 +0000 Subject: [PATCH 08/12] adopt-project.py: `check` names, and `execute` refuses before anything is made, a plan whose `source:` or `legs:` is not a mapping or whose `pins:`, `allow_empty_legs:` or `paths:` is not a list of what `plan` writes, so a scalar no longer ends both in a traceback and `pins: openGlass` no longer adopts with eight one-letter pins (#386) `Plan` read five keys of a plan file without asking their type: the constructor iterated `paths:` and `pins:`, `allowed_empty_legs` iterated `allow_empty_legs:`, `names` called `.get` on `legs:`, and `source_commit` and `open_source` on `source:`. The file is edited between `plan` and `execute` on purpose, and `x or []` rescues only the falsy values. So a number or a boolean ended `check` and `execute` in a `TypeError` or an `AttributeError`, exit 1, naming Python's error and not the key. Text, which is iterable, was read one character at a time: `pins: openGlass` passed `check`, adopted, and wrote eight pins, `G a e l n o p s`, that the root's own `validate-pins.py` refused. An item of `pins:` that is not text was written as Python spells it (`True`, `{'a': 1}`), and `allow_empty_legs: code` consented for four legs, `c`, `o`, `d` and `e`. Each key is now read through one reader that takes only what `plan` writes (`_plan_list`, `Plan.source`, `Plan.names`), and `_plan_key_findings` names what the reader left out in #293's words: a code per key (`plan-bad-source`, `plan-bad-legs`, `plan-bad-pins`, `plan-bad-allow-empty-legs`, `plan-bad-paths`), `_read_as` for what the file holds, one finding per item of `pins:` that is not text and of `allow_empty_legs:` that is not `spec` or `code`, and each key's remediation said once. An absent or bare key is not a finding. They are asked FIRST, right after `Plan.load`, and not in `_entry_findings` beside #293's and #294's. These keys are what the rest of the run is built from: the source is opened from `source:`, the names `execute` refuses on first come from `legs:`, and the constructor builds the entries and pins. Every later finding would be a statement about what the reader made of them (a `paths:` that is not a list reads as every source path uncovered). So `check` prints them, with #293's and #294's beside them so that every bad key of the plan is named in one run, and exits 1; `execute` refuses the same list in `_refuse_what_check_finds`' shape (the first finding's code, each finding as `check` prints it, capped), exit 2, before `_work_root` makes the work directory and before the source is opened or a leg exists. `legs:` is still kept by the constructor as it stands: the doctor's placement plan carries it as a list, and `tests/test_shape_doctor.py` hands that to `Plan` and `_leg_findings`. Only `check` and `execute`, reading a plan file, name it. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 248 +++++++++++++++++++- tests/test_adopt_execute_refusals.py | 324 +++++++++++++++++++++++++++ 2 files changed, 563 insertions(+), 9 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 041427f2..dc5b3ab1 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1855,10 +1855,15 @@ def __init__(self, path: Path, data: dict): "Remediation: re-run `plan`, which writes `mode: in-place`. " "A `new-root` mode is described in the tool's own docstring " "and is not implemented.") - self.entries = [e for e in (data.get("paths") or []) + # Each read takes only what `plan` writes, and `_plan_key_findings` + # names what it leaves out (#386). `legs:` is kept as it stands: the + # doctor's PLACEMENT plan, handed here by `tests/test_shape_doctor.py`, + # carries it as a list, and its readers ask its type themselves. + self.entries = [e for e in _plan_list(data, "paths") if isinstance(e, dict)] self.legs = data.get("legs") or {} - self.pins = [str(p) for p in (data.get("pins") or []) if p] + self.pins = [p for p in _plan_list(data, "pins") + if isinstance(p, str) and p] @classmethod def load(cls, path: Path) -> "Plan": @@ -1867,13 +1872,20 @@ def load(cls, path: Path) -> "Plan": raise Refusal("plan-unreadable", f"{path}: not a mapping") return cls(path, data) + @property + def source(self) -> dict: + """The plan's `source:` when it is a mapping, as `plan` writes it, and + `{}` for anything else, which `_plan_key_findings` names (#386).""" + source = self.data.get("source") + return source if isinstance(source, dict) else {} + @property def source_commit(self) -> str: - return str((self.data.get("source") or {}).get("commit") or "") + return str(self.source.get("commit") or "") def open_source(self, override: str | None, work_root: Path) -> Source: - spec = override or (self.data.get("source") or {}).get("local_path") \ - or (self.data.get("source") or {}).get("repository") + spec = override or self.source.get("local_path") \ + or self.source.get("repository") if not spec: raise Refusal("plan-no-source", f"{self.path}: `source:` names neither a local path " @@ -1886,8 +1898,14 @@ def names(self) -> dict[str, str]: They reach `gh repo create` and a push URL, so they are checked here rather than trusted because they came out of a file this tool wrote: the file is edited between `plan` and `execute`, on purpose. + + Only a `legs:` MAPPING holds them, as `_plan_leg_path` reads it: + anything else is read as no `legs:` at all, and never with `.get` on + a number or a list. `check` and `execute` name such a `legs:` first, + in `_plan_key_findings`, before this is asked (#386). """ - return {role: checked_value(f"legs.{role}", self.legs.get(role)) + legs = self.legs if isinstance(self.legs, dict) else {} + return {role: checked_value(f"legs.{role}", legs.get(role)) for role in ("assembly", "spec", "code")} @property @@ -1921,9 +1939,13 @@ def allowed_empty_legs(self) -> set[str]: """The legs this PLAN consents to having seeded. `execute` unions it with its own `--allow-empty-leg`, so consent can be given at either end — but it is never inferred from the entries being empty, which is - the state the consent exists to be deliberate about.""" - return {str(role) for role in (self.data.get("allow_empty_legs") or []) - if role} + the state the consent exists to be deliberate about. + + Each item of a LIST that is text, as `plan` writes it: text on the + key's own line is not read one character at a time, and what is left + out is named by `_plan_key_findings` (#386).""" + return {role for role in _plan_list(self.data, "allow_empty_legs") + if isinstance(role, str) and role} def seeding_record_disagreements(self, seeded: list[str]) -> list[str]: """Where the plan's `seeding:` record and its own entries disagree. @@ -2922,6 +2944,206 @@ def _plan_field_findings(plan: Plan) -> tuple[list[str], list[str]]: + ([FOLLOW_UPS_REMEDIATION] if followed else [])) +# --------------------------------------------------------------------------- +# The keys a plan is opened by are the types `plan` writes (#386) +# --------------------------------------------------------------------------- +# +# `Plan` read five keys of a plan file without asking their type. `plan` +# writes `source:` and `legs:` as mappings and `pins:`, `allow_empty_legs:` +# and `paths:` as lists, and the file is edited between `plan` and `execute` +# on purpose. `x or []` rescues only the falsy values: a number or a boolean +# ended `check` and `execute` in a `TypeError` or an `AttributeError`, exit 1, +# naming Python's error and not the key, and text, which is iterable, was read +# one character at a time. `pins: openGlass` passed `check`, adopted, and +# wrote eight one-letter neutral-product pins, `G`, `a`, `e`, `l`, `n`, `o`, +# `p` and `s`, which the project's own `validate-pins.py` then refused. +# +# So each is read through one reader that takes only what `plan` writes +# (`_plan_list`, `Plan.source`, `Plan.names`), and `_plan_key_findings` names +# what the reader left out in #293's words: a code per key, `plan-bad-` +# as `follow_ups:`' is `plan-bad-follow-ups`, `_read_as` for what the file +# holds, and each key's remediation said once. +# +# THEY ARE ASKED FIRST, right after `Plan.load`, and not in `_entry_findings` +# beside #293's and #294's. These keys are what the rest of the run is built +# from: the source is opened from `source:`, the names `execute` refuses on +# first come from `legs:`, and the constructor builds the entries and the +# pins. Every later finding is a statement about what they say, so a `paths:` +# that is not a list would be reported as every source path uncovered. +# `check` reports them, and stops, exit 1, with #293's and #294's findings +# beside them, so that every key of the plan that is not what `plan` writes +# is named in one run. `execute` refuses the same list, in +# `_refuse_what_check_finds`' shape, before the work directory, the source or +# a leg exists. + +#: `check`'s finding for each key below that is not what `plan` writes, and +#: the code `execute` refuses it under (#386). +BAD_SOURCE = "plan-bad-source" +BAD_LEGS = "plan-bad-legs" +BAD_PINS = "plan-bad-pins" +BAD_ALLOW_EMPTY_LEGS = "plan-bad-allow-empty-legs" +BAD_PATHS = "plan-bad-paths" + +#: Said once after the findings for each key. +SOURCE_REMEDIATION = ( + "Remediation: write `source:` in the plan file as `plan` writes it, a " + "mapping whose `local_path:` or `repository:` names the repository to " + "adopt and whose `commit:` is the one the plan was written against, or " + "re-run `plan`. `--source` says where to read that repository; it does " + "not stand in for the plan's record of it.") +LEGS_REMEDIATION = ( + "Remediation: write `legs:` in the plan file as `plan` writes it, a " + "mapping of the three repository names, `assembly:`, `spec:` and " + "`code:`, and the two mount paths, `spec_path:` and `code_path:`; or " + "re-run `plan`.") +PINS_REMEDIATION = ( + "Remediation: write `pins:` in the plan file as `plan` writes it, a list " + "of the neutral products' names, `pins: [openGlass]`, or `pins: []` for " + "none. `execute` writes each item into the assembly root's " + "`project.yaml` as one pin, and the repositories are named by them, so " + "it takes nothing else: text on the key's own line would be read one " + "character per pin, and an item the reader takes for a number, a boolean " + "or null (`2026`, `true`, `null`) is a name only once it is quoted.") +ALLOW_EMPTY_LEGS_REMEDIATION = ( + "Remediation: write `allow_empty_legs:` in the plan file as `plan` " + "writes it, a list of the legs that may be seeded, " + "`allow_empty_legs: [code]`, or `allow_empty_legs: []` for none; or " + "re-run `plan` with `--allow-empty-leg code`, or pass that to `execute`. " + "Each item is `spec` or `code`, the two legs `execute` extracts: text on " + "the key's own line would be read one character per leg, and consents " + "to nothing.") +PATHS_REMEDIATION = ( + "Remediation: `paths:` is the list of entries `plan` wrote, a `- path:` " + "and its `leg:` for each path of the source, which a value on the key's " + "own line replaced. Restore the block from the plan `plan` wrote, or " + "re-run `plan` and answer its questions again.") + +#: Said first in `execute`'s refusal of these findings. +PLAN_KEY_REFUSAL = ( + "Remediation: nothing was created and nothing was pushed: the plan was " + "refused before the work directory was made, the source was opened or a " + "leg repository was created. Correct each key named above in the plan " + "file and run `check` until it prints `plan ok`, then run `execute` " + "again.") + + +def _plan_list(data: dict, key: str) -> list: + """The plan's `key` when it is a LIST, as `plan` writes `paths:`, `pins:` + and `allow_empty_legs:`, and `[]` for anything else: THE ONE READ of the + three, so that none iterates a number or reads text one character at a + time. What it leaves out is named by `_plan_key_findings` (#386).""" + value = data.get(key) + return value if isinstance(value, list) else [] + + +def _not_a(code: str, key: str, value, kind: str, why: str) -> str: + """The finding for a `key:` that holds `value` where `plan` writes a + `kind`, in `_follow_ups_findings`' words.""" + return (f"FINDING {code}: `{key}:` is {_read_as(value)}, not a {kind}: " + f"{why}") + + +def _plan_key_findings(plan: Plan) -> tuple[list[str], list[str]]: + """A FINDING for each of the five keys `Plan` reads to open the source and + name the repositories that is present and is not what `plan` writes, in + the order `plan` writes them, and for each item of `pins:` or + `allow_empty_legs:` that is not text or not a leg; and what to print once + after them (#386). + + A key that is absent or bare is not a finding here, whichever it is: it + is read as it always was. For `source:` that is `plan-no-source`, unless + `--source` names the repository. + """ + data = plan.data + said: list[tuple[list[str], str]] = [] + + source, legs = data.get("source"), data.get("legs") + said.append(([] if source is None or isinstance(source, dict) else [ + _not_a(BAD_SOURCE, "source", source, "mapping", + "`check` and `execute` open the repository to adopt by its " + "`local_path:` or `repository:`, and hold it to its " + "`commit:`")], SOURCE_REMEDIATION)) + said.append(([] if legs is None or isinstance(legs, dict) else [ + _not_a(BAD_LEGS, "legs", legs, "mapping", + "the three repository names and the two mount paths are read " + "out of it by their keys")], LEGS_REMEDIATION)) + + pins = data.get("pins") + if pins is None: + found = [] + elif not isinstance(pins, list): + found = [_not_a(BAD_PINS, "pins", pins, "list", + "`execute` writes each item of a list into the " + "assembly root's `project.yaml` as one " + "neutral-product pin")] + else: + found = [f"FINDING {BAD_PINS}: item {number} of `pins:` is " + f"{_read_as(item)}, not text: `execute` writes each item " + "into the assembly root's `project.yaml` as the name of one " + "neutral product" + for number, item in enumerate(pins, 1) + if not isinstance(item, str)] + said.append((found, PINS_REMEDIATION)) + + allowed = data.get("allow_empty_legs") + if allowed is None: + found = [] + elif not isinstance(allowed, list): + found = [_not_a(BAD_ALLOW_EMPTY_LEGS, "allow_empty_legs", allowed, + "list", "each item of a list names one leg `execute` " + "may seed from the shape's template")] + else: + found = [f"FINDING {BAD_ALLOW_EMPTY_LEGS}: item {number} of " + f"`allow_empty_legs:` is {_read_as(item)}, not a leg " + "`execute` can seed, `spec` or `code`" + for number, item in enumerate(allowed, 1) + if item not in EXTRACTED_LEGS] + said.append((found, ALLOW_EMPTY_LEGS_REMEDIATION)) + + paths = data.get("paths") + said.append(([] if paths is None or isinstance(paths, list) else [ + _not_a(BAD_PATHS, "paths", paths, "list", + "each item of a list is one entry, a path of the source and " + "the leg it goes to")], PATHS_REMEDIATION)) + + return ([finding for found, _ in said for finding in found], + [remediation for found, remediation in said if found]) + + +def _plan_type_findings(plan: Plan) -> tuple[list[str], list[str]]: + """What `check` reports, and `execute` refuses, before it opens anything: + `_plan_key_findings`, and, when there is one, `_plan_field_findings` + too, so that every key of the plan that is not what `plan` writes is + named in one run and not the first of them (#386). `([], [])` for a plan + whose five keys `Plan` reads are what `plan` writes; #293's and #294's + keys are then asked in `_entry_findings`, beside everything else.""" + findings, remediation = _plan_key_findings(plan) + if not findings: + return [], [] + fields, field_remediation = _plan_field_findings(plan) + return findings + fields, remediation + field_remediation + + +def _refuse_a_mistyped_plan(plan: Plan) -> None: + """Refuse what `_plan_type_findings` finds, in the shape + `_refuse_what_check_finds` refuses `check`'s other findings -- the first + finding's code, every finding as `check` prints it, capped at + `REFUSED_FINDINGS_SHOWN` -- but first thing, before `_work_root` makes the + work directory or the source is opened or cloned (#386).""" + findings, remediation = _plan_type_findings(plan) + if not findings: + return + # `FINDING : ` is the one shape every finding is written in. + code = findings[0].split()[1].rstrip(":") + lines = [f"{len(findings)} finding(s) in {plan.path}, as `check` prints " + "them:"] + lines += [f" {finding}" for finding in findings[:REFUSED_FINDINGS_SHOWN]] + if len(findings) > REFUSED_FINDINGS_SHOWN: + lines.append(f" and {len(findings) - REFUSED_FINDINGS_SHOWN} more") + raise Refusal(code, "\n".join(lines), + "\n".join([PLAN_KEY_REFUSAL, *remediation])) + + #: What a plan whose entry `entry_path_problem` refuses is told to do. Said by #: `check`, once after its findings, and by `execute`'s refusal. UNSAFE_PATH_REMEDIATION = ( @@ -3042,6 +3264,13 @@ def _print_what_will_happen(plan: Plan, source: Source, names: dict, def cmd_check(args) -> int: plan = Plan.load(Path(args.plan)) + # FIRST, before anything is opened: the keys the rest is read from (#386). + mistyped, mistyped_remediation = _plan_type_findings(plan) + if mistyped: + for line in [*mistyped, *mistyped_remediation]: + print(line, file=sys.stderr) + print(f"\n{len(mistyped)} finding(s) in {plan.path}", file=sys.stderr) + return 1 work_root = _work_root(args) source = plan.open_source(args.source, work_root) naming = NamingPolicy.load(NAMING_POLICY) @@ -3903,6 +4132,7 @@ def _commit_the_split(plan: Plan, source: Source, assembly: Path, def cmd_execute(args) -> int: plan = Plan.load(Path(args.plan)) + _refuse_a_mistyped_plan(plan) work_root = _work_root(args) source = plan.open_source(args.source, work_root) naming = NamingPolicy.load(NAMING_POLICY) diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index bde46eb9..1cc40a9f 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -724,6 +724,299 @@ def test_both_findings_are_listed_and_the_code_is_the_first(adopter, world): "Remediation: write `follow_ups:`") == 1 +# --- the keys a plan is opened by (#386) ------------------------------------- + +#: The finding codes, one per key, as #293's is `plan-bad-follow-ups`. +BAD_SOURCE, BAD_LEGS, BAD_PINS = ("plan-bad-source", "plan-bad-legs", + "plan-bad-pins") +BAD_ALLOW, BAD_PATHS = "plan-bad-allow-empty-legs", "plan-bad-paths" + +#: What a hand edit can leave a key `Plan` reads holding, by test id: the key, +#: the block written in place of its own, the code, and what the finding must +#: begin with (#386). On #356's head (5a5011d) the scalars ended `check` and +#: `execute` in `TypeError: ... is not iterable` (`pins:`, `paths:`, +#: `allow_empty_legs:`) or `AttributeError: ... has no attribute 'get'` +#: (`legs:`, `source:`), exit 1; `pins: openGlass` passed `check` and adopted +#: with eight one-letter pins; an item `true` of `pins:` was written as the +#: pin `True`, and `allow_empty_legs: code` consented for four legs, `c`, `o`, +#: `d` and `e`. +BAD_KEYS = { + "pins-int": ("pins", "pins: 5\n", BAD_PINS, + "`pins:` is the number 5, not a list"), + "pins-true": ("pins", "pins: true\n", BAD_PINS, + "`pins:` is the boolean true, not a list"), + "pins-text": ("pins", "pins: openGlass\n", BAD_PINS, + "`pins:` is the text 'openGlass', not a list"), + "pins-item-true": ("pins", "pins: [openGlass, true]\n", BAD_PINS, + "item 2 of `pins:` is the boolean true, not text"), + "pins-item-int": ("pins", "pins:\n - 2026\n", BAD_PINS, + "item 1 of `pins:` is the number 2026, not text"), + "pins-item-null": ("pins", "pins: [null]\n", BAD_PINS, + "item 1 of `pins:` is null, not text"), + "pins-item-mapping": ("pins", "pins: [{a: 1}]\n", BAD_PINS, + "item 1 of `pins:` is a mapping of 1 key(s), not " + "text"), + "allow-int": ("allow_empty_legs", "allow_empty_legs: 5\n", BAD_ALLOW, + "`allow_empty_legs:` is the number 5, not a list"), + "allow-true": ("allow_empty_legs", "allow_empty_legs: true\n", BAD_ALLOW, + "`allow_empty_legs:` is the boolean true, not a list"), + "allow-text": ("allow_empty_legs", "allow_empty_legs: code\n", BAD_ALLOW, + "`allow_empty_legs:` is the text 'code', not a list"), + "allow-item-case": ("allow_empty_legs", "allow_empty_legs: [Code]\n", + BAD_ALLOW, "item 1 of `allow_empty_legs:` is the text " + "'Code', not a leg `execute` can seed"), + "allow-item-int": ("allow_empty_legs", "allow_empty_legs: [code, 5]\n", + BAD_ALLOW, "item 2 of `allow_empty_legs:` is the " + "number 5, not a leg `execute` can seed"), + "paths-int": ("paths", "paths: 5\n", BAD_PATHS, + "`paths:` is the number 5, not a list"), + "paths-true": ("paths", "paths: true\n", BAD_PATHS, + "`paths:` is the boolean true, not a list"), + "paths-text": ("paths", "paths: src/\n", BAD_PATHS, + "`paths:` is the text 'src/', not a list"), + "legs-int": ("legs", "legs: 5\n", BAD_LEGS, + "`legs:` is the number 5, not a mapping"), + "legs-list": ("legs", "legs: [spec, code]\n", BAD_LEGS, + "`legs:` is a list of 2 item(s), not a mapping"), + "source-int": ("source", "source: 5\n", BAD_SOURCE, + "`source:` is the number 5, not a mapping"), + "source-text": ("source", "source: Thing\n", BAD_SOURCE, + "`source:` is the text 'Thing', not a mapping"), +} +ONE_PER_BAD_KEY = pytest.mark.parametrize("edit", sorted(BAD_KEYS)) + +#: The line each key's remediation begins with, said once after its findings. +KEY_REMEDIATION = {key: f"Remediation: write `{key}:` in the plan file" + for key in ("source", "legs", "pins", "allow_empty_legs")} +KEY_REMEDIATION["paths"] = ("Remediation: `paths:` is the list of entries " + "`plan` wrote") + +#: What `execute`'s refusal of them says first. +NOTHING_MADE = ("nothing was created and nothing was pushed: the plan was " + "refused before the work directory was made, the source was " + "opened or a leg repository was created") + + +def with_key(text: str, key: str, block: str) -> str: + """`text` with the top-level `key:` and the block under it replaced by + `block`, or deleted when `block` is empty, as a hand edit would.""" + edited, done = re.subn(rf"^{key}:.*\n(?:[ \t]+.*\n)*", lambda _: block, + text, count=1, flags=re.M) + assert done == 1, f"the plan has no `{key}:` to edit" + return edited + + +def bad_key_text(world: World, edit: str) -> str: + key, block, _, _ = BAD_KEYS[edit] + return with_key(world.answered, key, block) + + +@ONE_PER_BAD_KEY +def test_check_finds_a_key_plan_reads_that_is_not_what_plan_writes(world, + edit): + """#386 (a): a FINDING that names the key and what the reader gave it, + exit 1, no traceback and no `plan ok`; and nothing else is reported, + because every other finding is read from these keys. On 5a5011d each was + a traceback naming Python's error, or `plan ok`.""" + key, _, code, said = BAD_KEYS[edit] + result = run_check(write_edited(world, bad_key_text(world, edit))) + assert result.returncode == 1, result.stderr + result.stdout + assert_no_traceback(result) + assert "plan ok" not in result.stdout + found = findings_of(result) + assert len(found) == 1, found + assert found[0].startswith(f"FINDING {code}: {said}"), found + assert result.stderr.count(KEY_REMEDIATION[key]) == 1, result.stderr + assert "1 finding(s) in" in result.stderr + + +@ONE_PER_BAD_KEY +def test_execute_refuses_a_key_plan_reads_before_anything_is_made( + adopter, world, edit): + """#386 (c): the same finding, refused under its own code before the + work directory, the source's reading or a leg; on 5a5011d each ended in + the traceback `check` printed, or, for the text and item rows of `pins:` + and `allow_empty_legs:`, adopted.""" + key, _, code, said = BAD_KEYS[edit] + world.plan.write_text(bad_key_text(world, edit), encoding=UTF8) + refusal = refused_by(adopter, execute_args(world.tmp, world.plan)) + assert refusal.code == code + assert f" FINDING {code}: {said}" in refusal.detail + assert refusal.detail.startswith(f"1 finding(s) in {world.plan}, as " + "`check` prints them:") + assert NOTHING_MADE in refusal.remediation + assert refusal.remediation.count(KEY_REMEDIATION[key]) == 1 + no_leg_was_created(world.tmp) + assert not (world.tmp / WORK).exists(), "not even the work directory" + assert source_branches(world.source) == ["main"] + + +def test_every_key_that_is_wrong_is_named_in_one_run(adopter, world): + """#386 (b): three of the five keys wrong at once, and #293's beside + them: one list, in the order `plan` writes the keys, then #293's, which + `check` would otherwise report only once these were corrected. `execute` + wears the first one's code and names every one, each remediation once.""" + text = with_key(world.answered, "pins", "pins: openGlass\n") + text = with_key(text, "legs", "legs: 5\n") + text = with_key(text, "paths", "paths: true\n") + text = with_follow_ups(text, "follow_ups: 5\n") + printed = check_findings(write_edited(world, text)) + assert [line.split(":", 1)[0] for line in printed] == [ + f"FINDING {BAD_LEGS}", f"FINDING {BAD_PINS}", f"FINDING {BAD_PATHS}", + f"FINDING {BAD_FOLLOW_UPS_CODE}"], printed + world.plan.write_text(text, encoding=UTF8) + refusal = refused_by(adopter, execute_args(world.tmp, world.plan)) + assert refusal.code == BAD_LEGS + for finding in printed: + assert f" {finding}" in refusal.detail + for key in ("legs", "pins", "paths"): + assert refusal.remediation.count(KEY_REMEDIATION[key]) == 1 + assert refusal.remediation.count("Remediation: write `follow_ups:`") == 1 + no_leg_was_created(world.tmp) + + +def test_a_wrong_source_is_named_when_source_says_where_to_read_it(world): + """`--source` says where to read the repository, so the plan's `source:` + is not read to open it; it is still the plan's record of the repository, + and its `commit:` is what `check` holds the tree to. On 5a5011d this was + `AttributeError` from `source_commit`.""" + text = with_key(world.answered, "source", "source: 5\n") + result = run_script(ADOPT, "check", "--plan", + str(write_edited(world, text)), + "--source", str(world.source)) + assert result.returncode == 1, result.stderr + result.stdout + assert_no_traceback(result) + [found] = findings_of(result) + assert found.startswith( + f"FINDING {BAD_SOURCE}: `source:` is the number 5, not a mapping"), \ + found + + +def test_a_spec_only_plan_that_consents_in_text_is_not_four_legs(adopter, + tmp_path): + """#386's `allow_empty_legs: code`, where it matters: a source with no + code, whose plan `plan` wrote with `--allow-empty-leg code`. Text on the + key's own line was read as four legs, `c`, `o`, `d` and `e`, so on + 5a5011d `check` said `plan ok` while `execute` refused + `adopt-empty-leg-unconsented` for the leg the person had named; both now + name the key.""" + repo = spec_only_source(tmp_path) + plan_path = tmp_path / "plan.yaml" + written = write_plan(repo, plan_path, project=PROJECT, + extra=("--allow-empty-leg", CODE)) + assert written.returncode == 0, written.stderr + written.stdout + text = with_key(plan_path.read_text(encoding=UTF8), "allow_empty_legs", + "allow_empty_legs: code\n") + plan_path.write_text(text, encoding=UTF8) + found = check_findings(plan_path) + assert found == [f"FINDING {BAD_ALLOW}: `allow_empty_legs:` is the text " + "'code', not a list: each item of a list names one leg " + "`execute` may seed from the shape's template"], found + refusal = refused_by(adopter, execute_args(tmp_path, plan_path)) + assert refusal.code == BAD_ALLOW + no_leg_was_created(tmp_path) + + +#: What `check` and `execute` still take, by test id: the key, the block +#: written in place of its own ("" deletes the key), and whether `check` +#: then passes. A `paths:` with no entries is valid as a TYPE and leaves every +#: source path uncovered, which is `plan-uncovered`'s to say, as it was. +GOOD_KEYS = { + "pins-empty": ("pins", "pins: []\n", True), + "pins-bare": ("pins", "pins:\n", True), + "pins-absent": ("pins", "", True), + "pins-flow": ("pins", "pins: [openGlass]\n", True), + "pins-block": ("pins", "pins:\n - openGlass\n", True), + "allow-empty": ("allow_empty_legs", "allow_empty_legs: []\n", True), + "allow-bare": ("allow_empty_legs", "allow_empty_legs:\n", True), + "allow-absent": ("allow_empty_legs", "", True), + "allow-both": ("allow_empty_legs", "allow_empty_legs: [spec, code]\n", + True), + "paths-empty": ("paths", "paths: []\n", False), + "paths-bare": ("paths", "paths:\n", False), + "paths-absent": ("paths", "", False), +} + + +@pytest.mark.parametrize("edit", sorted(GOOD_KEYS)) +def test_a_list_an_empty_list_a_bare_key_and_no_key_are_not_findings( + adopter, world, edit): + """#386 (d): what `plan` writes, and the ways of writing none, are not + findings of this kind; a plan with no entries is `plan-uncovered`, as it + was, and never `plan-bad-paths`.""" + key, block, passes = GOOD_KEYS[edit] + text = with_key(world.answered, key, block) + checked = run_check(write_edited(world, text)) + assert_no_traceback(checked) + assert "FINDING plan-bad-" not in checked.stderr, checked.stderr + if passes: + assert checked.returncode == 0, checked.stderr + checked.stdout + plan, source = open_plan(adopter, world, text) + assert adopter._refuse_what_check_finds(plan, source) is None + else: + assert checked.returncode == 1, checked.stderr + checked.stdout + assert {line.split(":", 1)[0] for line in findings_of(checked)} == { + f"FINDING {UNCOVERED}"} + + +def test_the_readers_ask_the_type_of_what_they_read(adopter, world): + """#386 (3): no `TypeError` or `AttributeError` from the reads themselves, + whoever constructs the plan: each takes only what `plan` writes. On + 5a5011d the constructor raised on `pins: 5` and `paths: 5`, + `allowed_empty_legs` on `allow_empty_legs: 5`, `names` on `legs: 5`, and + `source_commit` and `open_source` on `source: 5`; text was read one + character at a time.""" + base = {"kind": adopter.PLAN_KIND, "mode": "in-place"} + for value in (5, True, 1.5, "openGlass", {"a": 1}): + plan = adopter.Plan(Path("p.yaml"), { + **base, "pins": value, "paths": value, + "allow_empty_legs": value, "source": value}) + assert plan.pins == [] and plan.entries == [] + assert plan.allowed_empty_legs() == set() + assert plan.source_commit == "" + with pytest.raises(adopter.Refusal) as caught: + plan.open_source(None, world.tmp / "read") + assert caught.value.code == "plan-no-source" + assert plan.open_source(str(world.source), + world.tmp / "read").commit + plan = adopter.Plan(Path("p.yaml"), { + **base, "pins": ["openGlass", True, None, "", 5], + "allow_empty_legs": ["code", None, 5, "Spec"]}) + assert plan.pins == ["openGlass"] + assert plan.allowed_empty_legs() == {"code", "Spec"} + no_legs = adopter.Plan(Path("p.yaml"), dict(base)).names() + for legs in (5, "Northwind", [{"role": "spec"}]): + plan = adopter.Plan(Path("p.yaml"), {**base, "legs": legs}) + assert plan.legs == legs, "the doctor's list is kept as it stands" + assert plan.names() == no_legs, "read as no `legs:` at all" + + +def test_a_placement_plans_list_of_legs_still_constructs(adopter): + """#386's constraint: `shape-doctor.py --placement-plan` writes `legs:` as + a LIST and `tests/test_shape_doctor.py` hands it to `Plan` and + `_leg_findings`. The constructor refuses no `legs:`; only `check` and + `execute`, reading a plan FILE, name one that is not a mapping.""" + plan = adopter.Plan(Path("placement-plan.yaml"), { + "kind": adopter.PLAN_KIND, "mode": "in-place", + "legs": [{"role": "spec", "path": "spec"}], + "paths": [{"path": "spec/tool.py", "leg": "spec"}]}) + assert [entry["path"] for entry in plan.entries] == ["spec/tool.py"] + assert plan.legs == [{"role": "spec", "path": "spec"}] + assert adopter._leg_findings(plan, None) == [] + + +def test_the_refusal_comes_before_anyone_is_asked_to_say_yes_for_a_key( + adopter, world, monkeypatch): + """WITHOUT `--yes`, on a stream that is not a terminal: the refusal is + still the key's own, and not `adopt-unconfirmed`.""" + monkeypatch.setattr(sys, "stdin", io.StringIO("")) + world.plan.write_text(bad_key_text(world, "pins-text"), encoding=UTF8) + refusal = refused_by(adopter, + execute_args(world.tmp, world.plan, yes=False)) + assert refusal.code == BAD_PINS + no_leg_was_created(world.tmp) + + # --- a plan that is not UTF-8 (#192) ---------------------------------------- #: The byte `0xff` is in no UTF-8 text, so wherever it lands the file cannot be @@ -889,3 +1182,34 @@ def test_an_elected_on_it_cannot_read_creates_no_leg_and_the_corrected_plan_runs manifest = adopter.load_yaml(work / PROJECT / "project.yaml") assert manifest["reference"] == adopter.default_reference("2026-09-02") assert str(manifest["elected_on"]) == "2026-09-02" + + +@needs_filter_repo +def test_pins_written_as_text_creates_no_leg_and_the_corrected_plan_runs( + adopter, world): + """#386 (e) as a person met it, through `main`: `pins: openGlass` is exit 2 + and a REFUSED line, with no work directory, no leg and no split branch. + On 5a5011d it adopted, said `adoption verified`, and wrote eight pins, + `G`, `a`, `e`, `l`, `n`, `o`, `p` and `s`. The corrected plan, the list + `plan` writes, adopts with the one pin it names.""" + remotes, work = world.tmp / REMOTES, world.tmp / WORK + flags = ("--plan", str(world.plan), "--yes", "--local-remote-dir", + str(remotes), "--work-dir", str(work)) + world.plan.write_text(bad_key_text(world, "pins-text"), encoding=UTF8) + refused = run_script(ADOPT, "execute", *flags) + assert refused.returncode == 2, refused.stderr + refused.stdout + assert f"REFUSED {BAD_PINS}: " in refused.stderr + assert "`pins:` is the text 'openGlass', not a list" in refused.stderr + assert_no_traceback(refused) + assert "adoption verified" not in refused.stdout + assert not remotes.exists(), "not even the directory the legs go in" + assert not work.exists(), "and no work directory" + assert source_branches(world.source) == ["main"] + + world.plan.write_text(with_key(world.answered, "pins", + "pins: [openGlass]\n"), encoding=UTF8) + corrected = run_script(ADOPT, "execute", *flags) + assert corrected.returncode == 0, corrected.stderr + corrected.stdout + assert "adoption verified" in corrected.stdout + manifest = adopter.load_yaml(work / PROJECT / "project.yaml") + assert manifest["neutral_product_pins"] == ["openGlass"] From f8e6de11a7eef4e0c27721f6f1d3f7e1915a38d7 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:49:02 +0000 Subject: [PATCH 09/12] adopt-project.py: the tests now pin that only an absent or null `elected_on:` is "no date", so an empty text, `false`, `0`, `[]` and `{}` stay findings in `check` and refusals before any leg, and `if not elected_on:` no longer passes the suite (#294) Review of 5a5011d (F1): `_elected_on_findings` returns nothing for `elected_on is None` only, and `""` and the other falsy values reach `election_date`, so `check` exits 1 with `FINDING election-date-malformed` and `execute` exits 2 with no leg. No test pinned it: changing `is None` to `not elected_on` passed all 81 tests, and under that change `check` says `plan ok` and `execute` creates both legs before it refuses. `empty-text` is a row of BAD_DATES, so the `check` test and the `execute` test both take it. A `check` test of its own takes `false`, `0`, `[]` and `{}`, because BAD_DATES' assertion says "the text" and these are not text. Under the mutant the six new cases fail, `check` exiting 0 and `execute` having made the leg remotes; at this commit they pass. Tests only: adopt-project.py is unchanged. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_adopt_execute_refusals.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index bde46eb9..ac9e9929 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -559,12 +559,15 @@ def test_a_list_of_text_an_empty_list_and_a_bare_key_still_pass(adopter, #: as the plan holds it, as the reader reads it, and whether the plan keeps #: its `reference:` (#294). Without one, `execute` made both legs and only #: then refused, naming `--elected-on`; with one it never read the date and -#: wrote the word into the manifest. +#: wrote the word into the manifest. `empty-text` is falsy and not null: only +#: `elected_on:` ABSENT or null is "no date" (`execute` fills in today's), so +#: `""` is a date `election_date` cannot read and is refused like any other. BAD_DATES = { "word": ("someday", "someday", False), "notadate": ("notadate", "notadate", False), "unpadded": ('"2026-9-2"', "2026-9-2", False), "not-a-day": ('"2026-02-30"', "2026-02-30", False), + "empty-text": ('""', "", False), "word-reference-kept": ("someday", "someday", True), } ONE_PER_BAD_DATE = pytest.mark.parametrize("edit", sorted(BAD_DATES)) @@ -606,6 +609,27 @@ def test_check_finds_an_elected_on_it_cannot_read_under_the_plans_key(world, assert DATE_FLAG not in result.stderr + result.stdout +@pytest.mark.parametrize("value,read_as", [ + ("false", "the boolean false"), ("0", "the number 0"), + ("[]", "a list of 0 item(s)"), ("{}", "a mapping of 0 key(s)")]) +def test_check_finds_a_falsy_elected_on_that_is_not_null(world, value, + read_as): + """#294: only an `elected_on:` that is absent or null is "no date". Any + other falsy value is a date `election_date` cannot read, so it is a + FINDING that says what the plan holds; `if not elected_on:` in place of + `is None` would skip it and `check` would say `plan ok`. It is not a + `BAD_DATES` row because that test says "the text".""" + result = run_check(write_edited( + world, with_elected_on(world.answered, value, False))) + assert result.returncode == 1, result.stderr + result.stdout + assert_no_traceback(result) + found = findings_of(result) + assert len(found) == 1, found + assert found[0].startswith( + f"FINDING {DATE_CODE}: the plan's `elected_on:` is {read_as}, which " + "is not a day written YYYY-MM-DD"), found + + @ONE_PER_BAD_DATE def test_execute_refuses_an_elected_on_it_cannot_read_before_any_leg( adopter, world, edit): From dbfad7081799dd6a8dff43523babd52bed4270f2 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:49:11 +0000 Subject: [PATCH 10/12] adopt-project.py: the `elected_on:` finding claims only what `execute` writes ("the form `execute` writes into the project's `project.yaml`"), and `_plan_field_findings` says the order `plan` writes the two keys, so neither overstates what the code does (#294) Review of 5a5011d (N1, N2), both wording in this PR's own text. N1: the docstring said the findings come "in the order the plan file holds the two keys". The order is fixed, date then follow-ups, and it is the order `plan` writes them (`render_plan` emits `elected_on` before `follow_ups`). It now says so, and so does the both-findings test's docstring. N2: the finding called the date "the one form `execute` reads". That does not hold on Python 3.11 and later until #314 lands: `20261011` and `2026-W41-6` pass `election_date`, and `execute` writes them into `project.yaml`. It now says "the form `execute` writes into the project's `project.yaml`", which is true on every Python. The `check` test pins the new words and that the old ones are gone: at the reviewed adopt-project.py its six rows fail, and they pass here. The two `startswith` assertions match only up to "YYYY-MM-DD" and did not change. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 9 +++++---- tests/test_adopt_execute_refusals.py | 9 +++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 6379c1ba..3d62252a 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -3184,14 +3184,15 @@ def _elected_on_findings(plan: Plan) -> list[str]: "reference by it") return [f"FINDING {ELECTION_DATE_MALFORMED}: the plan's `elected_on:` " f"is {_read_as(elected_on)}, which is not a day written " - "YYYY-MM-DD, the one form `execute` reads: it writes the date " - f"into the project's `project.yaml`{chosen}"] + "YYYY-MM-DD, the form `execute` writes into the project's " + f"`project.yaml`{chosen}"] return [] def _plan_field_findings(plan: Plan) -> tuple[list[str], list[str]]: - """`_elected_on_findings` and `_follow_ups_findings`, in the order the - plan file holds the two keys, and what `check` prints once after them.""" + """`_elected_on_findings` and `_follow_ups_findings`, in the order `plan` + writes the two keys (`elected_on:` before `follow_ups:`), and what `check` + prints once after them.""" dated, followed = _elected_on_findings(plan), _follow_ups_findings(plan) return (dated + followed, ([ELECTED_ON_REMEDIATION] if dated else []) diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index ac9e9929..e4f248c0 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -604,6 +604,11 @@ def test_check_finds_an_elected_on_it_cannot_read_under_the_plans_key(world, f"FINDING {DATE_CODE}: the plan's `elected_on:` is the text " f"{read!r}, which is not a day written YYYY-MM-DD"), found assert ("the plan naming no `reference:`" in found[0]) is not reference + # the claim is about what `execute` WRITES, not "the one form it reads": + # on Python 3.11+ `election_date` reads `20261011` too, until #314 lands + assert ", the form `execute` writes into the project's `project.yaml`" in \ + found[0], found + assert "the one form" not in found[0], found assert "Remediation: edit `elected_on:` in the plan file" in \ result.stderr assert DATE_FLAG not in result.stderr + result.stdout @@ -729,8 +734,8 @@ def cannot_ask(elected_on): def test_both_findings_are_listed_and_the_code_is_the_first(adopter, world): - """Both keys wrong at once: one list, the date first, as the plan file - holds the two keys, and each remediation once.""" + """Both keys wrong at once: one list, the date first, as `plan` writes the + two keys, and each remediation once.""" text = with_follow_ups(with_elected_on(world.answered, "someday", False), "follow_ups: 5\n") printed = check_findings(write_edited(world, text)) From 5f177c9a39a613b5becf41a16a99b250d5c3bf4c Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:04:32 +0000 Subject: [PATCH 11/12] adopt-project.py: `plan` writes each `--pin` name through `y`, as it writes every other value, so `plan --pin 2026` no longer writes a plan its own `check` refuses as `plan-bad-pins`, and `--pin 007` is no longer declared as the pin `7` (#386) #386's item check names an item of `pins:` that the reader does not give back as text. `render_plan` wrote the line as `"pins: [" + ", ".join(pins) + "]"`, the one value in the plan not written through `y`, and `PIN_NAME_RE` takes names that are digits: `repo_shape.parse_yaml` reads a bare `2026` as the number 2026, `007` as the number 7 and `1.5` as a float. So at 4d8a149, `plan --pin 2026` (or `007`, or `1.5`) exited 0 and the plan it wrote failed `check` with `FINDING plan-bad-pins: item 1 of pins: is the number 2026, not text`. Before #386 `2026` was declared as `str(2026)`, and `007` silently as `7`. `y` double-quotes a name the reader would take for a number, so each comes back as the text `--pin` was given, which is the invariant the writer's own comment states: everything it emits is read back by `check`. A bare YAML keyword (`--pin true`, `--pin null`) is still written bare, as `y` writes one everywhere (#181); `check` now names it where the pin `True` used to be declared, or the pin dropped. The new `test_a_pin_plan_writes_reads_back_as_the_name_it_was_given` (`2026`, `007`, `1.5`, and `openGlass` as the control) runs `plan --pin`, answers the plan's questions and asks `check` and `Plan.load`: at 4d8a149 the three numeric names fail (`check` exit 1, `plan-bad-pins`); on #356's 0aff5ae `007` fails (`['7'] == ['007']`); all four pass here. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 9 ++++++++- tests/test_adopt_execute_refusals.py | 29 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/adopt-project.py b/adopt-project.py index 23809162..57381fbf 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1298,11 +1298,18 @@ def render_plan(args, source: Source, entries: list[Entry], names: dict, emit(lines, "code", names["code"], 2) emit(lines, "spec_path", args.spec_path, 2) emit(lines, "code_path", args.code_path, 2) + # Each name through `y`, as every other value here is written: `--pin` + # takes `2026` and `007` (`PIN_NAME_RE`), and written bare the reader + # takes them for the numbers 2026 and 7, which `check` names as an item + # that is not text (#386), so a plan `plan` wrote would fail its own + # `check`. A bare YAML keyword (`true`, `null`) is still written bare and + # read as a boolean or null, as everywhere `y` writes one (#181); `check` + # names it, where it used to declare the pin `True` or drop it. lines += ["", "# Neutral products this project declares a pin on. A", "# `` assembly root is a DESCENDANT only when", "# its `open` is listed here (2026-09-02).", - "pins: [" + ", ".join(pins) + "]", ""] + "pins: [" + ", ".join(y(pin) for pin in pins) + "]", ""] lines += [ "# A leg that NO entry below assigns a path to is SEEDED from the", diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index 1cc40a9f..303291e0 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -1017,6 +1017,35 @@ def test_the_refusal_comes_before_anyone_is_asked_to_say_yes_for_a_key( no_leg_was_created(world.tmp) +#: Names `plan --pin` takes (`PIN_NAME_RE`) that the plan's reader takes for +#: a number when they are written bare: `pins: [2026]` is the number 2026, +#: `[007]` the number 7 and `[1.5]` a float. `openGlass` is the control. +PIN_NAMES_PLAN_WRITES = ("2026", "007", "1.5", "openGlass") + + +@pytest.mark.parametrize("name", PIN_NAMES_PLAN_WRITES) +def test_a_pin_plan_writes_reads_back_as_the_name_it_was_given(adopter, + tmp_path, + name): + """`check` passes the plan `plan` wrote, and reads back the pin it was + given. On #356's head `pins: [007]` was read as the pin `7`; with #386's + item check and the line still written bare, `plan --pin 2026` wrote a + plan its own `check` refused as `plan-bad-pins`. `plan` now writes each + name as it writes every other value, quoted where the reader would take + it for something else.""" + source = make_source_repo(tmp_path / "Thing") + plan = tmp_path / "adoption-plan.yaml" + written = write_plan(source, plan, project=PROJECT, + extra=("--pin", name)) + assert written.returncode == 0, written.stderr + written.stdout + for path, leg in ANSWERS: + resolve(plan, path, leg) + checked = run_check(plan) + assert checked.returncode == 0, checked.stderr + checked.stdout + assert "plan ok" in checked.stdout + assert adopter.Plan.load(plan).pins == [name] + + # --- a plan that is not UTF-8 (#192) ---------------------------------------- #: The byte `0xff` is in no UTF-8 text, so wherever it lands the file cannot be From 5982bbe835d1ca9e963b9ad2f68e3c59a4c998c9 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 12 Oct 2026 21:06:27 +0000 Subject: [PATCH 12/12] adopt-project.py: `_manifest_values` asks the plan's `org:` and `id:` before its date guard, so `check` and `execute` refuse a plan with a bad `elected_on:` and a bad organisation or id for the same first problem (#294) The guard this PR added to `_manifest_values` (return None when `_elected_on_findings` has a finding, so #314's validator is not asked about a date this PR already reports) returned before the function's `checked_value("org", ...)` and before `_template_table` read `plan.project_id`. Those two are where `check` meets a bad organisation and a bad id; `execute` meets the organisation earlier, in `_repository_urls`, and the id here, through `_refuse_what_check_finds`. So with `elected_on: someday` beside them: - `org: "-evil"`: `check` exited 1 listing only the date, and `execute` exited 2 `REFUSED unsafe-value`; - `id: "North Wind"`: `check` exited 2 `REFUSED plan-bad-id` (from the topic line it prints later), and `execute` exited 2 `REFUSED election-date-malformed`. On main 580d7df both commands name `unsafe-value`, and `plan-bad-id`, for those plans. The organisation is now refused, and the id read, before the guard, so both commands name the same first problem again, as on main, and a date-only plan is still one finding in the plan key's own words, not `plan-manifest-invalid` beside it. The docstrings of `_manifest_values` and `_manifest_findings` now also say they return nothing for such a date. tests/test_adopt_execute_refusals.py: `test_a_bad_date_does_not_change_what_check_and_execute_refuse_first`, for the organisation and the id, with `reference:` kept and removed: `check` exits 2 on the code `execute` refuses with, and no leg is created. All 4 fail on 88c430c's code and pass here. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 22 +++++++++++---- tests/test_adopt_execute_refusals.py | 40 ++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index ecb9b4a9..cb747c17 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1899,7 +1899,9 @@ def _manifest_values(plan: Plan, naming: NamingPolicy) -> dict | None: would build it from this plan, FOR VALIDATION AND NEVER FOR WRITING; None where the plan's names are not ones the naming policy classifies, which `check` and `execute` report in their own words (`_check_names`) and the - manifest cannot be rendered without. + manifest cannot be rendered without, and None where its `elected_on:` is + a date `election_date` refuses, which `_elected_on_findings` reports in + the plan key's own words (#294). `_template_table` is asked, not copied, and `execute`'s `_template_values` is that same table with the real commit and digest, so the table cannot @@ -1909,7 +1911,8 @@ def _manifest_values(plan: Plan, naming: NamingPolicy) -> dict | None: `git` runs here and a checkout that cannot name its revision is asked the same question as any other. A refusal of the plan's own names, id, organisation or leg paths is raised here as it is by `check` and `execute` - a few lines later. + a few lines later. The organisation and the id are asked BEFORE the date + is: a bad date does not change which problem either command names first. """ if not isinstance(plan.legs, dict): # The doctor's PLACEMENT plan writes `legs:` as a list (see @@ -1922,12 +1925,21 @@ def _manifest_values(plan: Plan, naming: NamingPolicy) -> dict | None: _check_names(naming, names, pins) except Refusal: return None + # The organisation and the id are asked BEFORE the date's guard below. + # `checked_value` refuses an `org:` this tool will not put on a command + # line (`unsafe-value`; `execute` meets it first, in `_repository_urls`), + # and `Plan.project_id` refuses an `id:` that is not a project id + # (`plan-bad-id`; `_template_table` reads it again). After the guard, + # neither was asked of a plan whose date is a finding: `check` listed the + # date where `execute` refused the organisation, and refused the id where + # `execute` refused the date. + org = checked_value("org", plan.get("org")) + _ = plan.project_id if _elected_on_findings(plan): # Reported in the plan key's own words (#294), and the reference the # manifest records is chosen BY that date when the plan names none: # there is no manifest to ask about until it is one. return None - org = checked_value("org", plan.get("org")) repositories = {role: f"{org}/{name}" for role, name in names.items()} (_, spec_path), (_, code_path) = _plan_leg_paths(plan) return _template_table( @@ -1940,8 +1952,8 @@ def _manifest_findings(plan: Plan, misreads_elsewhere: bool = False ) -> tuple[list[str], list[str]]: """What the project's own validator would refuse of this plan's values, as `check` prints them (one FINDING each), and what it prints once after - them. `[]` and `[]` when it would pass, or when the plan's names are - refused in their own words (`_manifest_values`). + them. `[]` and `[]` when it would pass, or when the plan's names, or its + date, are refused in their own words (`_manifest_values`). `misreads_elsewhere` is for a caller whose list ALSO holds a misread rule's finding for each value the project's reader reads back as another diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index 2009634c..a1f5318a 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -670,6 +670,46 @@ def test_execute_refuses_an_elected_on_it_cannot_read_before_any_leg( assert DATE_FLAG not in str(refusal) +#: An `org:` or `id:` the plan refuses in its own words, by test id: the line +#: `plan` writes, the hand edit, and the code `check` and `execute` both +#: refuse it under. Beside an `elected_on:` that is a finding, the date's +#: guard in `_manifest_values` used to return before either was asked, so +#: the two commands named different first problems. +REFUSED_BESIDE_A_BAD_DATE = { + "org-unsafe": ("org: testorg", 'org: "-evil"', "unsafe-value"), + "id-bad": ("id: northwind", 'id: "North Wind"', "plan-bad-id"), +} + + +@pytest.mark.parametrize("reference", [True, False], + ids=["reference-kept", "no-reference"]) +@pytest.mark.parametrize("edit", sorted(REFUSED_BESIDE_A_BAD_DATE)) +def test_a_bad_date_does_not_change_what_check_and_execute_refuse_first( + adopter, world, edit, reference): + """A plan with `elected_on: someday` AND an `org:` or `id:` it refuses: + `check` and `execute` refuse it under the same code, the organisation's + or the id's, as they do with no date guard, and no leg is created. With + both asked after the guard, `check` listed only the date (exit 1) where + `execute` refused `unsafe-value`, and `check` refused `plan-bad-id` where + `execute` refused the date.""" + line, edited, code = REFUSED_BESIDE_A_BAD_DATE[edit] + text = with_elected_on(world.answered, "someday", reference) + assert line in text, f"`plan` wrote no `{line}` line" + plan = write_edited(world, text.replace(line, edited, 1)) + + checked = run_check(plan) + assert checked.returncode == 2, checked.stderr + checked.stdout + assert_no_traceback(checked) + first = next((said for said in checked.stderr.splitlines() + if said.startswith("REFUSED ")), "") + assert first.startswith(f"REFUSED {code}: "), checked.stderr + + refusal = refused_by(adopter, execute_args(world.tmp, plan)) + assert refusal.code == code + no_leg_was_created(world.tmp) + assert source_branches(world.source) == ["main"] + + @pytest.mark.parametrize("value,reference", [ ('"2026-09-02"', False), ('"2026-06-01"', False), ('"2026-09-02"', True), ("null", False)])