From c6700b7f50a4f3e63ae2b2bbc440f738af3fc965 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 17:16:32 +0000 Subject: [PATCH 1/2] family.py init and shape_materialize.py: where the holder's own YAML reader refuses the manifest `init` would write, `family-manifest-invalid` names the value on the line it refuses, `--org is '@x'`, and gives the reader's remedy, not the validator's (#929) The family half of #459's U119 (U151), which `64ec47c` (#817, #708) left when it did the same for the scaffold and `adopt-project.py`. The reader refuses `repository: @x/`, so `init --org @x --local-remote-dir ` (and `--dry-run`) said "the manifest this run would write is a file the holder's own YAML reader refuses: ...", named no value, and its remedy said "the words after each value are the validator's own", when the words were the reader's and the validator was never asked. - `shape_materialize._unread_sentences` takes the manifest's file name, the `CARRIED_BY` map and the words that name the reader, each the project's unless given, so its sentences and bytes for a project do not move. - `family.py`'s `_manifest_problems` gives each sentence a kind, as `_invalid_problems` does for a project: on the reader's refusal it asks `_unread_sentences` of `family.yaml`, whose `repository:` carries `--org` (`CARRIED_BY`), with "the holder's own YAML reader". `family_manifest_invalid` returns its sentences, as it returned them. - `refuse_unholdable_manifest` refuses `family-manifest-invalid` still, with the remedy for the kind (`KIND_REMEDIATIONS`): the validator's for a finding, and the new `MANIFEST_UNREADABLE_REMEDIATION` for the reader's refusal. That opens with `UNREADABLE_CHOICE`, the first sentence of the project's reader remedy, now its own constant, and says the words are the holder's reader's. Measured before, at 64464d31 (family.py as at 50178305): `init --org @x --family Orbit --created-by 'Test Human' --dry-run --local-remote-dir --work-dir ` exits 2 with the unnamed sentence and the validator's remedy. Now it says `--org is '@x', which the holder's own YAML reader refuses: a plain scalar cannot open with `@`, which is reserved in YAML: '@x/Orbit'.` under the reader's remedy, both runs, nothing created. Tests: tests/test_family_init_yaml_words.py. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 81 ++++++++++++++--- scripts/shape_materialize.py | 43 +++++---- tests/test_family_init_yaml_words.py | 129 +++++++++++++++++++++++++++ 3 files changed, 226 insertions(+), 27 deletions(-) create mode 100644 tests/test_family_init_yaml_words.py diff --git a/scripts/family.py b/scripts/family.py index e8ed8bd6..759b11a4 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -198,6 +198,13 @@ from shape_materialize import ( # noqa: E402 FAMILY_TEMPLATED, PLACEHOLDER_RE, render, ) +# Where the holder's own reader refuses the manifest `init` would write: the +# sentences that name the value on the line it refuses, and the first +# sentence of their remedy, the scaffold's and `adopt-project.py`'s since +# #708, asked of `family.yaml` (#929). +from shape_materialize import ( # noqa: E402 + KIND_INVALID, KIND_UNREADABLE, UNREADABLE_CHOICE, _unread_sentences, +) # How every command this prints for a person to paste spells its paths # (#486), the quoting the holder's `scripts/siblings.py` uses too. from repo_shape import shell_line, shell_quoted # noqa: E402 @@ -3272,17 +3279,50 @@ def family_manifest_invalid(values: dict, policy: NamingPolicy, what: dict, relates to another is asked as the holder asks it. `values` must be the complete table and `policy` the naming policy the holder carries a copy of. The validator's `Report` is asked for its findings alone: its notes - are what it says when nothing is wrong. + are what it says when nothing is wrong. Where the holder's own YAML + reader refuses the file before the validator can be asked, each + sentence is the reader's, naming the value on the line it refuses + (`_manifest_problems`, #929). + """ + return [sentence for _, sentence in _manifest_problems( + values, policy, what, template_dir)] + + +#: The placeholders that carry a value of `what` into `family.yaml` where no +#: line holds that value's own placeholder (#929): the organisation is the +#: owner half of the holder's `repository:`, so the reader's refusal of that +#: line is `--org`'s. `shape_materialize.CARRIED_BY` is the project's. +CARRIED_BY = {"FAMILY_REPOSITORY": "ORG"} + + +def _manifest_problems(values: dict, policy: NamingPolicy, what: dict, + template_dir: Path) -> list[tuple[str, str]]: + """`family_manifest_invalid`'s sentences, each with its kind, as + `shape_materialize` gives a project's (#708): `invalid` for a finding of + the validator's, and `unreadable` where the holder's own YAML reader + refuses the file the validator would read, whose words are the + reader's and whose remedy is its own (`MANIFEST_UNREADABLE_REMEDIATION`). + + THE VALUE IS NAMED (#929). The reader refuses `repository: @x/Orbit`, + so `init --org @x --local-remote-dir ` said only that the manifest + was a file the reader refuses, under a remedy calling the words the + validator's. `_unread_sentences`, the scaffold's and `adopt-project.py`'s + since #708, reads each line of `family.yaml` that carries a value alone + and names the value whose line the reader refuses, by its own + placeholder or by the one `CARRIED_BY` names; where none is, the + sentence is the reader's refusal of the whole file, as before. """ template = (Path(template_dir) / MANIFEST).read_text(encoding="utf-8") try: manifest = parse_yaml(render(template, values, MANIFEST)) except YamlError as exc: - return ["the manifest this run would write is a file the holder's " - f"own YAML reader refuses: {exc}."] + return [(KIND_UNREADABLE, sentence) for sentence in _unread_sentences( + values, what, template_dir, exc, manifest=MANIFEST, + carried_by=CARRIED_BY, reader="the holder's own YAML reader")] if not isinstance(manifest, dict): - return ["the manifest this run would write is not a mapping, which " - "is what the holder's own YAML reader makes of it."] + return [(KIND_INVALID, "the manifest this run would write is not a " + "mapping, which is what the holder's own YAML reader makes " + "of it.")] validator = family_validator(template_dir) # THE VALIDATOR'S OWN DEFECT, NEVER A FINDING. One that raises, one that # EXITS (a `sys.exit(0)` would end this run with a zero, having printed @@ -3305,7 +3345,8 @@ def family_manifest_invalid(values: dict, policy: NamingPolicy, what: dict, raise _validator_unusable( "answered with a finding that is not text", f"its type is {type(finding).__name__}") - return _sentences_of(findings, values, what) + return [(KIND_INVALID, sentence) + for sentence in _sentences_of(findings, values, what)] #: The remediation of a value the reader turns into something else, without @@ -3330,6 +3371,20 @@ def family_manifest_invalid(values: dict, policy: NamingPolicy, what: dict, "it refuses is red from its first commit, because `make validate` and " "the `validate` workflow run it first, and `init` cannot be run again " "over a pushed holder: there is no --force.") +#: Where the holder's own YAML reader refuses the manifest before its +#: validator can be asked (#929), the words are the reader's and a sentence +#: may name no value, so the remedy opens as the project's does +#: (`UNREADABLE_CHOICE`, #708) and says the words are the reader's. +MANIFEST_UNREADABLE_REMEDIATION = ( + f"{UNREADABLE_CHOICE} The words are the holder's own YAML reader's, " + "which refuses the manifest this run would write before its " + "validate-family.py can be asked. A holder whose manifest it refuses is " + "red from its first commit, because `make validate` and the `validate` " + "workflow run that validator first, and `init` cannot be run again over " + "a pushed holder: there is no --force.") +#: The remedy for each kind of problem `_manifest_problems` names. +KIND_REMEDIATIONS = {KIND_INVALID: MANIFEST_INVALID_REMEDIATION, + KIND_UNREADABLE: MANIFEST_UNREADABLE_REMEDIATION} def _manifest_flags(args) -> dict: @@ -3360,6 +3415,9 @@ def refuse_unholdable_manifest(args, values: dict, and alone, because the validator's finding about it (`id is None`) is its consequence; one the validator refuses (`--created-by ' '`, `--name 'Contoso Group'`) is `family-manifest-invalid`, in the validator's words. + A manifest the reader refuses before the validator can be asked (`--org + @x`, `repository: @x/`) is `family-manifest-invalid` still, in + the reader's words and with the reader's remedy (#929). """ flags = _manifest_flags(args) nothing = " Nothing was created, and `--dry-run` refuses the same." @@ -3367,11 +3425,12 @@ def refuse_unholdable_manifest(args, values: dict, if problems: raise Refusal("family-manifest-misread", " ".join(problems), MANIFEST_MISREAD_REMEDIATION + nothing) - problems = family_manifest_invalid( - values, NamingPolicy.load(NAMING_POLICY), flags, template_dir) - if problems: - raise Refusal("family-manifest-invalid", " ".join(problems), - MANIFEST_INVALID_REMEDIATION + nothing) + found = _manifest_problems(values, NamingPolicy.load(NAMING_POLICY), flags, + template_dir) + if found: + raise Refusal("family-manifest-invalid", + " ".join(sentence for _, sentence in found), + KIND_REMEDIATIONS[found[0][0]] + nothing) def cmd_init(args) -> int: diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 922f791a..c373070b 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -5049,11 +5049,19 @@ def _invalid_problems(values: dict, policy: NamingPolicy, what: dict, def _unread_sentences(values: dict, what: dict, template_dir: Path, - refusal: YamlError) -> list[str]: + refusal: YamlError, manifest: str = MANIFEST_TEMPLATE, + carried_by: dict | None = None, + reader: str = "the project's own YAML reader" + ) -> list[str]: """A sentence for each value of `what` that a line of the manifest the project's own reader refuses carries, in the reader's words, and the reader's `refusal` of the whole file where no such line is found. + `manifest`, `carried_by` (`CARRIED_BY` when None) and `reader`, the + words that name the reader, are the project's unless the caller gives + its own: `family.py init` asks it of the holder's `family.yaml`, whose + `repository:` carries `--org` (#929). + THE VALUE IS NAMED AGAIN (#708). Until #392 and #393 the reader took `repository: @x/Atlas` for text, so `--org @x` reached the validator, whose `manifest-leg-repository` names `--org`. Since then the reader @@ -5065,24 +5073,23 @@ def _unread_sentences(values: dict, what: dict, template_dir: Path, `_misread_rows` reads one, and a value is named where the reader refuses one of its lines. """ + carried_by = CARRIED_BY if carried_by is None else carried_by said: dict[str, list[str]] = {} - template = (Path(template_dir) / MANIFEST_TEMPLATE).read_text( - encoding="utf-8") + template = (Path(template_dir) / manifest).read_text(encoding="utf-8") for line in template.splitlines(): - carried = {CARRIED_BY.get(key, key) for key in _placeholders_of(line)} + carried = {carried_by.get(key, key) for key in _placeholders_of(line)} named = [key for key in what if key in carried and key in values] if not named or line.lstrip().startswith("#"): continue - reading = _reading_of(render(line, values, MANIFEST_TEMPLATE)) + reading = _reading_of(render(line, values, manifest)) if isinstance(reading, YamlError): for key in named: said.setdefault(key, []).append(f"{reading}.") if not said: - return ["the manifest this run would write is a file the project's " - f"own YAML reader refuses: {refusal}."] - return [f"{what[key]} is {str(values[key])!r}, which the project's own " - f"YAML reader refuses: {' '.join(words)}" - for key, words in said.items()] + return ["the manifest this run would write is a file " + f"{reader} refuses: {refusal}."] + return [f"{what[key]} is {str(values[key])!r}, which {reader} refuses: " + f"{' '.join(words)}" for key, words in said.items()] def _validator_notes(values: dict, policy: NamingPolicy, template_dir: Path, @@ -5163,13 +5170,17 @@ def manifest_judged(values: dict, policy: NamingPolicy, #: Where the project's own YAML reader refuses the manifest before its #: validator can be asked (#708), the words are the reader's, and a sentence #: may name no value (`_unread_sentences`), so the remedy says neither that -#: they are the validator's nor that a value is always named. -MANIFEST_UNREADABLE_REMEDIATION = ( +#: they are the validator's nor that a value is always named. Its first +#: sentence is `UNREADABLE_CHOICE`, which `family.py init`'s remedy for the +#: holder's reader opens with too (#929). +UNREADABLE_CHOICE = ( "Remediation: choose another value for each one named; where none is " - "named, the text the reader's words quote holds it. The words are the " - "project's own YAML reader's, which refuses the manifest this run would " - "write before its validate-manifest.py can be asked, and a project " - "whose manifest it refuses is one whose first bootstrap would fail.") + "named, the text the reader's words quote holds it.") +MANIFEST_UNREADABLE_REMEDIATION = ( + f"{UNREADABLE_CHOICE} The words are the project's own YAML reader's, " + "which refuses the manifest this run would write before its " + "validate-manifest.py can be asked, and a project whose manifest it " + "refuses is one whose first bootstrap would fail.") #: The remedy for each kind of problem, so a refusal names it by its kind #: (#708), with no conditional nested in another. KIND_REMEDIATIONS = {KIND_MISREAD: MANIFEST_MISREAD_REMEDIATION, diff --git a/tests/test_family_init_yaml_words.py b/tests/test_family_init_yaml_words.py new file mode 100644 index 00000000..ce23c81d --- /dev/null +++ b/tests/test_family_init_yaml_words.py @@ -0,0 +1,129 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`family.py init`: where the holder's own YAML reader refuses the manifest +`init` would write, the refusal names the value on the line it refuses and +gives the reader's remedy (#929, #459's U119 and U151). + +#817 (#708) did this for the scaffold and `adopt-project.py`. The family half +was left: the reader refuses `repository: @x/`, so `init --org @x +--local-remote-dir ` said only "the manifest this run would write is a +file the holder's own YAML reader refuses", named no value, and its remedy +said "the words after each value are the validator's own", when the words +were the reader's and the validator was never asked. Without +`--local-remote-dir`, `--org @x` is refused first as `family-bad-org` (#871), +so this wording is reached where no GitHub repository is made, or by a value +that passes GitHub's checks and that the reader still refuses. + +`init` now asks `shape_materialize._unread_sentences`, the scaffold's and +`adopt-project.py`'s since #708, of `family.yaml`, with `--org` carried by +the holder's `repository:`, and gives the reader's remedy, which opens with +the sentence the project's does (`UNREADABLE_CHOICE`). The code is +`family-manifest-invalid` still, as #708 kept `-manifest-invalid` for the +other two. A finding of the validator's keeps the validator's remedy. + +NO NETWORK AND NO GITHUB: `--local-remote-dir`, a template copy and the +suite's `gh` guard. +""" + +from __future__ import annotations + +import sys + +import pytest + +from conftest import REPO +from test_family_init_values import ( + HOLDER, TEMPLATE, family_module, given, init, nothing_was_created, table, + template_copy, +) +from test_reader_refusals_named import NOT_THE_VALIDATORS, OPENS + +sys.path.insert(0, str(REPO / "scripts")) +import shape_materialize # noqa: E402 + +#: The code all three cases are refused under. +INVALID = "family-manifest-invalid" + +#: The reader's remedy, as `init` gives it, before the sentence it ends with. +READER_REMEDY = ( + "Remediation: choose another value for each one named; where none is " + "named, the text the reader's words quote holds it. The words are the " + "holder's own YAML reader's, which refuses the manifest this run would " + "write before its validate-family.py can be asked. A holder whose " + "manifest it refuses is red from its first commit, because `make " + "validate` and the `validate` workflow run that validator first, and " + "`init` cannot be run again over a pushed holder: there is no --force.") + +#: The sentence every `init` remedy ends with, nothing having been made. +NOTHING = " Nothing was created, and `--dry-run` refuses the same." + +#: How the reader's words begin where they are the holder's. +HOLDERS_READER_REFUSES = "which the holder's own YAML reader refuses: " + + +@pytest.mark.parametrize("dry_run", [True, False], ids=["dry-run", "run"]) +@pytest.mark.parametrize("org", ["@x", "@opensoft"]) +def test_init_names_org_with_the_readers_remedy(tmp_path, org, dry_run): + """The issue's probe, both runs: `--org` is named, with the reader's + words for the holder's `repository:` line, under the reader's remedy, + and nothing is created.""" + result = init(tmp_path, "--org", org, dry_run=dry_run, + without=("--org",)) + assert result.returncode == 2, result.stderr + result.stdout + assert result.stdout == "" + assert (f"REFUSED {INVALID}: --org is {org!r}, {HOLDERS_READER_REFUSES}" + f"{OPENS} `@`, which is reserved in YAML: '{org}/{HOLDER}'.\n" + f"{READER_REMEDY}{NOTHING}\n") in result.stderr + assert NOT_THE_VALIDATORS not in result.stderr + nothing_was_created(tmp_path) + + +def test_a_refused_line_that_carries_no_value_names_the_whole_file( + tmp_path): + """Where the reader refuses a line that carries none of the values, the + sentence is its refusal of the whole file, as it was, and the remedy is + the reader's, which says where none is named, the text its words quote + holds it.""" + template = template_copy(tmp_path, "unreadable", quoted=True) + manifest = template / "family.yaml" + manifest.write_bytes(manifest.read_bytes() + b"notes: @nobody\n") + module = family_module() + with pytest.raises(module.Refusal) as caught: + module.refuse_unholdable_manifest(given(), table(), template) + assert caught.value.code == INVALID + assert caught.value.detail == ( + "the manifest this run would write is a file the holder's own YAML " + f"reader refuses: {OPENS} `@`, which is reserved in YAML: " + "'@nobody'.") + assert caught.value.remediation == READER_REMEDY + NOTHING + + +def test_a_finding_of_the_validator_keeps_the_validators_remedy(): + """The control: `--created-by ' '` reaches the validator, whose finding + is named in its own words under its own remedy, as before.""" + module = family_module() + with pytest.raises(module.Refusal) as caught: + module.refuse_unholdable_manifest(given(created_by=" "), + table(CREATED_BY=" "), TEMPLATE) + assert caught.value.code == INVALID + assert caught.value.detail.startswith( + "--created-by is ' ', which the holder's own validate-family.py " + "refuses: family-created-by: "), caught.value.detail + assert caught.value.remediation == ( + "Remediation: choose a value the holder's own validate-family.py " + "accepts; the words after each value are the validator's own. A " + "holder it refuses is red from its first commit, because `make " + "validate` and the `validate` workflow run it first, and `init` " + "cannot be run again over a pushed holder: there is no --force." + + NOTHING) + + +def test_the_projects_remedy_keeps_its_words(): + """The project's remedy is built from the sentence the holder's opens + with now, and says what it said under #708, byte for byte.""" + assert shape_materialize.MANIFEST_UNREADABLE_REMEDIATION == ( + "Remediation: choose another value for each one named; where none is " + "named, the text the reader's words quote holds it. The words are the " + "project's own YAML reader's, which refuses the manifest this run " + "would write before its validate-manifest.py can be asked, and a " + "project whose manifest it refuses is one whose first bootstrap would " + "fail.") From b8e62456021089005159f6af6c1f9e31b02ce7cf Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 19:58:36 +0000 Subject: [PATCH 2/2] tests/test_family_init_yaml_words.py: the manifest that is not a mapping keeps its kind and the validator's remedy, held by a test (#929) Round 1 of lane 5's review of #960 (issuecomment-6253381140), NIT 3. The not-a-mapping sentence is still `invalid` in `_manifest_problems`, so `init` gives it main's words and the validator's remedy, as the body says. The existing test of it holds only the sentence, so lane 5's mutant M2, which moved its kind to the reader's, survived. The new test holds the detail, the code and the remedy. The validator's remedy is spelled once in the file now, for this test and the validator control. Tests only. NITs 1 and 2 stay as answered, and NIT 4 is a body fix. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- tests/test_family_init_yaml_words.py | 37 ++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/tests/test_family_init_yaml_words.py b/tests/test_family_init_yaml_words.py index ce23c81d..27128516 100644 --- a/tests/test_family_init_yaml_words.py +++ b/tests/test_family_init_yaml_words.py @@ -53,6 +53,16 @@ "validate` and the `validate` workflow run that validator first, and " "`init` cannot be run again over a pushed holder: there is no --force.") +#: The validator's remedy, as `init` gives it, before the sentence it ends +#: with: what a finding of the validator's is told, and what the manifest +#: that is not a mapping is still told (lane 5's review of #960, NIT 3). +VALIDATOR_REMEDY = ( + "Remediation: choose a value the holder's own validate-family.py " + "accepts; the words after each value are the validator's own. A holder " + "it refuses is red from its first commit, because `make validate` and " + "the `validate` workflow run it first, and `init` cannot be run again " + "over a pushed holder: there is no --force.") + #: The sentence every `init` remedy ends with, nothing having been made. NOTHING = " Nothing was created, and `--dry-run` refuses the same." @@ -108,13 +118,26 @@ def test_a_finding_of_the_validator_keeps_the_validators_remedy(): assert caught.value.detail.startswith( "--created-by is ' ', which the holder's own validate-family.py " "refuses: family-created-by: "), caught.value.detail - assert caught.value.remediation == ( - "Remediation: choose a value the holder's own validate-family.py " - "accepts; the words after each value are the validator's own. A " - "holder it refuses is red from its first commit, because `make " - "validate` and the `validate` workflow run it first, and `init` " - "cannot be run again over a pushed holder: there is no --force." - + NOTHING) + assert caught.value.remediation == VALIDATOR_REMEDY + NOTHING + + +def test_a_manifest_that_is_not_a_mapping_keeps_its_kind_and_remedy( + tmp_path): + """LANE 5'S REVIEW OF #960, NIT 3. The manifest that is not a mapping + is still `invalid` in `_manifest_problems`, so it keeps main's words + and the validator's remedy, as the body says. The existing test of it + (`test_family_init_values.py`) holds only the sentence; this holds the + remedy too, so moving its kind to the reader's fails a test.""" + template = template_copy(tmp_path, "list", quoted=True) + (template / "family.yaml").write_bytes(b"- one\n- two\n") + module = family_module() + with pytest.raises(module.Refusal) as caught: + module.refuse_unholdable_manifest(given(), table(), template) + assert caught.value.code == INVALID + assert caught.value.detail == ( + "the manifest this run would write is not a mapping, which is what " + "the holder's own YAML reader makes of it.") + assert caught.value.remediation == VALIDATOR_REMEDY + NOTHING def test_the_projects_remedy_keeps_its_words():