From 60a258f3d14beff9b5b7bb1d9524d45869bc969f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 22:39:49 +0000 Subject: [PATCH 1/3] contracts/path-classification.yaml: a repository's own `single-repository.yaml` is asked drop or root, drop by default, under a rule of its own, so `adopt-project.py plan` no longer asks it the top-level data file's `spec`, `code` or `root` question for a record that no leg reads (#528) `plan` asked about a source's own `single-repository.yaml` under `ambiguous-top-level-yaml`: "a declaration the project has DECIDED (spec), configuration the implementation reads (code), or project-level wiring (root)". The file is none of those. It is this standard's optional record that a project STAYS one repository (`contracts/single-repository-record.yaml`), so a repository that carries one and is adopted into the Triad holds a record of the decision the adoption reverses, and its one reader, the Triad advisory, is silent in an assembly root whatever the file holds (`shape_advisory._read` answers `TRIAD_ROOT` from `project.yaml` first). `ambiguous-single-repository-record`, above `ambiguous-top-level-yaml`, names the file and asks drop or root, drop by default (Brett Heap's ruling of 2026-10-12, the A7 line of #457). The default is the question's own words and not a value the plan fills in: `plan` still writes `leg: null`, and `execute` still refuses with `plan-unresolved` while it is open, so nothing is dropped that a person did not answer `drop`. Nothing refuses `root`, unlike `ambiguous-project-yaml`: the file blocks nothing the shape writes. Only the name the advisory reads is asked this way; `release.yaml`, a record spelled `.yml` and a nested `single-repository.yaml` keep the rule they had. tests/test_adopt_single_repository_record.py (12): the rule names the file `shape_advisory.RECORD_FILE` reads and sits above the general question; the question offers drop and then root, says drop is the default and offers neither `spec` nor `code`; `plan` writes it with `leg: null`; `check` and `execute` say `plan-unresolved` while it is unanswered and `check` passes either answer; and `execute` adopts each answer, with the record in no repository for `drop` and the source's blob at the assembly root for `root`. Five are red on main bfed143 (the rule and question ones). Main: bfed143e89f97d3480cf5373051cb1ace82db312. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- contracts/path-classification.yaml | 41 ++++ tests/test_adopt_single_repository_record.py | 215 +++++++++++++++++++ 2 files changed, 256 insertions(+) create mode 100644 tests/test_adopt_single_repository_record.py diff --git a/contracts/path-classification.yaml b/contracts/path-classification.yaml index 83bf3b5d..57e5e69f 100644 --- a/contracts/path-classification.yaml +++ b/contracts/path-classification.yaml @@ -419,6 +419,47 @@ rules: `adopt-project.py` refuses a plan that keeps the repository's own file there. + # A REPOSITORY'S OWN `single-repository.yaml` is this standard's optional + # record that a project STAYS one repository + # (`contracts/single-repository-record.yaml`), so a repository that carries + # one and is adopted into the Triad holds a record of the very decision the + # adoption reverses (#528). The general question below offers `spec`, + # `code` and `root` for "a declaration, run-time configuration or wiring", + # and the file is none of those: its one reader is the Triad advisory + # (`scripts/shape_advisory.py`), which is silent in an assembly root + # whatever the file holds, so neither leg has any use for it. So it is asked + # a question of its own that names the file and offers drop or root, drop + # by default (Brett Heap's ruling of 2026-10-12, the A7 line of #457). + # + # THE DEFAULT IS THE QUESTION'S OWN WORDS, NOT A VALUE THE PLAN FILLS IN. + # `plan` still writes `leg: null` and `execute` still refuses while it is + # open, because an unanswered question is never an implicit anything and a + # path is dropped only when a person has answered `drop`. Nothing refuses + # `root` here, unlike `ambiguous-project-yaml`: the file blocks nothing the + # shape writes and no validator, gate or doctor row reads it. As for the + # rules above, what `execute` does is said of a repository WHEN IT IS + # ADOPTED; `shape-doctor.py --placement-plan` prints this question too, for + # a leg already split. + - id: ambiguous-single-repository-record + paths: + - "single-repository.yaml" + leg: ambiguous + reason: >- + A top-level `single-repository.yaml` is this standard's record that a + project STAYS a single repository: who decided it, when and why. + Adopting the repository into the Triad is the decision it records being + reversed, so the file no longer describes the project it sits in. It + confers nothing, and its only reader is the Triad advisory, which is + silent in an assembly root whatever the file holds. + question: >- + This file records that the repository stays a single repository, and + adopting it makes the project a Triad. Drop it (drop, the default: the + decision it records is the one being reversed), or keep it in the + assembly root (root) as a record of who decided to stay single and why? + Neither leg has any use for it, and nothing but the Triad advisory + reads it, which is silent in an assembly root whatever it says, so + keeping it breaks nothing. + - id: ambiguous-top-level-yaml paths: - "*.yaml" diff --git a/tests/test_adopt_single_repository_record.py b/tests/test_adopt_single_repository_record.py new file mode 100644 index 00000000..b6108551 --- /dev/null +++ b/tests/test_adopt_single_repository_record.py @@ -0,0 +1,215 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The source's own `single-repository.yaml`, asked about by name (#528). + +`plan` asked about it under `ambiguous-top-level-yaml`, whose question offers +`spec`, `code` and `root` for "a declaration the project has DECIDED, +configuration the implementation reads, or project-level wiring". The file is +none of those: it is this standard's optional record that a project STAYS one +repository (`contracts/single-repository-record.yaml`), and a repository being +adopted into the Triad holds a record of the decision the adoption reverses. +Its one reader is the Triad advisory, which is silent in an assembly root +whatever the file holds, so neither leg has any use for it. + +Ruled 2026-10-12 (the A7 line of #457): a path-policy rule names the file and +asks drop or root, defaulting to drop. That is `ambiguous-single-repository- +record`, above `ambiguous-top-level-yaml`. Its default is the question's own +words: `plan` still writes `leg: null` and `execute` still refuses while it is +open, because an unanswered question is never an implicit anything, and +nothing is dropped that a person did not answer `drop`. +""" + +from __future__ import annotations + +import importlib.util +import shutil +import sys +from argparse import Namespace + +import pytest + +from conftest import (ADOPT, REPO, findings, git, make_source_repo, planned, + run_script, write_plan) + +sys.path.insert(0, str(REPO / "scripts")) +from path_classify import PathPolicy # noqa: E402 +from repo_shape import load_yaml # noqa: E402 +from shape_advisory import RECORD_FILE # noqa: E402 + +needs_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git filter-repo is not installed: `pip install git-filter-repo`") + +PROJECT = "Northwind" +RULE = "ambiguous-single-repository-record" +GENERAL = "ambiguous-top-level-yaml" +POLICY = REPO / "contracts" / "path-classification.yaml" +SPLIT_BRANCH = "adopt/three-repo-shape" +#: A record as the schema's own example writes one. +RECORD = ("schema_version: 1\nkind: single-repository-record\n" + 'decided_by: "Dana Okafor"\ndecided_on: 2026-10-06\n' + 'reason: "A dotfiles repository."\n') +#: The source: a spec, the code, a README the root keeps, the record, and +#: another top-level YAML file that keeps the general question. +TREE = {"src/app.py": "print(1)\n", "docs/spec.md": "# Spec\n", + "README.md": "# Thing\n", RECORD_FILE: RECORD, + "release.yaml": "name: release\n"} + + +@pytest.fixture(scope="module") +def adopter(): + module_spec = importlib.util.spec_from_file_location( + "adopt_single_repository_record", ADOPT) + module = importlib.util.module_from_spec(module_spec) + module_spec.loader.exec_module(module) + return module + + +@pytest.fixture +def source(tmp_path): + return make_source_repo(tmp_path / "Thing", tree=TREE, edits=()) + + +def entry_of(plan, path: str) -> dict: + """The plan's entry for `path`, as the loader reads it.""" + entry, = [row for row in load_yaml(plan)["paths"] if row["path"] == path] + return entry + + +# --- the rule --------------------------------------------------------------- + +def test_the_rule_names_the_file_the_advisory_reads(): + """One name in two places: the rule asks about the file the advisory is + the only reader of, so a rename there is caught here.""" + rules = {rule["id"]: rule for rule, _ in PathPolicy.load(POLICY).rules} + assert rules[RULE]["paths"] == [RECORD_FILE] == ["single-repository.yaml"] + + +def test_the_rule_comes_before_the_general_question(): + """First match wins, so the order is the answer.""" + ids = [rule["id"] for rule, _ in PathPolicy.load(POLICY).rules] + assert ids.index(RULE) < ids.index(GENERAL) + + +def test_the_record_is_asked_a_question_of_its_own(): + verdict = PathPolicy.load(POLICY).classify_file(RECORD_FILE) + assert verdict.rule == RULE + assert verdict.leg is None and verdict.review_required + assert verdict.confidence == "review" + assert "`single-repository.yaml`" in verdict.reason + assert "Triad" in verdict.reason and "reversed" in verdict.reason + + +def test_the_question_offers_drop_and_root_and_drops_by_default(): + question = PathPolicy.load(POLICY).classify_file(RECORD_FILE).question + drop, root = question.index("(drop"), question.index("(root)") + assert drop < root, "the default is asked first" + assert "the default" in question[drop:root] + assert "stays a single repository" in question + assert "adopting it makes the project a Triad" in question + for general in ("(spec)", "(code)", "project-level wiring"): + assert general not in question, f"the general question's {general}" + + +def test_every_other_top_level_data_file_is_asked_as_before(): + """Only the record's name: `release.yaml` and a record spelled `.yml`, + which the advisory does not read, keep the question that offers `spec` + and `code`, and a nested record is not the project's.""" + policy = PathPolicy.load(POLICY) + for name in ("release.yaml", "single-repository.yml"): + verdict = policy.classify_file(name) + assert verdict.rule == GENERAL, name + assert "(spec)" in verdict.question and "(root)" in verdict.question + assert policy.classify_file("config/single-repository.yaml").rule != RULE + + +# --- the question `plan` asks ------------------------------------------------- + +def test_plan_asks_the_record_its_own_question(source, tmp_path): + out = tmp_path / "adoption-plan.yaml" + written = write_plan(source, out, project=PROJECT) + assert written.returncode == 0, written.stderr + written.stdout + entry = entry_of(out, RECORD_FILE) + assert entry["leg"] is None and entry["review_required"] is True + assert entry["rule"] == RULE + assert entry["question"] \ + == PathPolicy.load(POLICY).classify_file(RECORD_FILE).question + other = entry_of(out, "release.yaml") + assert other["rule"] == GENERAL + assert "(spec)" in other["question"] + + +def test_the_default_is_not_an_answer_check_says_so(source, tmp_path): + """Drop is the question's default and nothing more: a plan nobody + answered is unresolved, and the question is in the finding.""" + plan = planned(source, tmp_path, {"release.yaml": "spec"}) + (code, detail), = findings(plan) + assert code == "plan-unresolved" + assert detail.startswith("`single-repository.yaml` still has `leg: " + "null`. The question was: ") + assert PathPolicy.load(POLICY).classify_file(RECORD_FILE).question \ + in detail + + +def test_execute_refuses_it_unanswered_before_any_leg_exists( + adopter, source, tmp_path, monkeypatch): + plan = planned(source, tmp_path, {"release.yaml": "spec"}) + monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) + args = Namespace(plan=str(plan), source=None, + local_remote_dir=tmp_path / "remotes", + allow_empty_leg=[], yes=True, work_dir=tmp_path / "work") + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(args) + assert caught.value.code == "plan-unresolved" + assert caught.value.detail \ + == "1 path(s) still have `leg: null`: `single-repository.yaml`" + assert not (tmp_path / "remotes").exists() + + +@pytest.mark.parametrize("leg", ["drop", "root"]) +def test_both_answers_the_question_offers_check_ok(source, tmp_path, leg): + plan = planned(source, tmp_path, {"release.yaml": "spec", + RECORD_FILE: leg}) + assert findings(plan) == [] + + +# --- what each answer does ---------------------------------------------------- + +def adopted(source, tmp_path, leg: str): + plan = planned(source, tmp_path, {"release.yaml": "spec", + RECORD_FILE: leg}) + remotes = tmp_path / "remotes" + result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(remotes), + "--work-dir", str(tmp_path / "work")) + assert result.returncode == 0, result.stdout + result.stderr + assert "adoption verified" in result.stdout + return remotes + + +def files_of(repo, ref: str) -> list[str]: + return git("ls-tree", "-r", "--name-only", ref, + cwd=repo).stdout.splitlines() + + +@needs_filter_repo +def test_dropped_it_is_in_no_repository_of_the_assembly(source, tmp_path): + remotes = adopted(source, tmp_path, "drop") + assert RECORD_FILE not in files_of(source, SPLIT_BRANCH) + for role in ("spec", "code"): + leg = remotes / f"{PROJECT}-{role}.git" + assert RECORD_FILE not in files_of(leg, "main") + + +@needs_filter_repo +def test_kept_it_is_the_assembly_roots_with_the_sources_bytes( + source, tmp_path): + """Kept in the root, the record is the source's blob there, beside the + shape's manifest, and in neither leg.""" + remotes = adopted(source, tmp_path, "root") + ours = git("rev-parse", f"main:{RECORD_FILE}", cwd=source).stdout + assert git("rev-parse", f"{SPLIT_BRANCH}:{RECORD_FILE}", + cwd=source).stdout == ours + assert "project.yaml" in files_of(source, SPLIT_BRANCH) + for role in ("spec", "code"): + leg = remotes / f"{PROJECT}-{role}.git" + assert RECORD_FILE not in files_of(leg, "main") From 3f699ba76f503355900da8b72ba3fda2e9fe1293 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:19:39 +0000 Subject: [PATCH 2/3] adopt-project.py: `plan` writes a rule's default answer into the entry as its `leg:`, still `review_required: true` with the question beside it, so a source's own `single-repository.yaml` comes as `leg: drop`, which `check` passes and `execute` drops unless a person writes `root`, as Brett Heap ruled (#528) Brett Heap's ruling of 2026-10-12 (18:15:05Z, lane 3, the A7 line of #457) chose "Ask; default drop (Recommended)": "The default answer is drop". At 60a258f the default was only words inside the question: `plan` wrote `leg: null`, `check` found `plan-unresolved` and `execute` refused until a person answered, which is the option he declined, "Ask; no default" ("the person must answer drop or root"). The classifier could not express a default: `LEGS` has no `drop`, and `review_required` was written as `leg is None` (rv528 F1). - contracts/path-classification.yaml: the rule gains `default: drop`, and the policy header says a rule that asks may name a default, `drop` and nothing else, which the plan writes as `leg:` with the question kept. - scripts/path_classify.py: `Verdict` carries `default` beside the question and keeps `leg` None, so `review_required` is unchanged. A default is refused at load time (`path-policy-default`) on a rule that does not ask, or as anything but `drop`: an unanswered question is never an implicit `root`, and a rule that answers `spec` or `code` says so in `leg:`. A folded directory keeps its rule's default. - adopt-project.py: `Entry` keeps the verdict's `default` and `review_required`, and `with_defaults` writes the default in as `leg:` after both re-asks, so `ask_about_submodules` and `ask_about_refused_paths`, which carry none, are never answered by it, and `ask_about_refused_paths` cannot take a proposed `drop` for a person's. `walk` still leaves every question's `leg` None: the doctor's placement row walks a leg with it and calls a path misplaced when its `leg` is another, so a leg's copy stays a question there. `plan` prints each default with its question before the unresolved paths, and the plan's header says `execute` takes it as written. - AGENTS.md step 2: one sentence, so an assistant answers a default like any other question; the AGENTS.md cap moves 490 -> 492 with its dated reason in tests/test_repo_hygiene.py. Tests (tests/test_adopt_single_repository_record.py): `plan` writes `leg: drop` with `review_required: true`, the rule and the question, and prints it apart from the unresolved paths; `check` passes it unanswered; `execute` gets past every plan refusal with it, and adopts with the record in no repository and named among the split commit's dropped paths. The default is refused unless it is `drop` on a rule that asks; a folded directory keeps it; neither re-ask carries one, and in `plan` a default over a name `execute` will not extract is not written in; and the doctor's own `walked_findings` reads a leg's copy as a question. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 4 +- adopt-project.py | 75 +++++- contracts/path-classification.yaml | 23 +- scripts/path_classify.py | 48 +++- tests/test_adopt_single_repository_record.py | 247 +++++++++++++++---- tests/test_repo_hygiene.py | 15 +- 6 files changed, 345 insertions(+), 67 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e579245c..cbb68b71 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,7 +129,9 @@ install command). Seven steps, in order, and step 4 is the one you must not skip ``` 2. **You resolve the questions, in writing, in the plan.** Each unresolved - entry carries a `question:`. Set `leg:` to `spec`, `code`, `root` or `drop` + entry carries a `question:`; so does one whose `leg:` is its rule's default, + which `execute` takes as written (`drop` for a `single-repository.yaml` + record). Set `leg:` to `spec`, `code`, `root` or `drop` AND add a `resolution:` line saying why — read the repository to answer it (does the specification cite `examples/golden-run/`, or do the tests?). An answer with no reason beside it is a guess that will be read as a finding. diff --git a/adopt-project.py b/adopt-project.py index e4fa0451..020acfda 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -24,7 +24,10 @@ honestly call, every path holding one of the source's own submodules, and every path `execute` will not extract into a leg as it is named, carry `leg: null`, `review_required: true` and - the QUESTION to ask. + the QUESTION to ask. A question whose rule names a default + carries that default as its `leg:` instead, still + `review_required: true`: the source's own `single-repository.yaml` + record, `drop` (#528). A leg path over a path it keeps in the root itself is a NOTE, not a refusal: the answer is the person's to change. A `--pin @<40 hex>` is read as `scaffold-project.py @@ -1279,7 +1282,15 @@ def _default_branch(path: Path) -> str: class Entry: - """One row of the plan: a path, the leg proposed for it, and why.""" + """One row of the plan: a path, the leg proposed for it, and why. + + `default` is the answer the entry's rule names for its question, or + None, and `review_required` is the verdict's: whether a question was + asked. The two part only in `with_its_default`, the one place a + question's `leg:` is filled in, which `cmd_plan` alone calls; `walk` + leaves every question's `leg` None, so the doctor, which walks a leg + with it, still reads one as a question (#528). + """ def __init__(self, path: str, verdict: Verdict, files: int, size: int): self.path = path @@ -1288,9 +1299,23 @@ def __init__(self, path: str, verdict: Verdict, files: int, size: int): self.reason = verdict.reason self.confidence = verdict.confidence self.question = verdict.question + self.default = verdict.default + self.review_required = verdict.review_required self.files = files self.bytes = size + def with_its_default(self) -> "Entry": + """This question with its rule's default written in as its `leg:`, + still `review_required` and still carrying the question, the rule + and the reason, so a reader sees what was proposed and why.""" + out = Entry(self.path, Verdict(None, self.rule, self.reason, + self.confidence), self.files, + self.bytes) + out.question = self.question + out.default = self.default + out.leg = self.default + return out + def write(self, lines: list[str]) -> None: lines.append(f" - path: {y(self.path)}") emit(lines, "leg", self.leg, 4) @@ -1299,7 +1324,7 @@ def write(self, lines: list[str]) -> None: emit(lines, "reason", self.reason, 4) emit(lines, "files", self.files, 4) emit(lines, "bytes", self.bytes, 4) - emit(lines, "review_required", self.leg is None, 4) + emit(lines, "review_required", self.review_required, 4) if self.question: emit(lines, "question", self.question, 4) @@ -1337,6 +1362,23 @@ def walk(policy: PathPolicy, files: list[tuple[str, int]], return sorted(entries, key=lambda e: e.path) +def with_defaults(entries: list[Entry]) -> list[Entry]: + """`entries`, each question whose rule names a default answer written + with that default as its `leg:` (#528). + + RULED BY BRETT HEAP ON 2026-10-12: a source's own `single-repository.yaml` + is asked drop or root, "the default answer is drop". So `plan` writes + `leg: drop`, and keeps `review_required: true`, the rule and the + question beside it: `check` passes it, and `execute` drops the path + unless a person writes `root`. A path with no default is untouched, as + is every question `ask_about_submodules` or `ask_about_refused_paths` + asked in place of a rule's, because those carry none: called after + them, this fills in only what a rule proposed and nothing replaced. + """ + return [entry.with_its_default() if entry.leg is None and entry.default + else entry for entry in entries] + + def ask_about_submodules(policy: PathPolicy, entries: list[Entry], tree: list) -> list[Entry]: """`entries`, with every entry that holds a gitlink of the source's @@ -1430,6 +1472,9 @@ def _asked_about(entry: Entry, held: list[str], asked: Verdict, "own `leg:`. An entry added beside this one for a path " "it covers is `plan-covered-twice`." + _refused_replacements(own, held)) + # NO DEFAULT, from either rule (#528): the question is the submodules', + # which no rule's default answers, and one carried here would be + # written in as the entry's `leg:` by `with_its_default`. out = Entry(entry.path, Verdict(None, asked.rule, asked.reason, asked.confidence), entry.files, entry.bytes) @@ -1533,6 +1578,8 @@ def _refused_as_named(entry: Entry, problem: str, held: bool) -> Entry: "stands: `root` would keep a submodule in the assembly root, " "which keeps none of the source's" if held else "`root` or `drop` is the only answer `execute` takes for it") + # NO DEFAULT (#528): the rule's was an answer to its own question, and + # this one is why the name cannot go into a leg, which a person answers. out = Entry(entry.path, Verdict(None, entry.rule, entry.reason, entry.confidence), entry.files, entry.bytes) @@ -1796,6 +1843,8 @@ def render_plan(args, source: Source, entries: list[Entry], names: dict, "# human or an AI answers it by setting `leg:` to spec, code, root or", "# drop and adding one line of `resolution:` beside it. `check` then", "# has to pass, and `execute` REFUSES while any leg is still null.", + "# An entry whose rule names a default arrives with it as its `leg:`", + "# and `execute` takes it as written: confirm it or change it.", "", f"generated_on: {_dt.date.today().isoformat()}", f"tool: {SHAPE_REPOSITORY} adopt-project.py", @@ -3293,7 +3342,16 @@ def _print_plan_report(args, source: Source, tree: list, entries: list, for line in _tracking_branch_notes(args, source): print(line) unresolved = [e for e in entries if e.leg is None] + # A default is still a question (#528): it is printed with the question, + # before the paths `execute` refuses over, which stay last. + proposed = [e for e in entries if e.leg is not None and e.review_required] print(f"\nplan written to {out}") + if proposed: + print(f"{len(proposed)} path(s) carry their rule's default answer, " + "which `execute` takes as written unless it is changed:") + for entry in proposed: + print(f" {_shown(entry.path)} (leg: {entry.leg})\n" + f" {entry.question}") if unresolved: print(f"{len(unresolved)} path(s) need a human or an AI to answer a " "question before `execute` will run:") @@ -3328,6 +3386,13 @@ def cmd_plan(args) -> int: # or naming it as a spec directory, would be about nothing, and a leg it # alone fed is seeded -- which the plan records, as `check` will say of # the plan as written. A source with no such path plans as it did. + # + # The default a rule names is written into an entry last, after both + # re-asks (#528): `ask_about_refused_paths` passes over a path answered + # `drop`, so a default written in first would be taken for a person's + # answer there. What the rules imply is read without it, and it changes + # none of what is read from them, because `drop` puts the path in no leg + # and keeps nothing in the root. ruled = walk(policy, [(path, size) for path, _, _, size in tree]) implied = ask_about_refused_paths(ruled, tree) materialized = _predict_collisions( @@ -3336,8 +3401,8 @@ def cmd_plan(args) -> int: follow_ups = follow_ups_for(source, implied, names, args.spec_path, args.code_path, materialized) seeded = seeded_legs(assigned_paths(implied)) - entries = ask_about_refused_paths( - ask_about_submodules(policy, ruled, tree), tree) + entries = with_defaults(ask_about_refused_paths( + ask_about_submodules(policy, ruled, tree), tree)) out = Path(args.out) write_lf(out, render_plan(args, source, entries, names, pins, follow_ups, seeded)) diff --git a/contracts/path-classification.yaml b/contracts/path-classification.yaml index 57e5e69f..0fc70df9 100644 --- a/contracts/path-classification.yaml +++ b/contracts/path-classification.yaml @@ -33,6 +33,9 @@ kind: path-classification-policy # evidence or the code's test fixture?" is right every time and costs one # sentence of human attention. The plan marks those `review_required: true` # with `leg: null`, and `adopt-project.py execute` REFUSES while any remain. +# A rule that asks may name a `default:` answer, `drop` and nothing else: +# the plan writes it as `leg:`, still `review_required: true` with the +# question beside it, and `execute` takes it unless a person changes it. # # THE ROOT CLASS IS THE ONE PEOPLE GET WRONG. Ruled by Brett Heap on # 2026-09-02 for the MedxEHR conversion: `.specify/` (vendored Spec-Kit @@ -431,19 +434,21 @@ rules: # a question of its own that names the file and offers drop or root, drop # by default (Brett Heap's ruling of 2026-10-12, the A7 line of #457). # - # THE DEFAULT IS THE QUESTION'S OWN WORDS, NOT A VALUE THE PLAN FILLS IN. - # `plan` still writes `leg: null` and `execute` still refuses while it is - # open, because an unanswered question is never an implicit anything and a - # path is dropped only when a person has answered `drop`. Nothing refuses - # `root` here, unlike `ambiguous-project-yaml`: the file blocks nothing the - # shape writes and no validator, gate or doctor row reads it. As for the - # rules above, what `execute` does is said of a repository WHEN IT IS - # ADOPTED; `shape-doctor.py --placement-plan` prints this question too, for - # a leg already split. + # THE DEFAULT IS WRITTEN INTO THE PLAN, AND IT IS STILL A QUESTION. `plan` + # writes `leg: drop` with `review_required: true`, this rule and its + # question, so `check` passes it and `execute` drops the file unless a + # person writes `root`, and the split commit names it among the paths it + # dropped. Nothing refuses `root` here, unlike `ambiguous-project-yaml`: + # the file blocks nothing the shape writes and no validator, gate or + # doctor row reads it. As for the rules above, what `execute` does is said + # of a repository WHEN IT IS ADOPTED: `shape-doctor.py --placement-plan` + # prints this question too, by name, for a leg already split, and reads + # the leg's copy as a question, because the verdict's leg stays null. - id: ambiguous-single-repository-record paths: - "single-repository.yaml" leg: ambiguous + default: drop reason: >- A top-level `single-repository.yaml` is this standard's record that a project STAYS a single repository: who decided it, when and why. diff --git a/scripts/path_classify.py b/scripts/path_classify.py index 2ea818ce..a4ec75ba 100644 --- a/scripts/path_classify.py +++ b/scripts/path_classify.py @@ -23,6 +23,15 @@ owes a reader when the honest state is that it does not know; a guess dressed as a classification is worse, because nobody reviews a confident answer. +A QUESTION MAY CARRY A DEFAULT, AND IT IS STILL A QUESTION (#528). A rule +that asks may name the answer that holds when nobody gives another, as +`default:`. The verdict keeps `leg` None and carries the default beside it, +so whatever reads `leg` -- the doctor's placement row, walking a leg already +split -- still reads a question; `adopt-project.py plan` is what writes the +default into the plan, with the question still beside it. Only `drop` may +be one: an unanswered question is never an implicit `root`, and a rule that +can answer `spec` or `code` says so in `leg:`. + STANDARD LIBRARY ONLY, like everything else here. """ @@ -36,6 +45,11 @@ from repo_shape import Refusal, load_yaml # noqa: E402 LEGS = ("spec", "code", "root", "ambiguous") +#: The answers a rule that asks may name as its `default:` (#528). `drop` +#: alone: `root` unanswered would keep a path in the assembly root that no +#: person chose to keep, and a rule that answers `spec` or `code` says so in +#: `leg:` rather than asking. +DEFAULTS = ("drop",) DEFAULT_MAJORITY = 0.75 @@ -44,12 +58,18 @@ class Verdict: `leg` is `None` for `ambiguous`, because that is what the plan writes and a single spelling of "unknown" is one fewer thing to get wrong. + + `default` is the answer the rule names for its question, or None. It is + kept only beside a question (`leg` None) and never fills `leg` in: the + path is still one the rules could not call, and `review_required` says + so whatever the default is. """ - __slots__ = ("leg", "rule", "reason", "confidence", "question") + __slots__ = ("leg", "rule", "reason", "confidence", "question", "default") def __init__(self, leg: str | None, rule: str, reason: str, - confidence: str, question: str | None = None): + confidence: str, question: str | None = None, + default: str | None = None): self.leg = None if leg in (None, "ambiguous") else leg self.rule = rule self.reason = " ".join(str(reason).split()) @@ -59,6 +79,7 @@ def __init__(self, leg: str | None, rule: str, reason: str, # as a confident answer to a question nobody has answered. self.confidence = "review" if self.leg is None else confidence self.question = " ".join(str(question).split()) if question else None + self.default = default if self.leg is None else None @property def review_required(self) -> bool: @@ -114,6 +135,24 @@ def _compiled_rule(rule) -> tuple[dict, list[tuple[str, re.Pattern]]]: "path-policy-leg", f"rule {rule['id']!r} declares leg {rule.get('leg')!r}; the " f"classes are {list(LEGS)}") + # A DEFAULT IS AN ANSWER TO A QUESTION (#528), so only a rule that asks + # one may name it, and only one of DEFAULTS. Refused at load time like + # an unknown leg: a `default: root` that `plan` wrote unread would keep + # a path in the assembly root nobody chose to keep. + if "default" in rule: + if rule["leg"] != "ambiguous": + raise Refusal( + "path-policy-default", + f"rule {rule['id']!r} declares default " + f"{rule['default']!r} beside leg {rule['leg']!r}; only a " + "rule that asks (leg `ambiguous`) names a default answer") + if rule["default"] not in DEFAULTS: + raise Refusal( + "path-policy-default", + f"rule {rule['id']!r} declares default " + f"{rule['default']!r}; a default is one of {list(DEFAULTS)}, " + "because an unanswered question is never an implicit `root` " + "and a rule that answers `spec` or `code` says so in `leg:`") compiled = [(str(p), glob_to_regex(str(p))) for p in (rule.get("paths") or [])] if not compiled: @@ -170,7 +209,7 @@ def classify_file(self, path: str) -> Verdict: rule = self.rule_for(path) if rule is not None: return Verdict(rule["leg"], str(rule["id"]), rule.get("reason", ""), - "high", rule.get("question")) + "high", rule.get("question"), rule.get("default")) suffix = ("." + path.rsplit(".", 1)[1].lower()) if "." in \ Path(path).name.lstrip(".") else "" leg = self.extension_classes.get(suffix) @@ -208,7 +247,8 @@ def fold(self, verdicts: list[Verdict], files: list[str]) -> Verdict | None: rules = {v.rule for v in verdicts} if len(rules) == 1: return Verdict(leg, verdicts[0].rule, verdicts[0].reason, - verdicts[0].confidence, verdicts[0].question) + verdicts[0].confidence, verdicts[0].question, + verdicts[0].default) return Verdict( leg, "consensus", "every file under this path classifies as " diff --git a/tests/test_adopt_single_repository_record.py b/tests/test_adopt_single_repository_record.py index b6108551..a677527e 100644 --- a/tests/test_adopt_single_repository_record.py +++ b/tests/test_adopt_single_repository_record.py @@ -10,12 +10,14 @@ Its one reader is the Triad advisory, which is silent in an assembly root whatever the file holds, so neither leg has any use for it. -Ruled 2026-10-12 (the A7 line of #457): a path-policy rule names the file and -asks drop or root, defaulting to drop. That is `ambiguous-single-repository- -record`, above `ambiguous-top-level-yaml`. Its default is the question's own -words: `plan` still writes `leg: null` and `execute` still refuses while it is -open, because an unanswered question is never an implicit anything, and -nothing is dropped that a person did not answer `drop`. +Ruled 2026-10-12 by Brett Heap, "Ask; default drop": a path-policy rule names +the file and asks drop or root, and "the default answer is drop". That is +`ambiguous-single-repository-record`, above `ambiguous-top-level-yaml`, with +`default: drop`. `plan` writes `leg: drop` and keeps `review_required: true` +and the question beside it; `check` passes it, and `execute` drops the file +unless a person writes `root`. The verdict keeps `leg` None, so the doctor, +which walks a leg with the same `walk`, still reads a leg's copy as a +question. """ from __future__ import annotations @@ -27,12 +29,12 @@ import pytest -from conftest import (ADOPT, REPO, findings, git, make_source_repo, planned, - run_script, write_plan) +from conftest import (ADOPT, REPO, findings, git, load_without_bytecode, + make_source_repo, planned, run_script, write_plan) sys.path.insert(0, str(REPO / "scripts")) from path_classify import PathPolicy # noqa: E402 -from repo_shape import load_yaml # noqa: E402 +from repo_shape import Refusal, load_yaml # noqa: E402 from shape_advisory import RECORD_FILE # noqa: E402 needs_filter_repo = pytest.mark.skipif( @@ -75,6 +77,18 @@ def entry_of(plan, path: str) -> dict: return entry +def record_question() -> str: + return PathPolicy.load(POLICY).classify_file(RECORD_FILE).question + + +def one_rule_policy(**rule) -> PathPolicy: + """A policy holding `rule` alone, as the classifier loads one.""" + return PathPolicy({"kind": "path-classification-policy", + "rules": [{"id": "t", "paths": ["x", "notes/**"], + "reason": "why", "question": "Keep it?", + **rule}]}) + + # --- the rule --------------------------------------------------------------- def test_the_rule_names_the_file_the_advisory_reads(): @@ -90,17 +104,21 @@ def test_the_rule_comes_before_the_general_question(): assert ids.index(RULE) < ids.index(GENERAL) -def test_the_record_is_asked_a_question_of_its_own(): +def test_the_record_is_asked_a_question_with_drop_as_its_default(): + """The default rides beside the question and never fills `leg`: the + verdict is still one the rules could not call.""" verdict = PathPolicy.load(POLICY).classify_file(RECORD_FILE) assert verdict.rule == RULE - assert verdict.leg is None and verdict.review_required + assert verdict.leg is None + assert verdict.review_required + assert verdict.default == "drop" assert verdict.confidence == "review" assert "`single-repository.yaml`" in verdict.reason assert "Triad" in verdict.reason and "reversed" in verdict.reason def test_the_question_offers_drop_and_root_and_drops_by_default(): - question = PathPolicy.load(POLICY).classify_file(RECORD_FILE).question + question = record_question() drop, root = question.index("(drop"), question.index("(root)") assert drop < root, "the default is asked first" assert "the default" in question[drop:root] @@ -118,51 +136,87 @@ def test_every_other_top_level_data_file_is_asked_as_before(): for name in ("release.yaml", "single-repository.yml"): verdict = policy.classify_file(name) assert verdict.rule == GENERAL, name + assert verdict.default is None, name assert "(spec)" in verdict.question and "(root)" in verdict.question assert policy.classify_file("config/single-repository.yaml").rule != RULE -# --- the question `plan` asks ------------------------------------------------- +# --- a default the classifier takes, and one it refuses ----------------------- + +def test_a_rule_that_asks_may_name_drop_as_its_default(): + verdict = one_rule_policy(leg="ambiguous", default="drop").classify_file( + "x") + assert (verdict.leg, verdict.default) == (None, "drop") + assert verdict.review_required + + +@pytest.mark.parametrize("rule", [ + {"leg": "ambiguous", "default": "root"}, + {"leg": "ambiguous", "default": "spec"}, + {"leg": "ambiguous", "default": "code"}, + {"leg": "ambiguous", "default": None}, + {"leg": "spec", "default": "drop"}, + {"leg": "root", "default": "drop"}, +], ids=["root", "spec", "code", "empty", "beside-spec", "beside-root"]) +def test_any_other_default_is_refused_when_the_policy_loads(rule): + """`root` unanswered would keep a path in the assembly root nobody chose + to keep, a rule that answers says so in `leg:`, and a rule that does + not ask has no question for a default to answer.""" + with pytest.raises(Refusal) as caught: + one_rule_policy(**rule) + assert caught.value.code == "path-policy-default" + assert "'t'" in caught.value.detail -def test_plan_asks_the_record_its_own_question(source, tmp_path): + +def test_a_directory_the_rule_covers_whole_keeps_its_default(adopter): + """`walk` folds a directory whose files one rule asks about into one + entry; the default goes with the question, and `leg` stays None.""" + policy = one_rule_policy(leg="ambiguous", default="drop") + entry, = adopter.walk(policy, [("notes/a.md", 1), ("notes/b.md", 2)]) + assert (entry.path, entry.leg, entry.default) == ("notes/", None, "drop") + assert entry.review_required + + +# --- what `plan` writes -------------------------------------------------------- + +def test_plan_writes_drop_and_still_asks_the_question(source, tmp_path): out = tmp_path / "adoption-plan.yaml" written = write_plan(source, out, project=PROJECT) assert written.returncode == 0, written.stderr + written.stdout entry = entry_of(out, RECORD_FILE) - assert entry["leg"] is None and entry["review_required"] is True + assert entry["leg"] == "drop" + assert entry["review_required"] is True + assert entry["confidence"] == "review" assert entry["rule"] == RULE - assert entry["question"] \ - == PathPolicy.load(POLICY).classify_file(RECORD_FILE).question + assert entry["question"] == record_question() other = entry_of(out, "release.yaml") assert other["rule"] == GENERAL + assert other["leg"] is None assert "(spec)" in other["question"] -def test_the_default_is_not_an_answer_check_says_so(source, tmp_path): - """Drop is the question's default and nothing more: a plan nobody - answered is unresolved, and the question is in the finding.""" - plan = planned(source, tmp_path, {"release.yaml": "spec"}) - (code, detail), = findings(plan) - assert code == "plan-unresolved" - assert detail.startswith("`single-repository.yaml` still has `leg: " - "null`. The question was: ") - assert PathPolicy.load(POLICY).classify_file(RECORD_FILE).question \ - in detail - - -def test_execute_refuses_it_unanswered_before_any_leg_exists( - adopter, source, tmp_path, monkeypatch): +def test_plan_prints_the_default_with_its_question_and_not_as_unresolved( + source, tmp_path): + written = write_plan(source, tmp_path / "adoption-plan.yaml", + project=PROJECT) + assert written.returncode == 0, written.stderr + written.stdout + tail = written.stdout.split("\nplan written to ", 1)[1] + proposed, unresolved = tail.split( + " path(s) need a human or an AI to answer a question", 1) + assert ("1 path(s) carry their rule's default answer, which `execute` " + "takes as written unless it is changed:\n" + f" {RECORD_FILE} (leg: drop)\n {record_question()}\n" + ) in proposed + assert RECORD_FILE not in unresolved + assert "release.yaml" in unresolved + + +def test_check_passes_the_default_nobody_changed(source, tmp_path): + """The default is an answer `check` takes: only the general question + beside it was answered.""" plan = planned(source, tmp_path, {"release.yaml": "spec"}) - monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) - args = Namespace(plan=str(plan), source=None, - local_remote_dir=tmp_path / "remotes", - allow_empty_leg=[], yes=True, work_dir=tmp_path / "work") - with pytest.raises(adopter.Refusal) as caught: - adopter.cmd_execute(args) - assert caught.value.code == "plan-unresolved" - assert caught.value.detail \ - == "1 path(s) still have `leg: null`: `single-repository.yaml`" - assert not (tmp_path / "remotes").exists() + assert entry_of(plan, RECORD_FILE)["leg"] == "drop" + assert findings(plan) == [] @pytest.mark.parametrize("leg", ["drop", "root"]) @@ -172,11 +226,84 @@ def test_both_answers_the_question_offers_check_ok(source, tmp_path, leg): assert findings(plan) == [] +# --- a question asked in place of the rule's carries no default ---------------- + +def test_a_path_execute_will_not_extract_is_asked_with_no_default(adopter): + """`ask_about_refused_paths` asks why the name cannot go into a leg, and + the rule's default answers nothing there.""" + asked = adopter.Entry("-notes.md", adopter.Verdict( + None, "t", "why", "high", "Keep it?", "drop"), 1, 1) + out, = adopter.ask_about_refused_paths([asked], []) + assert (out.leg, out.default) == (None, None) + assert out.review_required + assert adopter.with_defaults([out])[0].leg is None + + +def test_a_record_that_is_a_submodule_is_asked_with_no_default(adopter): + """`ask_about_submodules` asks the `.gitmodules` question of a gitlink + whatever rule matched its path, and that question has no default.""" + policy = PathPolicy.load(POLICY) + ruled = [adopter.Entry(RECORD_FILE, policy.classify_file(RECORD_FILE), 1, + 0)] + tree = [(RECORD_FILE, adopter.GITLINK_MODE, "c" * 40, 0)] + out, = adopter.ask_about_submodules(policy, ruled, tree) + assert out.rule == "ambiguous-gitmodules" + assert (out.leg, out.default) == (None, None) + assert adopter.with_defaults([out])[0].leg is None + + +def test_plan_writes_a_default_only_where_no_question_replaced_it(tmp_path): + """In `plan` itself: a rule's default over a name `execute` will not + extract is not written in, because `ask_about_refused_paths` asks first + and passes over a path already answered `drop`.""" + policy = tmp_path / "policy.yaml" + policy.write_bytes( + b"schema_version: 1\nkind: path-classification-policy\nrules:\n" + b" - id: test-asked-with-a-default\n paths:\n" + b" - \"-notes.md\"\n leg: ambiguous\n default: drop\n" + b" reason: A test rule.\n question: Keep it?\n" + b" - id: test-everything-else\n paths:\n - \"**\"\n" + b" leg: spec\n reason: Everything else.\n") + here = make_source_repo(tmp_path / "Thing", + tree={"-notes.md": "# N\n", "src/a.py": "1\n"}, + edits=()) + out = tmp_path / "adoption-plan.yaml" + written = write_plan(here, out, extra=("--path-policy", str(policy))) + assert written.returncode == 0, written.stderr + written.stdout + entry = entry_of(out, "-notes.md") + assert entry["rule"] == "test-asked-with-a-default" + assert entry["leg"] is None + assert "`execute` will not extract" in entry["question"] + + +# --- the doctor still reads a leg's copy as a question ----------------------- + +def test_the_doctor_reads_a_legs_copy_as_a_question_and_not_misplaced( + adopter): + """The placement row walks a leg with `adopt-project.py`'s own `walk` + and calls a path misplaced when its `leg` is another one. The default + never fills `leg` there, so a copy in the spec leg is a question.""" + doctor = load_without_bytecode(REPO / "shape-doctor.py", + "shape_doctor_single_repository_record") + walked = adopter.walk(PathPolicy.load(POLICY), [(RECORD_FILE, 100)]) + misplaced, review, settled = doctor.walked_findings(walked, "spec", + "spec", None) + assert (misplaced, settled) == ([], []) + row, = review + assert row["path"] == f"spec/{RECORD_FILE}" + assert (row["rule"], row["classified_as"]) == (RULE, None) + assert row["review_required"] is True + + # --- what each answer does ---------------------------------------------------- -def adopted(source, tmp_path, leg: str): - plan = planned(source, tmp_path, {"release.yaml": "spec", - RECORD_FILE: leg}) +def adopted(source, tmp_path, leg: str | None): + """`execute` on the plan with the general question answered and the + record's left as `plan` wrote it (`None`), or answered `leg`.""" + answers = {"release.yaml": "spec"} + if leg is not None: + answers[RECORD_FILE] = leg + plan = planned(source, tmp_path, answers) remotes = tmp_path / "remotes" result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", "--local-remote-dir", str(remotes), @@ -192,12 +319,40 @@ def files_of(repo, ref: str) -> list[str]: @needs_filter_repo -def test_dropped_it_is_in_no_repository_of_the_assembly(source, tmp_path): - remotes = adopted(source, tmp_path, "drop") +def test_the_default_nobody_changed_drops_it_from_every_repository( + source, tmp_path): + """`execute` takes the default as written, and the split commit names + the record among the paths it dropped.""" + remotes = adopted(source, tmp_path, None) assert RECORD_FILE not in files_of(source, SPLIT_BRANCH) for role in ("spec", "code"): leg = remotes / f"{PROJECT}-{role}.git" assert RECORD_FILE not in files_of(leg, "main") + message = git("log", "-1", "--format=%B", SPLIT_BRANCH, + cwd=source).stdout + dropped = message.split("DROPPED (in no leg and no longer here):\n", 1)[1] + assert dropped.split("\n\n", 1)[0].splitlines() == [f" {RECORD_FILE}"] + + +def test_execute_takes_the_default_without_asking_again( + adopter, source, tmp_path, monkeypatch): + """No `plan-unresolved` for the record: `execute` gets past every + refusal of the plan to the confirmation, which `--yes` gives, and is + stopped there, before any leg exists.""" + plan = planned(source, tmp_path, {"release.yaml": "spec"}) + + def stop(*_args): + raise adopter.Refusal("confirm-reached", "past every plan refusal") + + monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) + monkeypatch.setattr(adopter, "_confirm", stop) + args = Namespace(plan=str(plan), source=None, + local_remote_dir=tmp_path / "remotes", + allow_empty_leg=[], yes=True, work_dir=tmp_path / "work") + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(args) + assert caught.value.code == "confirm-reached" + assert not (tmp_path / "remotes").exists() @needs_filter_repo diff --git a/tests/test_repo_hygiene.py b/tests/test_repo_hygiene.py index 8df3a3e6..c1338d03 100644 --- a/tests/test_repo_hygiene.py +++ b/tests/test_repo_hygiene.py @@ -616,9 +616,20 @@ def test_agents_md_is_short_enough_to_be_read(): no rule id is not a broken row; and a submodule moved on purpose stays MISPLACED against its row, which is the human's to change -- never a manifest edit an assistant makes to turn the row green, the hand-edited - pin arriving through a new door.""" + pin arriving through a new door. + + 2026-10-15: 490 -> 492 — step 2 of "Adopting an existing repository" + (#528). Two lines, inside that one numbered point. `plan` now writes a + rule's default into an entry as its `leg:`, still + `review_required: true` with the question beside it: a source's own + `single-repository.yaml` record comes as `drop`, by Brett Heap's + 2026-10-12 ruling. Step 2 said only that an UNRESOLVED entry carries a + question, and `check` passes a default, so an assistant briefed from + this file alone would let it through unread. The lines buy that it is + a question too, which `execute` takes as written, answered like the + others.""" lines = (REPO / "AGENTS.md").read_text().splitlines() - assert len(lines) <= 490, f"AGENTS.md is {len(lines)} lines; the cap is 490" + assert len(lines) <= 492, f"AGENTS.md is {len(lines)} lines; the cap is 492" def test_claude_md_points_at_agents_md(): From 19ffe4a1f3bdc54e6d12fa1d1a8183b7c6e0dbf2 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:20:29 +0000 Subject: [PATCH 3/3] adopt-project.py: `plan` asks the record's question, with its default, only of a top-level `single-repository.yaml` that declares `kind: single-repository-record` at the commit it plans, so a file of that name that is no record is asked the general question again and is not dropped by a default meant for the record (#528) In the standard `kind:` decides and not the filename (scripts/shape_advisory.py). At 60a258f a top-level `single-repository.yaml` declaring `kind: release-config` got the record's question: one `plan` run printed "NOTE single-repository.yaml does not silence this: it declares kind 'release-config'" and then told the person the file "records that the repository stays a single repository" (rv528 F2). With the default the previous commit writes in, that file would also have been dropped unread. - adopt-project.py: `ask_about_a_record_only` reads the file at the planned commit with `shape_advisory.read_record(Tree(path, commit))`, the advisory's own reading, and where it is no record (another kind, not a mapping, unparsable, not a file) classifies the entry as if the policy held no `ambiguous-single-repository-record` (`RECORD_RULE`): for a top-level YAML file, `ambiguous-top-level-yaml`'s question and `leg: null`, as before the rule existed. A source with no entry that rule answered is not read. It runs on `walk`'s entries before anything reads them, so the follow-ups, the seeding and the re-asks see the same entry the plan writes. - scripts/path_classify.py: `rule_for` and `classify_file` take `skip`, rule ids to pass over. - contracts/path-classification.yaml: the rule's comment says `kind:` decides, and that the doctor, which reads no file here, asks a leg's copy by name. Tests: each non-record body (another kind, not a mapping, does not parse) is planned under the general rule with its question and `leg: null`, and `check` finds it `plan-unresolved`; a record that leaves a field out is still the record; an uncommitted record over a committed non-record is not the record; and `RECORD_RULE` names the policy's rule. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 48 +++++++++++++- contracts/path-classification.yaml | 6 ++ scripts/path_classify.py | 13 +++- tests/test_adopt_single_repository_record.py | 69 ++++++++++++++++++-- 4 files changed, 126 insertions(+), 10 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 020acfda..f5ee91c6 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1362,6 +1362,44 @@ def walk(policy: PathPolicy, files: list[tuple[str, int]], return sorted(entries, key=lambda e: e.path) +#: The path policy's rule for the source's own staying-single record (#528). +#: Its question and its default are the record's, so `plan` asks them only +#: of a file that IS the record (`ask_about_a_record_only`). +RECORD_RULE = "ambiguous-single-repository-record" + + +def ask_about_a_record_only(policy: PathPolicy, entries: list[Entry], + source: "Source") -> list[Entry]: + """`entries`, with a top-level `single-repository.yaml` that is no + record asked the question the rules below the record's ask (#528). + + `kind:` DECIDES, NOT THE NAME, for the record's one reader, the Triad + advisory (`shape_advisory.read_record`). A file of that name declaring + `kind: release-config` silences nothing, and the same `plan` run said + so in a NOTE while the record's question told the person the file + records that the repository stays single, and its default would have + dropped it. So RECORD_RULE answers only a file that declares + `single-repository-record` in the commit being planned, read as the + advisory reads it there; any other is classified as if the policy held + no such rule, which for a top-level YAML file is the general question, + asked as before. A source with no entry RECORD_RULE answered is not + read at all. + """ + if not any(entry.path == shape_advisory.RECORD_FILE + and entry.rule == RECORD_RULE for entry in entries): + return entries + state, _, _ = shape_advisory.read_record( + shape_advisory.Tree(source.path, source.commit)) + if state == shape_advisory.RECORD_SILENCES: + return entries + return [Entry(entry.path, + policy.classify_file(entry.path, skip=(RECORD_RULE,)), + entry.files, entry.bytes) + if entry.path == shape_advisory.RECORD_FILE + and entry.rule == RECORD_RULE else entry + for entry in entries] + + def with_defaults(entries: list[Entry]) -> list[Entry]: """`entries`, each question whose rule names a default answer written with that default as its `leg:` (#528). @@ -3387,13 +3425,17 @@ def cmd_plan(args) -> int: # alone fed is seeded -- which the plan records, as `check` will say of # the plan as written. A source with no such path plans as it did. # - # The default a rule names is written into an entry last, after both - # re-asks (#528): `ask_about_refused_paths` passes over a path answered + # A `single-repository.yaml` that declares no record is asked as any + # other top-level data file is, before anything reads the entries + # (#528). The default a rule names is written into an entry last, after + # both re-asks: `ask_about_refused_paths` passes over a path answered # `drop`, so a default written in first would be taken for a person's # answer there. What the rules imply is read without it, and it changes # none of what is read from them, because `drop` puts the path in no leg # and keeps nothing in the root. - ruled = walk(policy, [(path, size) for path, _, _, size in tree]) + ruled = ask_about_a_record_only( + policy, walk(policy, [(path, size) for path, _, _, size in tree]), + source) implied = ask_about_refused_paths(ruled, tree) materialized = _predict_collisions( implied, attributes_link=any(path == ATTRIBUTES and mode == SYMLINK_MODE diff --git a/contracts/path-classification.yaml b/contracts/path-classification.yaml index 0fc70df9..0c97cf02 100644 --- a/contracts/path-classification.yaml +++ b/contracts/path-classification.yaml @@ -444,6 +444,12 @@ rules: # of a repository WHEN IT IS ADOPTED: `shape-doctor.py --placement-plan` # prints this question too, by name, for a leg already split, and reads # the leg's copy as a question, because the verdict's leg stays null. + # + # `kind:` DECIDES, NOT THE NAME, as it does for the advisory. `plan` asks + # this question only of a file that declares `kind: + # single-repository-record` at the commit it plans. One of this name that + # declares anything else, or does not parse, is no record, and is asked + # the general question below as any other top-level data file is. - id: ambiguous-single-repository-record paths: - "single-repository.yaml" diff --git a/scripts/path_classify.py b/scripts/path_classify.py index a4ec75ba..a61d5753 100644 --- a/scripts/path_classify.py +++ b/scripts/path_classify.py @@ -192,21 +192,28 @@ def load(cls, path: Path) -> "PathPolicy": # -- one path ----------------------------------------------------------- - def rule_for(self, path: str) -> dict | None: + def rule_for(self, path: str, skip=()) -> dict | None: """The FIRST rule whose globs match, or None. A rule written as `foo/**` also matches the bare directory `foo`, because a policy that classified every file under a directory but not the directory itself would descend into it for no reason. + + A rule whose id is in `skip` is passed over, as if the policy did not + hold it. `adopt-project.py plan` passes the record's rule over for a + file that has the record's name and declares another `kind:`, which + is then asked what the rules below that one ask (#528). """ for rule, compiled in self.rules: + if rule["id"] in skip: + continue for pattern, regex in compiled: if regex.match(path) or pattern == f"{path}/**": return rule return None - def classify_file(self, path: str) -> Verdict: - rule = self.rule_for(path) + def classify_file(self, path: str, skip=()) -> Verdict: + rule = self.rule_for(path, skip) if rule is not None: return Verdict(rule["leg"], str(rule["id"]), rule.get("reason", ""), "high", rule.get("question"), rule.get("default")) diff --git a/tests/test_adopt_single_repository_record.py b/tests/test_adopt_single_repository_record.py index a677527e..1acd24b0 100644 --- a/tests/test_adopt_single_repository_record.py +++ b/tests/test_adopt_single_repository_record.py @@ -17,7 +17,8 @@ and the question beside it; `check` passes it, and `execute` drops the file unless a person writes `root`. The verdict keeps `leg` None, so the doctor, which walks a leg with the same `walk`, still reads a leg's copy as a -question. +question. And `kind:` decides, not the name: a file of that name that is no +record at the commit `plan` reads is asked the general question. """ from __future__ import annotations @@ -55,6 +56,13 @@ TREE = {"src/app.py": "print(1)\n", "docs/spec.md": "# Spec\n", "README.md": "# Thing\n", RECORD_FILE: RECORD, "release.yaml": "name: release\n"} +#: Files named as the record is that are no record: `kind:` decides, as it +#: does for the advisory, and each of these silences nothing there. +NOT_A_RECORD = { + "another kind": "schema_version: 1\nkind: release-config\n", + "not a mapping": "- a\n- b\n", + "does not parse": "kind: [\n", +} @pytest.fixture(scope="module") @@ -91,11 +99,13 @@ def one_rule_policy(**rule) -> PathPolicy: # --- the rule --------------------------------------------------------------- -def test_the_rule_names_the_file_the_advisory_reads(): - """One name in two places: the rule asks about the file the advisory is - the only reader of, so a rename there is caught here.""" +def test_the_rule_names_the_file_the_advisory_reads(adopter): + """One name in three places: the rule asks about the file the advisory + is the only reader of, and `plan` asks the record's question of a record + only by that rule's id, so a rename in either is caught here.""" rules = {rule["id"]: rule for rule, _ in PathPolicy.load(POLICY).rules} assert rules[RULE]["paths"] == [RECORD_FILE] == ["single-repository.yaml"] + assert adopter.RECORD_RULE == RULE def test_the_rule_comes_before_the_general_question(): @@ -139,6 +149,9 @@ def test_every_other_top_level_data_file_is_asked_as_before(): assert verdict.default is None, name assert "(spec)" in verdict.question and "(root)" in verdict.question assert policy.classify_file("config/single-repository.yaml").rule != RULE + passed_over = policy.classify_file(RECORD_FILE, skip=(RULE,)) + assert passed_over.rule == GENERAL + assert passed_over.default is None # --- a default the classifier takes, and one it refuses ----------------------- @@ -226,6 +239,54 @@ def test_both_answers_the_question_offers_check_ok(source, tmp_path, leg): assert findings(plan) == [] +# --- `kind:` decides, not the name --------------------------------------------- + +@pytest.mark.parametrize("body", list(NOT_A_RECORD.values()), + ids=list(NOT_A_RECORD)) +def test_a_file_of_that_name_that_is_no_record_is_asked_as_before( + tmp_path, body): + """No record, no record's question and no default: the general one, + with `leg: null`, which `check` finds unanswered.""" + here = make_source_repo(tmp_path / "Thing", + tree={**TREE, RECORD_FILE: body}, edits=()) + plan = planned(here, tmp_path, {"release.yaml": "spec"}) + entry = entry_of(plan, RECORD_FILE) + general = PathPolicy.load(POLICY).classify_file("release.yaml") + assert entry["rule"] == GENERAL + assert entry["leg"] is None + assert entry["review_required"] is True + assert entry["question"] == general.question + assert entry["reason"] == general.reason + (code, detail), = findings(plan) + assert code == "plan-unresolved" + assert detail.startswith(f"`{RECORD_FILE}` still has `leg: null`.") + + +def test_a_record_that_leaves_a_field_out_is_still_the_record(tmp_path): + """The advisory's own reading: `kind:` decides, and a missing field is + a note there, not a reason to stop being the record.""" + partial = "schema_version: 1\nkind: single-repository-record\n" + here = make_source_repo(tmp_path / "Thing", + tree={**TREE, RECORD_FILE: partial}, edits=()) + out = tmp_path / "adoption-plan.yaml" + assert write_plan(here, out).returncode == 0 + entry = entry_of(out, RECORD_FILE) + assert (entry["rule"], entry["leg"]) == (RULE, "drop") + + +def test_the_commit_plan_reads_decides_and_not_the_working_tree(tmp_path): + """A record written over a committed non-record, and not committed, is + not in the repository being split, as the advisory says of it.""" + here = make_source_repo( + tmp_path / "Thing", + tree={**TREE, RECORD_FILE: NOT_A_RECORD["another kind"]}, edits=()) + (here / RECORD_FILE).write_bytes(RECORD.encode()) + out = tmp_path / "adoption-plan.yaml" + assert write_plan(here, out).returncode == 0 + entry = entry_of(out, RECORD_FILE) + assert (entry["rule"], entry["leg"]) == (GENERAL, None) + + # --- a question asked in place of the rule's carries no default ---------------- def test_a_path_execute_will_not_extract_is_asked_with_no_default(adopter):