From e9cb3633599734728329cc392e40bec0ffc2b6d4 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sun, 11 Oct 2026 23:49:01 +0000 Subject: [PATCH 01/11] scaffold-project.py and adopt-project.py: the values written into the new project's manifest are put through its own YAML reader and its own `validate-manifest.py` before anything is created, so `--id null` and `--tracking-branch true` no longer scaffold a project that reads them back as None and True, and a blank `--elected-by`, `--elected-on 2026-W41-6` or `--org test+org` no longer ends a scaffold or `adoption verified` over a project whose first bootstrap fails (#258) (#261) `project.yaml` writes `id:` and `tracking_branch:` bare, so `scaffold-project.py --id null` and `--id true` made three repositories whose first bootstrap failed `manifest-id`, and `--tracking-branch true`, `007` or `null` pushed every repository on the branch as typed while bootstrap read the manifest back as `True`, `7` or `main` and put each leg on a local branch the remotes do not have, with no upstream, exit 0. Both tools also accepted what the project's own `validate-manifest.py` refuses: a blank `--elected-by`, `--reference` or `--name`, `--elected-on 2026-W41-6` or `20261011` (which `date.fromisoformat` reads from Python 3.11), `--org test+org`, and a plan edited to `visibility: secret` or `elected_by: null` (written as `elected_by: ""`). A `--project` of 247 to 250 characters died in `Path.exists` with `File name too long` under `--local-remote-dir`, and `plan` admitted 251 to 255 that `check` refuses. `shape_materialize` now asks the two programs the project will use. A value is refused, never retyped, as #225, #181 and #251 refuse: - `manifest_misreads` renders each template line that carries a value and reads it with the project's reader (`parse_yaml`); the template says which placeholders are bare and which quoted, so quoting them (#259) later makes the refusal go away with no edit. The scaffold asks it (`scaffold-manifest-misread`); adopt's is #181's. - `manifest_invalid` renders `project.yaml` from the whole table and runs the validator the template ships, loaded from that file and not re-implemented, with no bytecode written into the template tree. The scaffold (`scaffold-manifest-invalid`), `adopt-project.py plan` (`adopt-manifest- invalid`) and `check` (`plan-manifest-invalid`, so `execute` refuses it through the one list, before a leg) say what it says, naming the value. - `election_date` admits only what the validator's `DATE_RE` admits, on every Python. - `refuse_unstorable_names`: a name is a directory's too, at most 255 bytes, `.git` included under `--local-remote-dir`. Refused rather than quoted in the templates: one decision for every tool that renders them is #259's, and numeric branch names (`2026`, `1.5`) that worked by accident are refused here as `adopt-project.py plan` refuses them on #181. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 172 +++++++- docs/cli.md | 13 +- scaffold-project.py | 85 +++- scripts/shape_materialize.py | 375 +++++++++++++++- tests/test_adopt_manifest_values.py | 392 +++++++++++++++++ tests/test_scaffold_manifest_values.py | 580 +++++++++++++++++++++++++ 6 files changed, 1606 insertions(+), 11 deletions(-) create mode 100644 tests/test_adopt_manifest_values.py create mode 100644 tests/test_scaffold_manifest_values.py diff --git a/adopt-project.py b/adopt-project.py index aa478d09..5f7c535f 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -49,6 +49,17 @@ any `leg:` is still null, and on every other finding `check` reports, before it has created a leg repository or pushed anything. +THE NEW PROJECT MUST BE ABLE TO HOLD WHAT `execute` WRITES INTO IT (#261). +`execute` puts the plan's values into the root's `project.yaml`, and the +project's own `scripts/validate-manifest.py` judges them at its first +bootstrap. So `plan` refuses, `check` finds and `execute` refuses a value that +validator would refuse -- a blank `elected_by` or `reference`, an `elected_on` +that is not `YYYY-MM-DD`, an organisation name it cannot hold, a `visibility` +other than `private`, `public` or `internal` -- asking the validator itself, +loaded from the template it ships, and not a copy of its rules. A name that +cannot be a directory's (a `--project` that makes a repository name of 256 +bytes with its `.git`) is refused the same way. + WHY `git filter-repo` AND NOT A VENDORED COPY. Extracting history correctly is a solved problem with one correct implementation, and a vendored copy of it would be a second implementation that drifts. It is a hard REQUIREMENT here, @@ -122,6 +133,13 @@ MOVE_THE_LEG, first_and_more as _first_and_more, path_overlap as _overlap, shape_path_collisions, ) +# What the project's own `validate-manifest.py` would say of the values written +# into its manifest, and whether a repository's name can be a directory's: +# asked of the plan's values by `plan`, `check` and `execute` (#261). +from shape_materialize import ( # noqa: E402 + MANIFEST_INVALID_REMEDIATION, manifest_value_problems, + refuse_manifest_values, refuse_unstorable_names, +) #: 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. @@ -1421,6 +1439,143 @@ def _checked_plan_inputs(args, source: Source, return names, pins +# --------------------------------------------------------------------------- +# The project must be able to hold what `execute` writes into it (#261) +# --------------------------------------------------------------------------- +# +# `execute` puts the plan's values into the assembly root's `project.yaml`, and +# the project's own `scripts/validate-manifest.py` then judges them at its +# first bootstrap. Nothing here asked it first, so a blank `--elected-by` or +# `--reference`, `--elected-on 2026-W41-6` or `20261011` (which +# `date.fromisoformat` reads from Python 3.11), `--org test+org`, and a plan +# edited to `visibility: secret` or `elected_by: null`, each ended +# `adoption verified` over a project whose own gate was red, with both legs +# already pushed; the way back was an edit to `project.yaml`, which AGENTS.md +# tells an agent never to make to turn a row green. +# +# `plan` (a flag), `check` (a FINDING) and `execute` (a refusal, through +# `_entry_findings`, before any leg exists) ask ONE question of the values: the +# `project.yaml` `execute` would write from them, put through the validator the +# template ships, through `shape_materialize.manifest_value_problems`, which +# loads that validator rather than copying a rule of it. The plan is read the +# way `execute` reads it (`_template_values`), so the table is the one `execute` +# renders from. Only the validator's question is asked here: whether the +# project's YAML reader reads a value back as the text it is -- `--id true`, +# `--tracking-branch 007` -- is #181's, and the scaffold asks both. + +#: How `plan` names each value the manifest records: the flag a person typed. +MANIFEST_FLAGS = { + "PROJECT_ID": "--id", "ELECTED_BY": "--elected-by", + "ELECTED_ON": "--elected-on", "REFERENCE": "--reference", + "VISIBILITY": "--visibility", "TRACKING_BRANCH": "--tracking-branch", + "ORG": "--org", +} + +#: And how `check` and `execute` name the same value: its key in the plan. +MANIFEST_PLAN_KEYS = { + "PROJECT_ID": "id", "ELECTED_BY": "elected_by", + "ELECTED_ON": "elected_on", "REFERENCE": "reference", + "VISIBILITY": "visibility", "TRACKING_BRANCH": "tracking_branch", + "ORG": "org", +} + + +def _flag_plan(args, names: dict, pins: list) -> "Plan": + """The plan `plan` is about to write, as `Plan` reads one: the values it + records, under the keys `render_plan` writes them to. + + Built from the values and not read back from the file, which lists every + path of the source: `plan` asks what `check` will ask of the same keys, + before a file exists to read. `tests/test_adopt_manifest_values.py` holds + the two to the same keys. + """ + return Plan(Path(args.out), { + "kind": PLAN_KIND, "mode": "in-place", "project": args.project, + "org": args.org, "id": args.id, "visibility": args.visibility, + "tracking_branch": args.tracking_branch, + "elected_by": args.elected_by, "elected_on": args.elected_on, + "reference": args.reference, + "legs": {**names, "spec_path": args.spec_path, + "code_path": args.code_path}, + "pins": list(pins)}) + + +def _manifest_values(plan: Plan, naming: NamingPolicy) -> dict | None: + """The substitution table `execute` renders the manifest from, as it + would build it from this plan; 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, or + where this tool's own checkout cannot name its revision (below). + + `_template_values` is asked, not copied, so the table cannot differ from + `execute`'s. The four leg commits and digests are not known before the + legs exist and are not in the manifest; 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. + """ + if not isinstance(plan.legs, dict): + # The doctor's PLACEMENT plan writes `legs:` as a list (see + # `_plan_leg_path`): it names no repositories, so there is no + # manifest to render from it. + return None + names = plan.names() + pins = set(plan.pins) + try: + _check_names(naming, names, pins) + except Refusal: + 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) + try: + shape_commit = git_out(["rev-parse", "HEAD"], cwd=SHAPE_ROOT).lower() + except Refusal: + # No checkout to read a revision from: `execute` cannot run either, + # and says so in its own words where it reads one. `plan` and `check` + # never needed one and are not made to. + return None + return _template_values( + plan, names, repositories, {"assembly": ""}, str(spec_path), + str(code_path), str(plan.get("tracking_branch", "main")), {}, {}, + shape_commit, pins, naming) + + +def _manifest_findings(plan: Plan) -> 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`).""" + naming = NamingPolicy.load(NAMING_POLICY) + values = _manifest_values(plan, naming) + if values is None: + return [], [] + problems = manifest_value_problems(values, naming, MANIFEST_PLAN_KEYS, + misreads=False) + if not problems: + return [], [] + return ([f"FINDING plan-manifest-invalid: {sentence}" + for _, sentence in problems], + [f"{MANIFEST_INVALID_REMEDIATION} `execute` writes the plan's " + "values into the project's `project.yaml` as they stand, so " + "correct each one named here in the plan, or re-run `plan` with " + "a value the validator accepts."]) + + +def _refuse_unholdable_values(args, names: dict, pins: list, + naming: NamingPolicy) -> None: + """`plan` refuses, before it writes a file, what `check` would find in the + plan it is about to write (#261): the three repository names it derives, + held to `checked_value` as `check` holds `legs:` (a `--project` of 251 to + 255 characters planned, then `check` refused `legs.spec`), and the + manifest's values, through the one question `check` asks of them.""" + plan = _flag_plan(args, names, pins) + values = _manifest_values(plan, naming) + if values is not None: + refuse_manifest_values( + "adopt", values, naming, MANIFEST_FLAGS, + "No plan was written.", misreads=False) + + def _print_triad_advisory(path: Path, commit: str | None = None) -> None: """The Triad advisory, once, about the repository this run reads. @@ -1508,6 +1663,7 @@ def cmd_plan(args) -> int: naming = NamingPolicy.load(NAMING_POLICY) names, pins = _checked_plan_inputs(args, source, naming) + _refuse_unholdable_values(args, names, pins, naming) tree = source.tree() # WHAT THE RULES IMPLY is read from `walk`'s entries, BEFORE @@ -2134,8 +2290,10 @@ def _entry_findings(plan: Plan, source: Source findings = (_coverage_findings([str(e.get("path")) for e in plan.entries], [path for path, _, _, _ in tree]) + _leg_findings(plan, tree)) + value_findings, value_remediation = _manifest_findings(plan) submodule_findings, remediation = _submodule_findings(plan, source, tree) - return findings + submodule_findings, remediation + return (findings + value_findings + submodule_findings, + value_remediation + remediation) def _topics_line(topic: str, local: bool) -> str: @@ -2653,6 +2811,14 @@ def _repository_urls(args, plan: Plan, names: dict, local = args.local_remote_dir is not None org = checked_value("org", plan.get("org")) repositories = {role: f"{org}/{name}" for role, name in names.items()} + # Each name is a directory's too: the work directory's for all three, and + # under `--local-remote-dir` a bare `.git` for the two new legs. A + # name of 247 to 250 characters passed every check and died in + # `Path.exists` with `File name too long`, a traceback, before any leg + # existed (#261). + refuse_unstorable_names( + "adopt-name-too-long", list(names.values()), + [names["spec"], names["code"]] if local else []) if local: # NOT created yet: nothing exists on disk until the human has said # yes, so a refused run leaves the directory it would have used @@ -3622,7 +3788,9 @@ def main(argv: list[str] | None = None) -> int: plan.add_argument("--id", default=None) plan.add_argument("--visibility", choices=VISIBILITY_CHOICES, default="private") - plan.add_argument("--elected-by", default=None) + plan.add_argument("--elected-by", default=None, + help="the human electing the shape (default: `git " + "config user.name`); not blank") plan.add_argument("--elected-on", default=None, help="YYYY-MM-DD") plan.add_argument("--reference", default=None, help="the document the election followed. Default: " diff --git a/docs/cli.md b/docs/cli.md index 5c50a686..b1be605b 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -246,7 +246,9 @@ options: -h, --help show this help message and exit --org ORG --project PROJECT the assembly-root name, e.g. Atlas - --id ID lowercase project id + --id ID lowercase project id (default: --project lowercased). + One the project's YAML reader would read as another + type -- `null`, `true`, `007` -- is refused. --name NAME display name --visibility {private,public,internal} default: private for a fresh scaffold; with --reuse- @@ -262,6 +264,9 @@ options: (owner/openProduct@) to override it for one pin without changing this default for the others. --elected-by ELECTED_BY + the human electing the shape (default: + GIT_AUTHOR_NAME, then `git config user.name`); not + blank --elected-on ELECTED_ON YYYY-MM-DD --reference REFERENCE @@ -273,6 +278,10 @@ options: does not claim it followed a document that did not exist yet. --tracking-branch TRACKING_BRANCH + the branch every repository is created on (default: + main). One the project's YAML reader would read as + another type -- `null`, `true`, `007`, `2026` -- is + refused. --spec-path SPEC_PATH --code-path CODE_PATH --dry-run @@ -352,6 +361,8 @@ options: --id ID --visibility {private,public,internal} --elected-by ELECTED_BY + the human electing the shape (default: `git config + user.name`); not blank --elected-on ELECTED_ON YYYY-MM-DD --reference REFERENCE diff --git a/scaffold-project.py b/scaffold-project.py index 032f7707..dea1b46f 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -29,6 +29,17 @@ digest-pinned in `contracts/shape-pin.yaml`, so "which openRepoShape is this a copy of, and has anyone edited it since" both have answers. +WHAT THE NEW PROJECT MUST BE ABLE TO HOLD (#258, #261). The manifest is +rendered from the values the run is about to use, read with the project's own +YAML reader and put through the validator the template ships, BEFORE anything +is created and under `--dry-run` alike. A value the reader turns into something +else (`--id null`, `--tracking-branch true` or `007`: the template writes them +bare) is refused as `scaffold-manifest-misread`, and one the validator refuses +(a blank `--elected-by`, `--reference` or `--name`, an `--org` it cannot hold) +as `scaffold-manifest-invalid`, in the validator's own words; an +`--elected-on` is read in the validator's form only, on every Python. A name +that cannot be a directory's is refused as `scaffold-name-too-long`. + IDEMPOTENCY. It refuses to write into a remote or a working directory that already exists. There is no --force: re-running over a live project is not a scaffold, and the failure mode of getting it wrong is silent data loss. @@ -64,6 +75,12 @@ ) # The leg-path rule `adopt-project.py` asks of its own paths (#179). from shape_materialize import refuse_bad_leg_paths # noqa: E402 +# What the new project's own reader and `validate-manifest.py` would make of +# the values written into its manifest, and whether a repository's name can +# be a directory's: asked before anything is created (#258, #261). +from shape_materialize import ( # noqa: E402 + refuse_manifest_values, refuse_unstorable_names, +) from shape_advisory import scaffold_lines # noqa: E402 # The shape half of the leg-path collision rule, which `adopt-project.py` # asks of its own mount paths (#172), imported as one unit (#197), with the @@ -72,6 +89,15 @@ assembly_root_paths, neutral_pin_path, shape_path_collisions, ) +#: How a refusal names each value the manifest records (#258, #261). The leg +#: paths are not here: `refuse_bad_leg_paths` asks for them (#179, #225). +MANIFEST_FLAGS = { + "PROJECT_ID": "--id", "PROJECT_NAME": "--name", + "ELECTED_BY": "--elected-by", "ELECTED_ON": "--elected-on", + "REFERENCE": "--reference", "VISIBILITY": "--visibility", + "TRACKING_BRANCH": "--tracking-branch", "ORG": "--org", +} + #: The pin argument: `openGlass@<40 hex>`, optionally organisation-qualified. PIN_ARG_RE = re.compile(r"^(?P[A-Za-z0-9_.-]+(?:/[A-Za-z0-9_.-]+)?)@(?P[0-9a-fA-F]{40})$") @@ -255,7 +281,11 @@ def main(argv: list[str] | None = None) -> int: parser.add_argument("--org", required=True) parser.add_argument("--project", required=True, help="the assembly-root name, e.g. Atlas") - parser.add_argument("--id", default=None, help="lowercase project id") + parser.add_argument("--id", default=None, + help="lowercase project id (default: --project " + "lowercased). One the project's YAML reader " + "would read as another type -- `null`, `true`, " + "`007` -- is refused.") parser.add_argument("--name", default=None, help="display name") parser.add_argument("--visibility", choices=VISIBILITY_CHOICES, default=None, @@ -272,7 +302,10 @@ def main(argv: list[str] | None = None) -> int: "--pin itself (owner/openProduct@) to " "override it for one pin without changing this " "default for the others.") - parser.add_argument("--elected-by", default=None) + parser.add_argument("--elected-by", default=None, + help="the human electing the shape (default: " + "GIT_AUTHOR_NAME, then `git config user.name`); " + "not blank") parser.add_argument("--elected-on", default=None, help="YYYY-MM-DD") parser.add_argument("--reference", default=None, help="the document the election followed. Default: " @@ -283,7 +316,11 @@ def main(argv: list[str] | None = None) -> int: "--elected-on chooses it, and a back-dated " "project does not claim it followed a document " "that did not exist yet.") - parser.add_argument("--tracking-branch", default="main") + parser.add_argument("--tracking-branch", default="main", + help="the branch every repository is created on " + "(default: main). One the project's YAML reader " + "would read as another type -- `null`, `true`, " + "`007`, `2026` -- is refused.") parser.add_argument("--spec-path", default="spec") parser.add_argument("--code-path", default="code") parser.add_argument("--dry-run", action="store_true") @@ -484,6 +521,35 @@ def _refuse_colliding_leg_paths(spec_path: str, code_path: str, _leg_collision_remediation(neutral_pins)) +def _refuse_unholdable_manifest(args, values: dict, policy) -> None: + """The manifest this run would write is one the project can hold, or this + refuses (#258, #261), before a work directory or a remote exists and + under `--dry-run` alike. + + `values` is the whole substitution table, so the project's own YAML + reader and `validate-manifest.py` are asked about the `project.yaml` the + run is about to write: a value the reader turns into something else + (`--id null`, `--tracking-branch 007`) is `scaffold-manifest-misread`, and + one the validator refuses (a blank `--elected-by`, `--org test+org`) is + `scaffold-manifest-invalid`. `shape_materialize.refuse_manifest_values` + asks both, and says why each is refused. + + A value a flag did not supply is named by where it came from, so the + sentence does not point at a flag nobody typed. + """ + flags = dict(MANIFEST_FLAGS) + for key, flag, given, source in ( + ("PROJECT_ID", "--id", args.id, "--project lowercased"), + ("PROJECT_NAME", "--name", args.name, "--project"), + ("ELECTED_BY", "--elected-by", args.elected_by, + "GIT_AUTHOR_NAME or `git config user.name`")): + if not given: + flags[key] = f"{flag} (not given, so {source})" + refuse_manifest_values( + "scaffold", values, policy, flags, + "Nothing was created, and `--dry-run` refuses the same.") + + def _elector(args) -> str: """(a) Whose act electing this shape was, or a refusal. @@ -1038,6 +1104,13 @@ def _scaffold(args) -> int: names = {"assembly": project, "spec": f"{project}-spec", "code": f"{project}-code"} + # Each name is a directory's too, in the work directory and, under + # `--local-remote-dir`, as a bare `.git` (#261): a `--project` of + # 247 to 250 characters died in `Path.exists` with a traceback, and one + # of 251 or more in the work directory, where adopt's `check` refused it. + refuse_unstorable_names( + "scaffold-name-too-long", list(names.values()), + list(names.values()) if args.local_remote_dir is not None else []) policy = NamingPolicy.load(SHAPE_ROOT / "contracts" / "repository-naming.yaml") if not PROJECT_ID_RE.match(project_id): raise Refusal("scaffold-bad-id", @@ -1099,6 +1172,12 @@ def _scaffold(args) -> int: "CODE_NAMING": naming_block(policy, names["code"], "code", declared_pins), } + # THE PROJECT MUST BE ABLE TO HOLD WHAT IS ABOUT TO BE WRITTEN (#258, + # #261): the manifest is rendered from this very table and put through + # the project's own reader and `validate-manifest.py`, so what its first + # bootstrap would refuse is refused here, before the plan is printed. + _refuse_unholdable_manifest(args, values, policy) + _print_plan(args, values, names, repositories, urls, local, policy, declared_pins, chain, link_pins, pins) if args.dry_run: diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 54b2c55c..b55b4bd5 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -243,18 +243,32 @@ def election_date(elected_on: str) -> _dt.date: A date this cannot read is not one it will guess at: the guess would pick the reference, and the reference is a claim about a human's act. + + THE FORM IS THE PROJECT'S OWN VALIDATOR'S (#261), on every Python. + `date.fromisoformat` reads `2026-W41-6` and `20261011` since Python 3.11, + and the project's `validate-manifest.py` refuses both: its `elected_on` + must match its own `DATE_RE`, asked here through the validator the + template carries (`manifest_validator`) and not copied. A date is read + only when that rule admits it AND it is a day on the calendar, so the + answer is the same on 3.9 and 3.14. """ + text = str(elected_on) try: - return _dt.date.fromisoformat(str(elected_on)) + if not manifest_validator().DATE_RE.match(text): + raise ValueError(f"{text!r} is not YYYY-MM-DD") + return _dt.date.fromisoformat(text) except ValueError as exc: raise Refusal( "election-date-malformed", - f"--elected-on {elected_on!r} is not a date; it takes YYYY-MM-DD " + f"--elected-on {elected_on!r} is not a YYYY-MM-DD date " "(and a plan file's `elected_on:` carries the same form)", "Remediation: re-run with a zero-padded ISO date, e.g. " - "--elected-on 2026-09-02 — not 2026-9-2 and not a word like " - "'yesterday'. The `reference:` the manifest records is chosen BY " - "that date, so it has to be readable before anything is written.", + "--elected-on 2026-09-02 — not 2026-9-2, not an ISO week or " + "basic-format date (`2026-W41-6`, `20261011`) that Python 3.11 " + "and later reads and the project's own validator refuses, and " + "not a word like 'yesterday'. The `reference:` the manifest " + "records is chosen BY that date, so it has to be readable before " + "anything is written.", ) from exc @@ -1725,3 +1739,354 @@ def refuse_bad_leg_paths(refusal: str, spec: tuple, code: tuple) -> None: problems = leg_path_problems(spec, code) if problems: raise Refusal(refusal, " ".join(problems), LEG_PATH_REMEDIATION) + + +# --------------------------------------------------------------------------- +# What a tool may write into the new project's manifest (#258, #261) +# --------------------------------------------------------------------------- +# +# `scaffold-project.py` and `adopt-project.py` fill `templates/assembly-root/` +# from values a person typed (`--id`, `--tracking-branch`, `--elected-by`, +# `--elected-on`, `--reference`, `--org`, `--name`) or edited into a plan +# (`visibility:`, `elected_by:`), and the new project then reads the result +# with two programs of its own: its YAML reader (`repo_shape`) and its +# `scripts/validate-manifest.py`. Neither tool asked either of them anything, +# so a value the project could not hold was found by the project's FIRST +# bootstrap, after the repositories existed: +# +# * READ BACK AS SOMETHING ELSE (#258). The templates write `id:` and +# `tracking_branch:` bare, so `--id null`, `--id true`, `--tracking-branch +# true` and `--tracking-branch 007` became None, True and 7. The id left +# the project's gate red; the branch was silent, each leg sitting on a +# local `True` or `7` the remote does not have, with no upstream. +# * REFUSED BY THE VALIDATOR (#261). A blank `--elected-by` or `--reference`, +# `--elected-on 2026-W41-6` or `20261011` (which `date.fromisoformat` reads +# from Python 3.11), `--org test+org`, a plan's `visibility: secret` or +# `elected_by: null`: all accepted, and `adoption verified` or a scaffold +# exit 0 followed by a bootstrap that said no. +# +# THE CURE IS TO ASK, BEFORE ANYTHING IS CREATED, AND TO REFUSE. The two +# questions below take the values a tool is about to record and put them +# through the very programs the project will use, so there is no second +# definition of either rule to keep in step: +# +# `manifest_misreads` renders each template line that carries one of the +# values, reads that line with the project's reader (`repo_shape. +# parse_yaml`, of which every project carries a copy) and refuses a +# value that does not come back as the same text. The TEMPLATE says +# which values are written bare and which quoted: a placeholder the +# template quotes is asked as the quoted line it is, so the day the +# templates quote `id:` and `tracking_branch:` (#259), those values +# read back and this refuses nothing, with no edit here. +# `manifest_invalid` renders `project.yaml` from the whole substitution +# table, reads it, and runs the validator the template ships +# (`templates/assembly-root/scripts/validate-manifest.py`, loaded from +# that file, never re-implemented) over it. Whatever the validator says, +# now or after it learns another rule, is refused here in its words. +# +# REFUSED, NOT QUOTED OR CORRECTED, as #225, #181 and #251 refuse: a value +# the manifest cannot hold has no spelling that is the same value (`007` as a +# branch is a legal git name and a different number), and rewriting a +# person's value for them is the silent retyping this refuses. Quoting the +# templates is #259's decision, and one decision for every tool that renders +# them. +# +# IT LIVES HERE AND NOT IN `scripts/repo_shape.py` OR THE VALIDATOR, which are +# SHAPE COPIES: the materializer writes both into every project and +# `contracts/shape-pin.yaml` digests them, so a rule moved into either would +# put an `upstream-changed` row in front of every project in the estate for a +# question no project's own copy asks of itself. The validator is LOADED, so +# its definition stays the one place the rule is written. + +import importlib.util # noqa: E402 +from repo_shape import YamlError, parse_yaml # noqa: E402 + +#: The assembly root's template directory, and the two files in it these +#: questions read: the manifest, and the validator the project is given. +ASSEMBLY_TEMPLATE = (Path(__file__).resolve().parent.parent / "templates" + / "assembly-root") +MANIFEST_TEMPLATE = "project.yaml" +MANIFEST_VALIDATOR = "scripts/validate-manifest.py" + +#: The templated files the project's reader reads: the manifest and the two +#: leg pins. The materializer's own list, so a templated YAML file added there +#: is asked about here without an edit. +TEMPLATED_YAML = tuple(rel for rel in TEMPLATED if rel.endswith(".yaml")) + +_PLACEHOLDER_KEY_RE = re.compile(r"\{\{([A-Z_]+)\}\}") + +#: The validator modules already loaded, by the file they were loaded from. +_VALIDATORS: dict = {} + + +def manifest_validator(template_dir: Path = ASSEMBLY_TEMPLATE): + """The new project's own `scripts/validate-manifest.py`, as a module. + + LOADED FROM THE TEMPLATE IT IS COPIED FROM, so what is asked is the rule + the project will apply and no second spelling of it exists. By path, + because the filename has a hyphen and cannot be imported by name + (`shape-doctor.py` loads the tools it asks the same way). The directory + the validator puts at the front of `sys.path` for its own sibling imports + is taken off again: `bootstrap` and `validate-pins` are modules of the + project and must not shadow anything of this process's. + + NO BYTECODE IS WRITTEN. Importing a source file makes CPython write + `scripts/__pycache__/` beside it, and this file is a TEMPLATE: a `.pyc` + in `templates/assembly-root/` is a file nobody put there, which every + reader of that tree then meets (`tests/test_scaffold_leg_path_collisions.py` + reads each file under it as text). The same reason `shape-doctor.py` keeps + the interpreter's cache out of a tree it only reads. + """ + path = Path(template_dir) / MANIFEST_VALIDATOR + module = _VALIDATORS.get(path) + if module is None: + spec = importlib.util.spec_from_file_location( + "openreposhape_validate_manifest", path) + module = importlib.util.module_from_spec(spec) + saved_path, saved_flag = list(sys.path), sys.dont_write_bytecode + sys.dont_write_bytecode = True + try: + spec.loader.exec_module(module) + finally: + sys.path[:] = saved_path + sys.dont_write_bytecode = saved_flag + _VALIDATORS[path] = module + return module + + +def _reading_in_words(reading) -> str: + """What the project's reader made of a value, as a sentence says it.""" + if isinstance(reading, YamlError): + return f"nothing at all, because it refuses the line ({reading})" + if reading is None: + return "no value at all" + if isinstance(reading, bool): + return "the boolean " + ("true" if reading else "false") + if isinstance(reading, (int, float)): + return f"the number {reading!r}" + if isinstance(reading, str): + return f"the text {reading!r}" + return "a list" if isinstance(reading, list) else "a mapping" + + +def _line_shown(line: str) -> str: + """`line` in backticks, or as Python spells it where it holds a character + a terminal shows as something else, so that one finding stays one line.""" + return f"`{line}`" if line.isprintable() else repr(line) + + +def _reading_of(rendered: str): + """What the project's reader makes of the one `key: value` line + `rendered`: the value, the `YamlError` it raised, or the whole answer + when it is not one key with one value (a value with a line break in it + can carry a second key into the file).""" + try: + parsed = parse_yaml(rendered.lstrip() + "\n") + except YamlError as exc: + return exc + if isinstance(parsed, dict) and len(parsed) == 1: + return next(iter(parsed.values())) + return parsed + + +def manifest_misreads(values: dict, what: dict, + template_dir: Path = ASSEMBLY_TEMPLATE) -> list[str]: + """A sentence for each value the project's reader would read back as + anything but that same text; `[]` when every one reads back. + + `values` is the substitution table; `what` names the placeholders to ask + about and how the caller calls each (`{"PROJECT_ID": "--id"}`), and a + placeholder it does not name is not asked: a block value + (`NEUTRAL_PRODUCT_PINS`, a naming block) is not one line's scalar, and a + value a rule of its own already judged is not asked twice. + + Each template line that carries exactly one of them is rendered with the + value, as `render` writes it, and read as the project reads it. + """ + found: dict[str, list] = {} + for rel in TEMPLATED_YAML: + text = (Path(template_dir) / rel).read_text(encoding="utf-8") + for line in text.splitlines(): + keys = _PLACEHOLDER_KEY_RE.findall(line) + if (len(keys) != 1 or keys[0] not in what + or keys[0] not in values + or line.lstrip().startswith("#")): + continue + key, value = keys[0], str(values[keys[0]]) + rendered = render(line, {key: value}, rel).lstrip() + reading = _reading_of(rendered) + if not (isinstance(reading, str) and reading == value): + found.setdefault(key, []).append((rel, rendered, reading)) + return [ + f"{what[key]} is {str(values[key])!r}, which is written " + + " and ".join(f"into {rel} as {_line_shown(line)}" + for rel, line, _ in rows) + + ", where the project's own YAML reader reads it back as " + + f"{_reading_in_words(rows[0][2])} and not as that text." + for key, rows in found.items()] + + +#: Which placeholder carries the value a finding of the validator is about, +#: for the ones a tool is handed a value for. A finding with no row is still +#: refused, in the validator's words and without a flag to point at. +FINDING_PLACEHOLDER = { + "manifest-id": "PROJECT_ID", "manifest-topic": "PROJECT_ID", + "manifest-name": "PROJECT_NAME", "manifest-elected-by": "ELECTED_BY", + "manifest-elected-on": "ELECTED_ON", "manifest-reference": "REFERENCE", + "manifest-visibility": "VISIBILITY", "manifest-leg-repository": "ORG", +} + +_FINDING_RE = re.compile(r"^FINDING (?P[a-z0-9-]+): (?P.*)$", + re.DOTALL) + + +def _sentences_of(findings: list[str], values: dict, + what: dict) -> list[str]: + """The FINDINGs of the validator as sentences: one for each value the + caller handed over, naming it and holding every finding about it (`--org` + is judged once for each of the three legs it names), and one for each + finding that is about none of them.""" + by_value: dict[str, list[str]] = {} + sentences: list[str] = [] + for finding in findings: + match = _FINDING_RE.match(finding) + if match is None: + sentences.append(finding) + continue + code, detail = match.group("code"), match.group("detail") + said = f"{code}: {detail if detail.endswith('.') else detail + '.'}" + key = FINDING_PLACEHOLDER.get(code) + if key in what and key in values: + by_value.setdefault(key, []).append(said) + else: + sentences.append("the manifest this run would write is one the " + f"project's own validate-manifest.py refuses: " + f"{said}") + return [f"{what[key]} is {str(values[key])!r}, which the project's own " + f"validate-manifest.py refuses: {' '.join(said)}" + for key, said in by_value.items()] + sentences + + +def manifest_invalid(values: dict, policy: NamingPolicy, what: dict, + template_dir: Path = ASSEMBLY_TEMPLATE) -> list[str]: + """A sentence for each FINDING the project's own validator would make of + the `project.yaml` these `values` render; `[]` when it would pass. + + The WHOLE manifest is rendered and read, so a rule about how one value + relates to another (`topic` to `id`, a leg's repository to its role) is + asked as the project asks it. `values` must be the complete table; + `policy` is the naming policy the project carries a copy of. A WARNING of + the validator (a chain link it cannot check offline) is a note and never + a refusal, exactly as the project's own exit code treats it. + """ + template = (Path(template_dir) / MANIFEST_TEMPLATE).read_text( + encoding="utf-8") + try: + manifest = parse_yaml(render(template, values, MANIFEST_TEMPLATE)) + except YamlError as exc: + return ["the manifest this run would write is a file the project's " + f"own YAML reader refuses: {exc}."] + notes = manifest_validator(template_dir)._findings(manifest, policy, None) + return _sentences_of([note for note in notes + if not note.startswith("WARNING")], values, what) + + +#: The remediation, without the sentence only the caller can write (what was +#: or was not created, whether `--dry-run` refuses the same). +MANIFEST_MISREAD_REMEDIATION = ( + "Remediation: choose another value for each one named, as nothing here " + "will retype it for you. The templates write these values unquoted, so " + "the project would read `null` and `~` as nothing, `true` and `false` as " + "booleans and `007` or `2026` as numbers, and its first bootstrap would " + "fail, or put its legs on a branch the remotes do not have. An id or a " + "branch name the reader takes for text (`atlas`, `release-1`, `v2`) " + "passes.") +MANIFEST_INVALID_REMEDIATION = ( + "Remediation: choose a value the project's own validate-manifest.py " + "accepts; the words after each value are the validator's own, and a " + "project it refuses is one whose first bootstrap would fail.") + + +def manifest_value_problems(values: dict, policy: NamingPolicy, what: dict, + *, misreads: bool = True, + template_dir: Path = ASSEMBLY_TEMPLATE + ) -> list[tuple[str, str]]: + """`(kind, sentence)` for everything wrong with the values, `kind` being + `misread` or `invalid`; `[]` when the project would hold every one. + + A misread comes FIRST and alone: a value the reader turns into something + else makes the manifest invalid in ways that are its consequence (`id: + null` fails the validator as `id is None`), so the sentence about the + value itself is the one worth reading. `misreads=False` asks only the + validator's question, for a tool whose YAML-misread rule is its own. + """ + if misreads: + misread = manifest_misreads(values, what, template_dir) + if misread: + return [("misread", sentence) for sentence in misread] + return [("invalid", sentence) for sentence + in manifest_invalid(values, policy, what, template_dir)] + + +def refuse_manifest_values(prefix: str, values: dict, policy: NamingPolicy, + what: dict, tail: str, *, + misreads: bool = True) -> None: + """Raise `Refusal(-manifest-misread)` or `-manifest- + invalid`, naming every value, when the project could not hold them; the + caller's own `prefix` (`scaffold`, `adopt`) says which tool refused, and + its `tail` finishes the remediation with what only it knows.""" + problems = manifest_value_problems(values, policy, what, + misreads=misreads) + if problems: + kind = problems[0][0] + remedy = (MANIFEST_MISREAD_REMEDIATION if kind == "misread" + else MANIFEST_INVALID_REMEDIATION) + raise Refusal(f"{prefix}-manifest-{kind}", + " ".join(sentence for _, sentence in problems), + f"{remedy} {tail}") + + +# --------------------------------------------------------------------------- +# A repository's name is also a directory's (#261) +# --------------------------------------------------------------------------- +# +# `checked_value` holds a name to 255 characters, `SAFE_ARG_RE`'s limit, and +# the tools then make a directory of it: `/` for each of the +# three, and under `--local-remote-dir` a bare `.git` beside them. A +# name is at most 255 bytes on ext4, APFS and NTFS alike, so a `--project` of +# 247 to 250 characters passed every check and died in `Path.exists` with +# `OSError: File name too long`, a traceback and exit 1, where the first +# directory is asked about; the scaffold died the same way from 247, and from +# 251 characters in the work directory, where adopt's `check` had already +# refused the name `legs.spec` with `unsafe-value`. + +#: The longest name a file or a directory may have, in bytes. +NAME_MAX = 255 + + +def refuse_unstorable_names(refusal: str, directories: list[str], + bare: list[str] = ()) -> None: + """Raise `Refusal(refusal)` when a repository name cannot be a directory + name: each of `directories` is made as it stands under the work + directory, each of `bare` as `.git` under `--local-remote-dir`.""" + problems = [] + for names, suffix, where in ( + (directories, "", "the directory this run makes for it under " + "its work directory"), + (bare, ".git", "the bare repository's directory under " + "--local-remote-dir")): + for name in names: + size = len(f"{name}{suffix}".encode("utf-8")) + if size > NAME_MAX: + shown = name if len(name) <= 24 else name[:24] + "..." + problems.append( + f"{shown}{suffix} is {size} bytes long, and {where} " + f"may be at most {NAME_MAX}") + if problems: + raise Refusal( + refusal, "; ".join(problems) + ".", + "Remediation: nothing was created. Use a shorter --project: the " + "three repositories are named ``, `-spec` and " + "`-code`, and a disk keeps a name of at most " + f"{NAME_MAX} bytes, which under --local-remote-dir includes the " + "`.git` each bare repository adds.") diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py new file mode 100644 index 00000000..eeaca585 --- /dev/null +++ b/tests/test_adopt_manifest_values.py @@ -0,0 +1,392 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`adopt-project.py` refuses a value the new project's validator refuses (#261). + +`plan`, `check` and `execute` took values the project's own +`scripts/validate-manifest.py` refuses, and `execute` said `adoption verified` +over a project whose first bootstrap then failed, with both legs already +pushed: a blank `--elected-by` or `--reference`, `--elected-on 2026-W41-6` or +`20261011` (which `date.fromisoformat` reads from Python 3.11), `--org +test+org`, and a plan edited to `visibility: secret` or `elected_by: null` +(which `execute` wrote as `elected_by: ""`). The way back was an edit to +`project.yaml`, which AGENTS.md tells an agent never to make to turn a row +green. + +`plan` also admitted a `--project` of 251 to 255 characters, which `check` then +refused as `unsafe-value: legs.spec`, because it held `--project` to +`checked_value` and not the names it derives; and under `--local-remote-dir` a +`--project` of 247 to 250 characters died in `Path.exists` with `OSError: File +name too long`, before any leg existed. + +Now `plan` refuses before it writes a file, `check` makes a FINDING of each +(`plan-manifest-invalid`), and `execute` refuses every one through `check`'s +own list (#168), before a leg repository exists. All three ask the project's +own validator, loaded from the template it ships, about the `project.yaml` the +plan's values would render. + +NO NETWORK AND NO GITHUB. `plan` and `check` read a local source. `execute` +runs only against `--local-remote-dir`, and every `execute` here is a +refusal that comes before the first leg; the tests that run one skip without +`git filter-repo`, as the other `execute` tests do. +""" + +from __future__ import annotations + +import importlib.util +import shutil +import sys +from argparse import Namespace +from pathlib import Path + +import pytest + +from conftest import (ADOPT, REPO, git, make_source_repo, resolve, run_script, + write_plan) + +sys.path.insert(0, str(REPO / "scripts")) +import shape_materialize # noqa: E402 + +PROJECT = "Northwind" +UTF8 = "utf-8" +INVALID = "adopt-manifest-invalid" +FINDING = "FINDING plan-manifest-invalid" +REMOTES, WORK = "remotes", "work" + +#: What `execute` needs and `plan` and `check` do not. +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`") + +ANSWERS = (("examples/", "spec"), (".claude/", "root"), + ("release.yaml", "root")) + +#: `(extra `plan` flags, what the refusal names)`: values the validator +#: refuses, which `plan` used to write into a plan that `check` passed. +PLAN_REFUSED = [ + pytest.param(["--elected-by= "], (INVALID, "--elected-by is ' '", + "manifest-elected-by"), + id="elected-by-blank"), + pytest.param(["--elected-by=\u3000"], (INVALID, "--elected-by is " + "'\\u3000'", "manifest-elected-by"), + id="elected-by-ideographic-space"), + pytest.param(["--reference= "], (INVALID, "--reference is ' '", + "manifest-reference"), + id="reference-blank"), + pytest.param(["--org=test+org"], (INVALID, "--org is 'test+org'", + "manifest-leg-repository"), + id="org-plus"), + pytest.param(["--org=test@org"], (INVALID, "--org is 'test@org'", + "manifest-leg-repository"), + id="org-at"), + pytest.param(["--elected-on=2026-W41-6"], ("election-date-malformed", + "2026-W41-6"), + id="elected-on-iso-week"), + pytest.param(["--elected-on=20261011"], ("election-date-malformed", + "20261011"), + id="elected-on-basic-format"), +] + +#: `(old line, new line, what `check` names)`: the same values, edited into a +#: plan by hand, which is what a plan is for. +PLAN_EDITS = [ + pytest.param("visibility: private", "visibility: secret", + ("visibility is 'secret'", "manifest-visibility"), + id="visibility-secret"), + pytest.param('elected_by: "Test Human"', "elected_by: null", + ("elected_by is ''", "manifest-elected-by"), + id="elected-by-null"), + pytest.param('elected_by: "Test Human"', 'elected_by: " "', + ("elected_by is ' '", "manifest-elected-by"), + id="elected-by-blank"), + pytest.param('elected_by: "Test Human"', 'elected_by: ""', + ("elected_by is ''", "manifest-elected-by"), + id="elected-by-empty"), + pytest.param('reference: "openxFactory docs/project-repo-schema.md"', + 'reference: " "', ("reference is ' '", "manifest-reference"), + id="reference-blank"), + pytest.param('elected_on: "2026-09-02"', "elected_on: 20261011", + ("elected_on is '20261011'", "manifest-elected-on"), + id="elected-on-basic-format"), + pytest.param('elected_on: "2026-09-02"', "elected_on: 2026-W41-6", + ("elected_on is '2026-W41-6'", "manifest-elected-on"), + id="elected-on-iso-week"), + pytest.param("org: testorg", "org: test+org", + ("org is 'test+org'", "manifest-leg-repository"), + id="org-plus"), +] + + +@pytest.fixture(scope="module") +def adopter(): + spec = importlib.util.spec_from_file_location("adopt_manifest_values", + ADOPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@pytest.fixture +def world(tmp_path): + """A source, and a plan for it with every question ANSWERED: the plan + `check` passes. Each test edits its own copy of the text.""" + source = make_source_repo(tmp_path / "Thing") + plan = tmp_path / "adoption-plan.yaml" + written = write_plan(source, plan, project=PROJECT) + assert written.returncode == 0, written.stderr + written.stdout + for path, leg in ANSWERS: + resolve(plan, path, leg) + return {"source": source, "plan": plan, "tmp": tmp_path, + "text": plan.read_text(encoding=UTF8)} + + +def edited(world: dict, old: str, new: str) -> Path: + """The plan with the one line `old` replaced, written beside it.""" + assert old in world["text"], f"{old!r} is not a line of the plan" + path = world["tmp"] / "edited-plan.yaml" + with path.open("w", encoding=UTF8, newline="\n") as handle: + handle.write(world["text"].replace(old, new, 1)) + return path + + +def execute(world: dict, plan: Path): + return run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(world["tmp"] / REMOTES), + "--work-dir", str(world["tmp"] / WORK)) + + +def no_leg_was_created(world: dict) -> None: + assert not (world["tmp"] / REMOTES).exists() + for role in ("spec", "code"): + assert not (world["tmp"] / WORK / f"{PROJECT}-{role}").exists() + + +# --- `plan` refuses before it writes anything ----------------------------- + +@pytest.mark.parametrize("extra, named", PLAN_REFUSED) +def test_plan_refuses_a_value_the_validator_refuses_and_writes_no_plan( + tmp_path, extra, named): + source = make_source_repo(tmp_path / "Thing") + out = tmp_path / "plan.yaml" + result = write_plan(source, out, extra=tuple(extra)) + assert result.returncode == 2, result.stderr + result.stdout + for fragment in named: + assert fragment in result.stderr, result.stderr + assert not out.exists(), "a refused `plan` wrote a plan" + + +def test_plan_says_it_in_the_validators_own_words(tmp_path): + validator = shape_materialize.manifest_validator() + ((finding,),) = [validator._elected_by_finding({"elected_by": " "})] + source = make_source_repo(tmp_path / "Thing") + result = write_plan(source, tmp_path / "plan.yaml", + extra=("--elected-by= ",)) + assert finding.split(": ", 1)[1] in result.stderr + + +def test_plan_still_writes_a_plan_for_values_the_validator_accepts(tmp_path): + source = make_source_repo(tmp_path / "Thing") + out = tmp_path / "plan.yaml" + result = write_plan(source, out, org="my_org.x", + extra=("--elected-by=Robert Smith", + "--reference=docs/project-repo-schema.md")) + assert result.returncode == 0, result.stderr + result.stdout + checked = run_script(ADOPT, "check", "--plan", str(out)) + assert checked.returncode in (0, 1), checked.stderr + assert FINDING not in checked.stderr + + +@pytest.mark.parametrize("length", [251, 255]) +def test_plan_refuses_a_project_whose_derived_names_check_would_refuse( + tmp_path, length): + """`plan` held `--project` to 255 characters and not `-spec`, so + it wrote a plan that `check` refused as `unsafe-value: legs.spec`. It now + refuses the same value, in the same words, before the plan exists.""" + source = make_source_repo(tmp_path / "Thing") + out = tmp_path / "plan.yaml" + result = write_plan(source, out, project="N" * length) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED unsafe-value: legs.spec is " in result.stderr + assert not out.exists() + + +def test_plan_asks_what_check_asks_of_the_same_keys(adopter, tmp_path, + monkeypatch, capsys): + """`plan` builds the plan it is about to write from its values, before a + file exists to read. Held to `render_plan`'s own keys: each key the + built one carries is what the file `plan` writes then says.""" + built = [] + real = adopter._flag_plan + monkeypatch.setattr(adopter, "_flag_plan", + lambda *a: built.append(real(*a)) or built[-1]) + source = make_source_repo(tmp_path / "Thing") + out = tmp_path / "plan.yaml" + code = adopter.main(["plan", "--source", str(source), "--project", + PROJECT, "--org", "testorg", "--elected-by", + "Test Human", "--elected-on", "2026-09-02", + "--visibility", "internal", "--tracking-branch", + "trunk", "--pin", "openGlass", "--out", str(out)]) + assert code == 0, capsys.readouterr().err + (flag_plan,) = built + written = adopter.Plan.load(out) + for key, value in flag_plan.data.items(): + if key == "legs": + for leg, leg_value in value.items(): + assert written.data["legs"][leg] == leg_value, leg + else: + assert written.data[key] == value, key + + +# --- `check` finds, `execute` refuses, both before a leg ------------------- + +def test_a_plan_with_no_such_edit_still_passes_check(world): + result = run_script(ADOPT, "check", "--plan", str(world["plan"])) + assert result.returncode == 0, result.stderr + result.stdout + assert "plan ok" in result.stdout + + +@pytest.mark.parametrize("old, new, named", PLAN_EDITS) +def test_check_finds_a_value_the_validator_refuses(world, old, new, named): + plan = edited(world, old, new) + result = run_script(ADOPT, "check", "--plan", str(plan)) + assert result.returncode == 1, result.stderr + result.stdout + findings = [line for line in result.stderr.splitlines() + if line.startswith("FINDING ")] + assert [f for f in findings if f.startswith(FINDING)], result.stderr + for fragment in named: + assert fragment in result.stderr, result.stderr + assert "correct each one named here in the plan" in result.stderr + + +@needs_filter_repo +@pytest.mark.parametrize("old, new, named", PLAN_EDITS) +def test_execute_refuses_it_before_any_leg_exists(world, old, new, named): + """Each row ended `adoption verified` on main, with both legs pushed and a + first bootstrap that failed.""" + plan = edited(world, old, new) + result = execute(world, plan) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED plan-manifest-invalid" in result.stderr + for fragment in named: + assert fragment in result.stderr, result.stderr + assert "adoption verified" not in result.stdout + assert "Traceback" not in result.stderr + no_leg_was_created(world) + assert git("branch", "--format=%(refname:short)", + cwd=world["source"]).stdout.split() == ["main"] + + +def test_two_bad_values_are_two_findings(world): + text = world["text"].replace("visibility: private", "visibility: secret") + text = text.replace('elected_by: "Test Human"', "elected_by: null") + plan = world["tmp"] / "two.yaml" + with plan.open("w", encoding=UTF8, newline="\n") as handle: + handle.write(text) + result = run_script(ADOPT, "check", "--plan", str(plan)) + assert result.returncode == 1 + found = [line for line in result.stderr.splitlines() + if line.startswith(FINDING)] + assert len(found) == 2, result.stderr + assert "2 finding(s)" in result.stderr + + +def test_the_org_is_one_finding_not_one_for_each_leg(world): + """`org: test+org` makes all three repositories `test+org/`; the + validator says so three times and the sentence holds them under the one + value that is wrong.""" + plan = edited(world, "org: testorg", "org: test+org") + result = run_script(ADOPT, "check", "--plan", str(plan)) + assert result.stderr.count(FINDING) == 1, result.stderr + for leg in ("assembly", "spec", "code"): + assert f"leg '{leg}'" in result.stderr + + +def test_a_plan_the_validator_would_pass_raises_no_refusal(adopter, world): + """THE CONTRACT THAT MUST NOT BREAK: what `check` passes, `execute` + runs. (`_refuse_what_check_finds` reads the very list `check` prints.)""" + plan = adopter.Plan.load(world["plan"]) + source = plan.open_source(str(world["source"]), world["tmp"] / "read") + assert adopter._refuse_what_check_finds(plan, source) is None + + +def test_check_and_execute_read_one_list(adopter, world): + plan_path = edited(world, "visibility: private", "visibility: secret") + plan = adopter.Plan.load(plan_path) + source = plan.open_source(str(world["source"]), world["tmp"] / "read") + findings, remediation = adopter._entry_findings(plan, source) + assert [f for f in findings if f.startswith(FINDING)] + with pytest.raises(adopter.Refusal) as caught: + adopter._refuse_what_check_finds(plan, source) + assert caught.value.code == "plan-manifest-invalid" + for finding in findings: + assert finding in caught.value.detail + assert remediation[0] in caught.value.remediation + + +# --- a repository's name is a directory's ---------------------------------- + +@needs_filter_repo +@pytest.mark.parametrize("length", [247, 250]) +def test_execute_refuses_a_name_whose_bare_repository_cannot_be_made( + tmp_path, length): + """`plan` cannot know `--local-remote-dir`, so a 247 to 250 character + project plans and checks; `execute` died in `Path.exists` with `OSError: + File name too long` and a traceback. It is now refused before a leg.""" + source = make_source_repo(tmp_path / "Thing") + plan = tmp_path / "plan.yaml" + written = write_plan(source, plan, project="N" * length) + assert written.returncode == 0, written.stderr + written.stdout + for path, leg in ANSWERS: + resolve(plan, path, leg) + checked = run_script(ADOPT, "check", "--plan", str(plan)) + assert checked.returncode == 0, checked.stderr + checked.stdout + result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(tmp_path / REMOTES), + "--work-dir", str(tmp_path / WORK)) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED adopt-name-too-long" in result.stderr + assert "Traceback" not in result.stderr + assert not (tmp_path / REMOTES).exists() + + +def test_one_definition_serves_both_tools(adopter): + assert adopter.refuse_unstorable_names is ( + shape_materialize.refuse_unstorable_names) + assert adopter.refuse_manifest_values is ( + shape_materialize.refuse_manifest_values) + assert adopter.manifest_value_problems is ( + shape_materialize.manifest_value_problems) + + +def test_the_flags_and_the_plan_keys_name_the_same_values(adopter): + assert set(adopter.MANIFEST_FLAGS) == set(adopter.MANIFEST_PLAN_KEYS) + assert adopter.MANIFEST_FLAGS["ELECTED_BY"] == "--elected-by" + assert adopter.MANIFEST_PLAN_KEYS["ELECTED_BY"] == "elected_by" + # every value the validator's findings are mapped to is nameable here + mapped = set(shape_materialize.FINDING_PLACEHOLDER.values()) + assert mapped - {"PROJECT_NAME"} <= set(adopter.MANIFEST_FLAGS) + + +def test_a_checkout_that_cannot_name_its_revision_skips_the_question( + adopter, world, monkeypatch): + """`plan` and `check` never needed a revision and are not made to: where + this tool's own checkout cannot give one (`execute` could not run either, + and says so where it reads one), the manifest cannot be rendered and + nothing is asked, rather than `check` dying where it succeeded.""" + def no_revision(*args, **kwargs): + raise adopter.Refusal("not-a-checkout", "no git here") + monkeypatch.setattr(adopter, "git_out", no_revision) + plan = adopter.Plan.load(edited(world, "visibility: private", + "visibility: secret")) + assert adopter._manifest_findings(plan) == ([], []) + + +def test_a_placement_plan_names_no_repository_so_nothing_is_asked(adopter): + """The doctor's placement plan writes `legs:` as a LIST of the mounted + legs it audited and is handed to `Plan` by `shape-doctor.py`; it has no + `legs.spec` to hold to anything, and `.get` on a list is an + `AttributeError`, not an answer.""" + plan = adopter.Plan(Path("placement.yaml"), { + "kind": adopter.PLAN_KIND, "mode": "in-place", + "legs": [{"role": "spec", "path": "spec"}]}) + naming = adopter.NamingPolicy.load(adopter.NAMING_POLICY) + assert adopter._manifest_values(plan, naming) is None + assert adopter._manifest_findings(plan) == ([], []) diff --git a/tests/test_scaffold_manifest_values.py b/tests/test_scaffold_manifest_values.py new file mode 100644 index 00000000..dc77d020 --- /dev/null +++ b/tests/test_scaffold_manifest_values.py @@ -0,0 +1,580 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A scaffold refuses a value the new project's manifest cannot hold (#258, #261). + +`scaffold-project.py` writes `--id` and `--tracking-branch` into +`project.yaml` bare, so `--id null` and `--id true` scaffolded a project whose +first bootstrap failed `manifest-id`, and `--tracking-branch true`, `007` or +`null` pushed every repository on the branch as typed while bootstrap read the +manifest back as `True`, `7` or `main` and put each leg on a local branch the +remotes do not have, with no upstream. It exited 0 and said nothing (#258). + +It also accepted what the project's own `validate-manifest.py` refuses -- a +blank `--elected-by`, `--reference` or `--name`, `--elected-on 2026-W41-6` or +`20261011` (which `date.fromisoformat` reads from Python 3.11), `--org +test+org` -- and exited 0 over a project whose first bootstrap failed (#261). +A `--project` of 247 to 250 characters died in `Path.exists` with `OSError: +File name too long` under `--local-remote-dir`, and one of 251 or more in the +work directory. + +The rule applied elsewhere in this repository (#179, #225, #251) is applied +here: REFUSE what the result cannot hold, BEFORE anything is created, under +`--dry-run` alike, naming the value and why. The questions are asked of the +programs the new project will use -- its YAML reader and the validator the +template ships -- not of a copy of either rule. + +NO NETWORK AND NO GITHUB. Every run uses `--local-remote-dir`, which makes +bare repositories on disk, or `--dry-run`, which makes nothing; the one test +that proves `gh` is never asked puts a fake one on PATH. +""" + +from __future__ import annotations + +import importlib.util +import os +import random +import re +import shutil +import stat +import sys +import textwrap +from pathlib import Path + +import pytest + +from conftest import (FILE_PROTOCOL, ORG, REPO, SCAFFOLD, WINDOWS_SKIP, git, + run_script) + +sys.path.insert(0, str(REPO / "scripts")) +import shape_materialize # noqa: E402 +from repo_shape import load_yaml, parse_yaml # noqa: E402 + +MISREAD = "REFUSED scaffold-manifest-misread" +INVALID = "REFUSED scaffold-manifest-invalid" +TOO_LONG = "REFUSED scaffold-name-too-long" +BAD_DATE = "REFUSED election-date-malformed" + +#: Where a test's scaffold writes, under `tmp_path`: the bare remotes, the +#: tool's work directory, and the temporary directory the tool would +#: otherwise make that work directory in. +REMOTES, WORK, SCRATCH = "remotes", "work", "tmp" + +#: Everything but the flags under test. A flag given again after these wins, +#: because argparse takes the last: `--elected-by=` below replaces the +#: one here. +IDENTITY = ["--org", ORG, "--project", "Atlas", "--elected-by", "Test Human", + "--elected-on", "2026-09-02"] + +#: `(flag, value, what the project's reader makes of it)`. Each is a legal git +#: branch name or a legal id, the reader takes it for something else, and the +#: manifest writes it bare. +MISREAD_VALUES = [ + pytest.param("--id", "null", "no value at all", id="id-null"), + pytest.param("--id", "true", "the boolean true", id="id-true"), + pytest.param("--id", "false", "the boolean false", id="id-false"), + pytest.param("--id", "007", "the number 7", id="id-007"), + pytest.param("--id", "2026", "the number 2026", id="id-2026"), + pytest.param("--tracking-branch", "true", "the boolean true", + id="branch-true"), + pytest.param("--tracking-branch", "True", "the boolean true", + id="branch-True"), + pytest.param("--tracking-branch", "TRUE", "the boolean true", + id="branch-TRUE"), + pytest.param("--tracking-branch", "false", "the boolean false", + id="branch-false"), + pytest.param("--tracking-branch", "False", "the boolean false", + id="branch-False"), + pytest.param("--tracking-branch", "null", "no value at all", + id="branch-null"), + pytest.param("--tracking-branch", "Null", "no value at all", + id="branch-Null"), + pytest.param("--tracking-branch", "NULL", "no value at all", + id="branch-NULL"), + pytest.param("--tracking-branch", "~", "no value at all", id="branch-tilde"), + pytest.param("--tracking-branch", "007", "the number 7", id="branch-007"), + pytest.param("--tracking-branch", "2026", "the number 2026", + id="branch-2026"), + pytest.param("--tracking-branch", "1.5", "the number 1.5", id="branch-1.5"), + pytest.param("--tracking-branch", "1.50", "the number 1.5", + id="branch-1.50"), + pytest.param("--tracking-branch", "0", "the number 0", id="branch-0"), +] + +#: Values the reader takes for text, so the manifest holds what was typed. +#: `yes` and `on` are YAML 1.1 booleans and this reader reads them as text; +#: `0x1F`, `1e3` and `.inf` are numbers elsewhere and text here. They are not +#: refused: the reader is asked, not a list of its words. +TEXT_BRANCHES = ["trunk", "release-1", "feature/x", "v1.2.3", "1.5.2", "yes", + "on", "0x1F", "1e3", ".inf"] +TEXT_IDS = ["atlas-2", "v2", "null-island", "truelove", "yes", "on", "1e3"] + +#: `(flag args, code, what the refusal names)`: values the new project's own +#: validator refuses, which the scaffold used to accept. +INVALID_VALUES = [ + pytest.param(["--elected-by= "], INVALID, ("--elected-by is ' '", + "manifest-elected-by"), + id="elected-by-blank"), + pytest.param(["--elected-by=\t"], INVALID, ("--elected-by is '\\t'", + "manifest-elected-by"), + id="elected-by-tab"), + pytest.param(["--reference= "], INVALID, ("--reference is ' '", + "manifest-reference"), + id="reference-blank"), + pytest.param(["--name= "], INVALID, ("--name is ' '", "manifest-name"), + id="name-blank"), + pytest.param(["--org=test+org"], INVALID, ("--org is 'test+org'", + "manifest-leg-repository"), + id="org-plus"), + pytest.param(["--org=test@org"], INVALID, ("--org is 'test@org'", + "manifest-leg-repository"), + id="org-at"), + pytest.param(["--org=test~org"], INVALID, ("--org is 'test~org'", + "manifest-leg-repository"), + id="org-tilde"), + pytest.param(["--elected-on=2026-W41-6"], BAD_DATE, ("2026-W41-6",), + id="elected-on-iso-week"), + pytest.param(["--elected-on=20261011"], BAD_DATE, ("20261011",), + id="elected-on-basic-format"), + pytest.param(["--elected-on=2026-9-2"], BAD_DATE, ("2026-9-2",), + id="elected-on-unpadded"), +] + +#: Whitespace that `str.strip` takes off and a human cannot see: the +#: validator's `.strip()` decides, so the scaffold must agree on every one of +#: them rather than on a space and a tab. +INVISIBLE = ["\u00a0", "\u2003", "\u3000"] + +#: And the ones that are also LINE BREAKS to `str.splitlines`, so the project's +#: reader splits the manifest at them: the manifest is refused as unreadable +#: before the validator is asked, which is a `misread` here, and becomes +#: `invalid` the day `render` spells such a value as an escape (#250). +LINE_BREAKS = ["\x0b", "\x1c", "\u2028"] + + +def load(name: str, path: Path): + spec = importlib.util.spec_from_file_location(name, path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@pytest.fixture(scope="module") +def scaffolder(): + return load("scaffold_for_manifest_values", SCAFFOLD) + + +@pytest.fixture(scope="module") +def adopter(): + return load("adopt_for_scaffold_manifest_values", REPO / "adopt-project.py") + + +def scaffold(tmp_path: Path, *args: str, work: bool = False, + local: bool = True): + """One scaffold of `Atlas` into local bare remotes under `tmp_path`. + + Without `work` no `--work-dir` is given, so the tool would make its own in + a temporary directory; `TMPDIR` (and Windows' `TEMP` and `TMP`) point at + an empty `tmp_path/tmp` instead, which is how a test sees whether one was + made. + """ + scratch = tmp_path / SCRATCH + scratch.mkdir(exist_ok=True) + flags = [*IDENTITY, *args] + if local: + flags += ["--local-remote-dir", str(tmp_path / REMOTES)] + if work: + flags += ["--work-dir", str(tmp_path / WORK)] + return run_script(SCAFFOLD, *flags, env={ + "TMPDIR": str(scratch), "TMP": str(scratch), "TEMP": str(scratch)}) + + +def assert_nothing_was_created(tmp_path: Path) -> None: + """No bare remote, no work directory of the tool's own and none of ours.""" + found = sorted(p.name for p in tmp_path.iterdir()) + assert found == [SCRATCH], f"a refused scaffold created something: {found}" + assert list((tmp_path / SCRATCH).iterdir()) == [], ( + "a refused scaffold made a temporary work directory") + + +def assert_refused(result, code: str) -> None: + assert result.returncode == 2, result.stderr + result.stdout + assert code in result.stderr, result.stderr + assert result.stdout == "", "a plan was printed for a refused scaffold" + + +# --- #258: a value the project's reader turns into something else --------- + +@pytest.mark.parametrize("flag, value, reading", MISREAD_VALUES) +def test_a_value_the_reader_misreads_is_refused_before_anything_is_made( + tmp_path, flag, value, reading): + # `=` form, so a value that begins with `~` or `-` reaches the tool as a + # value and argparse does not take it for another option. + result = scaffold(tmp_path, f"{flag}={value}") + assert_refused(result, MISREAD) + assert f"{flag} is {value!r}" in result.stderr + assert f"reads it back as {reading} and not as that text" in result.stderr + assert "Remediation: choose another value" in result.stderr + assert_nothing_was_created(tmp_path) + + +@pytest.mark.parametrize("flag, value", [("--id", "null"), + ("--tracking-branch", "007")]) +def test_a_dry_run_refuses_a_misread_value_as_the_real_run_would( + tmp_path, flag, value): + """`--dry-run` is a rehearsal: it must not print a plan for a scaffold the + real run refuses (the issue's own `--dry-run` accepted every one).""" + result = scaffold(tmp_path, "--dry-run", f"{flag}={value}") + assert_refused(result, MISREAD) + assert_nothing_was_created(tmp_path) + + +@pytest.mark.parametrize("project, derived, reading", [ + ("True", "true", "the boolean true"), + ("Null", "null", "no value at all"), +]) +def test_an_id_derived_from_the_project_name_is_asked_too( + tmp_path, project, derived, reading): + """With no `--id` the id is the project name lowercased, so `--project + Null` is `id: null`; the sentence says where the id came from, so it does + not point at a flag nobody typed.""" + result = scaffold(tmp_path, "--project", project) + assert_refused(result, MISREAD) + assert (f"--id (not given, so --project lowercased) is {derived!r}" + in result.stderr) + assert reading in result.stderr + assert_nothing_was_created(tmp_path) + + +def test_an_explicit_id_is_the_way_round_for_such_a_project_name(tmp_path): + result = scaffold(tmp_path, "--project", "Null", "--id", "null-project", + "--dry-run") + assert result.returncode == 0, result.stderr + result.stdout + assert "project Null (null-project)" in result.stdout + + +@pytest.mark.parametrize("branch", TEXT_BRANCHES) +def test_a_branch_the_reader_takes_for_text_is_still_accepted(tmp_path, + branch): + result = scaffold(tmp_path, "--dry-run", f"--tracking-branch={branch}") + assert result.returncode == 0, result.stderr + result.stdout + + +@pytest.mark.parametrize("project_id", TEXT_IDS) +def test_an_id_the_reader_takes_for_text_is_still_accepted(tmp_path, + project_id): + result = scaffold(tmp_path, "--dry-run", f"--id={project_id}") + assert result.returncode == 0, result.stderr + result.stdout + assert f"({project_id})" in result.stdout + + +@pytest.mark.parametrize("branch, project_id", [ + ("release-1", "null-island"), + ("v1.2.3", "atlas-2"), +]) +def test_what_is_typed_is_what_the_project_holds_and_bootstrap_agrees( + tmp_path, branch, project_id): + """The whole round trip, with the project's own tools: the manifest holds + the branch and the id as typed, its validator passes, and bootstrap puts + each leg on that branch with an upstream. On main `--tracking-branch + true` was the opposite of every one of these.""" + result = scaffold(tmp_path, f"--tracking-branch={branch}", + f"--id={project_id}", work=True) + assert result.returncode == 0, result.stderr + result.stdout + clone = tmp_path / "Atlas" + cloned = git(*FILE_PROTOCOL, "clone", "-q", "--recurse-submodules", + str(tmp_path / REMOTES / "Atlas.git"), str(clone), + cwd=tmp_path, check=False) + assert cloned.returncode == 0, cloned.stderr + manifest = load_yaml(clone / "project.yaml") + assert manifest["id"] == project_id + assert manifest["tracking_branch"] == branch + assert manifest["topic"] == f"xf-project-{project_id}" + booted = run_script(clone / "scripts" / "bootstrap.py", cwd=clone) + assert booted.returncode == 0, booted.stderr + booted.stdout + for leg in ("spec", "code"): + on = git("rev-parse", "--abbrev-ref", "HEAD", cwd=clone / leg) + assert on.stdout.strip() == branch + upstream = git("rev-parse", "--abbrev-ref", "@{u}", cwd=clone / leg, + check=False) + assert upstream.stdout.strip() == f"origin/{branch}", upstream.stderr + for name in ("Atlas", "Atlas-spec", "Atlas-code"): + heads = git("for-each-ref", "--format=%(refname:short)", "refs/heads", + cwd=tmp_path / REMOTES / f"{name}.git").stdout.split() + assert heads == [branch] + + +# --- #261: a value the project's own validator refuses -------------------- + +@pytest.mark.parametrize("args, code, named", INVALID_VALUES) +def test_a_value_the_validator_refuses_is_refused_before_anything_is_made( + tmp_path, args, code, named): + result = scaffold(tmp_path, *args) + assert_refused(result, code) + for fragment in named: + assert fragment in result.stderr + assert_nothing_was_created(tmp_path) + + +@pytest.mark.parametrize("args, code, named", INVALID_VALUES[:3]) +def test_a_dry_run_refuses_what_the_validator_refuses(tmp_path, args, code, + named): + result = scaffold(tmp_path, "--dry-run", *args) + assert_refused(result, code) + assert_nothing_was_created(tmp_path) + + +def test_the_refusal_says_it_in_the_validators_own_words(tmp_path): + """Not a sentence that happens to agree: the words after the value are the + validator's own finding, read from the validator the template ships.""" + validator = shape_materialize.manifest_validator() + expected = { + "elected_by": validator._elected_by_finding({"elected_by": " "}), + "reference": validator._reference_finding({"reference": " "}), + "name": validator._name_finding({"name": " "}), + } + for key, flag in (("elected_by", "--elected-by"), + ("reference", "--reference"), ("name", "--name")): + (finding,) = expected[key] + detail = finding.split(": ", 1)[1] + result = scaffold(tmp_path, "--dry-run", f"{flag}= ") + assert detail in result.stderr, (detail, result.stderr) + + +@pytest.mark.parametrize("blank", INVISIBLE) +def test_whitespace_a_person_cannot_see_is_judged_as_the_validator_judges_it( + tmp_path, blank): + """`"\\u3000".strip()` is empty, so the validator refuses an `elected_by` + of it. The scaffold agrees on each, because the validator is asked and no + second definition of blank is kept.""" + result = scaffold(tmp_path, "--dry-run", f"--elected-by={blank}") + assert_refused(result, INVALID) + assert "manifest-elected-by" in result.stderr + + +@pytest.mark.parametrize("blank", LINE_BREAKS) +def test_a_blank_that_is_also_a_line_break_is_refused_too(tmp_path, blank): + """It cannot be written into the manifest as it stands: the reader splits + the file at it. Refused either way, before anything is made, and never + scaffolded into a project whose every reader refuses `project.yaml`.""" + result = scaffold(tmp_path, "--dry-run", f"--elected-by={blank}") + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED scaffold-manifest-" in result.stderr + assert "--elected-by is " in result.stderr + assert result.stdout == "" + assert_nothing_was_created(tmp_path) + + +def test_an_organisation_the_validator_accepts_is_still_accepted(tmp_path): + """`_` and `.` are in the validator's `QUALIFIED_RE`: the scaffold does + not refuse more than the project does.""" + result = scaffold(tmp_path, "--dry-run", "--org=my_org.x") + assert result.returncode == 0, result.stderr + result.stdout + + +def test_an_elector_and_a_reference_with_text_in_them_still_scaffold(tmp_path): + result = scaffold(tmp_path, "--dry-run", "--elected-by=Robert Smith", + "--reference=docs/project-repo-schema.md") + assert result.returncode == 0, result.stderr + result.stdout + + +# --- the election date: the validator's form, on every Python ------------- + +def test_the_election_date_is_read_in_the_validators_form_only(): + """`date.fromisoformat` reads an ISO week date and the basic format from + Python 3.11 and the validator refuses both; a date is read only when the + validator's own `DATE_RE` admits it AND it is a real day.""" + for text in ("2026-W41-6", "2026W416", "2026-W41", "20261011", + "2026-9-2", "2026-13-45", "2026-09-02\n", " 2026-09-02", + "\u0662\u0660\u0662\u0666-\u0660\u0669-\u0660\u0662", ""): + with pytest.raises(shape_materialize.Refusal) as caught: + shape_materialize.election_date(text) + assert caught.value.code == "election-date-malformed", repr(text) + assert str(shape_materialize.election_date("2026-09-02")) == "2026-09-02" + + +def test_the_election_date_never_accepts_what_the_validator_refuses(): + """Seeded: any string over the alphabet of dates and their ISO variants. + Whatever `election_date` reads, `DATE_RE` admits.""" + date_re = shape_materialize.manifest_validator().DATE_RE + rng = random.Random(41) + alphabet = "0123456789-WT:/ ." + for _ in range(4000): + text = "".join(rng.choice(alphabet) + for _ in range(rng.randint(6, 11))) + try: + shape_materialize.election_date(text) + except shape_materialize.Refusal: + continue + assert date_re.match(text), repr(text) + # Dates are rare in noise, so the ones that must be read are planted. + for text in ("2026-09-02", "2026-01-31", "1999-12-31", "2024-02-29"): + assert date_re.match(text) + shape_materialize.election_date(text) + + +def test_the_election_date_asks_the_validator_not_a_copy_of_its_rule( + monkeypatch): + """Make the validator's rule stricter and the answer follows: nothing in + `election_date` holds a second `YYYY-MM-DD` pattern to keep in step.""" + validator = shape_materialize.manifest_validator() + monkeypatch.setattr(validator, "DATE_RE", + re.compile(r"^2027-\d\d-\d\d$")) + with pytest.raises(shape_materialize.Refusal): + shape_materialize.election_date("2026-09-02") + assert str(shape_materialize.election_date("2027-09-02")) == "2027-09-02" + + +# --- a repository's name is a directory's --------------------------------- + +@pytest.mark.parametrize("length", [247, 248, 250]) +def test_a_project_whose_bare_repository_name_is_too_long_is_refused( + tmp_path, length): + """`-spec.git` is `length + 9` bytes: 256 and more fails on every + disk. On main this was `OSError: File name too long` and a traceback.""" + result = scaffold(tmp_path, "--project", "N" * length) + assert_refused(result, TOO_LONG) + assert "Traceback" not in result.stderr + assert ".git is" in result.stderr + assert "--local-remote-dir" in result.stderr + assert_nothing_was_created(tmp_path) + + +@pytest.mark.parametrize("length", [251, 255]) +def test_a_project_whose_work_directory_name_is_too_long_is_refused_too( + tmp_path, length): + """`-spec` alone is over 255 bytes from 251 characters, so the + work directory cannot hold it, with or without local remotes.""" + for local in (True, False): + result = scaffold(tmp_path, "--project", "N" * length, local=local, + work=True) + assert_refused(result, TOO_LONG) + assert "Traceback" not in result.stderr + assert "work directory" in result.stderr + assert not (tmp_path / WORK).exists() + + +def test_the_longest_name_a_disk_keeps_is_still_accepted(tmp_path): + """246 characters is `-spec.git` at exactly 255 bytes. A dry run + only: a real tree this deep is a Windows path the OS will not open.""" + result = scaffold(tmp_path, "--project", "N" * 246, "--dry-run") + assert result.returncode == 0, result.stderr + result.stdout + without_remotes = scaffold(tmp_path, "--project", "N" * 250, "--dry-run", + local=False) + assert without_remotes.returncode == 0, without_remotes.stderr + + +def test_the_name_rule_is_the_same_bytes_the_tools_make(): + refuse = shape_materialize.refuse_unstorable_names + assert refuse("r", ["a" * 255], []) is None + assert refuse("r", ["a" * 251], ["a" * 251]) is None + with pytest.raises(shape_materialize.Refusal): + refuse("r", ["a" * 256], []) + with pytest.raises(shape_materialize.Refusal) as caught: + refuse("r", ["a" * 252], ["a" * 252]) + assert caught.value.code == "r" + assert "256 bytes" in caught.value.detail + + +# --- never GitHub ---------------------------------------------------------- + +@WINDOWS_SKIP # the `gh` below is a `#!` script, which Windows does not run +@pytest.mark.parametrize("flag", ["--id=null", "--tracking-branch=true", + "--elected-by= ", "--org=test+org"]) +def test_a_refused_value_never_reaches_gh(tmp_path, flag): + """Local remotes are the test path; the real one calls `gh repo create`. + Each refusal comes before the first `gh` call, so no repository is made + on the forge for a project that was never going to scaffold.""" + bin_dir, log = tmp_path / "bin", tmp_path / "gh.log" + bin_dir.mkdir() + shim = bin_dir / "gh" + shim.write_text(textwrap.dedent("""\ + #!/bin/sh + echo "$@" >> "$FAKE_GH_LOG" + echo "fake gh: no network here" >&2 + exit 1 + """), encoding="utf-8") + shim.chmod(shim.stat().st_mode | stat.S_IXUSR) + result = run_script( + SCAFFOLD, *IDENTITY, flag, "--work-dir", str(tmp_path / WORK), + env={"PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + "FAKE_GH_LOG": str(log)}) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED" in result.stderr + assert not log.exists(), f"gh was asked: {log.read_text()}" + assert not (tmp_path / WORK).exists() + + +# --- one definition, two readers ------------------------------------------ + +def test_both_tools_ask_the_one_definition(scaffolder, adopter): + for name in ("refuse_manifest_values", "refuse_unstorable_names"): + defined = getattr(shape_materialize, name) + assert defined.__module__ == "shape_materialize" + assert getattr(scaffolder, name) is defined + assert adopter.refuse_unstorable_names is ( + shape_materialize.refuse_unstorable_names) + assert adopter.manifest_value_problems is ( + shape_materialize.manifest_value_problems) + + +def test_the_validator_asked_is_the_one_the_template_ships(): + module = shape_materialize.manifest_validator() + assert Path(module.__file__).resolve() == ( + REPO / "templates" / "assembly-root" / "scripts" + / "validate-manifest.py") + assert shape_materialize.manifest_validator() is module, ( + "it is loaded once") + + +def test_loading_the_validator_leaves_sys_path_as_it_found_it(tmp_path): + """The validator puts its own directory first on `sys.path` for the + sibling modules it imports (`bootstrap`, `validate-pins` live there too), + and that must not outlive the load. A copy of the template is a path not + loaded yet, so this loads for real.""" + copy = tmp_path / "assembly-root" + shutil.copytree(REPO / "templates" / "assembly-root", copy) + before = list(sys.path) + module = shape_materialize.manifest_validator(copy) + assert Path(module.__file__).resolve().parent.parent == ( + copy.resolve()), "the copy was not what was loaded" + assert sys.path == before + assert not list(copy.rglob("__pycache__")), ( + "loading the validator wrote bytecode into the template tree") + + +def test_every_name_a_refusal_uses_is_a_value_the_tools_hold(scaffolder, + adopter): + """The placeholders each tool names, and the ones the validator's findings + are mapped to, are all keys of the substitution table: a key that no + longer existed would be a value asked about nothing.""" + template = "\n".join( + path.read_text(encoding="utf-8") + for path in (REPO / "templates" / "assembly-root").rglob("*") + if path.is_file() and path.suffix in (".yaml", ".md")) + known = set(re.findall(r"\{\{([A-Z_]+)\}\}", template)) + known.add("ORG") # a key of both tools' tables, in no template + for table in (scaffolder.MANIFEST_FLAGS, adopter.MANIFEST_FLAGS, + adopter.MANIFEST_PLAN_KEYS): + assert set(table) <= known, set(table) - known + assert set(shape_materialize.FINDING_PLACEHOLDER.values()) <= known + assert set(adopter.MANIFEST_FLAGS) == set(adopter.MANIFEST_PLAN_KEYS) + + +def test_quoting_the_template_would_make_the_refusal_go_away(tmp_path): + """#259 proposes quoting `id:` and `tracking_branch:` in the template. The + question is asked of the TEMPLATE, so the day it does, those values read + back and nothing here refuses them, with no edit to this rule.""" + copy = tmp_path / "assembly-root" + shutil.copytree(REPO / "templates" / "assembly-root", copy) + manifest = copy / "project.yaml" + text = manifest.read_text(encoding="utf-8") + quoted = (text.replace("id: {{PROJECT_ID}}", 'id: "{{PROJECT_ID}}"') + .replace("tracking_branch: {{TRACKING_BRANCH}}", + 'tracking_branch: "{{TRACKING_BRANCH}}"')) + assert quoted != text + manifest.write_text(quoted, encoding="utf-8") + values = {"PROJECT_ID": "null", "TRACKING_BRANCH": "007"} + what = {"PROJECT_ID": "--id", "TRACKING_BRANCH": "--tracking-branch"} + assert len(shape_materialize.manifest_misreads(values, what)) == 2 + assert shape_materialize.manifest_misreads(values, what, copy) == [] + # and what the quoted line holds is the text, through the project's reader + assert parse_yaml('tracking_branch: "007"\n') == {"tracking_branch": "007"} From 3e32a572db2bc5b9cc610d4b6e7fe93091e34edb Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sun, 11 Oct 2026 23:50:55 +0000 Subject: [PATCH 02/11] tests/test_adopt_manifest_values.py: a plan whose names the naming policy does not classify is `check`'s naming finding and no crash, and no second finding about a manifest nobody could write (#261) The question asked of a plan's values renders the manifest from them, which needs names that classify. `_manifest_values` returns None for those, and `check` reports them in its own words; this holds that. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_manifest_values.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index eeaca585..fc3e6cad 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -390,3 +390,17 @@ def test_a_placement_plan_names_no_repository_so_nothing_is_asked(adopter): naming = adopter.NamingPolicy.load(adopter.NAMING_POLICY) assert adopter._manifest_values(plan, naming) is None assert adopter._manifest_findings(plan) == ([], []) + + +def test_a_plan_whose_names_do_not_classify_is_the_naming_findings_not_a_crash( + world): + """The manifest cannot be rendered from names the naming policy does not + classify, and `check` reports those in its own words. The question asked + of the values must not turn that finding into a traceback, or into a + second finding about a manifest nobody could write.""" + plan = edited(world, " assembly: Northwind\n", " assembly: north_wind\n") + result = run_script(ADOPT, "check", "--plan", str(plan)) + assert result.returncode == 1, result.stderr + result.stdout + assert "FINDING naming-unclassified" in result.stderr + assert FINDING not in result.stderr + assert "Traceback" not in result.stderr From 9bfe2e54b404f42c3fab59bd263e6bf04704204e Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:03:15 +0000 Subject: [PATCH 03/11] adopt-project.py: `check` does not say a leg's mount path twice, once as `plan-bad-leg-path` and again as the validator's `manifest-leg-path`, so a plan with `spec_path: ../spec` is one finding as it was (#261) The question asked of a plan's values ran the validator over the whole manifest, which also refuses a `path:` that is absolute, has a `..`, is `.` or is empty. `_leg_findings` already reports each of those as `plan-bad-leg-path` in the same list, so `tests/test_adopt_leg_paths.py` (twelve rows of `test_check_reports_a_non_canonical_path_as_a_finding`) saw two findings for one value. `manifest_invalid` takes the codes a caller reports by a rule of its own and `check` and `execute` pass the two leg-path codes. The scaffold and `plan` judge a leg path before the manifest is rendered and ignore nothing, so a rule they did not apply would still refuse. The two tests that copy the template tree now leave out `__pycache__`, which a stale one in the source tree would otherwise put in the copy. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 7 ++++++- scripts/shape_materialize.py | 25 ++++++++++++++++++++----- tests/test_adopt_manifest_values.py | 14 ++++++++++++++ tests/test_scaffold_manifest_values.py | 6 ++++-- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 5f7c535f..561c29a3 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1471,6 +1471,11 @@ def _checked_plan_inputs(args, source: Source, "ORG": "--org", } +#: The validator's findings about a leg's mount path, which `check` already +#: reports as `plan-bad-leg-path` and `plan-leg-path-collides` from the same +#: list (`_leg_findings`): said once, in the words of the rule that judges it. +LEG_PATH_CODES = ("manifest-leg-path", "manifest-leg-path-collision") + #: And how `check` and `execute` name the same value: its key in the plan. MANIFEST_PLAN_KEYS = { "PROJECT_ID": "id", "ELECTED_BY": "elected_by", @@ -1550,7 +1555,7 @@ def _manifest_findings(plan: Plan) -> tuple[list[str], list[str]]: if values is None: return [], [] problems = manifest_value_problems(values, naming, MANIFEST_PLAN_KEYS, - misreads=False) + misreads=False, ignore=LEG_PATH_CODES) if not problems: return [], [] return ([f"FINDING plan-manifest-invalid: {sentence}" diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index b55b4bd5..b66a7f72 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -1940,6 +1940,13 @@ def manifest_misreads(values: dict, what: dict, re.DOTALL) +def _code_of(finding: str) -> str | None: + """The code of one `FINDING : `, or None for a line that is + not one.""" + match = _FINDING_RE.match(finding) + return match.group("code") if match else None + + def _sentences_of(findings: list[str], values: dict, what: dict) -> list[str]: """The FINDINGs of the validator as sentences: one for each value the @@ -1968,10 +1975,17 @@ def _sentences_of(findings: list[str], values: dict, def manifest_invalid(values: dict, policy: NamingPolicy, what: dict, - template_dir: Path = ASSEMBLY_TEMPLATE) -> list[str]: + template_dir: Path = ASSEMBLY_TEMPLATE, + ignore: tuple = ()) -> list[str]: """A sentence for each FINDING the project's own validator would make of the `project.yaml` these `values` render; `[]` when it would pass. + `ignore` is the codes of findings the CALLER already reports by a rule of + its own in the same list (`adopt-project.py check` finds a leg path + `plan-bad-leg-path` and must not say it a second time as the validator's + `manifest-leg-path`); a caller that judges nothing earlier ignores none, + so that a rule it did not apply still refuses. + The WHOLE manifest is rendered and read, so a rule about how one value relates to another (`topic` to `id`, a leg's repository to its role) is asked as the project asks it. `values` must be the complete table; @@ -1987,8 +2001,9 @@ def manifest_invalid(values: dict, policy: NamingPolicy, what: dict, return ["the manifest this run would write is a file the project's " f"own YAML reader refuses: {exc}."] notes = manifest_validator(template_dir)._findings(manifest, policy, None) - return _sentences_of([note for note in notes - if not note.startswith("WARNING")], values, what) + return _sentences_of( + [note for note in notes if not note.startswith("WARNING") + and _code_of(note) not in ignore], values, what) #: The remediation, without the sentence only the caller can write (what was @@ -2008,7 +2023,7 @@ def manifest_invalid(values: dict, policy: NamingPolicy, what: dict, def manifest_value_problems(values: dict, policy: NamingPolicy, what: dict, - *, misreads: bool = True, + *, misreads: bool = True, ignore: tuple = (), template_dir: Path = ASSEMBLY_TEMPLATE ) -> list[tuple[str, str]]: """`(kind, sentence)` for everything wrong with the values, `kind` being @@ -2025,7 +2040,7 @@ def manifest_value_problems(values: dict, policy: NamingPolicy, what: dict, if misread: return [("misread", sentence) for sentence in misread] return [("invalid", sentence) for sentence - in manifest_invalid(values, policy, what, template_dir)] + in manifest_invalid(values, policy, what, template_dir, ignore)] def refuse_manifest_values(prefix: str, values: dict, policy: NamingPolicy, diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index fc3e6cad..018fee06 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -404,3 +404,17 @@ def test_a_plan_whose_names_do_not_classify_is_the_naming_findings_not_a_crash( assert "FINDING naming-unclassified" in result.stderr assert FINDING not in result.stderr assert "Traceback" not in result.stderr + + +@pytest.mark.parametrize("value", ["../spec", "/spec", ".", '""', "spec/"]) +def test_a_bad_leg_path_is_one_finding_in_the_words_of_its_own_rule(world, + value): + """The validator also refuses such a `path:` (`manifest-leg-path`), and the + plan's `legs.spec_path` is already `plan-bad-leg-path` in the same list: + said twice would be two findings for one value.""" + plan = edited(world, " spec_path: spec\n", f" spec_path: {value}\n") + result = run_script(ADOPT, "check", "--plan", str(plan)) + assert result.returncode == 1, result.stderr + result.stdout + assert "FINDING plan-bad-leg-path" in result.stderr + assert FINDING not in result.stderr + assert "1 finding(s)" in result.stderr diff --git a/tests/test_scaffold_manifest_values.py b/tests/test_scaffold_manifest_values.py index dc77d020..1f17e19b 100644 --- a/tests/test_scaffold_manifest_values.py +++ b/tests/test_scaffold_manifest_values.py @@ -531,7 +531,8 @@ def test_loading_the_validator_leaves_sys_path_as_it_found_it(tmp_path): and that must not outlive the load. A copy of the template is a path not loaded yet, so this loads for real.""" copy = tmp_path / "assembly-root" - shutil.copytree(REPO / "templates" / "assembly-root", copy) + shutil.copytree(REPO / "templates" / "assembly-root", copy, + ignore=shutil.ignore_patterns("__pycache__", "*.pyc")) before = list(sys.path) module = shape_materialize.manifest_validator(copy) assert Path(module.__file__).resolve().parent.parent == ( @@ -564,7 +565,8 @@ def test_quoting_the_template_would_make_the_refusal_go_away(tmp_path): question is asked of the TEMPLATE, so the day it does, those values read back and nothing here refuses them, with no edit to this rule.""" copy = tmp_path / "assembly-root" - shutil.copytree(REPO / "templates" / "assembly-root", copy) + shutil.copytree(REPO / "templates" / "assembly-root", copy, + ignore=shutil.ignore_patterns("__pycache__", "*.pyc")) manifest = copy / "project.yaml" text = manifest.read_text(encoding="utf-8") quoted = (text.replace("id: {{PROJECT_ID}}", 'id: "{{PROJECT_ID}}"') From ccc0960b55950e1e255550c42a913ee646467f2c Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:09:16 +0000 Subject: [PATCH 04/11] adopt-project.py: `check` and `execute` ask the validator about the plan's values that are text, or no value, and leave a value the reader read as a number, a boolean or a list to the misread rule, so a plan's `id: 007` is not echoed as `'7'` (#261) `execute` writes `str` of what the plan's reader read, so `id: 007` (the number 7) or `elected_on: 20260902` reached the validator as text the plan never said, and the finding echoed it (`id is '7'`). A value read as another kind is a misread, which `adopt-project.py` on #181 judges in words that name what was read and never echo the retyped text, and a merge of the two put both findings on one value and failed that branch's own test of the echo. The values asked about are the ones the plan holds as text, and as no value at all (`elected_by: null` is written blank, which is the finding wanted). The plan edits that make a bad `elected_on` are quoted in the tests, so they are text on every tree. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 11 ++++++++++- scripts/shape_materialize.py | 25 +++++++++++++++++++------ tests/test_adopt_manifest_values.py | 22 ++++++++++++++++++++-- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 561c29a3..38cb82d6 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1554,8 +1554,17 @@ def _manifest_findings(plan: Plan) -> tuple[list[str], list[str]]: values = _manifest_values(plan, naming) if values is None: return [], [] + # A value the plan's reader read as a number, a boolean or a collection is + # a MISREAD (`id: 007` is 7), and what the template would then write is + # `str` of what was read, which the plan never said: it is not asked about + # here, in words that would echo that text. + not_text = tuple( + placeholder for placeholder, key in MANIFEST_PLAN_KEYS.items() + if plan.get(key) is not None + and not isinstance(plan.get(key), str)) problems = manifest_value_problems(values, naming, MANIFEST_PLAN_KEYS, - misreads=False, ignore=LEG_PATH_CODES) + misreads=False, ignore=LEG_PATH_CODES, + not_text=not_text) if not problems: return [], [] return ([f"FINDING plan-manifest-invalid: {sentence}" diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index b66a7f72..465ad387 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -1947,12 +1947,13 @@ def _code_of(finding: str) -> str | None: return match.group("code") if match else None -def _sentences_of(findings: list[str], values: dict, - what: dict) -> list[str]: +def _sentences_of(findings: list[str], values: dict, what: dict, + not_text: tuple = ()) -> list[str]: """The FINDINGs of the validator as sentences: one for each value the caller handed over, naming it and holding every finding about it (`--org` is judged once for each of the three legs it names), and one for each - finding that is about none of them.""" + finding that is about none of them. A finding about a placeholder in + `not_text` is left out, as `manifest_invalid` says.""" by_value: dict[str, list[str]] = {} sentences: list[str] = [] for finding in findings: @@ -1963,6 +1964,8 @@ def _sentences_of(findings: list[str], values: dict, code, detail = match.group("code"), match.group("detail") said = f"{code}: {detail if detail.endswith('.') else detail + '.'}" key = FINDING_PLACEHOLDER.get(code) + if key is not None and key in not_text: + continue if key in what and key in values: by_value.setdefault(key, []).append(said) else: @@ -1976,7 +1979,7 @@ def _sentences_of(findings: list[str], values: dict, def manifest_invalid(values: dict, policy: NamingPolicy, what: dict, template_dir: Path = ASSEMBLY_TEMPLATE, - ignore: tuple = ()) -> list[str]: + ignore: tuple = (), not_text: tuple = ()) -> list[str]: """A sentence for each FINDING the project's own validator would make of the `project.yaml` these `values` render; `[]` when it would pass. @@ -1986,6 +1989,14 @@ def manifest_invalid(values: dict, policy: NamingPolicy, what: dict, `manifest-leg-path`); a caller that judges nothing earlier ignores none, so that a rule it did not apply still refuses. + `not_text` is the placeholders whose value the caller READ as something + that is not text (a plan's `id: 007` is the number 7, and `str` of it is + what the template would write): that is a misread, which is judged in + words that say what was read and never in the retyped text, so the + validator's finding about such a value is left to that rule. A value the + plan holds as no value at all is not one of these: `execute` writes it as + blank, and the validator's finding about a blank is the one wanted. + The WHOLE manifest is rendered and read, so a rule about how one value relates to another (`topic` to `id`, a leg's repository to its role) is asked as the project asks it. `values` must be the complete table; @@ -2003,7 +2014,7 @@ def manifest_invalid(values: dict, policy: NamingPolicy, what: dict, notes = manifest_validator(template_dir)._findings(manifest, policy, None) return _sentences_of( [note for note in notes if not note.startswith("WARNING") - and _code_of(note) not in ignore], values, what) + and _code_of(note) not in ignore], values, what, not_text) #: The remediation, without the sentence only the caller can write (what was @@ -2024,6 +2035,7 @@ def manifest_invalid(values: dict, policy: NamingPolicy, what: dict, def manifest_value_problems(values: dict, policy: NamingPolicy, what: dict, *, misreads: bool = True, ignore: tuple = (), + not_text: tuple = (), template_dir: Path = ASSEMBLY_TEMPLATE ) -> list[tuple[str, str]]: """`(kind, sentence)` for everything wrong with the values, `kind` being @@ -2040,7 +2052,8 @@ def manifest_value_problems(values: dict, policy: NamingPolicy, what: dict, if misread: return [("misread", sentence) for sentence in misread] return [("invalid", sentence) for sentence - in manifest_invalid(values, policy, what, template_dir, ignore)] + in manifest_invalid(values, policy, what, template_dir, ignore, + not_text)] def refuse_manifest_values(prefix: str, values: dict, policy: NamingPolicy, diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index 018fee06..c2885c63 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -103,10 +103,10 @@ pytest.param('reference: "openxFactory docs/project-repo-schema.md"', 'reference: " "', ("reference is ' '", "manifest-reference"), id="reference-blank"), - pytest.param('elected_on: "2026-09-02"', "elected_on: 20261011", + pytest.param('elected_on: "2026-09-02"', 'elected_on: "20261011"', ("elected_on is '20261011'", "manifest-elected-on"), id="elected-on-basic-format"), - pytest.param('elected_on: "2026-09-02"', "elected_on: 2026-W41-6", + pytest.param('elected_on: "2026-09-02"', 'elected_on: "2026-W41-6"', ("elected_on is '2026-W41-6'", "manifest-elected-on"), id="elected-on-iso-week"), pytest.param("org: testorg", "org: test+org", @@ -418,3 +418,21 @@ def test_a_bad_leg_path_is_one_finding_in_the_words_of_its_own_rule(world, assert "FINDING plan-bad-leg-path" in result.stderr assert FINDING not in result.stderr assert "1 finding(s)" in result.stderr + + +@pytest.mark.parametrize("old, new", [ + ("visibility: private", "visibility: [secret]"), + ('elected_on: "2026-09-02"', "elected_on: 20260902"), + ("id: northwind", "id: 007"), +]) +def test_a_value_the_reader_reads_as_another_kind_is_not_this_questions( + world, old, new): + """`id: 007` is the number 7, `elected_on: 20260902` another number and + `visibility: [secret]` a list: what the template would write is `str` of + what was read, which the plan never said, so a finding about it would + echo text nobody typed. That is a misread and is judged as one; the + validator is asked about the values the plan holds as text, and as no + value at all.""" + plan = edited(world, old, new) + result = run_script(ADOPT, "check", "--plan", str(plan)) + assert FINDING not in result.stderr, result.stderr From ca21b57b4ced6bce9f4ba0ff6b2490f293c2b82c Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:17:27 +0000 Subject: [PATCH 05/11] tests/test_adopt_manifest_values.py: `execute`'s refusal is asked for the validator's finding and not for the code of the first finding in the list, so a value a second rule also judges does not move it (#261) `execute` refuses under the code of the first finding `check` prints (#168). A plan's `elected_on` of `"20261011"` is also a misread on #181, which lists its finding first, so the refusal wore that code on a merge of the two. What the test is about is that this finding is in the refusal. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_manifest_values.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index c2885c63..249d8851 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -264,7 +264,12 @@ def test_execute_refuses_it_before_any_leg_exists(world, old, new, named): plan = edited(world, old, new) result = execute(world, plan) assert result.returncode == 2, result.stderr + result.stdout - assert "REFUSED plan-manifest-invalid" in result.stderr + # The refusal wears the code of the FIRST finding in the list (#168). A + # value another rule also judges (a number-looking `elected_on` is a + # misread on #181) may be listed before this one, so what is asked is + # that this finding is in it. + assert "REFUSED plan-manifest-" in result.stderr + assert f"{FINDING}: " in result.stderr for fragment in named: assert fragment in result.stderr, result.stderr assert "adoption verified" not in result.stdout From 526d8d7f7387703d568f814de9d6243d12cdaab5 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:06:27 +0000 Subject: [PATCH 06/11] scripts/shape_materialize.py: the remediation of `scaffold-manifest-misread` says what holds of every value it refuses, that the project's reader reads back something other than what was typed, and no longer says `--tracking-branch 2026` or `1.5` fails the first bootstrap or lands the legs on a branch the remotes lack (#258) The review of ca21b57 found the text false for exactly the values this refusal newly takes from people. On main both scaffold, bootstrap with exit 0, and get the upstream origin/2026 or origin/1.5, because every reader of `tracking_branch` does `str()` and `str(2026)` is `2026` again. The refusal stays, as #181 and #266 refuse the same class and quoting the templates is #259's to decide; its reason is now the true one. It names the failures that do hold (an id that is not text fails validate-manifest.py, `007` as a branch reads back as `7`), says that a number which survives `str()` works only because of the tools that read it and is still a number to the project's own reader, and gives the way forward: a value the reader takes for text. Tests: the wording is asserted for `--tracking-branch 2026`, `1.5` and `007` and `--id 2026` and `null` (five rows that fail on the old text), and the claim about `str()` is asked of the project's own reader. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/shape_materialize.py | 25 +++++++++++--- tests/test_scaffold_manifest_values.py | 46 ++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index e6de20e0..46fee5fe 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -2154,14 +2154,29 @@ def manifest_invalid(values: dict, policy: NamingPolicy, what: dict, #: The remediation, without the sentence only the caller can write (what was #: or was not created, whether `--dry-run` refuses the same). +#: +#: WHAT IT SAYS IS ONLY WHAT HOLDS FOR EVERY VALUE THE RULE REFUSES: the +#: project's reader reads back something other than what was typed. What that +#: does next differs by value and by reader, and it is NOT always a failure. +#: `--tracking-branch 2026` and `1.5` scaffold, bootstrap and get their +#: upstream today, because every reader of `tracking_branch` does `str()` and +#: `str(2026)` is `2026` again (while `007` reads back as `7`, `1.50` as +#: `1.5` and `0` as `main`). So the text names only the failures that do hold +#: (an id that is not text fails the validator; `007` as a branch becomes the +#: branch `7`) and says that a number which survives `str()` works because of +#: the reader and is still a number to the project's own. MANIFEST_MISREAD_REMEDIATION = ( "Remediation: choose another value for each one named, as nothing here " "will retype it for you. The templates write these values unquoted, so " - "the project would read `null` and `~` as nothing, `true` and `false` as " - "booleans and `007` or `2026` as numbers, and its first bootstrap would " - "fail, or put its legs on a branch the remotes do not have. An id or a " - "branch name the reader takes for text (`atlas`, `release-1`, `v2`) " - "passes.") + "the project's own YAML reader takes `null` and `~` for no value, `true` " + "and `false` for booleans, and `007`, `2026` or `1.5` for numbers: what " + "it reads back is not what was typed. What that does differs: an id " + "that is not text fails the project's own validate-manifest.py, and " + "`007` as a branch reads back as the branch `7`; `2026` or `1.5` as a " + "branch happens to work today, because the tools that read it turn the " + "number back into text, but it is still a number to the project's " + "reader and not a name. A value the reader takes for text (`atlas`, " + "`release-2026`, `v2`) passes.") MANIFEST_INVALID_REMEDIATION = ( "Remediation: choose a value the project's own validate-manifest.py " "accepts; the words after each value are the validator's own, and a " diff --git a/tests/test_scaffold_manifest_values.py b/tests/test_scaffold_manifest_values.py index 1f17e19b..af973da9 100644 --- a/tests/test_scaffold_manifest_values.py +++ b/tests/test_scaffold_manifest_values.py @@ -227,6 +227,52 @@ def test_a_dry_run_refuses_a_misread_value_as_the_real_run_would( assert_nothing_was_created(tmp_path) +@pytest.mark.parametrize("flag, value", [ + ("--tracking-branch", "2026"), ("--tracking-branch", "1.5"), + ("--tracking-branch", "007"), ("--id", "2026"), ("--id", "null"), +]) +def test_the_remediation_says_what_is_true_of_every_value_it_refuses( + tmp_path, flag, value): + """`--tracking-branch 2026` and `1.5` are the values this refusal newly + takes from people: they scaffolded, bootstrapped and got their upstream, + because every reader of `tracking_branch` does `str()` and `str(2026)` is + `2026` again. The remediation once said the first bootstrap would fail or + put the legs on a branch the remotes lack, which is false of exactly + those, so it now says only what holds of all of them -- the project's + reader reads back something other than what was typed -- names the + failures that do hold, says plainly that a number which survives `str()` + is a number all the same, and gives the way forward: a value that is text + to the reader.""" + result = scaffold(tmp_path, "--dry-run", f"{flag}={value}") + assert_refused(result, MISREAD) + said = result.stderr + assert "what it reads back is not what was typed" in said + assert ("an id that is not text fails the project's own " + "validate-manifest.py") in said + assert ("`2026` or `1.5` as a branch happens to work today, because the " + "tools that read it turn the number back into text, but it is " + "still a number to the project's reader and not a name") in said + assert ("A value the reader takes for text (`atlas`, `release-2026`, " + "`v2`) passes.") in said + assert "first bootstrap would fail" not in said + assert "branch the remotes do not have" not in said + + +@pytest.mark.parametrize("typed, read_back", [ + ("2026", "2026"), ("1.5", "1.5"), + ("007", "7"), ("1.50", "1.5"), +]) +def test_the_remediations_claim_about_str_is_what_the_reader_does( + typed, read_back): + """The remediation says a branch that is a number survives `str()` only + when `str()` of the number is the text typed. That is a claim about the + project's own reader, so it is asked of the reader: `2026` and `1.5` come + back as typed, `007` and `1.50` as another branch.""" + value = parse_yaml(f"tracking_branch: {typed}\n")["tracking_branch"] + assert not isinstance(value, str), "the reader takes it for text" + assert str(value) == read_back + + @pytest.mark.parametrize("project, derived, reading", [ ("True", "true", "the boolean true"), ("Null", "null", "no value at all"), From e34158dbee897623e2d04c33cfc07c56161c4d5f Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:14:15 +0000 Subject: [PATCH 07/11] shape_materialize.py and adopt-project.py: a template validator that is missing, broken or raising is the named refusal `manifest-validator-unusable` and not a traceback, `check` leaves a value a misread rule judges to that rule once one is in the list, a name too long is shown at both ends, a placeholder is what `PLACEHOLDER_RE` finds, and the other branch of `_line_shown` is tested (#258) (#261) Four findings of the adversarial review of ca21b57, each small. SHOULD-3, the double finding with #181. `_manifest_findings` takes `misreads_elsewhere` (default False). Set, the placeholders `shape_materialize.misread_placeholders` names, which are the ones `manifest_misreads` refuses, are not asked of the validator, so `elected_on: "20261011"`, `visibility: "true"` and `id: "007"` get one finding from a list that also holds the misread rule's, and not a pair that says the retyped text (`id is 7`). It is False here, because the review's reading (always leave such a value to the misread rule) makes this branch alone accept `id: "007"` at `check`, where it is refused today: six tests of this branch fail when the parameter is made unconditional. Whoever lands second passes it True where the two lists meet; nothing is lost in either order. NIT-7. `manifest_validator` loads the template's `validate-manifest.py`, and `manifest_invalid` asks it. A missing, unreadable or broken file, one that is missing what is asked of it (`DATE_RE`, `_findings`), or an exception from the question was a traceback and exit 1 (a validator without `DATE_RE` was an `AttributeError` in `election_date`). Each is now `REFUSED manifest-validator-unusable`, naming the file and saying the copy of the standard is the defect, before anything is created. A failed load is not cached. NIT-3. `scaffold-name-too-long` cut each name at its first 24 characters, which lost `-spec` and `-code`, so a 251-character project read as two identical pairs of sentences. A name is shown at both ends. NIT-5. `_PLACEHOLDER_KEY_RE` was a second copy of `PLACEHOLDER_RE`'s alphabet, which #260 widens to digits. `_placeholders_of` reads a line with `PLACEHOLDER_RE` itself, so the two cannot drift. NIT-4. The non-printable branch of `_line_shown` was guarded by no test (a mutant that always used backticks survived 84 tests); it is now, for a vertical tab, a file separator and a line separator. Tests: each of the above fails when its change is reverted (ten mutants, all killed, in the report). Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 20 ++- scripts/shape_materialize.py | 104 +++++++++++--- tests/test_adopt_manifest_values.py | 133 +++++++++++++++++ tests/test_scaffold_manifest_values.py | 188 +++++++++++++++++++++++++ 4 files changed, 424 insertions(+), 21 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 87747fd6..ac08f7f9 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -139,7 +139,7 @@ # asked of the plan's values by `plan`, `check` and `execute` (#261). from shape_materialize import ( # noqa: E402 MANIFEST_INVALID_REMEDIATION, manifest_value_problems, - refuse_manifest_values, refuse_unstorable_names, + misread_placeholders, refuse_manifest_values, refuse_unstorable_names, ) #: The naming policy this tool classifies leg names against. One constant, @@ -1679,11 +1679,23 @@ def _manifest_values(plan: Plan, naming: NamingPolicy) -> dict | None: shape_commit, pins, naming) -def _manifest_findings(plan: Plan) -> tuple[list[str], list[str]]: +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`).""" + 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 + type (`elected_on: "20261011"`, `visibility: "true"`, `id: "007"`: text in + the plan, a number or a boolean to the reader once the template writes it + bare), so that such a value is judged once and not twice. It is False + wherever nothing else asks that question, so that this rule alone still + refuses the value, with the validator's finding as the only one. Who is + left out is the reader's answer (`shape_materialize.manifest_misreads`, + the rule the scaffold applies) and not a list. + """ naming = NamingPolicy.load(NAMING_POLICY) values = _manifest_values(plan, naming) if values is None: @@ -1696,6 +1708,8 @@ def _manifest_findings(plan: Plan) -> tuple[list[str], list[str]]: placeholder for placeholder, key in MANIFEST_PLAN_KEYS.items() if plan.get(key) is not None and not isinstance(plan.get(key), str)) + if misreads_elsewhere: + not_text += misread_placeholders(values, MANIFEST_PLAN_KEYS) problems = manifest_value_problems(values, naming, MANIFEST_PLAN_KEYS, misreads=False, ignore=LEG_PATH_CODES, not_text=not_text) diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 46fee5fe..3f3662fc 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -1943,12 +1943,27 @@ def refuse_bad_leg_paths(refusal: str, spec: tuple, code: tuple) -> None: MANIFEST_TEMPLATE = "project.yaml" MANIFEST_VALIDATOR = "scripts/validate-manifest.py" +#: The names this module asks of the validator: its `elected_on` pattern and +#: the function that makes every FINDING. +VALIDATOR_ASKS = ("DATE_RE", "_findings") + #: The templated files the project's reader reads: the manifest and the two #: leg pins. The materializer's own list, so a templated YAML file added there #: is asked about here without an edit. TEMPLATED_YAML = tuple(rel for rel in TEMPLATED if rel.endswith(".yaml")) -_PLACEHOLDER_KEY_RE = re.compile(r"\{\{([A-Z_]+)\}\}") + +def _placeholders_of(line: str) -> list[str]: + """The names of the placeholders `render` finds in `line`. + + From `PLACEHOLDER_RE`, the pattern `render` itself uses to find one, and + not from a second spelling of its alphabet: a template line this counted + as one placeholder while `render` saw two would be rendered here with a + one-key table and refused as `template-unsubstituted`. When the pattern + is widened (#260 admits digits, for `{{SPEC_TREE_SHA256}}`) this follows. + """ + return [token[2:-2] for token in PLACEHOLDER_RE.findall(line)] + #: The validator modules already loaded, by the file they were loaded from. _VALIDATORS: dict = {} @@ -1982,13 +1997,39 @@ def manifest_validator(template_dir: Path = ASSEMBLY_TEMPLATE): sys.dont_write_bytecode = True try: spec.loader.exec_module(module) + except Exception as exc: # a missing, unreadable or broken file + raise _validator_unusable( + "could not be loaded", f"{type(exc).__name__}: {exc}" + ) from exc finally: sys.path[:] = saved_path sys.dont_write_bytecode = saved_flag + # What this module asks of it, so that a file that loads and lacks one + # is the same refusal and not an `AttributeError` where it is used. + for name in VALIDATOR_ASKS: + if not hasattr(module, name): + raise _validator_unusable( + "could not be used", f"it has no `{name}`") _VALIDATORS[path] = module return module +def _validator_unusable(what: str, why: str) -> Refusal: + """The refusal for a template validator that cannot be asked, and not a + traceback. It is a defect of the copy of the standard that was run and + not of any value, and it is said so. Nothing was created, because the + question is asked before anything exists; and a validator that cannot + answer lets no manifest through unasked.""" + return Refusal( + "manifest-validator-unusable", + f"templates/assembly-root/{MANIFEST_VALIDATOR}, the validator this " + f"run asks about the manifest it would write, {what}: {why}", + "Remediation: this is a defect in the copy of this standard you ran, " + "not in your values: the file is missing, unreadable or broken " + "there, and nothing was created. Restore it from the standard's " + "repository, or reinstall the standard, and run again.") + + def _reading_in_words(reading) -> str: """What the project's reader made of a value, as a sentence says it.""" if isinstance(reading, YamlError): @@ -2024,6 +2065,28 @@ def _reading_of(rendered: str): return parsed +def _misread_rows(values: dict, what: dict, template_dir: Path + ) -> dict[str, list[tuple]]: + """For each placeholder of `what` whose value the project's reader would + not read back as that same text: `(file, rendered line, reading)` for each + template line that carries it.""" + found: dict[str, list] = {} + for rel in TEMPLATED_YAML: + text = (Path(template_dir) / rel).read_text(encoding="utf-8") + for line in text.splitlines(): + keys = _placeholders_of(line) + if (len(keys) != 1 or keys[0] not in what + or keys[0] not in values + or line.lstrip().startswith("#")): + continue + key, value = keys[0], str(values[keys[0]]) + rendered = render(line, {key: value}, rel).lstrip() + reading = _reading_of(rendered) + if not (isinstance(reading, str) and reading == value): + found.setdefault(key, []).append((rel, rendered, reading)) + return found + + def manifest_misreads(values: dict, what: dict, template_dir: Path = ASSEMBLY_TEMPLATE) -> list[str]: """A sentence for each value the project's reader would read back as @@ -2038,27 +2101,23 @@ def manifest_misreads(values: dict, what: dict, Each template line that carries exactly one of them is rendered with the value, as `render` writes it, and read as the project reads it. """ - found: dict[str, list] = {} - for rel in TEMPLATED_YAML: - text = (Path(template_dir) / rel).read_text(encoding="utf-8") - for line in text.splitlines(): - keys = _PLACEHOLDER_KEY_RE.findall(line) - if (len(keys) != 1 or keys[0] not in what - or keys[0] not in values - or line.lstrip().startswith("#")): - continue - key, value = keys[0], str(values[keys[0]]) - rendered = render(line, {key: value}, rel).lstrip() - reading = _reading_of(rendered) - if not (isinstance(reading, str) and reading == value): - found.setdefault(key, []).append((rel, rendered, reading)) return [ f"{what[key]} is {str(values[key])!r}, which is written " + " and ".join(f"into {rel} as {_line_shown(line)}" for rel, line, _ in rows) + ", where the project's own YAML reader reads it back as " + f"{_reading_in_words(rows[0][2])} and not as that text." - for key, rows in found.items()] + for key, rows in _misread_rows(values, what, template_dir).items()] + + +def misread_placeholders(values: dict, what: dict, + template_dir: Path = ASSEMBLY_TEMPLATE + ) -> tuple[str, ...]: + """The placeholders of `what` whose value `manifest_misreads` would refuse, + for a caller that leaves those values to a misread rule of its own and + must not have the validator's question asked a second time of them + (`manifest_invalid`'s `not_text`).""" + return tuple(_misread_rows(values, what, template_dir)) #: Which placeholder carries the value a finding of the validator is about, @@ -2146,7 +2205,13 @@ def manifest_invalid(values: dict, policy: NamingPolicy, what: dict, except YamlError as exc: return ["the manifest this run would write is a file the project's " f"own YAML reader refuses: {exc}."] - notes = manifest_validator(template_dir)._findings(manifest, policy, None) + validator = manifest_validator(template_dir) + try: + notes = validator._findings(manifest, policy, None) + except Exception as exc: # the validator's own defect, never a finding + raise _validator_unusable( + "raised when it was asked", f"{type(exc).__name__}: {exc}" + ) from exc return _sentences_of( [note for note in notes if not note.startswith("WARNING") and _code_of(note) not in ignore], values, what, not_text) @@ -2256,7 +2321,10 @@ def refuse_unstorable_names(refusal: str, directories: list[str], for name in names: size = len(f"{name}{suffix}".encode("utf-8")) if size > NAME_MAX: - shown = name if len(name) <= 24 else name[:24] + "..." + # BOTH ENDS: a name this long differs from the others by its + # tail, `-spec` or `-code`, which a cut at the front loses. + shown = (name if len(name) <= 27 + else f"{name[:12]}...{name[-12:]}") problems.append( f"{shown}{suffix} is {size} bytes long, and {where} " f"may be at most {NAME_MAX}") diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index 249d8851..0412c24b 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -441,3 +441,136 @@ def test_a_value_the_reader_reads_as_another_kind_is_not_this_questions( plan = edited(world, old, new) result = run_script(ADOPT, "check", "--plan", str(plan)) assert FINDING not in result.stderr, result.stderr + + +# --- a text value the reader misreads: judged once, not twice --------------- + +#: `(old line, new line, what the validator says of it)`: TEXT in the plan, +#: which the template writes bare and the project's reader reads as a number +#: or a boolean. The validator refuses each; where a misread rule is also in +#: the list (#181) it refuses the same value, and the sentence should be one. +TEXT_THE_READER_MISREADS = [ + pytest.param("id: northwind", 'id: "007"', "manifest-id: id is 7", + id="id-007"), + pytest.param("visibility: private", 'visibility: "true"', + "manifest-visibility: visibility is True", + id="visibility-true"), + pytest.param('elected_on: "2026-09-02"', 'elected_on: "20261011"', + "manifest-elected-on: elected_on is 20261011", + id="elected-on-basic-format"), +] + + +@pytest.mark.parametrize("old, new, named", TEXT_THE_READER_MISREADS) +def test_a_text_value_the_reader_misreads_is_one_finding_on_this_rule_alone( + adopter, world, old, new, named): + """With nothing else asking the misread question the validator's finding + is the only one, so `check` and `execute` still refuse the value: it must + not be dropped for want of a rule that is not here.""" + plan = adopter.Plan.load(edited(world, old, new)) + findings, remediation = adopter._manifest_findings(plan) + assert len(findings) == 1, findings + assert findings[0].startswith(FINDING) and named in findings[0] + assert remediation + + +@pytest.mark.parametrize("old, new, named", TEXT_THE_READER_MISREADS) +def test_where_a_misread_rule_judges_it_the_validator_is_not_asked_again( + adopter, world, old, new, named): + """`misreads_elsewhere`: a list that holds the misread rule's finding for + such a value (#181's `plan-manifest-misread`) is not given a second one + about the same value, in the validator's words and with the retyped text + (`id is 7`) that the misread finding exists to avoid.""" + plan = adopter.Plan.load(edited(world, old, new)) + assert adopter._manifest_findings(plan, misreads_elsewhere=True) == ( + [], []) + + +def test_a_value_that_is_not_a_misread_is_still_asked_where_one_judges_the_rest( + adopter, world): + """Only what the reader misreads is left out. `visibility: secret` is text + to the reader and the validator refuses it, so it is still a finding, and + so is a blank `elected_by`; `id: "007"` beside them is not.""" + text = world["text"].replace("id: northwind", 'id: "007"') + text = text.replace("visibility: private", "visibility: secret") + text = text.replace('elected_by: "Test Human"', 'elected_by: " "') + path = world["tmp"] / "mixed.yaml" + with path.open("w", encoding=UTF8, newline="\n") as handle: + handle.write(text) + plan = adopter.Plan.load(path) + alone, _ = adopter._manifest_findings(plan) + shared, _ = adopter._manifest_findings(plan, misreads_elsewhere=True) + assert len(alone) == 3, alone + assert len(shared) == 2, shared + assert any("visibility is 'secret'" in f for f in shared) + assert any("elected_by is ' '" in f for f in shared) + assert not any("manifest-id" in f for f in shared) + + +def test_the_entry_findings_of_this_rule_alone_are_unchanged(adopter, world): + """`_entry_findings` does not pass `misreads_elsewhere`: nothing else + here asks the misread question, so the list `check` prints and `execute` + refuses on still holds the validator's finding for `id: "007"`.""" + plan = adopter.Plan.load(edited(world, "id: northwind", 'id: "007"')) + source = plan.open_source(str(world["source"]), world["tmp"] / "read") + findings, _ = adopter._entry_findings(plan, source) + assert len([f for f in findings if f.startswith(FINDING)]) == 1, findings + + +# --- a validator that cannot be asked --------------------------------------- + +def a_copy_of_the_standard(base: Path, remake) -> Path: + """The standard, as its own git checkout (the tools name their revision), + with its template's `validate-manifest.py` made over by `remake`, which is + given the real text and returns the text to put there, or None to remove + the file.""" + copy = base / "standard" + shutil.copytree(REPO, copy, symlinks=True, ignore=shutil.ignore_patterns( + ".git", "__pycache__", ".pytest_cache", "tests", "docs", "handoffs")) + git("init", "-q", cwd=copy) + git("add", "-A", cwd=copy) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", + "--no-gpg-sign", "-m", "a copy of the standard", cwd=copy) + path = copy / "templates" / "assembly-root" / "scripts" / ( + "validate-manifest.py") + remade = remake(path.read_text(encoding=UTF8)) + if remade is None: + path.unlink() + else: + with path.open("w", encoding=UTF8, newline="\n") as handle: + handle.write(remade) + return copy + + +BROKEN_VALIDATORS = [ + pytest.param(lambda real: None, "could not be loaded: FileNotFoundError", + id="missing"), + pytest.param(lambda real: real + "\n\ndef _findings(manifest, policy, " + "other):\n raise KeyError('boom')\n", + "raised when it was asked: KeyError: 'boom'", + id="raises-when-asked"), +] + + +@pytest.mark.parametrize("remake, said", BROKEN_VALIDATORS) +def test_plan_and_check_name_a_validator_that_cannot_be_asked( + world, tmp_path, remake, said): + """The validator the template ships is asked of the values in `plan`, + `check` and `execute`; one that is missing or raises was a traceback and + exit 1. `plan` writes no plan and `check` reads none, and both say that the + defect is the copy's.""" + copy = a_copy_of_the_standard(tmp_path, remake) + out = tmp_path / "not-written.yaml" + planned = run_script( + copy / "adopt-project.py", "plan", "--source", str(world["source"]), + "--project", PROJECT, "--org", "testorg", "--elected-by", "Test Human", + "--elected-on", "2026-09-02", "--out", str(out)) + checked = run_script(copy / "adopt-project.py", "check", "--plan", + str(world["plan"])) + for result in (planned, checked): + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED manifest-validator-unusable" in result.stderr + assert said in result.stderr, result.stderr + assert "a defect in the copy of this standard you ran" in result.stderr + assert "Traceback" not in result.stderr + assert not out.exists(), "a plan was written" diff --git a/tests/test_scaffold_manifest_values.py b/tests/test_scaffold_manifest_values.py index af973da9..a7db4206 100644 --- a/tests/test_scaffold_manifest_values.py +++ b/tests/test_scaffold_manifest_values.py @@ -520,6 +520,25 @@ def test_the_name_rule_is_the_same_bytes_the_tools_make(): assert "256 bytes" in caught.value.detail +def test_the_names_a_refusal_shows_are_told_apart_by_their_tails(): + """The three repositories are ``, `-spec` and + `-code`. Cut at the front, `-spec` and `-code` were lost and two + of the sentences read the same; each name is shown at both ends.""" + name = "N" * 251 + with pytest.raises(shape_materialize.Refusal) as caught: + shape_materialize.refuse_unstorable_names( + "r", [name, f"{name}-spec", f"{name}-code"]) + sentences = caught.value.detail.rstrip(".").split("; ") + assert len(sentences) == 2 and len(set(sentences)) == 2, sentences + assert sentences[0].startswith("NNNNNNNNNNNN...NNNNNNN-spec is 256 bytes") + assert sentences[1].startswith("NNNNNNNNNNNN...NNNNNNN-code is 256 bytes") + # the bare repository's directory adds `.git`, after the tail + with pytest.raises(shape_materialize.Refusal) as caught: + shape_materialize.refuse_unstorable_names("r", [], [f"{name}-code"]) + assert caught.value.detail.startswith( + "NNNNNNNNNNNN...NNNNNNN-code.git is 260 bytes") + + # --- never GitHub ---------------------------------------------------------- @WINDOWS_SKIP # the `gh` below is a `#!` script, which Windows does not run @@ -626,3 +645,172 @@ def test_quoting_the_template_would_make_the_refusal_go_away(tmp_path): assert shape_materialize.manifest_misreads(values, what, copy) == [] # and what the quoted line holds is the text, through the project's reader assert parse_yaml('tracking_branch: "007"\n') == {"tracking_branch": "007"} + + +# --- one alphabet for a placeholder ---------------------------------------- + +def test_a_placeholder_is_what_render_calls_one(tmp_path, monkeypatch): + """The question counts the placeholders of a template line with the + pattern `render` finds them by, not with a second spelling of its + alphabet. #260 widens `PLACEHOLDER_RE` to admit digits + (`{{SPEC_TREE_SHA256}}`); a line holding one asked placeholder and one + with a digit must then count as two and not be asked, as a line that is + asked is rendered with a one-key table and refused as + `template-unsubstituted`.""" + monkeypatch.setattr(shape_materialize, "PLACEHOLDER_RE", + re.compile(r"\{\{[A-Z0-9_]+\}\}")) + assert shape_materialize._placeholders_of( + "x: {{PROJECT_ID}}-{{SPEC_TREE_SHA256}}") == [ + "PROJECT_ID", "SPEC_TREE_SHA256"] + copy = tmp_path / "assembly-root" + shutil.copytree(REPO / "templates" / "assembly-root", copy, + ignore=shutil.ignore_patterns("__pycache__", "*.pyc")) + manifest = copy / "project.yaml" + manifest.write_text( + manifest.read_text(encoding="utf-8") + + "extra: {{PROJECT_ID}}-{{SPEC_TREE_SHA256}}\n", encoding="utf-8") + values = {"PROJECT_ID": "null", "SPEC_TREE_SHA256": "0" * 64} + what = {"PROJECT_ID": "--id"} + # only the line that holds the one placeholder is asked, and nothing raises + sentences = shape_materialize.manifest_misreads(values, what, copy) + assert len(sentences) == 1 + assert "id: null" in sentences[0] and "extra" not in sentences[0] + + +# --- what is shown of a line ---------------------------------------------- + +def test_a_line_a_terminal_would_show_as_something_else_is_quoted_by_python(): + """A character a terminal shows as something else (a vertical tab, a + line separator) is spelled the way Python does, so that one sentence stays + one line and the character can be seen; a printable line is in + backticks.""" + shown = shape_materialize._line_shown + assert shown("id: atlas") == "`id: atlas`" + assert shown("tracking_branch: a\x0bb") == "'tracking_branch: a\\x0bb'" + assert shown("id: a\u2028b") == "'id: a\\u2028b'" + + +@pytest.mark.parametrize("char", ["\x0b", "\x1c", "\u2028"]) +def test_a_misread_sentence_holding_a_line_break_is_one_printable_line(char): + """The manifest is split at the break, so the reader refuses it; the + sentence says so without printing the break itself.""" + what = {"TRACKING_BRANCH": "--tracking-branch"} + (sentence,) = shape_materialize.manifest_misreads( + {"TRACKING_BRANCH": f"a{char}b"}, what) + assert len(sentence.splitlines()) == 1 and sentence.isprintable() + assert "into project.yaml as 'tracking_branch: a\\" in sentence + # a printable misread is quoted in backticks, as before + (plain,) = shape_materialize.manifest_misreads( + {"TRACKING_BRANCH": "007"}, what) + assert "into project.yaml as `tracking_branch: 007`" in plain + + +# --- the placeholders a misread rule would refuse --------------------------- + +def test_the_misread_placeholders_are_the_ones_the_misread_rule_refuses(): + """What a caller that leaves misreads to a rule of its own asks of the + validator is whatever `manifest_misreads` would refuse, as an answer of + the project's reader and not a list of values.""" + what = {"PROJECT_ID": "--id", "TRACKING_BRANCH": "--tracking-branch", + "ELECTED_BY": "--elected-by", "ELECTED_ON": "--elected-on", + "VISIBILITY": "--visibility", "ORG": "--org"} + values = {"PROJECT_ID": "007", "TRACKING_BRANCH": "2026", + "ELECTED_BY": " ", "ELECTED_ON": "2026-09-02", + "VISIBILITY": "true", "ORG": "acme"} + asked = shape_materialize.misread_placeholders(values, what) + assert sorted(asked) == ["PROJECT_ID", "TRACKING_BRANCH", "VISIBILITY"] + assert len(shape_materialize.manifest_misreads(values, what)) == len(asked) + assert shape_materialize.misread_placeholders( + {"PROJECT_ID": "atlas"}, what) == () + + +# --- a validator that cannot be asked ------------------------------------- + +#: `(how the template's validator is made over, what the refusal says)`: the +#: function is given the real file's text and returns the new text, or None to +#: remove the file. +BROKEN_VALIDATORS = [ + pytest.param(lambda real: None, "could not be loaded: FileNotFoundError", + id="missing"), + pytest.param(lambda real: "def (:\n", + "could not be loaded: SyntaxError", id="syntax"), + pytest.param(lambda real: "raise RuntimeError('boom at import')\n", + "could not be loaded: RuntimeError: boom at import", + id="raises-at-import"), + pytest.param(lambda real: "ANSWER = 42\n", + "could not be used: it has no `DATE_RE`", + id="lacks-what-is-asked"), + pytest.param(lambda real: real + "\n\ndef _findings(manifest, policy, " + "other):\n raise KeyError('boom')\n", + "raised when it was asked: KeyError: 'boom'", + id="raises-when-asked"), +] + + +def a_copy_of_the_standard(base: Path, remake) -> Path: + """The standard, as its own git checkout (the tools name their revision), + with its template's `validate-manifest.py` made over by `remake`, which is + given the real text and returns the text to put there, or None to remove + the file.""" + copy = base / "standard" + shutil.copytree(REPO, copy, symlinks=True, ignore=shutil.ignore_patterns( + ".git", "__pycache__", ".pytest_cache", "tests", "docs", "handoffs")) + git("init", "-q", cwd=copy) + git("add", "-A", cwd=copy) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", + "--no-gpg-sign", "-m", "a copy of the standard", cwd=copy) + path = copy / "templates" / "assembly-root" / "scripts" / ( + "validate-manifest.py") + remade = remake(path.read_text(encoding="utf-8")) + if remade is None: + path.unlink() + else: + with path.open("w", encoding="utf-8", newline="\n") as handle: + handle.write(remade) + return copy + + +@pytest.mark.parametrize("remake, said", BROKEN_VALIDATORS) +def test_a_validator_that_cannot_be_asked_is_a_named_refusal_not_a_traceback( + tmp_path, remake, said): + """The validator the template ships is loaded and asked, so a copy of the + standard that lost it, or holds one that raises, would have been a + traceback and exit 1 from deep inside the tool. Nothing is created + either way; it is now a refusal that names the file and says the defect is + the copy's, not the values'.""" + copy = a_copy_of_the_standard(tmp_path, remake) + run = tmp_path / "run" + (run / SCRATCH).mkdir(parents=True) + result = run_script( + copy / "scaffold-project.py", *IDENTITY, + "--local-remote-dir", str(run / REMOTES), + "--work-dir", str(run / WORK), + env={"TMPDIR": str(run / SCRATCH), "TMP": str(run / SCRATCH), + "TEMP": str(run / SCRATCH)}) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED manifest-validator-unusable" in result.stderr + assert "validate-manifest.py" in result.stderr + assert said in result.stderr, result.stderr + assert "a defect in the copy of this standard you ran" in result.stderr + assert "Traceback" not in result.stderr + assert result.stdout == "", "a plan was printed for a refused scaffold" + assert sorted(p.name for p in run.iterdir()) == [SCRATCH] + assert list((run / SCRATCH).iterdir()) == [] + assert not list((copy / "templates").rglob("__pycache__")), ( + "bytecode was written into the template tree") + + +def test_a_validator_that_failed_to_load_is_not_remembered(tmp_path): + """A failed load is not cached: the file put right, the next ask loads + it.""" + copy = tmp_path / "assembly-root" + shutil.copytree(REPO / "templates" / "assembly-root", copy, + ignore=shutil.ignore_patterns("__pycache__", "*.pyc")) + validator = copy / "scripts" / "validate-manifest.py" + good = validator.read_bytes() + validator.write_text("def (:\n", encoding="utf-8") + with pytest.raises(shape_materialize.Refusal) as caught: + shape_materialize.manifest_validator(copy) + assert caught.value.code == "manifest-validator-unusable" + validator.write_bytes(good) + assert hasattr(shape_materialize.manifest_validator(copy), "_findings") From 7defac7c915f901046262acbe4a481e18524e252 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:20:50 +0000 Subject: [PATCH 08/11] tests/test_adopt_manifest_values.py: a text value the reader misreads (`id: "007"`, `visibility: "true"`, `elected_on: "20261011"`) is asserted to be ONE finding in the list `check` prints, and `check` and `execute` are asked only that they refuse it, so a merge with #181 that does not pass `misreads_elsewhere` fails here and one that does edits no test (#261) The first cut pinned the validator's own finding for `elected_on: "20261011"` in the rows every plan edit is run through, and the unwired call site in a test of its own. A throwaway merge with #181 (e337f44) that passed `misreads_elsewhere=True` then failed three tests of this branch for the reason the parameter exists: the value is now the misread rule's finding. Now the three values are rows of one list, asked at three levels: `_manifest_findings` (this rule alone: one finding in the validator's words; `misreads_elsewhere`: none), `check` and `execute` (refuse the value and name it, whichever rule says so), and `_entry_findings` (exactly one finding names the value, not zero and not two). On this branch alone all of them hold. In the throwaway merge with #181 they hold with the call site passing `misreads_elsewhere=True`, and the third fails for each of the three values, two findings and not one, with the plain keep-both resolution: the test is the reminder to whoever lands second. `elected-on-basic-format` leaves PLAN_EDITS, the rows that name the validator's words in `check` and `execute`; the same value is a row of the new list. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_manifest_values.py | 54 ++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index 0412c24b..94995f4e 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -87,6 +87,10 @@ #: `(old line, new line, what `check` names)`: the same values, edited into a #: plan by hand, which is what a plan is for. +#: A text value the reader reads as a number (`elected_on: "20261011"`) is +#: not here but in `TEXT_THE_READER_MISREADS` below: where a misread rule is +#: also in the list (#181) it is that rule's finding, and these rows name the +#: validator's. PLAN_EDITS = [ pytest.param("visibility: private", "visibility: secret", ("visibility is 'secret'", "manifest-visibility"), @@ -103,9 +107,6 @@ pytest.param('reference: "openxFactory docs/project-repo-schema.md"', 'reference: " "', ("reference is ' '", "manifest-reference"), id="reference-blank"), - pytest.param('elected_on: "2026-09-02"', 'elected_on: "20261011"', - ("elected_on is '20261011'", "manifest-elected-on"), - id="elected-on-basic-format"), pytest.param('elected_on: "2026-09-02"', 'elected_on: "2026-W41-6"', ("elected_on is '2026-W41-6'", "manifest-elected-on"), id="elected-on-iso-week"), @@ -461,6 +462,35 @@ def test_a_value_the_reader_reads_as_another_kind_is_not_this_questions( ] +@pytest.mark.parametrize("old, new, named", TEXT_THE_READER_MISREADS) +def test_check_refuses_a_text_value_the_reader_misreads(world, old, new, + named): + """Whichever rule says it: the validator's finding here, or a misread + rule's (#181) where one is in the list. What is asked is that `check` + finds the value, and names it.""" + plan = edited(world, old, new) + result = run_script(ADOPT, "check", "--plan", str(plan)) + assert result.returncode == 1, result.stderr + result.stdout + value = new.split(": ", 1)[1].strip('"') + findings = [line for line in result.stderr.splitlines() + if line.startswith("FINDING ")] + assert findings and any(value in f for f in findings), result.stderr + + +@needs_filter_repo +@pytest.mark.parametrize("old, new, named", TEXT_THE_READER_MISREADS) +def test_execute_refuses_a_text_value_the_reader_misreads_before_any_leg( + world, old, new, named): + plan = edited(world, old, new) + result = execute(world, plan) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED plan-manifest-" in result.stderr + assert new.split(": ", 1)[1].strip('"') in result.stderr + assert "adoption verified" not in result.stdout + assert "Traceback" not in result.stderr + no_leg_was_created(world) + + @pytest.mark.parametrize("old, new, named", TEXT_THE_READER_MISREADS) def test_a_text_value_the_reader_misreads_is_one_finding_on_this_rule_alone( adopter, world, old, new, named): @@ -507,14 +537,20 @@ def test_a_value_that_is_not_a_misread_is_still_asked_where_one_judges_the_rest( assert not any("manifest-id" in f for f in shared) -def test_the_entry_findings_of_this_rule_alone_are_unchanged(adopter, world): - """`_entry_findings` does not pass `misreads_elsewhere`: nothing else - here asks the misread question, so the list `check` prints and `execute` - refuses on still holds the validator's finding for `id: "007"`.""" - plan = adopter.Plan.load(edited(world, "id: northwind", 'id: "007"')) +@pytest.mark.parametrize("old, new, named", TEXT_THE_READER_MISREADS) +def test_a_text_value_the_reader_misreads_is_one_finding_in_the_list_check_prints( + adopter, world, old, new, named): + """NOT ZERO AND NOT TWO, whatever rules the list holds. This rule alone, + `_entry_findings` does not pass `misreads_elsewhere`, and the validator's + finding is the one. With #181 in the list as well, the call there must pass + it, or this is the pair the two rules each make of one value: this test + fails on a merge that has not.""" + plan = adopter.Plan.load(edited(world, old, new)) source = plan.open_source(str(world["source"]), world["tmp"] / "read") findings, _ = adopter._entry_findings(plan, source) - assert len([f for f in findings if f.startswith(FINDING)]) == 1, findings + value = new.split(": ", 1)[1].strip('"') + about = [f for f in findings if value in f] + assert len(about) == 1, findings # --- a validator that cannot be asked --------------------------------------- From cf3e8d3b13abf8facba16aa787c14862810435b9 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 02:38:56 +0000 Subject: [PATCH 09/11] adopt-project.py: `plan` and `check` ask the project's validator about the manifest with a fixed, well-formed commit and digest and run no `git`, so a checkout that cannot name its revision is asked the question and not skipped in silence, and SonarCloud's `pythonsecurity:S8705` on the `git rev-parse HEAD` this branch added to `_manifest_values` has nothing left to flag (#261) `_manifest_values` rendered the manifest FOR VALIDATION with the standard's real commit, read with `git rev-parse HEAD`, and (through `_template_values`) the real digest of its tree, read with `git ls-tree -r`. The validator the template ships (`validate-manifest.py`, `_shape_findings`) holds `shape.commit` to `COMMIT_RE` and `shape.digests.tree_sha256` to `SHA256_RE` and compares neither with anything, `manifest_misreads` is asked only about the placeholders a tool names to a person (`MANIFEST_FLAGS`, `MANIFEST_PLAN_KEYS`, neither of which holds either one), and `manifest_invalid` passes the values through. So the two reads bought nothing, and where `git` failed (a `git archive` copy, a downloaded zip) the function returned None and `plan` and `check` skipped the whole question without a word: `plan --elected-by ' '` exited 0 and wrote the plan (NIT-9 of the review). Now the table is built by `_template_table`, which is handed its commit and digest and runs nothing, and `_manifest_values` hands it two module constants, `_VALIDATION_SHAPE_COMMIT` and `_VALIDATION_SHAPE_DIGEST`: lowercase hex of the right length with a letter in it, which the project's own YAML reader reads as text whether the template quotes it or not. The `Refusal` fallback is gone, so `plan` and `check` always ask. No new function of this branch reaches `run`, `git_out`, `tree_digest` or `subprocess` any more (the call-graph of the functions this branch adds or changes, before and after, is in the report). `execute` is untouched where it matters: it still reads `git rev-parse HEAD` and calls `_template_values`, which is `_template_table` with the real commit and `tree_digest(SHAPE_ROOT, commit)`, so what it writes into `project.yaml` and the two leg pins is what it wrote before. `test_execute_still_renders_ the_real_commit_and_digest` stops `execute` where the legs would be built and asserts the table it holds carries the standard's real HEAD and the real tree digest and neither constant. tests/test_adopt_manifest_values.py: the test that asserted the old behaviour (`test_a_checkout_that_cannot_name_its_revision_skips_the_question`) is replaced by 16, of which 14 fail on the previous head and pass now: - `plan` refuses a blank `--elected-by` or `--reference` and an `--org` of `test+org`, and `check` finds each of the seven plan edits, run from a copy of the tool tree with NO `.git`, with the same REFUSED and FINDING lines as the same tool in a checkout; a good plan still plans and checks `plan ok` there; - with `git_out` and `tree_digest` raising, `_manifest_findings` still finds the value, and with every way of starting a process (`subprocess.run`, `Popen`, `check_output`, `check_call`, `call`, `os.system`) raising it still answers; - the constants are 40 and 64 lowercase hex with a letter, read as text by the project's reader quoted or bare, and are what `_manifest_values` holds; - the validator gives the same sentences for any well-formed commit and digest, refuses `not-a-commit` for its form, and neither value is a placeholder either tool names to a person. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 68 ++++++--- tests/test_adopt_manifest_values.py | 229 ++++++++++++++++++++++++++-- 2 files changed, 268 insertions(+), 29 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index ac08f7f9..d33a423a 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1639,18 +1639,37 @@ def _flag_plan(args, names: dict, pins: list) -> "Plan": "pins": list(pins)}) +#: The commit and the tree digest the manifest rendered FOR VALIDATION holds +#: in `shape.commit` and `shape.digests.tree_sha256`, in place of the real +#: ones `execute` reads and writes (`_template_values`). The validator the +#: template ships holds each to its FORM (`COMMIT_RE`, `SHA256_RE`) and +#: compares neither with anything, and no tool names either to a person, so +#: reading the real ones (`git rev-parse HEAD`, then `git ls-tree -r` of the +#: whole standard) bought `plan` and `check` nothing, and made them skip the +#: question silently from a tree with no git revision. Lowercase hex with a +#: letter in it, so that the project's YAML reader reads each as text and never +#: as a number (forty digits would be an integer), quoted or not. +_VALIDATION_SHAPE_COMMIT = "0123456789abcdef0123456789abcdef01234567" +_VALIDATION_SHAPE_DIGEST = ("0123456789abcdef0123456789abcdef" + "0123456789abcdef0123456789abcdef") + + def _manifest_values(plan: Plan, naming: NamingPolicy) -> dict | None: """The substitution table `execute` renders the manifest from, as it - would build it from this plan; 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, or - where this tool's own checkout cannot name its revision (below). - - `_template_values` is asked, not copied, so the table cannot differ from - `execute`'s. The four leg commits and digests are not known before the - legs exist and are not in the manifest; 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. + 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. + + `_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 + differ from `execute`'s. The four leg commits and digests are not known + before the legs exist and are not in the manifest; the standard's own + commit and tree digest are replaced by the fixed pair above, so that no + `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. """ if not isinstance(plan.legs, dict): # The doctor's PLACEMENT plan writes `legs:` as a list (see @@ -1666,17 +1685,10 @@ def _manifest_values(plan: Plan, naming: NamingPolicy) -> dict | 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) - try: - shape_commit = git_out(["rev-parse", "HEAD"], cwd=SHAPE_ROOT).lower() - except Refusal: - # No checkout to read a revision from: `execute` cannot run either, - # and says so in its own words where it reads one. `plan` and `check` - # never needed one and are not made to. - return None - return _template_values( + return _template_table( plan, names, repositories, {"assembly": ""}, str(spec_path), str(code_path), str(plan.get("tracking_branch", "main")), {}, {}, - shape_commit, pins, naming) + _VALIDATION_SHAPE_COMMIT, _VALIDATION_SHAPE_DIGEST, pins, naming) def _manifest_findings(plan: Plan, misreads_elsewhere: bool = False @@ -3649,6 +3661,22 @@ def cmd_execute(args) -> int: def _template_values(plan: Plan, names, repositories, urls, spec_path, code_path, tracking, leg_commits, leg_digests, shape_commit, pins, naming) -> dict[str, str]: + """The substitution table the new project's files are WRITTEN from: the + standard's real `shape_commit` and the digest of its tree, read here. + `execute` is the one caller. A table to ask a question about and not to + write is `_template_table`'s, which reads neither (`_manifest_values`).""" + return _template_table(plan, names, repositories, urls, spec_path, + code_path, tracking, leg_commits, leg_digests, + shape_commit, tree_digest(SHAPE_ROOT, shape_commit), + pins, naming) + + +def _template_table(plan: Plan, names, repositories, urls, spec_path, + code_path, tracking, leg_commits, leg_digests, + shape_commit, shape_digest, pins, naming + ) -> dict[str, str]: + """The substitution table, from the plan and the commit and digest it is + handed, running nothing: no `git`, no `gh`.""" project = names["assembly"] project_id = plan.project_id elected_on = str(plan.get("elected_on", _dt.date.today().isoformat())) @@ -3680,7 +3708,7 @@ def _template_values(plan: Plan, names, repositories, urls, spec_path, "CODE_REPOSITORY": repositories["code"], "SHAPE_REPOSITORY": SHAPE_REPOSITORY, "SHAPE_COMMIT": shape_commit, - "SHAPE_TREE_SHA256": tree_digest(SHAPE_ROOT, shape_commit), + "SHAPE_TREE_SHA256": shape_digest, "DIGEST_DEFINITION": TREE_DIGEST_DEFINITION, "CLONE_URL": urls["assembly"], "ASSEMBLY_CLONE_URL": urls["assembly"], diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index 94995f4e..da56ce6b 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -32,7 +32,10 @@ from __future__ import annotations import importlib.util +import os +import re import shutil +import subprocess import sys from argparse import Namespace from pathlib import Path @@ -43,6 +46,7 @@ write_plan) sys.path.insert(0, str(REPO / "scripts")) +import repo_shape # noqa: E402 import shape_materialize # noqa: E402 PROJECT = "Northwind" @@ -371,18 +375,225 @@ def test_the_flags_and_the_plan_keys_name_the_same_values(adopter): assert mapped - {"PROJECT_NAME"} <= set(adopter.MANIFEST_FLAGS) -def test_a_checkout_that_cannot_name_its_revision_skips_the_question( +# --- the question is asked with no git at all (NIT-9, SonarCloud S8705) ------ +# +# The manifest is rendered here to be ASKED ABOUT and never written: the +# validator the template ships holds `shape.commit` and `shape.digests. +# tree_sha256` to their FORM (`COMMIT_RE`, `SHA256_RE`) and compares neither +# with anything, and neither is a value a tool names to a person, so +# `manifest_misreads` is not asked about them. `plan` and `check` therefore put +# a fixed, well-formed pair into the table they render and run no `git` to read +# the real ones. `execute` still reads the real ones and writes them. + +@pytest.fixture(scope="module") +def no_git_standard(tmp_path_factory): + """The standard as a `git archive` or a downloaded zip holds it: every + tool, template and contract, and no `.git`, so no revision to read.""" + copy = tmp_path_factory.mktemp("fixV2-no-git") / "standard" + shutil.copytree(REPO, copy, symlinks=True, ignore=shutil.ignore_patterns( + ".git", "__pycache__", ".pytest_cache", "tests", "docs", "handoffs")) + assert not (copy / ".git").exists() + return copy + + +def verdict_of(result) -> list[str]: + """What a run said about the values, without the lines git or the host add + around it.""" + return [line for line in result.stderr.splitlines() + if line.startswith(("REFUSED ", "FINDING ", "Remediation: "))] + + +def test_a_checkout_that_cannot_name_its_revision_is_still_asked( adopter, world, monkeypatch): - """`plan` and `check` never needed a revision and are not made to: where - this tool's own checkout cannot give one (`execute` could not run either, - and says so where it reads one), the manifest cannot be rendered and - nothing is asked, rather than `check` dying where it succeeded.""" - def no_revision(*args, **kwargs): - raise adopter.Refusal("not-a-checkout", "no git here") - monkeypatch.setattr(adopter, "git_out", no_revision) + """`plan` and `check` need no revision, so a checkout that cannot give one + (or a git that fails) is asked the same question as any other, and a plan + the validator refuses is found there as it is in a checkout. It skipped + the whole question and said nothing, so a blank `--elected-by` passed + `plan` from a tree that had no `.git`.""" + def no_git(*args, **kwargs): + raise adopter.Refusal("git-failed", "no git here") + monkeypatch.setattr(adopter, "git_out", no_git) + monkeypatch.setattr(adopter, "tree_digest", no_git) plan = adopter.Plan.load(edited(world, "visibility: private", "visibility: secret")) - assert adopter._manifest_findings(plan) == ([], []) + naming = adopter.NamingPolicy.load(adopter.NAMING_POLICY) + assert adopter._manifest_values(plan, naming) is not None + findings, remediation = adopter._manifest_findings(plan) + assert len(findings) == 1, findings + assert findings[0].startswith(FINDING) + assert "visibility is 'secret'" in findings[0] and remediation + # and it is no finding for a plan the validator would pass + good = adopter.Plan.load(world["plan"]) + assert adopter._manifest_findings(good) == ([], []) + + +def test_the_question_runs_no_process_at_all(adopter, world, monkeypatch): + """No `git`, no `gh`, no shell: asking what the validator would say of a + plan's values reads files and runs the validator's own Python, and the + table it asks about is built by a function that is handed its commit and + digest. (It ran `git rev-parse HEAD` and then `git ls-tree -r` of the + whole standard, with `plan` and `check` skipping the question where that + failed.)""" + refused = adopter.Plan.load(edited(world, "visibility: private", + "visibility: secret")) + good = adopter.Plan.load(world["plan"]) + naming = adopter.NamingPolicy.load(adopter.NAMING_POLICY) + shape_materialize.manifest_validator() # its own load is a read as well + + def no_process(*args, **kwargs): + raise AssertionError(f"a process was started: {args!r}") + for name in ("run", "Popen", "check_output", "check_call", "call"): + monkeypatch.setattr(subprocess, name, no_process) + monkeypatch.setattr(os, "system", no_process) + assert adopter._manifest_values(good, naming) is not None + assert adopter._manifest_findings(good) == ([], []) + assert adopter._manifest_findings(good, misreads_elsewhere=True) == ( + [], []) + (finding,), remediation = adopter._manifest_findings(refused) + assert "visibility is 'secret'" in finding and remediation + + +@pytest.mark.parametrize("flag, value, named", [ + ("--elected-by", " ", "--elected-by is ' '"), + ("--reference", " ", "--reference is ' '"), + ("--org", "test+org", "--org is 'test+org'"), +]) +def test_plan_refuses_it_from_a_tree_with_no_git_as_in_a_checkout( + world, tmp_path, no_git_standard, flag, value, named): + out = tmp_path / "not-written.yaml" + given = {"--org": "testorg", "--elected-by": "Test Human", + "--elected-on": "2026-09-02", flag: value} + args = ["plan", "--source", str(world["source"]), "--project", PROJECT, + "--out", str(out), *(f"{k}={v}" for k, v in given.items())] + without_git = run_script(no_git_standard / "adopt-project.py", *args) + assert without_git.returncode == 2, (without_git.stderr + + without_git.stdout) + assert f"REFUSED {INVALID}: {named}" in without_git.stderr + assert "Traceback" not in without_git.stderr + assert not out.exists(), "a plan was written" + in_checkout = run_script(ADOPT, *args) + assert in_checkout.returncode == 2, in_checkout.stderr + assert verdict_of(without_git) == verdict_of(in_checkout) != [] + assert not out.exists() + + +def test_plan_and_check_pass_a_good_plan_from_a_tree_with_no_git( + world, tmp_path, no_git_standard): + """The question is asked and not skipped, and asking it makes no refusal + of a plan the validator accepts.""" + out = tmp_path / "plan.yaml" + planned = run_script( + no_git_standard / "adopt-project.py", "plan", "--source", + str(world["source"]), "--project", PROJECT, "--org", "testorg", + "--elected-by", "Test Human", "--elected-on", "2026-09-02", "--out", + str(out)) + assert planned.returncode == 0, planned.stderr + planned.stdout + assert out.is_file() + for path, leg in ANSWERS: + resolve(out, path, leg) + checked = run_script(no_git_standard / "adopt-project.py", "check", + "--plan", str(out)) + assert checked.returncode == 0, checked.stderr + checked.stdout + assert "plan ok" in checked.stdout + assert FINDING not in checked.stderr + + +@pytest.mark.parametrize("old, new, named", PLAN_EDITS) +def test_check_finds_it_from_a_tree_with_no_git_as_in_a_checkout( + world, no_git_standard, old, new, named): + plan = edited(world, old, new) + without_git = run_script(no_git_standard / "adopt-project.py", "check", + "--plan", str(plan)) + assert without_git.returncode == 1, (without_git.stderr + + without_git.stdout) + assert FINDING in without_git.stderr, without_git.stderr + for fragment in named: + assert fragment in without_git.stderr, without_git.stderr + in_checkout = run_script(ADOPT, "check", "--plan", str(plan)) + assert in_checkout.returncode == 1 + assert verdict_of(without_git) == verdict_of(in_checkout) != [] + + +def test_what_plan_and_check_put_in_the_table_is_a_well_formed_pair( + adopter, world): + """A fixed commit and digest in the form the validator holds the real ones + to, which the project's YAML reader reads as text whether the template + quotes it or not (forty digits would be one number, and `12345e6789...` + another).""" + commit, digest = (adopter._VALIDATION_SHAPE_COMMIT, + adopter._VALIDATION_SHAPE_DIGEST) + assert re.fullmatch(r"[0-9a-f]{40}", commit) + assert re.fullmatch(r"[0-9a-f]{64}", digest) + for value in (commit, digest): + assert re.search(r"[a-f]", value) + for line in (f"k: {value}\n", f'k: "{value}"\n'): + assert shape_materialize.parse_yaml(line) == {"k": value} + plan = adopter.Plan.load(world["plan"]) + naming = adopter.NamingPolicy.load(adopter.NAMING_POLICY) + values = adopter._manifest_values(plan, naming) + assert values["SHAPE_COMMIT"] == commit + assert values["SHAPE_TREE_SHA256"] == digest + # it is not this checkout's own: no claim about it is made + assert commit != git("rev-parse", "HEAD", cwd=REPO).stdout.strip() + + +def test_the_validator_compares_the_shape_commit_with_nothing_and_no_tool_names_it( + adopter, world): + """What the fixed pair rests on, asked of the validator itself: any + well-formed commit and digest give the same sentences for the same values, + a malformed one is refused for its FORM, and neither value is one a tool + names to a person (so `manifest_misreads` is never asked about it).""" + plan = adopter.Plan.load(edited(world, "visibility: private", + "visibility: secret")) + naming = adopter.NamingPolicy.load(adopter.NAMING_POLICY) + values = adopter._manifest_values(plan, naming) + keys = adopter.MANIFEST_PLAN_KEYS + asked = shape_materialize.manifest_invalid(values, naming, keys) + assert len(asked) == 1 and "visibility is 'secret'" in asked[0] + head = git("rev-parse", "HEAD", cwd=REPO).stdout.strip() + for other in ({"SHAPE_COMMIT": head, "SHAPE_TREE_SHA256": "f" * 64}, + {"SHAPE_COMMIT": "A" * 40, "SHAPE_TREE_SHA256": "0" * 64}, + {"SHAPE_COMMIT": "1" * 39 + "e", + "SHAPE_TREE_SHA256": "e" * 64}): + assert shape_materialize.manifest_invalid( + {**values, **other}, naming, keys) == asked + malformed = shape_materialize.manifest_invalid( + {**values, "SHAPE_COMMIT": "not-a-commit"}, naming, keys) + assert any("shape.commit is 'not-a-commit', not 40 hex" in sentence + for sentence in malformed), malformed + for table in (adopter.MANIFEST_FLAGS, keys): + assert "SHAPE_COMMIT" not in table + assert "SHAPE_TREE_SHA256" not in table + # and were it asked, the fixed pair reads back as the text it is + assert shape_materialize.manifest_misreads( + values, {"SHAPE_COMMIT": "--shape-commit", + "SHAPE_TREE_SHA256": "--shape-digest"}) == [] + + +@needs_filter_repo +def test_execute_still_renders_the_real_commit_and_digest( + adopter, world, monkeypatch, capsys): + """`execute` reads the standard's own revision and the digest of its tree + and writes THEM into the files it makes: the fixed pair is for the + question only. Stopped where the legs would be built, with the table + `execute` hands them in hand.""" + seen = [] + + def stop_before_the_legs(source, names, repositories, urls, values, *rest): + seen.append(dict(values)) + return None + monkeypatch.setattr(adopter, "_build_the_legs", stop_before_the_legs) + code = adopter.main(["execute", "--plan", str(world["plan"]), "--yes", + "--local-remote-dir", str(world["tmp"] / REMOTES), + "--work-dir", str(world["tmp"] / WORK)]) + capsys.readouterr() + assert code == 2 + (values,) = seen + head = git("rev-parse", "HEAD", cwd=REPO).stdout.strip().lower() + assert values["SHAPE_COMMIT"] == head + assert values["SHAPE_TREE_SHA256"] == repo_shape.tree_digest(REPO, head) + assert values["SHAPE_COMMIT"] != adopter._VALIDATION_SHAPE_COMMIT + assert values["SHAPE_TREE_SHA256"] != adopter._VALIDATION_SHAPE_DIGEST def test_a_placement_plan_names_no_repository_so_nothing_is_asked(adopter): From ab66bc0e44eb5e030c7945795c6e7bd055652abc Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 03:19:07 +0000 Subject: [PATCH 10/11] adopt-project.py, shape_materialize.py, tests: the comment above `MANIFEST_FLAGS` names `_template_table` as what builds the table, the two test files lose a lane-private token and eight composite assertions, and `scripts/validate-manifest.py` is spelled once in `shape_materialize` (#261) Review round 3 of this branch found four small things and no defect in the code. None of this changes what any tool does. - adopt-project.py: the comment above `MANIFEST_FLAGS` said the plan is read "the way `execute` reads it (`_template_values`)". `_template_values` is now the wrapper that reads the real commit and digest; the table is built by `_template_table`, which both it and `_manifest_values` call. The comment says that. - tests/test_adopt_manifest_values.py: a section comment and a `mktemp` prefix named review-round identifiers that mean nothing in the repository. The section comment now says what the section tests, and the temporary directory is `no-git-standard`. - SonarCloud `python:S9073` (split a composite assertion): `a and b` is now `assert a` and `assert b`, with the same operands and the same failure message, at five places in tests/test_adopt_manifest_values.py and three in tests/test_scaffold_manifest_values.py. - SonarCloud `python:S1192` in scripts/shape_materialize.py: the literal "scripts/validate-manifest.py" was spelled in `COPIED_VERBATIM`, in `EXECUTABLE` and in `MANIFEST_VALIDATOR`. `MANIFEST_VALIDATOR` is now defined once, beside `BOOTSTRAP` and the other path names those two lists share, and both lists use it. Its value is unchanged (checked against the previous module), and `run()` is not touched. Tests (venv with git-filter-repo 2.47.0 on PATH): tests/test_adopt_manifest_values.py, tests/test_scaffold_manifest_values.py and tests/test_scaffold_leg_path_collisions.py 289 passed; tests/test_manifest_validator.py, tests/test_repo_hygiene.py and tests/test_cli_reference.py 102 passed; tests/test_update_shape.py and tests/test_family.py, which use the moved constant, 176 passed. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 11 ++++++----- scripts/shape_materialize.py | 11 +++++++---- tests/test_adopt_manifest_values.py | 19 ++++++++++++------- tests/test_scaffold_manifest_values.py | 9 ++++++--- 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index d33a423a..a06bf3a2 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1591,11 +1591,12 @@ def _checked_plan_inputs(args, source: Source, # `_entry_findings`, before any leg exists) ask ONE question of the values: the # `project.yaml` `execute` would write from them, put through the validator the # template ships, through `shape_materialize.manifest_value_problems`, which -# loads that validator rather than copying a rule of it. The plan is read the -# way `execute` reads it (`_template_values`), so the table is the one `execute` -# renders from. Only the validator's question is asked here: whether the -# project's YAML reader reads a value back as the text it is -- `--id true`, -# `--tracking-branch 007` -- is #181's, and the scaffold asks both. +# loads that validator rather than copying a rule of it. The table is built by +# `_template_table`, which `execute`'s `_template_values` also calls, so it is +# the one `execute` renders from. Only the validator's question is asked here: +# whether the project's YAML reader reads a value back as the text it is -- +# `--id true`, `--tracking-branch 007` -- is #181's, and the scaffold asks +# both. #: How `plan` names each value the manifest records: the flag a person typed. MANIFEST_FLAGS = { diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 3f3662fc..7de08013 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -389,6 +389,9 @@ def default_reference(elected_on: str) -> str: #: rather than importing one across a boundary neither module crosses #: otherwise. BOOTSTRAP = "scripts/bootstrap.py" +#: And again for the project's manifest validator, which `COPIED_VERBATIM` and +#: `EXECUTABLE` both carry and `manifest_validator` loads from the template. +MANIFEST_VALIDATOR = "scripts/validate-manifest.py" #: Copied out of openRepoShape's OWN tree, so the project carries the standard #: it was cut from rather than a link to it. @@ -400,7 +403,7 @@ def default_reference(elected_on: str) -> str: #: Copied VERBATIM out of the assembly-root template (no substitution). COPIED_VERBATIM = ( "scripts/validate-pins.py", - "scripts/validate-manifest.py", + MANIFEST_VALIDATOR, BOOTSTRAP, "Makefile", ".gitignore", @@ -443,7 +446,7 @@ def default_reference(elected_on: str) -> str: #: Rendered only when the project DECLARES a neutral-product pin, and written #: once per declared pin at `neutral_pin_path(product)`. NEUTRAL_PIN_TEMPLATE = "contracts/neutral-product-pin.yaml" -EXECUTABLE = ("scripts/validate-pins.py", "scripts/validate-manifest.py", +EXECUTABLE = ("scripts/validate-pins.py", MANIFEST_VALIDATOR, BOOTSTRAP, VALIDATE_NAMING) #: The copy pin, rendered LAST by `_materialize` over the copies it digests. #: Named once because `assembly_root_paths` lists it beside the lists above, @@ -1937,11 +1940,11 @@ def refuse_bad_leg_paths(refusal: str, spec: tuple, code: tuple) -> None: from repo_shape import YamlError, parse_yaml # noqa: E402 #: The assembly root's template directory, and the two files in it these -#: questions read: the manifest, and the validator the project is given. +#: questions read: the manifest, and the validator the project is given +#: (`MANIFEST_VALIDATOR`, named beside the copy lists that carry it). ASSEMBLY_TEMPLATE = (Path(__file__).resolve().parent.parent / "templates" / "assembly-root") MANIFEST_TEMPLATE = "project.yaml" -MANIFEST_VALIDATOR = "scripts/validate-manifest.py" #: The names this module asks of the validator: its `elected_on` pattern and #: the function that makes every FINDING. diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index da56ce6b..020ef16b 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -375,7 +375,7 @@ def test_the_flags_and_the_plan_keys_name_the_same_values(adopter): assert mapped - {"PROJECT_NAME"} <= set(adopter.MANIFEST_FLAGS) -# --- the question is asked with no git at all (NIT-9, SonarCloud S8705) ------ +# --- the question is asked with no git at all ------------------------------- # # The manifest is rendered here to be ASKED ABOUT and never written: the # validator the template ships holds `shape.commit` and `shape.digests. @@ -389,7 +389,7 @@ def test_the_flags_and_the_plan_keys_name_the_same_values(adopter): def no_git_standard(tmp_path_factory): """The standard as a `git archive` or a downloaded zip holds it: every tool, template and contract, and no `.git`, so no revision to read.""" - copy = tmp_path_factory.mktemp("fixV2-no-git") / "standard" + copy = tmp_path_factory.mktemp("no-git-standard") / "standard" shutil.copytree(REPO, copy, symlinks=True, ignore=shutil.ignore_patterns( ".git", "__pycache__", ".pytest_cache", "tests", "docs", "handoffs")) assert not (copy / ".git").exists() @@ -421,7 +421,8 @@ def no_git(*args, **kwargs): findings, remediation = adopter._manifest_findings(plan) assert len(findings) == 1, findings assert findings[0].startswith(FINDING) - assert "visibility is 'secret'" in findings[0] and remediation + assert "visibility is 'secret'" in findings[0] + assert remediation # and it is no finding for a plan the validator would pass good = adopter.Plan.load(world["plan"]) assert adopter._manifest_findings(good) == ([], []) @@ -450,7 +451,8 @@ def no_process(*args, **kwargs): assert adopter._manifest_findings(good, misreads_elsewhere=True) == ( [], []) (finding,), remediation = adopter._manifest_findings(refused) - assert "visibility is 'secret'" in finding and remediation + assert "visibility is 'secret'" in finding + assert remediation @pytest.mark.parametrize("flag, value, named", [ @@ -549,7 +551,8 @@ def test_the_validator_compares_the_shape_commit_with_nothing_and_no_tool_names_ values = adopter._manifest_values(plan, naming) keys = adopter.MANIFEST_PLAN_KEYS asked = shape_materialize.manifest_invalid(values, naming, keys) - assert len(asked) == 1 and "visibility is 'secret'" in asked[0] + assert len(asked) == 1, asked + assert "visibility is 'secret'" in asked[0] head = git("rev-parse", "HEAD", cwd=REPO).stdout.strip() for other in ({"SHAPE_COMMIT": head, "SHAPE_TREE_SHA256": "f" * 64}, {"SHAPE_COMMIT": "A" * 40, "SHAPE_TREE_SHA256": "0" * 64}, @@ -685,7 +688,8 @@ def test_check_refuses_a_text_value_the_reader_misreads(world, old, new, value = new.split(": ", 1)[1].strip('"') findings = [line for line in result.stderr.splitlines() if line.startswith("FINDING ")] - assert findings and any(value in f for f in findings), result.stderr + assert findings, result.stderr + assert any(value in f for f in findings), result.stderr @needs_filter_repo @@ -711,7 +715,8 @@ def test_a_text_value_the_reader_misreads_is_one_finding_on_this_rule_alone( plan = adopter.Plan.load(edited(world, old, new)) findings, remediation = adopter._manifest_findings(plan) assert len(findings) == 1, findings - assert findings[0].startswith(FINDING) and named in findings[0] + assert findings[0].startswith(FINDING) + assert named in findings[0] assert remediation diff --git a/tests/test_scaffold_manifest_values.py b/tests/test_scaffold_manifest_values.py index a7db4206..6f69ed6f 100644 --- a/tests/test_scaffold_manifest_values.py +++ b/tests/test_scaffold_manifest_values.py @@ -529,7 +529,8 @@ def test_the_names_a_refusal_shows_are_told_apart_by_their_tails(): shape_materialize.refuse_unstorable_names( "r", [name, f"{name}-spec", f"{name}-code"]) sentences = caught.value.detail.rstrip(".").split("; ") - assert len(sentences) == 2 and len(set(sentences)) == 2, sentences + assert len(sentences) == 2, sentences + assert len(set(sentences)) == 2, sentences assert sentences[0].startswith("NNNNNNNNNNNN...NNNNNNN-spec is 256 bytes") assert sentences[1].startswith("NNNNNNNNNNNN...NNNNNNN-code is 256 bytes") # the bare repository's directory adds `.git`, after the tail @@ -674,7 +675,8 @@ def test_a_placeholder_is_what_render_calls_one(tmp_path, monkeypatch): # only the line that holds the one placeholder is asked, and nothing raises sentences = shape_materialize.manifest_misreads(values, what, copy) assert len(sentences) == 1 - assert "id: null" in sentences[0] and "extra" not in sentences[0] + assert "id: null" in sentences[0] + assert "extra" not in sentences[0] # --- what is shown of a line ---------------------------------------------- @@ -697,7 +699,8 @@ def test_a_misread_sentence_holding_a_line_break_is_one_printable_line(char): what = {"TRACKING_BRANCH": "--tracking-branch"} (sentence,) = shape_materialize.manifest_misreads( {"TRACKING_BRANCH": f"a{char}b"}, what) - assert len(sentence.splitlines()) == 1 and sentence.isprintable() + assert len(sentence.splitlines()) == 1 + assert sentence.isprintable() assert "into project.yaml as 'tracking_branch: a\\" in sentence # a printable misread is quoted in backticks, as before (plain,) = shape_materialize.manifest_misreads( From a5438a5cb791da0c71fd394bc16be473110d2c5e Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:15:11 +0000 Subject: [PATCH 11/11] tests/test_scaffold_manifest_values.py: an `--elected-by` of a TAB, `\x0b`, `\x1c` or U+2028 is expected to be refused as `scaffold-unwritable-value`, now that #282's refusal fires first (#258) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #351 (#282, d82a83e) refuses an `--elected-by`, `--reference` or `--name` that no file can hold as one line of text in `_identity`, as `scaffold-unwritable-value`. That happens before this branch renders the manifest and asks the project's validator. A bare TAB, `\x0b`, `\x1c` or U+2028 as `--elected-by` is blank to the validator AND unwritable, so on the merged tree #282's refusal is the one the person sees. Five test cases still expected `scaffold-manifest-invalid` or `scaffold-manifest-` for those values: - `test_a_value_the_validator_refuses_is_refused_before_anything_is_made[elected-by-tab]`; - `test_a_dry_run_refuses_what_the_validator_refuses[elected-by-tab]`; - `test_a_blank_that_is_also_a_line_break_is_refused_too[\x0b|\x1c|
]`. They now expect `scaffold-unwritable-value` with #282's own words ("it holds a control character", "it holds a line break"), and the comment above `LINE_BREAKS` says why. This is lane 5's compose patch for the two PRs, written with #351 for whichever landed second. It is applied unchanged; `git apply --check` was clean. No product code changes: adopt asks the two questions in the same order since the previous merge (`plan-unwritable-value` before `plan-manifest-invalid`). Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_scaffold_manifest_values.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/test_scaffold_manifest_values.py b/tests/test_scaffold_manifest_values.py index 6f69ed6f..34aa19d5 100644 --- a/tests/test_scaffold_manifest_values.py +++ b/tests/test_scaffold_manifest_values.py @@ -52,6 +52,10 @@ INVALID = "REFUSED scaffold-manifest-invalid" TOO_LONG = "REFUSED scaffold-name-too-long" BAD_DATE = "REFUSED election-date-malformed" +#: A free-text value no file can hold as one line of text is refused in +#: `_identity`, before the manifest is rendered (#282), so a blank that is +#: also a control character or a line break is refused there. +UNWRITABLE = "REFUSED scaffold-unwritable-value" #: Where a test's scaffold writes, under `tmp_path`: the bare remotes, the #: tool's work directory, and the temporary directory the tool would @@ -113,8 +117,8 @@ pytest.param(["--elected-by= "], INVALID, ("--elected-by is ' '", "manifest-elected-by"), id="elected-by-blank"), - pytest.param(["--elected-by=\t"], INVALID, ("--elected-by is '\\t'", - "manifest-elected-by"), + pytest.param(["--elected-by=\t"], UNWRITABLE, ("--elected-by is '\\t'", + "it holds a control character"), id="elected-by-tab"), pytest.param(["--reference= "], INVALID, ("--reference is ' '", "manifest-reference"), @@ -143,10 +147,9 @@ #: them rather than on a space and a tab. INVISIBLE = ["\u00a0", "\u2003", "\u3000"] -#: And the ones that are also LINE BREAKS to `str.splitlines`, so the project's -#: reader splits the manifest at them: the manifest is refused as unreadable -#: before the validator is asked, which is a `misread` here, and becomes -#: `invalid` the day `render` spells such a value as an escape (#250). +#: And the ones that are also LINE BREAKS to `str.splitlines`: no file can +#: hold one as one line of text, so `_identity` refuses each as +#: `scaffold-unwritable-value` before the manifest is rendered (#282). LINE_BREAKS = ["\x0b", "\x1c", "\u2028"] @@ -403,8 +406,8 @@ def test_a_blank_that_is_also_a_line_break_is_refused_too(tmp_path, blank): scaffolded into a project whose every reader refuses `project.yaml`.""" result = scaffold(tmp_path, "--dry-run", f"--elected-by={blank}") assert result.returncode == 2, result.stderr + result.stdout - assert "REFUSED scaffold-manifest-" in result.stderr - assert "--elected-by is " in result.stderr + assert f"{UNWRITABLE}: --elected-by is " in result.stderr + assert "it holds a line break" in result.stderr assert result.stdout == "" assert_nothing_was_created(tmp_path)