From 233dc71942bc78f978cbb014bde346ffaa1f1352 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:44:17 +0000 Subject: [PATCH 1/7] templates: `project.yaml`, both leg pins and `family.yaml` write their text placeholders between double quotes, so `--id 007`, `--tracking-branch 2026`, `--spec-path true` or `--org null` are read back as typed instead of as a number, a boolean or no value, and `family.py add` quotes a member's id the same way (#259) The assembly root's `project.yaml` wrote `id`, `elected_on`, `visibility`, `tracking_branch` and both legs' `path` bare, each leg pin its `submodule_path`, and `family.yaml` its `id`, `org`, `created_on`, `visibility` and `tracking_branch`. Every reader of those files reads them with `repo_shape`'s YAML subset, which takes a bare `007` as 7, `2026` and `20261011` as numbers, `1.5` as a float, `true` as True and `null` as None, and the checks on the flags admit all of them. So on c7c02fa: - `scaffold-project.py --id 007` (or `2026`, `true`, `null`, `20261011`) exited 0 with a project whose own gate failed `manifest-id`, and `--spec-path 2026` one that failed `manifest-leg-path` and `pin-path-mismatch`; - `--tracking-branch 007` pushed each leg on `007`, then the root's own `bootstrap.py` put the legs on a local `7` with no upstream, and `true` and `null` did the same on `True` and `main`; - `adopt-project.py execute` wrote the same lines under `adoption verified`; - `family.py init --id 007` or `--org null` pushed a holder its own `validate-family.py` refused (`family-id`, `family-org`). Brett Heap ruled on 2026-10-12, "quote #259". `render` already spells a value between a YAML template's double quotes with `double_quoted` (#284), so each of these values now reads back exactly; an ordinary value renders byte for byte as before apart from the two quote characters. `org:` is quoted with the family's other text fields: it is the same defect in the same file (`--org 2026` and `--org null` pass `checked_value`). `family.py`'s `members:` writer records each member's own `project.yaml` id. That id is read back exactly now, so written bare `007` came back as 7 and the holder reported `member-id-mismatch` for a member it had just pinned. `_member_id` quotes an id only where the reader would not take the bare text back, so no existing holder's block moves by a byte. No pinned digest moves: the four files are TEMPLATED, not COPIED, so no row of any project's `contracts/shape-pin.yaml` names them. Projects made before this keep their bare lines, which read correctly for every plain-text value, so the readers keep their `str()` fallbacks. Tests: `tests/test_template_text_placeholders.py` (new) renders every quoted field with eight values the reader misreads, and runs `scaffold-project.py`, `adopt-project.py` `plan`/`check`/`execute` and `family.py init`/`add` into local bare remotes, then a clone, the root's own `bootstrap.py`, its validators and `bump-leg.py`; all 123 fail on c7c02fa. Fourteen existing tests that asserted or edited a bare rendered line now spell it quoted. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 33 +- .../assembly-root/contracts/code-pin.yaml | 2 +- .../assembly-root/contracts/spec-pin.yaml | 2 +- templates/assembly-root/project.yaml | 12 +- templates/family-root/family.yaml | 10 +- tests/test_family.py | 3 +- tests/test_manifest_validator.py | 17 +- tests/test_pin_validator.py | 4 +- tests/test_reference_default.py | 2 +- tests/test_scaffold_e2e.py | 4 +- tests/test_setup_project_py.py | 2 +- tests/test_setup_sh.py | 2 +- tests/test_shape_doctor.py | 6 +- tests/test_template_text_placeholders.py | 390 ++++++++++++++++++ 14 files changed, 454 insertions(+), 35 deletions(-) create mode 100644 tests/test_template_text_placeholders.py diff --git a/scripts/family.py b/scripts/family.py index 583037b2..07915f41 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -79,8 +79,9 @@ from repo_shape import ( # noqa: E402 free_plan_secret_hint, COMMIT_RE, PROJECT_ID_RE, PYTHON, TREE_DIGEST_DEFINITION, - VISIBILITY_CHOICES, NamingPolicy, Refusal, checked_value, git_out, - load_yaml, recorded_gitlink, repo_basename, same_repository, tree_digest, + VISIBILITY_CHOICES, NamingPolicy, Refusal, YamlError, checked_value, + double_quoted, git_out, load_yaml, parse_yaml, recorded_gitlink, + repo_basename, same_repository, tree_digest, ) from shape_materialize import ( # noqa: E402 RULESET_HINT, SHAPE_REPOSITORY, CommandFailed, check_program, @@ -247,12 +248,38 @@ def rewritten(self, rows: list[dict]) -> str: return "\n".join(out) + "\n" +def _member_id(value) -> str: + """A row's `id:` as `_member_lines` spells it: bare where the holder's + reader takes the bare text back as that same text, and between double + quotes where it does not (#259). + + THE MEMBER'S OWN `project.yaml` QUOTES ITS ID NOW. A project scaffolded + with `--id 007` used to write `id: 007`, which every reader took as the + number 7; this row then recorded `str(7)`, and the two agreed on a + misread. The template writes `id: "007"` since #259, so the id read here + is `007`, and written bare it would come back as 7: `validate-family.py` + would report `member-id-mismatch` for a member it had just pinned, and so + would `2026`, `20261011`, `true` and `null`. The reader is asked, not a + list of the words it treats as something else, so a word it learns later + is quoted with no edit here. An id that reads back bare, which is every + ordinary one, is written as it always was, so no existing holder's + `members:` block moves by a byte. + """ + text = str(value) + try: + if parse_yaml(f"id: {text}\n") == {"id": text}: + return text + except YamlError: + pass + return double_quoted(text) + + def _member_lines(row: dict) -> list[str]: """One member as the YAML subset `repo_shape.parse_yaml` reads back.""" pin = row.get("pin") or {} return [ f" - project: {row['project']}", - f" id: {row['id']}", + f" id: {_member_id(row['id'])}", f" repository: {row['repository']}", f" path: {row['path']}", " pin:", diff --git a/templates/assembly-root/contracts/code-pin.yaml b/templates/assembly-root/contracts/code-pin.yaml index aedb47cf..f498b7b2 100644 --- a/templates/assembly-root/contracts/code-pin.yaml +++ b/templates/assembly-root/contracts/code-pin.yaml @@ -19,7 +19,7 @@ kind: pinned_contract_manifest leg_role: code source_repository: {{CODE_REPOSITORY}} -submodule_path: {{CODE_PATH}} +submodule_path: "{{CODE_PATH}}" commit: "{{CODE_COMMIT}}" revision_kind: commit diff --git a/templates/assembly-root/contracts/spec-pin.yaml b/templates/assembly-root/contracts/spec-pin.yaml index 796ecd2d..418dd63a 100644 --- a/templates/assembly-root/contracts/spec-pin.yaml +++ b/templates/assembly-root/contracts/spec-pin.yaml @@ -19,7 +19,7 @@ kind: pinned_contract_manifest leg_role: spec source_repository: {{SPEC_REPOSITORY}} -submodule_path: {{SPEC_PATH}} +submodule_path: "{{SPEC_PATH}}" commit: "{{SPEC_COMMIT}}" revision_kind: commit diff --git a/templates/assembly-root/project.yaml b/templates/assembly-root/project.yaml index 75888d19..c4a4b0e0 100644 --- a/templates/assembly-root/project.yaml +++ b/templates/assembly-root/project.yaml @@ -28,7 +28,7 @@ kind: project-manifest # identically and owes no declaration. # =========================================================================== -id: {{PROJECT_ID}} +id: "{{PROJECT_ID}}" name: "{{PROJECT_NAME}}" # The elected schema, and the document the election followed. For a project @@ -44,7 +44,7 @@ reference: "{{REFERENCE}}" # Electing the shape is a human's act (a `PA` decision, `CA`-constrained and # `PM`-sequenced), so the manifest records whose act it was and when. elected_by: "{{ELECTED_BY}}" -elected_on: {{ELECTED_ON}} +elected_on: "{{ELECTED_ON}}" # The GitHub topic every one of this project's repositories carries. Names # carry the human eye, topics carry the organisation's search, and the @@ -59,12 +59,12 @@ topic: {{TOPIC}} # in-place adoption it is the value the two NEW legs were created with — the # assembly root IS the adopted source and keeps whatever visibility it # already had on GitHub, unchanged by this tool. -visibility: {{VISIBILITY}} +visibility: "{{VISIBILITY}}" # The branch `scripts/bootstrap.py` places each leg on, AT its pinned commit. # The pin stays exact: `.gitmodules` carries no `branch=`, so there is exactly # one authoritative answer to "what commit is this project". -tracking_branch: {{TRACKING_BRANCH}} +tracking_branch: "{{TRACKING_BRANCH}}" # The openRepoShape revision this project was scaffolded from, pinned by # commit and digest under the same rule the family's neutral products use: a @@ -156,9 +156,9 @@ legs: {{ASSEMBLY_NAMING}} - role: spec repository: {{SPEC_REPOSITORY}} - path: {{SPEC_PATH}} + path: "{{SPEC_PATH}}" {{SPEC_NAMING}} - role: code repository: {{CODE_REPOSITORY}} - path: {{CODE_PATH}} + path: "{{CODE_PATH}}" {{CODE_NAMING}} diff --git a/templates/family-root/family.yaml b/templates/family-root/family.yaml index 3600b9f8..0aff4ac8 100644 --- a/templates/family-root/family.yaml +++ b/templates/family-root/family.yaml @@ -30,25 +30,25 @@ kind: family-manifest # this file assert anything about a tree it does not own. # =========================================================================== -id: {{FAMILY_ID}} +id: "{{FAMILY_ID}}" name: "{{FAMILY_NAME}}" -org: {{ORG}} +org: "{{ORG}}" repository: {{FAMILY_REPOSITORY}} # Creating a family is a human's act, like electing the shape, so the manifest # records whose act it was and when. created_by: "{{CREATED_BY}}" -created_on: {{CREATED_ON}} +created_on: "{{CREATED_ON}}" # `private`, `public` or `internal`. NOT ENFORCED HERE — GitHub is the source # of truth for what a repository's visibility actually is — but recording it # is what lets a reader tell why the members need a credential in CI. -visibility: {{VISIBILITY}} +visibility: "{{VISIBILITY}}" # The branch `scripts/bootstrap.py` places each member on, AT its pinned # commit. `.gitmodules` carries no `branch=`: there is exactly one # authoritative answer to "what commit of this member is this family". -tracking_branch: {{TRACKING_BRANCH}} +tracking_branch: "{{TRACKING_BRANCH}}" # The directory every member is mounted under. One name, so a reader and a # validator cannot disagree about where to look. diff --git a/tests/test_family.py b/tests/test_family.py index 43ef3a41..efa586f8 100644 --- a/tests/test_family.py +++ b/tests/test_family.py @@ -1927,7 +1927,8 @@ def test_siblings_skips_a_clone_whose_project_id_is_not_the_rows(estate): sibling = estate["folder"] / "IRSS" manifest_path = sibling / "project.yaml" manifest_path.write_text( - manifest_path.read_text().replace("id: irss", "id: something-else", 1)) + manifest_path.read_text().replace('id: "irss"', 'id: "something-else"', + 1)) result = siblings(estate["holder"]) assert result.returncode == 1, result.stdout + result.stderr assert "WRONG PROJECT" in result.stdout diff --git a/tests/test_manifest_validator.py b/tests/test_manifest_validator.py index b79bc4f8..5fba4ffc 100644 --- a/tests/test_manifest_validator.py +++ b/tests/test_manifest_validator.py @@ -41,7 +41,8 @@ def test_a_topic_that_is_not_derived_from_the_id_is_a_finding(project): def test_a_missing_role_is_a_finding(project): edit(project, - ' - role: code\n repository: testorg/Atlas-code\n path: code\n', + ' - role: code\n repository: testorg/Atlas-code\n' + ' path: "code"\n', "") result = validate(project) assert result.returncode == 1 @@ -103,7 +104,7 @@ def test_an_election_with_no_elector_is_a_finding(project): def test_a_bad_id_is_a_finding(project): - edit(project, "id: atlas", "id: Atlas") + edit(project, 'id: "atlas"', 'id: "Atlas"') result = validate(project) assert result.returncode == 1 assert "manifest-id" in result.stderr @@ -221,8 +222,8 @@ def test_a_declared_descendant_may_be_the_assembly_root(project): def test_the_pin_file_makes_the_referent_real(project): edit(project, "neutral_product_pins: []", "neutral_product_pins:\n - openChart") - edit(project, "repository: testorg/Atlas-code\n path: code", - "repository: testorg/MedxChart\n path: code") + edit(project, 'repository: testorg/Atlas-code\n path: "code"', + 'repository: testorg/MedxChart\n path: "code"') edit(project, " form: project-leg\n role: code\n" " also_matches: []", " form: domain-descendant\n role: ~\n" @@ -257,8 +258,8 @@ def test_the_x_stem_spelling_of_a_referent_is_accepted_in_a_manifest(project): the CANONICAL `openFactory` spelling for the same referent set.""" edit(project, "neutral_product_pins: []", "neutral_product_pins:\n - openxFactory") - edit(project, "repository: testorg/Atlas-code\n path: code", - "repository: testorg/codexFactory\n path: code") + edit(project, 'repository: testorg/Atlas-code\n path: "code"', + 'repository: testorg/codexFactory\n path: "code"') edit(project, " form: project-leg\n role: code\n" " also_matches: []", " form: domain-descendant\n role: ~\n" @@ -493,8 +494,8 @@ def test_a_neutral_product_root_that_drops_the_overlap_is_a_finding(project): def test_a_neutral_product_is_still_not_a_spec_or_code_leg(project): """The edge of the relaxation, checked where a manifest can actually express it: the admission is `assembly` alone.""" - edit(project, "repository: testorg/Atlas-code\n path: code", - "repository: testorg/openDox\n path: code") + edit(project, 'repository: testorg/Atlas-code\n path: "code"', + 'repository: testorg/openDox\n path: "code"') result = validate(project) assert result.returncode == 1 assert "naming-not-a-leg" in result.stderr diff --git a/tests/test_pin_validator.py b/tests/test_pin_validator.py index 70254fc3..cefcc200 100644 --- a/tests/test_pin_validator.py +++ b/tests/test_pin_validator.py @@ -237,8 +237,8 @@ def test_a_missing_shape_pin_refuses(project): def test_a_pin_path_that_disagrees_with_the_manifest_is_a_finding(project): - edit_pin(project, "spec-pin.yaml", "submodule_path: spec", - "submodule_path: specification") + edit_pin(project, "spec-pin.yaml", 'submodule_path: "spec"', + 'submodule_path: "specification"') result = validate(project) assert result.returncode == 1 assert "pin-path-mismatch" in result.stderr diff --git a/tests/test_reference_default.py b/tests/test_reference_default.py index 764727eb..37c6941b 100644 --- a/tests/test_reference_default.py +++ b/tests/test_reference_default.py @@ -99,7 +99,7 @@ def test_the_scaffold_writes_the_reference_the_election_date_chooses( anything is created, and says that the date is what chose it.""" result, manifest = scaffold(tmp_path, "--elected-on", elected_on) assert f'reference: "{expected}"' in manifest - assert f"elected_on: {elected_on}" in manifest + assert f'elected_on: "{elected_on}"' in manifest assert f"reference {expected}" in result.stdout assert "(chosen by the election date)" in result.stdout diff --git a/tests/test_scaffold_e2e.py b/tests/test_scaffold_e2e.py index cb8ca028..df9b4a75 100644 --- a/tests/test_scaffold_e2e.py +++ b/tests/test_scaffold_e2e.py @@ -82,7 +82,7 @@ def test_a_descendant_form_project_scaffolds_and_records_the_overlap(tmp_path): # a leg whose name does NOT match the claim form records an empty overlap assert (" - role: spec\n" " repository: MedxSoft/MedxScribe-spec\n" - " path: spec\n" + ' path: "spec"\n' " naming:\n" " form: project-leg\n" " role: spec\n" @@ -132,7 +132,7 @@ def test_a_neutral_product_elects_the_shape_and_carries_three_repositories(tmp_p # matches the neutral form not at all. " - role: spec\n" " repository: testorg/openDox-spec\n" - " path: spec\n" + ' path: "spec"\n' " naming:\n" " form: project-leg\n" " role: spec\n" diff --git a/tests/test_setup_project_py.py b/tests/test_setup_project_py.py index a65ba2d9..7285ae98 100644 --- a/tests/test_setup_project_py.py +++ b/tests/test_setup_project_py.py @@ -272,7 +272,7 @@ def test_an_internal_visibility_is_accepted(tmp_path): assert result.returncode == 0, result.stderr + result.stdout assert "visibility internal" in result.stdout manifest = (tmp_path / "work" / "Sample" / "project.yaml").read_text() - assert "visibility: internal" in manifest + assert 'visibility: "internal"' in manifest def test_a_name_outside_the_naming_policy_stops_before_the_scaffold(tmp_path): diff --git a/tests/test_setup_sh.py b/tests/test_setup_sh.py index 338807e3..28c3b9ae 100644 --- a/tests/test_setup_sh.py +++ b/tests/test_setup_sh.py @@ -212,7 +212,7 @@ def test_an_internal_visibility_is_accepted(tmp_path): assert result.returncode == 0, result.stderr + result.stdout assert "visibility internal" in result.stdout manifest = (tmp_path / "work" / "Sample" / "project.yaml").read_text() - assert "visibility: internal" in manifest + assert 'visibility: "internal"' in manifest def test_a_name_outside_the_naming_policy_stops_before_the_scaffold(tmp_path): diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index ed390940..0c8f362f 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2256,8 +2256,8 @@ def test_a_sibling_at_the_right_origin_with_another_id_is_not_the_member( root, sibling = member_beside(holder, tmp_path) manifest = sibling / "project.yaml" manifest.write_text( - manifest.read_text(encoding="utf-8").replace("id: irrs", - "id: somethingelse"), + manifest.read_text(encoding="utf-8").replace('id: "irrs"', + 'id: "somethingelse"'), encoding="utf-8") row = rows_of(doctor(standard, root, "--json"))["members"] entry = row["detail"]["members"][0] @@ -3517,7 +3517,7 @@ def test_a_leg_the_manifest_points_outside_the_root_is_never_read(standard, manifest = project / "project.yaml" manifest.write_text( manifest.read_text(encoding="utf-8").replace( - "path: spec", "path: ../outside", 1), encoding="utf-8") + 'path: "spec"', 'path: "../outside"', 1), encoding="utf-8") row = rows_of(doctor(standard, project, "--json"))["placement"] legs = {entry["role"]: entry for entry in row["detail"]["legs"]} assert "outside" in legs["spec"]["state"], legs diff --git a/tests/test_template_text_placeholders.py b/tests/test_template_text_placeholders.py new file mode 100644 index 00000000..704e359d --- /dev/null +++ b/tests/test_template_text_placeholders.py @@ -0,0 +1,390 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A text value YAML would read as another type reads back as typed (#259). + +The assembly root's `project.yaml`, its two leg pins and a family holder's +`family.yaml` wrote their text placeholders bare: `id: {{PROJECT_ID}}`, +`tracking_branch: {{TRACKING_BRANCH}}`, `path: {{SPEC_PATH}}`, +`submodule_path: {{SPEC_PATH}}`. Every reader of those files reads them with +`repo_shape`'s YAML subset, which takes a bare `007` as the number 7, `2026` +and `20261011` as numbers, `1.5` as a float, `true` as True and `null` as +None. The checks on the flags (`PROJECT_ID_RE`, `checked_value`, the leg-path +rule) admit every one of them, so on main: + +- `scaffold-project.py --id 007` wrote a project whose own gate failed + `manifest-id`, and `--spec-path 2026` one that failed `manifest-leg-path`; +- `--tracking-branch 007` pushed each leg on `007`, and the root's own + `bootstrap.py` then put the legs on a local `7` with no upstream (`true` + and `null` did the same on `True` and `main`); +- `adopt-project.py execute` wrote the same lines, and `family.py init` wrote + a holder its own `validate-family.py` refused (`family-id`, `family-org`). + +Brett Heap ruled on 2026-10-12, "quote #259". The templates write each of +those fields between double quotes, `render` spells the value there with +`double_quoted` (#284), and the reader takes it back exactly. A project made +before this keeps its bare lines, which read correctly for every name that is +plain text, so the readers keep their `str()`. + +NO NETWORK AND NO GITHUB: templates rendered in memory, and scaffolds, +adoptions and families into local bare remotes (`--local-remote-dir`), which +never call `gh`. +""" + +from __future__ import annotations + +import re +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import (ADOPT, FILE_PROTOCOL, REPO, SCAFFOLD, git, + make_source_repo, run_script) + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import load_yaml, parse_yaml # noqa: E402 +from shape_materialize import render # noqa: E402 + +FAMILY = REPO / "scripts" / "family.py" +BUMP = REPO / "scripts" / "bump-leg.py" +PROJECT = "Atlas" +ADOPTED = "Northwind" +HOLDER = "Contoso" +ORG = "testorg" +ADOPT_BRANCH = "adopt/three-repo-shape" + +PLACEHOLDER = re.compile(r"\{\{([A-Z0-9_]+)\}\}") + +#: Text the reader takes as something else when it stands bare after a key: +#: numbers, a float, the keywords it knows, and the dates no ISO form spells. +MISREAD = ("2026", "007", "1.5", "true", "null", "20261011", "~", "False") + +#: `(template, placeholder, where the loaded document holds it)` for every +#: text field a tool fills from a value it was given. A leg is found by its +#: role, never by its position in `legs:`. +TEXT_FIELDS = [ + ("templates/assembly-root/project.yaml", "PROJECT_ID", ("id",)), + ("templates/assembly-root/project.yaml", "ELECTED_ON", ("elected_on",)), + ("templates/assembly-root/project.yaml", "VISIBILITY", ("visibility",)), + ("templates/assembly-root/project.yaml", "TRACKING_BRANCH", + ("tracking_branch",)), + ("templates/assembly-root/project.yaml", "SPEC_PATH", + ("legs", "spec", "path")), + ("templates/assembly-root/project.yaml", "CODE_PATH", + ("legs", "code", "path")), + ("templates/assembly-root/contracts/spec-pin.yaml", "SPEC_PATH", + ("submodule_path",)), + ("templates/assembly-root/contracts/code-pin.yaml", "CODE_PATH", + ("submodule_path",)), + ("templates/family-root/family.yaml", "FAMILY_ID", ("id",)), + ("templates/family-root/family.yaml", "ORG", ("org",)), + ("templates/family-root/family.yaml", "CREATED_ON", ("created_on",)), + ("templates/family-root/family.yaml", "VISIBILITY", ("visibility",)), + ("templates/family-root/family.yaml", "TRACKING_BRANCH", + ("tracking_branch",)), +] + + +def held(document, where: tuple): + """The value at `where`: a key, or under `legs` the leg with that role.""" + node = document + for step in where: + if isinstance(node, list): + node = next(leg for leg in node if leg.get("role") == step) + else: + node = node[step] + return node + + +def rendered(template: str, placeholder: str, value: str) -> str: + """`template` with `value` in `placeholder` and a harmless stand-in in + every other placeholder: a line of its own for a block placeholder.""" + text = (REPO / template).read_text(encoding="utf-8") + values = {} + for key in set(PLACEHOLDER.findall(text)): + if key == placeholder: + values[key] = value + elif re.search(r"^\{\{" + key + r"\}\}", text, re.M): + values[key] = f" stand_in_{key.lower()}: 1" + else: + values[key] = f"v-{key.lower()}" + return render(text, values, template) + + +@pytest.mark.parametrize("value", MISREAD) +@pytest.mark.parametrize( + "template,placeholder,where", TEXT_FIELDS, + ids=[f"{t.split('/')[-1]}:{'.'.join(w)}" for t, _, w in TEXT_FIELDS]) +def test_a_text_field_reads_back_as_the_text_it_was_given(template, + placeholder, where, + value): + assert held(parse_yaml(rendered(template, placeholder, value)), + where) == value + + +# --- scaffold-project.py, end to end --------------------------------------- +# +# Each row puts a different value in each field, so that across the rows +# every field holds every value its flag admits: `--id` refuses `1.5` (a dot +# is not in `PROJECT_ID_RE`), and every other field takes all six. + +SCAFFOLDS = [ + # --id, --tracking-branch, --spec-path, --code-path + ("true", "2026", "007", "1.5"), + ("null", "007", "1.5", "true"), + ("20261011", "1.5", "true", "null"), + ("2026", "true", "null", "20261011"), + ("007", "null", "20261011", "2026"), + ("007", "20261011", "2026", "007"), +] + + +def heads(bare: Path) -> list[str]: + return git("for-each-ref", "--format=%(refname:short)", "refs/heads", + cwd=bare).stdout.split() + + +def clone(remote: Path, target: Path, *extra: str) -> Path: + proc = subprocess.run( + ["git", *FILE_PROTOCOL, "clone", "-q", "--recurse-submodules", + *extra, str(remote), str(target)], + capture_output=True, text=True, check=False) + assert proc.returncode == 0, proc.stderr + return target + + +def assert_the_root_reads_what_it_was_given(manifest: dict, pins: dict, + project_id: str, branch: str, + spec: str, code: str) -> None: + assert manifest["id"] == project_id + assert manifest["tracking_branch"] == branch + assert held(manifest, ("legs", "spec", "path")) == spec + assert held(manifest, ("legs", "code", "path")) == code + assert pins == {"spec": spec, "code": code} + + +def pin_paths(root: Path, show=None) -> dict: + """Each leg pin's `submodule_path`, read from disk or through `show`.""" + out = {} + for role in ("spec", "code"): + rel = f"contracts/{role}-pin.yaml" + pin = parse_yaml(show(rel)) if show else load_yaml(root / rel) + out[role] = pin["submodule_path"] + return out + + +def assert_the_clone_works(root: Path, branch: str, spec: str, + code: str) -> None: + """The root's own bootstrap puts each leg on `branch` with its upstream, + and the root's own three validators pass.""" + booted = run_script(root / "scripts" / "bootstrap.py", cwd=root) + assert booted.returncode == 0, booted.stdout + booted.stderr + for path in (spec, code): + leg = root / path + assert git("symbolic-ref", "--short", "HEAD", cwd=leg).stdout.strip() \ + == branch, f"the leg at {path!r} is not on {branch!r}" + assert git("rev-parse", "--abbrev-ref", "@{upstream}", + cwd=leg).stdout.strip() == f"origin/{branch}" + for validator, args in (("validate-manifest.py", ()), + ("validate-pins.py", ()), + ("validate-repository-naming.py", + ("--project", "project.yaml"))): + checked = run_script(root / "scripts" / validator, *args, cwd=root) + assert checked.returncode == 0, \ + f"{validator}: {checked.stdout}{checked.stderr}" + + +@pytest.mark.parametrize("project_id,branch,spec,code", SCAFFOLDS, + ids=["-".join(row) for row in SCAFFOLDS]) +def test_a_scaffold_reads_back_every_value_it_was_given(tmp_path, project_id, + branch, spec, code): + remotes, work = tmp_path / "remotes", tmp_path / "work" + result = run_script(SCAFFOLD, "--org", ORG, "--project", PROJECT, + "--id", project_id, "--tracking-branch", branch, + "--spec-path", spec, "--code-path", code, + "--elected-by", "Test Human", + "--elected-on", "2026-09-02", + "--local-remote-dir", str(remotes), + "--work-dir", str(work)) + assert result.returncode == 0, result.stderr + result.stdout + written = work / PROJECT + assert_the_root_reads_what_it_was_given( + load_yaml(written / "project.yaml"), pin_paths(written), project_id, + branch, spec, code) + for name in (PROJECT, f"{PROJECT}-spec", f"{PROJECT}-code"): + assert heads(remotes / f"{name}.git") == [branch], name + + root = clone(remotes / f"{PROJECT}.git", tmp_path / "clone" / PROJECT) + assert_the_clone_works(root, branch, spec, code) + + # `bump-leg.py` reads the branch and the leg's path out of the same + # manifest. On the tracking branch it refuses, which it can only do if it + # read the branch the root is on; on a branch of its own it finds the + # leg's checkout at its path and reports the pin it already holds. + pinned = git("rev-parse", f"HEAD:{spec}", cwd=root).stdout.strip() + bump = ("--root", str(root), "--leg", "spec", "--to", pinned, + "--local-remote-dir", str(remotes)) + refused = run_script(BUMP, *bump) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert "bump-leg-on-tracking-branch" in refused.stderr + git("switch", "-q", "-c", "bump/spec", cwd=root) + again = run_script(BUMP, *bump) + assert again.returncode == 1, again.stdout + again.stderr + assert "nothing to do" in again.stdout + assert f"at {spec}/" in again.stdout + + +# --- adopt-project.py, end to end ------------------------------------------ +# +# `plan` writes a value the plan's own reader would misread between double +# quotes when it starts with a digit, so `2026`, `007`, `1.5` and `20261011` +# go through the flags. It writes `true` and `null` bare, and the plan's +# reader then takes them as a boolean and as no value: that is the PLAN's +# writer, #181's to fix. Those two rows quote the value in the plan by hand, +# as #181's writer will, and ask `check` and `execute` the rest. + +ADOPTIONS = [ + # --id, --tracking-branch, --spec-path, --code-path, by hand + ("007", "2026", "1.5", "20261011", False), + ("20261011", "007", "2026", "007", False), + ("2026", "1.5", "007", "2026", False), + ("2026", "20261011", "20261011", "1.5", False), + ("true", "true", "null", "true", True), + ("null", "null", "true", "null", True), +] + +pytest_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git filter-repo is not installed: `pip install git-filter-repo`") + +SOURCE_TREE = {"README.md": "# Thing\n", "specs/feature.md": "# Feature\n", + "src/main.py": "VALUE = 1\n"} + + +def quote_in_plan(plan: Path, edits: dict) -> None: + """Replace each `: ` line of the plan with the quoted value, + in bytes, so the plan stays LF on every platform.""" + data = plan.read_bytes() + for line, value in edits.items(): + key = line.split(b":", 1)[0] + assert data.count(line) == 1, line + data = data.replace(line, key + b': "' + value.encode() + b'"\n') + plan.write_bytes(data) + + +@pytest_filter_repo +@pytest.mark.parametrize("project_id,branch,spec,code,by_hand", ADOPTIONS, + ids=["-".join(row[:4]) for row in ADOPTIONS]) +def test_an_adoption_reads_back_every_value_it_was_given(tmp_path, project_id, + branch, spec, code, + by_hand): + source = make_source_repo(tmp_path / "Thing", tree=SOURCE_TREE, edits=()) + plan = tmp_path / "adoption-plan.yaml" + flags = () if by_hand else ("--id", project_id, "--tracking-branch", + branch, "--spec-path", spec, + "--code-path", code) + planned = run_script(ADOPT, "plan", "--source", str(source), + "--project", ADOPTED, "--org", ORG, + "--elected-by", "Test Human", + "--elected-on", "2026-09-02", + "--out", str(plan), *flags) + assert planned.returncode == 0, planned.stderr + planned.stdout + if by_hand: + quote_in_plan(plan, {b"id: northwind\n": project_id, + b"tracking_branch: main\n": branch, + b" spec_path: spec\n": spec, + b" code_path: code\n": code}) + checked = run_script(ADOPT, "check", "--plan", str(plan)) + assert checked.returncode == 0, checked.stderr + checked.stdout + remotes = tmp_path / "remotes" + executed = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(remotes), + "--work-dir", str(tmp_path / "work")) + assert executed.returncode == 0, executed.stderr + executed.stdout + + def show(rel: str) -> str: + return git("show", f"{ADOPT_BRANCH}:{rel}", cwd=source).stdout + + assert_the_root_reads_what_it_was_given( + parse_yaml(show("project.yaml")), pin_paths(source, show), project_id, + branch, spec, code) + for leg in ("spec", "code"): + assert heads(remotes / f"{ADOPTED}-{leg}.git") == [branch], leg + + root = clone(source, tmp_path / "clone" / ADOPTED, "-b", ADOPT_BRANCH) + assert_the_clone_works(root, branch, spec, code) + + +# --- family.py, end to end ------------------------------------------------- + +FAMILIES = [ + # --id, --org, --tracking-branch + ("007", "2026", "true"), + ("2026", "true", "null"), + ("true", "null", "007"), + ("null", "007", "1.5"), + ("20261011", "1.5", "20261011"), + ("007", "20261011", "2026"), +] + + +@pytest.mark.parametrize("family_id,org,branch", FAMILIES, + ids=["-".join(row) for row in FAMILIES]) +def test_a_family_holder_reads_back_every_value_it_was_given(tmp_path, + family_id, org, + branch): + remotes = tmp_path / "remotes" + created = run_script(FAMILY, "init", "--org", org, "--family", HOLDER, + "--id", family_id, "--tracking-branch", branch, + "--created-by", "Test Human", + "--created-on", "2026-09-04", + "--local-remote-dir", str(remotes), + "--work-dir", str(tmp_path / "work")) + assert created.returncode == 0, created.stderr + created.stdout + root = tmp_path / "work" / HOLDER + manifest = load_yaml(root / "family.yaml") + assert (manifest["id"], manifest["org"], manifest["tracking_branch"]) \ + == (family_id, org, branch) + assert heads(remotes / f"{HOLDER}.git") == [branch] + checked = run_script(root / "scripts" / "validate-family.py", cwd=root) + assert checked.returncode == 0, checked.stdout + checked.stderr + + +def test_a_member_whose_id_yaml_would_misread_is_pinned_under_that_id( + tmp_path): + """`family.py add` records the member's OWN `project.yaml` id in its row. + That id is read back exactly now, so the row has to be too: written bare, + `007` came back as 7 and the holder's validator reported + `member-id-mismatch` for a member it had just pinned. An ordinary id is + written as it always was, so no existing holder's block moves.""" + remotes, work = tmp_path / "remotes", tmp_path / "work" + members = {"IRRS": "007", "IRSS": "true", "IRQS": None} + for project, project_id in members.items(): + flags = ("--id", project_id) if project_id else () + made = run_script(SCAFFOLD, "--org", ORG, "--project", project, + *flags, "--elected-by", "Test Human", + "--elected-on", "2026-09-04", + "--local-remote-dir", str(remotes), + "--work-dir", str(work)) + assert made.returncode == 0, made.stderr + made.stdout + created = run_script(FAMILY, "init", "--org", ORG, "--family", HOLDER, + "--created-by", "Test Human", + "--created-on", "2026-09-04", + "--local-remote-dir", str(remotes), + "--work-dir", str(tmp_path / "fam")) + assert created.returncode == 0, created.stderr + created.stdout + root = tmp_path / "fam" / HOLDER + for project in members: + added = run_script(FAMILY, "add", "--family-root", str(root), + "--member", f"{ORG}/{project}", + "--local-remote-dir", str(remotes)) + assert added.returncode == 0, added.stderr + added.stdout + rows = {row["project"]: row["id"] + for row in load_yaml(root / "family.yaml")["members"]} + assert rows == {"IRRS": "007", "IRSS": "true", "IRQS": "irqs"} + lines = (root / "family.yaml").read_bytes().splitlines() + assert b' id: "007"' in lines + assert b" id: irqs" in lines + checked = run_script(root / "scripts" / "validate-family.py", cwd=root) + assert checked.returncode == 0, checked.stdout + checked.stderr From 3894aa80b1eff4eadd61275675759ba22647c1e6 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:16:12 +0000 Subject: [PATCH 2/7] scripts/family.py: `_member_id`'s docstring says a legacy row whose bare id the reader misreads is rewritten quoted, and a test hands `rewritten` a row whose id the reader raises on, so the false "no existing holder's `members:` block moves by a byte" is gone and deleting the `except YamlError` fails 7 tests (#259) The review of 233dc71 found two nits in `_member_id`. N1. The docstring said no existing holder's `members:` block moves by a byte. It does move once: a holder written before #259 holds `id: 2026` for a member scaffolded with `--id 2026`, and the next `bump`, `add` or `remove` rewrites that row as `id: "2026"`. That is harmless, because `validate-family.py` compares the id with `str()` and passes before and after. The docstring now says so, and a row whose id reads back as text does not move. The member test's docstring had the same overstatement ("no existing holder's block moves") and is corrected the same way. The `except` carries a comment naming the ids that reach it. N3. The `try/except YamlError` had no test: with it removed every test still passed. `[`, `{`, `&a`, `*a`, `!t` and an unclosed `'` or `"` raise `YamlError` when read bare, and none of them is a `PROJECT_ID_RE` id, so no tool writes one, but a hand-edited holder can hold one between double quotes. `test_a_member_id_the_reader_refuses_bare_is_rewritten_between_quotes` (7 cases) opens such a holder, rewrites its `members:` block with `Family.rewritten`, and asks that the id read back exactly and the text come out byte for byte. With the `try/except` removed all 7 fail with the reader's `YamlError`, and with the `except` returning the bare text all 7 fail too. No behaviour changes: the `.py` change is a docstring and a comment. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- scripts/family.py | 12 ++++- tests/test_template_text_placeholders.py | 59 +++++++++++++++++++++++- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/scripts/family.py b/scripts/family.py index 07915f41..ca8ca85c 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -262,14 +262,22 @@ def _member_id(value) -> str: would `2026`, `20261011`, `true` and `null`. The reader is asked, not a list of the words it treats as something else, so a word it learns later is quoted with no edit here. An id that reads back bare, which is every - ordinary one, is written as it always was, so no existing holder's - `members:` block moves by a byte. + ordinary one, is written as it always was, so a row whose id reads back + as text does not move. + + A ROW WHOSE BARE ID THE READER MISREADS MOVES, ONCE. A holder written + before #259 holds `id: 2026` for a member scaffolded with `--id 2026`, + and the next `bump`, `add` or `remove` of that holder rewrites the row as + `id: "2026"`. That is harmless: `validate-family.py` compares the id with + `str()`, and passes before and after. """ text = str(value) try: if parse_yaml(f"id: {text}\n") == {"id": text}: return text except YamlError: + # Bare, `[`, `{`, `&a`, `*a`, `!t` and an unclosed quote are not read + # as another type: the reader raises, and the holder would not load. pass return double_quoted(text) diff --git a/tests/test_template_text_placeholders.py b/tests/test_template_text_placeholders.py index 704e359d..c8115bf2 100644 --- a/tests/test_template_text_placeholders.py +++ b/tests/test_template_text_placeholders.py @@ -41,9 +41,12 @@ from conftest import (ADOPT, FILE_PROTOCOL, REPO, SCAFFOLD, git, make_source_repo, run_script) +from test_family import family_module sys.path.insert(0, str(REPO / "scripts")) -from repo_shape import load_yaml, parse_yaml # noqa: E402 +from repo_shape import ( # noqa: E402 + YamlError, double_quoted, load_yaml, parse_yaml, +) from shape_materialize import render # noqa: E402 FAMILY = REPO / "scripts" / "family.py" @@ -357,7 +360,8 @@ def test_a_member_whose_id_yaml_would_misread_is_pinned_under_that_id( That id is read back exactly now, so the row has to be too: written bare, `007` came back as 7 and the holder's validator reported `member-id-mismatch` for a member it had just pinned. An ordinary id is - written as it always was, so no existing holder's block moves.""" + written as it always was, so a row whose id reads back as text does not + move.""" remotes, work = tmp_path / "remotes", tmp_path / "work" members = {"IRRS": "007", "IRSS": "true", "IRQS": None} for project, project_id in members.items(): @@ -388,3 +392,54 @@ def test_a_member_whose_id_yaml_would_misread_is_pinned_under_that_id( assert b" id: irqs" in lines checked = run_script(root / "scripts" / "validate-family.py", cwd=root) assert checked.returncode == 0, checked.stdout + checked.stderr + + +#: Ids the holder's reader RAISES on, rather than reads as another type, when +#: they stand bare after `id:`. `PROJECT_ID_RE` keeps every one out of a +#: scaffold and out of `family.py add`, so no tool writes one; a hand-edited +#: row holds one between double quotes, and the next `bump`, `add` or +#: `remove` spells the whole block again. +UNREADABLE_BARE_IDS = ("[", "{", "&a", "*a", "!t", "'a", '"a') + +HAND_EDITED_HOLDER = """\ +name: Contoso +kind: family-manifest +members: + - project: IRRS + id: {spelled} + repository: testorg/IRRS + path: members/IRRS + pin: + revision_kind: commit + commit: "{commit}" + digest_algorithm: sha256 + digest_definition: sorted-ls-tree-r-v1 + tree_sha256: "{tree}" +""" + + +@pytest.mark.parametrize( + "member_id", UNREADABLE_BARE_IDS, + ids=["flow-sequence", "flow-mapping", "anchor", "alias", "tag", + "open-single-quote", "open-double-quote"]) +def test_a_member_id_the_reader_refuses_bare_is_rewritten_between_quotes( + tmp_path, member_id): + """`_member_id` asks the reader whether the bare id reads back as that + text, and an id the reader RAISES on is not read back as anything: it + gets the quotes, and the rewrite goes on. A call that let the error out + would stop `bump`, `add` and `remove` on a holder that holds such a row, + though the row itself is a quoted scalar the reader takes exactly.""" + with pytest.raises(YamlError): + parse_yaml(f"id: {member_id}\n") + spelled = double_quoted(member_id) + text = HAND_EDITED_HOLDER.format(spelled=spelled, commit="a" * 40, + tree="b" * 64) + (tmp_path / "family.yaml").write_bytes(text.encode("utf-8")) + family = family_module().Family.open(tmp_path) + assert [row["id"] for row in family.members] == [member_id] + + again = family.rewritten(family.members) + + assert parse_yaml(again)["members"][0]["id"] == member_id + assert f" id: {spelled}\n" in again + assert again == text From 77cd0b2093fd2be8876cc032189d3d6702838d90 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 12 Oct 2026 18:07:01 +0000 Subject: [PATCH 3/7] tests/test_scaffold_manifest_values.py, tests/test_adopt_manifest_values.py and scaffold-project.py: #314's tests compose with the quoted templates, so the 40 values it refused as misreads are asserted held as typed, `visibility: "true"` and `elected_on: "20261011"` stay refused in the validator's words, `--tracking-branch=~` is a strict xfail until #355, and `--help` no longer says `--id 007` or `--tracking-branch null` is refused (#259) #314's misread gate (`manifest_misreads`) asks the TEMPLATE how a value is written. With `project.yaml` quoting `id`, `elected_on`, `visibility` and `tracking_branch`, it refuses nothing on the shipped templates, and 45 of #314's tests failed on the merge: 36 in the scaffold file and 9 in the adopt file, the same 45 ids the composition note counted on #314's older head. They are not one kind: - 40 expected a refusal the value no longer needs. Each now asserts that the value is ACCEPTED. A scaffold of `--id null|true|false|007|2026`, or of `--tracking-branch true|True|TRUE|false|False|null|Null|NULL|007|2026|1.5|1.50|0`, exits 0. The manifest it pushes holds the text, and each branch is the one every remote has. `--dry-run` prints the plan, and an id derived from `--project True|Null` is held as `"true"`/`"null"`. `--id=null` and `--tracking-branch=true` reach the forge's `gh repo create`. Adopt's `id: "007"` checks clean and `execute` writes it as typed. No coverage of the rule is dropped. A module copy of the standard writes the four lines bare again, as before #259, and there the rule's sentence, its remediation, the derived-id wording, the line-break sentence and `misread_placeholders` are asserted as before. The shipped templates are asserted to give no misread for any of those values. - 4 are still refused, by #314's validator gate, in other words: `visibility: "true"` and `elected_on: "20261011"` are now text the validator refuses (`visibility is 'true'`, `elected_on is '20261011'`, no longer `True` and `20261011` the number). Their premise that a misread rule judges them, so that the validator is not asked again, no longer holds, and the test now asserts the validator's one finding with or without `misreads_elsewhere`. - 1 is a real break: `--tracking-branch=~` reads back as `~` and dies at `git init -b ~`, after `gh repo create` against GitHub. That is #283, and PR #355's `git check-ref-format --branch` refuses it. No branch check is added here. The row is `xfail(strict=True)`; no test file used xfail before. It asserts the refusal #355 gives, and on a local merge with #355's head it reports XPASS(strict), so #355's CI says when to take the mark off. The adopt file has one test fewer (78). Two `[id-007]` rows, the flipped `..._on_this_rule_alone` and the passing `misreads_elsewhere` one, are one test that asserts both. `scaffold-project.py`'s `--id` and `--tracking-branch` help said a value the reader would read as another type, `null`, `true`, `007`, is refused, and its module docstring and `_refuse_unholdable_manifest` said the same. On this tree each is accepted, so the sentences are dropped or reworded, and `docs/cli.md` is regenerated. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- docs/cli.md | 8 +- scaffold-project.py | 37 +-- tests/test_adopt_manifest_values.py | 158 ++++++++---- tests/test_scaffold_manifest_values.py | 340 +++++++++++++++++++------ 4 files changed, 398 insertions(+), 145 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 08721707..badbfb87 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -246,9 +246,7 @@ 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 (default: --project lowercased). - One the project's YAML reader would read as another - type -- `null`, `true`, `007` -- is refused. + --id ID lowercase project id (default: --project lowercased) --name NAME display name --visibility {private,public,internal} default: private for a fresh scaffold; with --reuse- @@ -279,9 +277,7 @@ options: 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. + main) --spec-path SPEC_PATH where the spec leg is mounted, as a path in the assembly root (default: spec). A path that is, holds diff --git a/scaffold-project.py b/scaffold-project.py index 670b07b8..2ddb284d 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -39,13 +39,16 @@ 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`. +is created and under `--dry-run` alike. A value the reader would turn into +something else where a template line writes it bare is refused as +`scaffold-manifest-misread`. The template writes `id` and `tracking_branch` +between double quotes (#259), so `--id null` or `--tracking-branch 007` is +held as typed, and that refusal guards the template rather than the flags. A +value the validator refuses (a blank `--elected-by`, `--reference` or +`--name`, an `--org` it cannot hold) is refused 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 @@ -299,9 +302,7 @@ def main(argv: list[str] | None = None) -> int: help="the assembly-root name, e.g. Atlas") 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.") + "lowercased)") parser.add_argument("--name", default=None, help="display name") parser.add_argument("--visibility", choices=VISIBILITY_CHOICES, default=None, @@ -334,9 +335,7 @@ def main(argv: list[str] | None = None) -> int: "that did not exist yet.") 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.") + "(default: main)") parser.add_argument("--spec-path", default="spec", help="where the spec leg is mounted, as a path in " "the assembly root (default: spec). A path that " @@ -653,11 +652,13 @@ def _refuse_unholdable_manifest(args, values: dict, policy) -> None: `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. + run is about to write: a value the reader would turn into something else + where a template line writes it bare is `scaffold-manifest-misread` (the + template quotes `id` and `tracking_branch`, #259, so `--id null` and + `--tracking-branch 007` are held as typed), 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. diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index 491ada1d..dd0adce0 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -91,10 +91,9 @@ #: `(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. +#: A text value the reader read as a number while the template wrote it bare +#: (`elected_on: "20261011"`) is not here but in `TEXT_THE_VALIDATOR_REFUSES` +#: below, with the other values the template quotes now (#259). PLAN_EDITS = [ pytest.param("visibility: private", "visibility: secret", ("visibility is 'secret'", "manifest-visibility"), @@ -658,30 +657,39 @@ def test_a_value_the_reader_reads_as_another_kind_is_not_this_questions( assert FINDING not in result.stderr, result.stderr -# --- a text value the reader misreads: judged once, not twice --------------- +# --- a text value the template once wrote bare: judged once, not twice ------ +# +# Each value below is TEXT in the plan that the template wrote bare until #259, +# so the project's reader read it as a number or a boolean: the validator +# refused each in the retyped words (`id is 7`), and a misread rule (#181) +# would have refused the same value. The template quotes all three now, so the +# reader reads each back as typed, and no misread rule refuses any of them. +# What is left is the validator's own answer about the text itself. + +#: `(old line, new line)`: text the validator accepts once it reads it back as +#: typed, so `check` passes it and `execute` writes it into the manifest. +TEXT_THE_TEMPLATE_NOW_HOLDS = [ + pytest.param("id: northwind", 'id: "007"', id="id-007"), +] -#: `(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"), +#: `(old line, new line, what the validator says of it)`: text the validator +#: refuses as the text it is, so it is still refused, by the validator, in its +#: words about that text: `visibility is 'true'`, no longer `visibility is +#: True`. +TEXT_THE_VALIDATOR_REFUSES = [ pytest.param("visibility: private", 'visibility: "true"', - "manifest-visibility: visibility is 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", + "manifest-elected-on: elected_on is '20261011'", id="elected-on-basic-format"), ] -@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.""" +@pytest.mark.parametrize("old, new, named", TEXT_THE_VALIDATOR_REFUSES) +def test_check_refuses_a_text_value_the_validator_refuses(world, old, new, + named): + """`check` finds the value, and names it, in the validator's words.""" plan = edited(world, old, new) result = run_script(ADOPT, "check", "--plan", str(plan)) assert result.returncode == 1, result.stderr + result.stdout @@ -690,28 +698,60 @@ def test_check_refuses_a_text_value_the_reader_misreads(world, old, new, if line.startswith("FINDING ")] assert findings, result.stderr assert any(value in f for f in findings), result.stderr + assert any(f.startswith(FINDING) and named in f for f in findings), ( + result.stderr) + + +@pytest.mark.parametrize("old, new", TEXT_THE_TEMPLATE_NOW_HOLDS) +def test_check_passes_a_text_value_the_template_now_holds(world, old, new): + """`id: "007"` was the number 7 to the reader while the template wrote + `id:` bare, and the validator found `manifest-id: id is 7`. Quoted, it is + the id `007`, which the validator accepts, so the plan checks clean.""" + plan = edited(world, old, new) + result = run_script(ADOPT, "check", "--plan", str(plan)) + assert result.returncode == 0, result.stderr + result.stdout + assert "plan ok" in result.stdout + assert "FINDING " not in 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( +@pytest.mark.parametrize("old, new, named", TEXT_THE_VALIDATOR_REFUSES) +def test_execute_refuses_a_text_value_the_validator_refuses_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 named 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( +@needs_filter_repo +@pytest.mark.parametrize("old, new", TEXT_THE_TEMPLATE_NOW_HOLDS) +def test_execute_writes_a_text_value_the_template_now_holds_as_typed( + adopter, world, old, new): + """`execute` refused this before any leg while the template wrote it + bare. It adopts it now, and the manifest it commits holds the text that + was in the plan.""" + plan = edited(world, old, new) + result = execute(world, plan) + assert result.returncode == 0, result.stderr + result.stdout + assert "adoption verified" in result.stdout + key, value = new.split(": ", 1) + shown = git("show", f"{adopter.ADOPT_BRANCH}:project.yaml", + cwd=world["source"]).stdout + assert repo_shape.parse_yaml(shown)[key] == value.strip('"') + + +@pytest.mark.parametrize("old, new, named", TEXT_THE_VALIDATOR_REFUSES) +def test_a_text_value_the_validator_refuses_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.""" + """With nothing else asking about the value 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 @@ -720,23 +760,43 @@ def test_a_text_value_the_reader_misreads_is_one_finding_on_this_rule_alone( 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.""" +@pytest.mark.parametrize("old, new", TEXT_THE_TEMPLATE_NOW_HOLDS) +def test_a_text_value_the_template_now_holds_is_no_finding_either_way( + adopter, world, old, new): + """The validator accepts it, and the reader reads it back, so it is no + finding on this rule alone, and none where a misread rule judges the rest + (`misreads_elsewhere`).""" plan = adopter.Plan.load(edited(world, old, new)) + assert adopter._manifest_findings(plan) == ([], []) assert adopter._manifest_findings(plan, misreads_elsewhere=True) == ( [], []) +@pytest.mark.parametrize("old, new, named", TEXT_THE_VALIDATOR_REFUSES) +def test_where_a_misread_rule_judges_the_rest_the_validator_still_answers( + adopter, world, old, new, named): + """`misreads_elsewhere` leaves out only what the project's reader would + misread, for a list that holds a misread rule's finding about it (#181's + `plan-manifest-misread`). While the template wrote these bare that was + each of them, and the validator was not asked again. It quotes them now + (#259): the reader reads each back as typed, no misread rule refuses it, + and the validator's finding, in words about the text, is the one with or + without such a rule in the list.""" + plan = adopter.Plan.load(edited(world, old, new)) + shared = adopter._manifest_findings(plan, misreads_elsewhere=True) + assert shared == adopter._manifest_findings(plan) + (finding,), remediation = shared + assert named in finding + assert remediation + + 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.""" + so is a blank `elected_by`. `id: "007"` beside them was a misread while the + template wrote `id:` bare; quoted (#259) it is the id `007`, and no finding + on either list.""" text = world["text"].replace("id: northwind", 'id: "007"') text = text.replace("visibility: private", "visibility: secret") text = text.replace('elected_by: "Test Human"', 'elected_by: " "') @@ -746,21 +806,22 @@ def test_a_value_that_is_not_a_misread_is_still_asked_where_one_judges_the_rest( 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 len(alone) == 2, alone + assert shared == alone 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) + assert not any("manifest-id" in f for f in alone) -@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( +@pytest.mark.parametrize("old, new, named", TEXT_THE_VALIDATOR_REFUSES) +def test_a_text_value_the_validator_refuses_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.""" + finding is the one. A misread rule in the list as well (#181) reads the + template, which writes these values quoted (#259), so it has nothing to + say of them; one that said something would be the pair two rules each make + of one value, and this test fails on a merge that has it.""" plan = adopter.Plan.load(edited(world, old, new)) source = plan.open_source(str(world["source"]), world["tmp"] / "read") findings, _, _ = adopter._entry_findings(plan, source) @@ -769,6 +830,17 @@ def test_a_text_value_the_reader_misreads_is_one_finding_in_the_list_check_print assert len(about) == 1, findings +@pytest.mark.parametrize("old, new", TEXT_THE_TEMPLATE_NOW_HOLDS) +def test_a_text_value_the_template_now_holds_is_no_finding_in_the_list_check_prints( + adopter, world, old, new): + """Not one finding but none: the whole list `check` prints is empty for a + plan whose only edit is a value the validator accepts.""" + 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 findings == [], findings + + # --- a validator that cannot be asked --------------------------------------- def a_copy_of_the_standard(base: Path, remake) -> Path: diff --git a/tests/test_scaffold_manifest_values.py b/tests/test_scaffold_manifest_values.py index 34aa19d5..87e414d8 100644 --- a/tests/test_scaffold_manifest_values.py +++ b/tests/test_scaffold_manifest_values.py @@ -1,13 +1,25 @@ # 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 +`scaffold-project.py` wrote `--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). +THE TEMPLATE NOW QUOTES THEM (#259). `project.yaml` writes `id`, +`elected_on`, `visibility` and `tracking_branch` between double quotes, so +the reader reads each of those values back as typed, and the misread question, +which is asked of the TEMPLATE, refuses none of them: `--id null` and +`--tracking-branch 007` scaffold, and the manifest holds `"null"` and `"007"`. +The question is still asked. A template line that writes such a value bare is +refused again, with no edit to the rule: the tests below show it on a copy of +this standard whose `project.yaml` writes those four lines bare, as it did +before #259. `--tracking-branch=~` now reads back as `~`, which git refuses +as a branch name; that is #283's, and its row is an expected failure until +the fix for #283 (PR #355) lands. + 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 @@ -23,8 +35,8 @@ 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. +bare repositories on disk, or `--dry-run`, which makes nothing; the two tests +that ask whether `gh` is reached put a fake one on PATH. """ from __future__ import annotations @@ -68,10 +80,11 @@ 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 = [ +#: `(flag, value, what the project's reader makes of it bare)`. Each is a +#: legal git branch name or a legal id that the reader takes for something +#: else where a line writes it bare, as `project.yaml` did before #259. The +#: template quotes both fields now, so the manifest holds each one as typed. +ONCE_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"), @@ -93,7 +106,6 @@ 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"), @@ -103,6 +115,37 @@ pytest.param("--tracking-branch", "0", "the number 0", id="branch-0"), ] +#: `(flag, value)`: a value the misread rule refused while the template wrote +#: it bare (`~` is no value at all to the reader) that reads back as typed now, +#: and is still no name git gives a branch. Refusing it is not this file's +#: rule: it is #283's, whose fix asks `git check-ref-format --branch` (PR +#: #355). Until that lands the scaffold passes it and dies at `git init -b ~`, +#: which against GitHub is after `gh repo create` has made all three +#: repositories, so the row is an expected failure that turns into an +#: unexpected pass, and fails, the day #355 refuses it. +NOT_A_BRANCH = [ + pytest.param("--tracking-branch", "~", id="branch-tilde", + marks=pytest.mark.xfail( + strict=True, + reason="#283: `~` reads back as text once the template " + "quotes `tracking_branch` (#259), and git refuses " + "it as a branch name, so the scaffold dies at " + "`git init -b ~` (after `gh repo create` against " + "GitHub); PR #355 refuses it first with `git " + "check-ref-format --branch`. Take this mark off " + "when #355 lands.")), +] + +#: The placeholder each flag of `ONCE_MISREAD_VALUES` fills. +PLACEHOLDER_OF = {"--id": "PROJECT_ID", "--tracking-branch": "TRACKING_BRANCH"} + +#: The `project.yaml` lines #259 quoted that this file's rules name, as +#: `(key, placeholder)`. `written_bare` writes them bare again, as the +#: template did before #259. +QUOTED_BY_259 = (("id", "PROJECT_ID"), ("elected_on", "ELECTED_ON"), + ("visibility", "VISIBILITY"), + ("tracking_branch", "TRACKING_BRANCH")) + #: 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 @@ -171,8 +214,9 @@ def adopter(): def scaffold(tmp_path: Path, *args: str, work: bool = False, - local: bool = True): - """One scaffold of `Atlas` into local bare remotes under `tmp_path`. + local: bool = True, tool: Path = SCAFFOLD): + """One scaffold of `Atlas` into local bare remotes under `tmp_path`, by + this standard's `scaffold-project.py` or, as `tool`, a copy's. 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 @@ -186,16 +230,17 @@ def scaffold(tmp_path: Path, *args: str, work: bool = False, flags += ["--local-remote-dir", str(tmp_path / REMOTES)] if work: flags += ["--work-dir", str(tmp_path / WORK)] - return run_script(SCAFFOLD, *flags, env={ + return run_script(tool, *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 found == [SCRATCH], ( + f"a scaffold that makes nothing created something: {found}") assert list((tmp_path / SCRATCH).iterdir()) == [], ( - "a refused scaffold made a temporary work directory") + "a scaffold that makes nothing made a temporary work directory") def assert_refused(result, code: str) -> None: @@ -204,29 +249,103 @@ def assert_refused(result, code: str) -> None: assert result.stdout == "", "a plan was printed for a refused scaffold" -# --- #258: a value the project's reader turns into something else --------- +def manifest_in(tmp_path: Path, project: str = "Atlas") -> dict: + """The `project.yaml` a scaffold under `tmp_path` pushed, as git holds it + in the bare remote, read by the project's own reader.""" + bare = tmp_path / REMOTES / f"{project}.git" + return parse_yaml(git("show", "HEAD:project.yaml", cwd=bare).stdout) + -@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): +def heads(bare: Path) -> list[str]: + """The branches a bare remote holds.""" + return git("for-each-ref", "--format=%(refname:short)", "refs/heads", + cwd=bare).stdout.split() + + +def written_bare(text: str) -> str: + """`project.yaml`'s text with the lines #259 quoted (`QUOTED_BY_259`) + written bare again, as the template wrote them before it.""" + for key, placeholder in QUOTED_BY_259: + quoted = f'\n{key}: "{{{{{placeholder}}}}}"\n' + assert text.count(quoted) == 1, f"{quoted!r} is not one line of it" + text = text.replace(quoted, f"\n{key}: {{{{{placeholder}}}}}\n") + return text + + +@pytest.fixture(scope="module") +def bare_standard(tmp_path_factory): + """This standard as its own checkout, with `project.yaml` writing the + four lines #259 quoted bare again. The misread rule is asked of the + template, so there it refuses what it refused before #259, and its tools + run from there to show it.""" + return a_copy_of_the_standard(tmp_path_factory.mktemp("bare"), + written_bare, "project.yaml") + + +def bare_template(standard: Path) -> Path: + """The assembly-root template of a copy of the standard.""" + return standard / "templates" / "assembly-root" + + +# --- #258: a value the project's reader turned into something else -------- + +@pytest.mark.parametrize("flag, value, reading", ONCE_MISREAD_VALUES) +def test_a_value_the_reader_once_misread_is_held_as_typed( + tmp_path, bare_standard, flag, value, reading): + """The misread rule refused each of these while `project.yaml` wrote it + bare, and it still refuses each where a template line does (asked here of + the copy of the standard that writes them bare). The template quotes them + now (#259), so this standard scaffolds each one, the manifest it pushes + holds the value as the text that was typed, and a branch is the one every + repository is created on.""" + placeholder = PLACEHOLDER_OF[flag] + (sentence,) = shape_materialize.manifest_misreads( + {placeholder: value}, {placeholder: flag}, + bare_template(bare_standard)) + assert f"{flag} is {value!r}" in sentence + assert f"reads it back as {reading} and not as that text" in sentence + assert shape_materialize.manifest_misreads( + {placeholder: value}, {placeholder: flag}) == [] # `=` 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 result.returncode == 0, result.stderr + result.stdout + manifest = manifest_in(tmp_path) + if flag == "--id": + assert manifest["id"] == value + assert manifest["topic"] == f"xf-project-{value}" + else: + assert manifest["tracking_branch"] == value + for name in ("Atlas", "Atlas-spec", "Atlas-code"): + assert heads(tmp_path / REMOTES / f"{name}.git") == [value], name + + +@pytest.mark.parametrize("flag, value", NOT_A_BRANCH) +def test_a_value_git_refuses_as_a_branch_is_refused_before_anything_is_made( + tmp_path, flag, value): + """No longer a misread, and still not a branch: it must be refused before + anything exists, by the rule that asks git (#283), and not die at the + first `git init -b` as `a git or gh command failed`.""" + result = scaffold(tmp_path, f"{flag}={value}") + assert_refused(result, "REFUSED ") + assert "a git or gh command failed" not in result.stderr + assert flag 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( +def test_a_dry_run_accepts_a_value_once_misread_as_the_real_run_does( 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).""" + """`--dry-run` is a rehearsal, and it answers as the real run does. It + refused these as the real run did while the template wrote them bare; the + template quotes them now (#259), so it prints the plan the real run + carries out, and makes nothing.""" result = scaffold(tmp_path, "--dry-run", f"{flag}={value}") - assert_refused(result, MISREAD) + assert result.returncode == 0, result.stderr + result.stdout + assert "--dry-run: nothing was created." in result.stdout + if flag == "--id": + assert f"Atlas ({value})" in result.stdout assert_nothing_was_created(tmp_path) @@ -235,9 +354,9 @@ def test_a_dry_run_refuses_a_misread_value_as_the_real_run_would( ("--tracking-branch", "007"), ("--id", "2026"), ("--id", "null"), ]) def test_the_remediation_says_what_is_true_of_every_value_it_refuses( - tmp_path, flag, value): + tmp_path, bare_standard, flag, value): """`--tracking-branch 2026` and `1.5` are the values this refusal newly - takes from people: they scaffolded, bootstrapped and got their upstream, + took 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 @@ -245,9 +364,18 @@ def test_the_remediation_says_what_is_true_of_every_value_it_refuses( 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}") + to the reader. + + The template quotes `id:` and `tracking_branch:` now (#259), so this + standard refuses none of them and the remediation is not said; it is + asked of the copy whose template writes them bare, where it is.""" + accepted = scaffold(tmp_path, "--dry-run", f"{flag}={value}") + assert accepted.returncode == 0, accepted.stderr + accepted.stdout + assert "Remediation" not in accepted.stderr + result = scaffold(tmp_path, "--dry-run", f"{flag}={value}", + tool=bare_standard / "scaffold-project.py") assert_refused(result, MISREAD) + assert_nothing_was_created(tmp_path) 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 " @@ -280,17 +408,26 @@ def test_the_remediations_claim_about_str_is_what_the_reader_does( ("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): +def test_an_id_derived_from_the_project_name_is_held_as_typed_too( + tmp_path, bare_standard, 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) + Null` is the id `null`. Where a template line writes `id:` bare it is + refused, and the sentence says where the id came from, so it does not + point at a flag nobody typed (asked of the copy that writes it bare). The + template quotes it now (#259), so this standard scaffolds the project and + the manifest holds the id as that text.""" + refused = scaffold(tmp_path, "--project", project, + tool=bare_standard / "scaffold-project.py") + assert_refused(refused, MISREAD) assert (f"--id (not given, so --project lowercased) is {derived!r}" - in result.stderr) - assert reading in result.stderr + in refused.stderr) + assert reading in refused.stderr assert_nothing_was_created(tmp_path) + result = scaffold(tmp_path, "--project", project) + assert result.returncode == 0, result.stderr + result.stdout + manifest = manifest_in(tmp_path, project) + assert manifest["id"] == derived + assert manifest["topic"] == f"xf-project-{derived}" def test_an_explicit_id_is_the_way_round_for_such_a_project_name(tmp_path): @@ -545,13 +682,10 @@ def test_the_names_a_refusal_shows_are_told_apart_by_their_tails(): # --- 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.""" +def scaffold_against_a_fake_gh(tmp_path: Path, flag: str): + """A scaffold with no `--local-remote-dir`, so it goes to the forge, with + a fake `gh` first on PATH that logs each call and answers no. Returns the + run and the log's path, which exists only if `gh` was asked.""" bin_dir, log = tmp_path / "bin", tmp_path / "gh.log" bin_dir.mkdir() shim = bin_dir / "gh" @@ -566,12 +700,40 @@ def test_a_refused_value_never_reaches_gh(tmp_path, flag): SCAFFOLD, *IDENTITY, flag, "--work-dir", str(tmp_path / WORK), env={"PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", "FAKE_GH_LOG": str(log)}) + return result, log + + +@WINDOWS_SKIP # the `gh` below is a `#!` script, which Windows does not run +@pytest.mark.parametrize("flag", ["--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.""" + result, log = scaffold_against_a_fake_gh(tmp_path, flag) 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() +@WINDOWS_SKIP # the `gh` below is a `#!` script, which Windows does not run +@pytest.mark.parametrize("flag", ["--id=null", "--tracking-branch=true"]) +def test_a_value_once_misread_reaches_gh_as_an_accepted_one_does(tmp_path, + flag): + """These two were refused before the first `gh` call while the template + wrote them bare. It quotes them now (#259), so they pass every question + the scaffold asks, and the run goes on to the forge as an accepted value's + does: the fake `gh` is asked to create the first leg and says no. That is + also the fake answering where `test_a_refused_value_never_reaches_gh` + would have seen a call.""" + result, log = scaffold_against_a_fake_gh(tmp_path, flag) + assert result.returncode == 2, result.stderr + result.stdout + assert "manifest-misread" not in result.stderr + assert log.exists(), "gh was never asked" + assert f"repo create {ORG}/Atlas-spec " in log.read_text() + assert f"gh repo create {ORG}/Atlas-spec " in result.stderr + + # --- one definition, two readers ------------------------------------------ def test_both_tools_ask_the_one_definition(scaffolder, adopter): @@ -629,24 +791,29 @@ def test_every_name_a_refusal_uses_is_a_value_the_tools_hold(scaffolder, 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, - 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}}"') - .replace("tracking_branch: {{TRACKING_BRANCH}}", - 'tracking_branch: "{{TRACKING_BRANCH}}"')) - assert quoted != text - manifest.write_text(quoted, encoding="utf-8") +def test_the_template_quotes_them_so_the_misread_rule_refuses_nothing_there( + scaffolder, bare_standard): + """#259 quoted `id:` and `tracking_branch:` in the template, and + `elected_on:` and `visibility:` with them. The question is asked of the + TEMPLATE, so since it did, no value of any field the scaffold names is + read back as anything but itself, and the rule refuses none, with no edit + to it. Written bare again, the same lines are refused again.""" + text = (REPO / "templates" / "assembly-root" / "project.yaml").read_text( + encoding="utf-8") + for key, placeholder in QUOTED_BY_259: + assert f'\n{key}: "{{{{{placeholder}}}}}"\n' in text, key + once_misread = {param.values[1] for param in ONCE_MISREAD_VALUES} + # and `~`, a basic-format date, and line breaks `render` spells escaped + also = {"~", "20261011", "a\x0bb", "a\u2028b"} + for value in sorted(once_misread | also): + values = dict.fromkeys(scaffolder.MANIFEST_FLAGS, value) + assert shape_materialize.manifest_misreads( + values, scaffolder.MANIFEST_FLAGS) == [], repr(value) 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) == [] + assert shape_materialize.manifest_misreads(values, what) == [] + assert len(shape_materialize.manifest_misreads( + values, what, bare_template(bare_standard))) == 2 # and what the quoted line holds is the text, through the project's reader assert parse_yaml('tracking_branch: "007"\n') == {"tracking_branch": "007"} @@ -660,7 +827,9 @@ def test_a_placeholder_is_what_render_calls_one(tmp_path, monkeypatch): (`{{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`.""" + `template-unsubstituted`. The template quotes `id:` (#259), which the + reader then reads back, so the copy writes it bare again to have a line + whose value is refused.""" monkeypatch.setattr(shape_materialize, "PLACEHOLDER_RE", re.compile(r"\{\{[A-Z0-9_]+\}\}")) assert shape_materialize._placeholders_of( @@ -671,7 +840,7 @@ def test_a_placeholder_is_what_render_calls_one(tmp_path, monkeypatch): ignore=shutil.ignore_patterns("__pycache__", "*.pyc")) manifest = copy / "project.yaml" manifest.write_text( - manifest.read_text(encoding="utf-8") + written_bare(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"} @@ -696,38 +865,53 @@ def test_a_line_a_terminal_would_show_as_something_else_is_quoted_by_python(): @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.""" +def test_a_misread_sentence_holding_a_line_break_is_one_printable_line( + bare_standard, char): + """Where a template line writes the value bare, the manifest is split at + the break, so the reader refuses it; the sentence says so without printing + the break itself. The template quotes `tracking_branch:` now (#259), and + `render` spells the break as an escape between the quotes, so the reader + reads it back and the rule refuses nothing; the sentence is asked of the + copy that writes the line bare.""" what = {"TRACKING_BRANCH": "--tracking-branch"} + assert shape_materialize.manifest_misreads( + {"TRACKING_BRANCH": f"a{char}b"}, what) == [] + bare = bare_template(bare_standard) (sentence,) = shape_materialize.manifest_misreads( - {"TRACKING_BRANCH": f"a{char}b"}, what) + {"TRACKING_BRANCH": f"a{char}b"}, what, bare) 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( - {"TRACKING_BRANCH": "007"}, what) + {"TRACKING_BRANCH": "007"}, what, bare) 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(): +def test_the_misread_placeholders_are_the_ones_the_misread_rule_refuses( + bare_standard): """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.""" + the project's reader and not a list of values: none on this standard's + template, which quotes all three (#259), and the three where a template + writes them bare.""" 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 shape_materialize.misread_placeholders(values, what) == () + assert shape_materialize.manifest_misreads(values, what) == [] + bare = bare_template(bare_standard) + asked = shape_materialize.misread_placeholders(values, what, bare) assert sorted(asked) == ["PROJECT_ID", "TRACKING_BRANCH", "VISIBILITY"] - assert len(shape_materialize.manifest_misreads(values, what)) == len(asked) + assert len(shape_materialize.manifest_misreads(values, what, bare)) == ( + len(asked)) assert shape_materialize.misread_placeholders( - {"PROJECT_ID": "atlas"}, what) == () + {"PROJECT_ID": "atlas"}, what, bare) == () # --- a validator that cannot be asked ------------------------------------- @@ -753,11 +937,12 @@ def test_the_misread_placeholders_are_the_ones_the_misread_rule_refuses(): ] -def a_copy_of_the_standard(base: Path, remake) -> Path: +def a_copy_of_the_standard(base: Path, remake, + rel: str = "scripts/validate-manifest.py") -> 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.""" + with its template's `validate-manifest.py`, or the template file `rel`, + 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")) @@ -765,8 +950,7 @@ def a_copy_of_the_standard(base: Path, remake) -> Path: 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") + path = copy / "templates" / "assembly-root" / rel remade = remake(path.read_text(encoding="utf-8")) if remade is None: path.unlink() From 4d647947e12a64c929ef910c809513f6b720f5c9 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 12 Oct 2026 19:07:55 +0000 Subject: [PATCH 4/7] tests/test_family_add_member_id.py: #395's quoted `id: "{{PROJECT_ID}}"` composes with #381's `member-duplicate-id` test, whose rewrite now matches the id as the template writes it and keeps its quoting, so the full suite no longer fails `test_a_member_whose_id_changes_between_the_probe_and_the_mount_is_unwound` on Linux (#259) #381 landed `member-duplicate-id` with a test that renumbers a member between the probe and the mount by rewriting `id: irqs` to `id: irrs` in the member's `project.yaml` with `re.sub(rb"(?m)^id: irqs$", ...)`, and asserts the line changed. #395 makes the assembly-root template write `id: "irqs"`, so that pattern matched nothing and the assertion failed. The two PRs were green separately and met only when #395 merged main, and the targeted runs that carried #395 never included this file. The rewrite is now `re.sub(rb'(?m)^id: ("?)irqs\1$', rb"id: \1irrs\1", text)`: it matches the id bare or quoted and writes the new id back in the quoting the old one had, so the rewritten member still loads. The `assert changed != text` stays as it was, so the test still fails loudly if the line ever stops matching. Swept `tests/` for every other regex or literal that matches an id, org, created_on, elected_on, visibility, tracking_branch, path or submodule_path line of a template-written file in its unquoted form. The only other one that matches such a line is `tests/test_shape_doctor.py`'s `_bad_family_id` (`^id: .*$` to `id: Bad Id`): it matches the quoted line too, and `Bad Id` is a plain scalar that loads, so it still makes `family-id` the one finding. Nothing else needed a change. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_family_add_member_id.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_family_add_member_id.py b/tests/test_family_add_member_id.py index 88033bc1..5393473d 100644 --- a/tests/test_family_add_member_id.py +++ b/tests/test_family_add_member_id.py @@ -246,7 +246,11 @@ def renumbered(*args, **kwargs): git("clone", "-q", str(remotes / f"{SPARE}.git"), str(work), cwd=tmp_path) text = (work / "project.yaml").read_bytes() - changed = re.sub(rb"(?m)^id: irqs$", b"id: irrs", text) + # The template writes the id quoted (`id: "irqs"`, #259), and a + # hand-written manifest writes it bare: match either, and write the + # new id back in the quoting the old one had, so the member still + # loads and this still fails loudly if the line ever stops matching. + changed = re.sub(rb'(?m)^id: ("?)irqs\1$', rb"id: \1irrs\1", text) assert changed != text (work / "project.yaml").write_bytes(changed) git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qam", From 6cbbac607d11b68516fc5b4de9f1c572c3c5aef5 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Tue, 13 Oct 2026 04:51:36 +0000 Subject: [PATCH 5/7] tests: test_template_text_placeholders.py's member test switches the holder to a branch before add, so the test holds once #487's tracking-branch refusal lands (PR #507) `test_a_member_whose_id_yaml_would_misread_is_pinned_under_that_id` runs `family.py init` and then three real `add`s. PR #507 (#487) makes `add`, `bump` and `remove` refuse on the holder's tracking branch as `family-on-tracking-branch`, so the test now switches the new holder to `family/members` first, in the file's own idiom and as #507's own fixtures do. On main this changes nothing: `add` commits on any branch. Neither PR has to edit this test, whichever lands first. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_template_text_placeholders.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_template_text_placeholders.py b/tests/test_template_text_placeholders.py index c8115bf2..cde1a81b 100644 --- a/tests/test_template_text_placeholders.py +++ b/tests/test_template_text_placeholders.py @@ -379,6 +379,9 @@ def test_a_member_whose_id_yaml_would_misread_is_pinned_under_that_id( "--work-dir", str(tmp_path / "fam")) assert created.returncode == 0, created.stderr + created.stdout root = tmp_path / "fam" / HOLDER + # On a branch of its own, as a person adds members for a pull request: + # #487 (PR #507) makes `add` refuse the holder's tracking branch. + git("switch", "-q", "-c", "family/members", cwd=root) for project in members: added = run_script(FAMILY, "add", "--family-root", str(root), "--member", f"{ORG}/{project}", From 7968f9028d3d168a7eb70f05b0c708b85ac9f760 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Tue, 13 Oct 2026 06:40:29 +0000 Subject: [PATCH 6/7] tests: test_adopt_github_exits.py matches the quoted tracking_branch line the template renders (#259), as main's new test (56713e21) reads the adopted root's project.yaml test_a_source_on_another_branch_opens_its_pull_request_against_it[master] and [trunk] read `git show :project.yaml`, the adopted root's project.yaml, which `execute` renders from templates/assembly-root/ project.yaml. This branch quotes that template's `tracking_branch:` line, so the root says `tracking_branch: "master"`, and the test asked for the bare line. It asks for the quoted one; the branch it expects and every other assertion are unchanged. Left as they were, because a PLAN file is written by adopt-project.py's own `emit`/`y`, not by a template, and `y` leaves `main`, `master` and `trunk` bare: test_adopt_github_exits.py:743 reads the plan `plan --out` wrote, and test_adopt_tracking_branch.py:73-76 edits the plan and reads it back. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_github_exits.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_adopt_github_exits.py b/tests/test_adopt_github_exits.py index 63725dcc..5c757f12 100644 --- a/tests/test_adopt_github_exits.py +++ b/tests/test_adopt_github_exits.py @@ -672,7 +672,7 @@ def test_a_source_on_another_branch_opens_its_pull_request_against_it( assert_pushed(github, branch) root = git("show", f"{SPLIT_BRANCH}:project.yaml", cwd=github / ORG / f"{NORTHWIND}.git").stdout - assert f"\ntracking_branch: {branch}\n" in root, root + assert f'\ntracking_branch: "{branch}"\n' in root, root assert "Base ref must be a branch" not in result.stderr From e5e768bc8fb3b8ceee217d00240212b24bf64bd0 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Tue, 13 Oct 2026 06:58:10 +0000 Subject: [PATCH 7/7] tests: test_template_text_placeholders.py's adoption read-back gives the source the branch each row tracks, which adopt now requires (#479, 56713e21) test_an_adoption_reads_back_every_value_it_was_given plans, checks and executes with a tracking branch of 2026, 007, 1.5, 20261011, true or null, from `--tracking-branch` or by hand in the plan, against a source that had only `main`. Main's 56713e21 (#479, in #493) makes `check` refuse a tracking branch the source lacks, as plan-tracking-branch-missing, and `execute` ask the remote for it (under --local-remote-dir that remote is the source), so all six rows died at `check`. The source now gets a branch of that name before `plan`. Nothing the test asserts changed. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_template_text_placeholders.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_template_text_placeholders.py b/tests/test_template_text_placeholders.py index cde1a81b..4a68103b 100644 --- a/tests/test_template_text_placeholders.py +++ b/tests/test_template_text_placeholders.py @@ -283,6 +283,10 @@ def test_an_adoption_reads_back_every_value_it_was_given(tmp_path, project_id, branch, spec, code, by_hand): source = make_source_repo(tmp_path / "Thing", tree=SOURCE_TREE, edits=()) + # Adopt refuses a tracking branch the source lacks, as + # `plan-tracking-branch-missing` (#479, main's 56713e21), so the source + # carries the branch this row adopts onto: flag rows and by-hand rows. + git("branch", branch, cwd=source) plan = tmp_path / "adoption-plan.yaml" flags = () if by_hand else ("--id", project_id, "--tracking-branch", branch, "--spec-path", spec,