From daff221f39141e0ee776345d6c01823ec0f7396c Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sun, 11 Oct 2026 22:40:42 +0000 Subject: [PATCH 01/11] shape-doctor.py: a root's own `contracts/shape-pin.yaml` that does not read as a pin (a merge conflict, a non-UTF-8 byte, a `commit:` that is not 40 hex, a deleted file) is `INVALID`, exit 1, with `update-shape.py check` as its NEXT, instead of `CANNOT ANSWER`, exit 3, sending the reader to `git fetch --all` in the standard (#237) `check_shape_currency` took every refusal `update-shape.py prepare` raises for this checkout failing to ask. `prepare` also reads the ROOT's own pin, and those refusals are about their repository. The doctor now reads the pin first with `read_pin` (the function `prepare` calls for it), so the two kinds are told apart by which read raised them, not by a code list or by text. A refusal from the pin is a FINDING with no `environment`, quoting `REFUSED : ` without its remediation (the same wording family #232 gives an unloadable manifest); a refusal from the standard stays `CANNOT ANSWER`, exit 3, unchanged. The NEXT is `update-shape.py check` with the file at fault named, not `validate-pins.py`: that validator passes a `digest_definition` this tool will not recompute, so there the shape-currency row is the only red one and the verdict is `INVALID (shape-currency)`. WIP state at commit time: the 13 new tests pass at this head and fail on main 39ac59c (`assert 3 == 1`); tests/test_update_shape*.py and tests/test_cli_reference.py are 133 passed; the full tests/test_shape_doctor.py run was still in progress with no failure so far. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- README.md | 6 +- shape-doctor.py | 73 ++++++++++++ tests/test_shape_doctor.py | 222 +++++++++++++++++++++++++++++++++++++ 3 files changed, 300 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 095a1b9e..c906f2eb 100644 --- a/README.md +++ b/README.md @@ -1160,7 +1160,11 @@ way, with its own next command. `MISPLACED` sits between the two: a path in the wrong leg is a fact about the shape that no validator, pin or manifest here can see, while a red validator names itself in the table whether or not it also names the verdict line. `--json` is the same report as one object, -each row carrying its `id`. +each row carrying its `id`. A root's own `contracts/shape-pin.yaml` that does +not read as a pin — a merge conflict in it, a `commit:` that is not 40 hex, a +deleted file — is not exit 3, which is for a standard that cannot answer: it is +a statement about the root, a `FINDING` on **shape currency** whose next +command is `update-shape.py check`, and the verdict is `INVALID`, exit 1. **`placement` is the row that reads what is *inside* the legs.** Brett Heap, 2026-09-10: *"we have to look for code in spec and spec in code"*. It runs the diff --git a/shape-doctor.py b/shape-doctor.py index 3f748571..a073a805 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -1081,6 +1081,64 @@ def what_a_re_pin_would_move(ctx: Context, us, repository: str, return "the pin alone" +#: The root's own pin, as a path under the root: one spelling for the row's +#: sentence, its `--json` detail and its next command (#237). +SHAPE_PIN = "contracts/shape-pin.yaml" + + +def unreadable_pin_row(ctx: Context, exc: Refusal) -> Row: + """The `shape currency` row for a root whose OWN pin does not read as one + (#237): a FINDING, the refusal quoted whole, and the file to put right. + + A STATEMENT ABOUT THEIR REPOSITORY, NOT A FAILURE TO ASK. The pin is a + file in the root: a merge conflict left in it, a byte that is not UTF-8, + a `commit:` that is not 40 hex, a deleted file are what somebody did to + THEIR tree, and no `git fetch` in the standard changes any of them. So + this row carries no `environment`, which is the only thing that makes + `verdict_for` answer CANNOT ANSWER, exit 3, before it looks at a single + other row. The ladder then reaches the gates that are red -- `pins` and + `manifest kinds` say the same about the same file where they can read it, + and where neither can (a `digest_definition` this tool does not + recompute, which `validate-pins.py` passes) this row is the only red one + and the verdict is `INVALID (shape-currency)`. + + THE REFUSAL, WHOLE, AND NOT ITS REMEDIATION. `REFUSED : ` + is what `read_pin` raised, in the spelling the project's own validator + prints for the same file, and it names the line or the field at fault. + Its `Remediation:` trailer is left off on purpose: `yaml-unparsable` + carries `repo_shape`'s default one, about submodules and the lockstep + invariant, which would send a person resolving a merge conflict to `git + submodule update`. This is the wording `unloaded_manifest_row` gives a + `project.yaml` that does not load, so the two files are answered alike. + + THE NEXT IS THE TOOL THAT REFUSED, NOT THE VALIDATOR AND NEVER A FETCH. + `update-shape.py check` asks exactly the question this row could not + answer, so it refuses the same way until the file reads as a pin and + compares the copies the moment it does -- including for the refusals + `validate-pins.py` does not make, which is why it is not that script + named here (the `pins` row names it whenever it is red). The comment + names the file, because the file is the fix, and says what a fix is not: + which side of a merge conflict, what a hand edit meant, whether a deleted + pin is restored or the project was never pinned -- all of it a person's + call, and the doctor writes nothing. + """ + pin = ctx.root / SHAPE_PIN + detail = {"error": exc.code, "pin": SHAPE_PIN, + "refusal": {"code": exc.code, "detail": exc.detail}} + return Row("shape-currency", LABEL_SHAPE_CURRENCY, FINDING, + f"the copies cannot be compared, because {SHAPE_PIN} does not " + f"read as a pin: REFUSED {exc.code}: {exc.detail}", + f"{PYTHON} {quote_arg(ctx.shape / 'update-shape.py')} check " + f"--root {quote_arg(ctx.root)} --upstream " + f"{quote_arg(ctx.shape)} # the file at fault is " + f"{quote_arg(pin)}, and the line above says what is wrong " + "with it; this refuses it the same way until it reads as a " + "pin, and compares the copies the moment it does. What it " + "should say -- a merge conflict resolved, a deleted file " + "restored, a hand edit put right -- is the person's call", + detail) + + def check_shape_currency(ctx: Context) -> Row: """`update-shape.py check`, summarised, against THIS checkout. @@ -1089,10 +1147,25 @@ def check_shape_currency(ctx: Context) -> Row: the shape this project is compared against is the checkout this file sits in, at its HEAD. What that costs is honesty about which standard answered, which is why both commits are in the row. + + TWO KINDS OF REFUSAL, TOLD APART BY WHICH READ RAISED THEM (#237). + `prepare` asks two things: what does the ROOT's own pin say, and can THIS + checkout of the standard answer about the commit it names. A refusal from + the first is about their repository and is `unreadable_pin_row`'s; one + from the second is this checkout failing to ask and stays CANNOT ANSWER. + The pin is read here first, on its own, with `read_pin` -- the function + `prepare` calls for it -- so the split is made by what was being read and + not by a list of refusal codes or a match on their text, and a refusal + `read_pin` or `open_upstream` learns later lands on the right side + without this file being told. """ us = ctx.update_shape() args = argparse.Namespace(root=str(ctx.root), upstream=str(ctx.shape), at=None) + try: + us.read_pin(ctx.root) + except Refusal as exc: + return unreadable_pin_row(ctx, exc) try: _root, _pin, rows, additions, upstream, pinned, target, _kind = \ us.prepare(args) diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index ac50577e..fbac0b6b 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2206,6 +2206,228 @@ def test_a_pin_this_standard_cannot_resolve_is_exit_three(standard, project, assert "fetch --all" in plain.stderr +# --- a root's OWN pin that does not read is the root's fault (#237) --------- +# +# `check_shape_currency` took EVERY refusal `update-shape.py prepare` raised +# for this checkout failing to ask. Most of them are: the pin names a commit +# this standard does not carry, the standard cannot be read. But `prepare` +# also reads the ROOT's own `contracts/shape-pin.yaml`, and a merge conflict +# in it, a `commit:` that is not 40 hex, a deleted file are somebody's repository +# being wrong. They answered CANNOT ANSWER, exit 3, outranking every red +# gate, with a NEXT that sent the reader to `git fetch --all` in the +# standard. AGENTS.md says exit 3 is "never a statement about their +# repository"; each case below is one, and is INVALID, exit 1. + +SHAPE_PIN = "contracts/shape-pin.yaml" + + +def pin_with(old: str, new: str): + """A mutation of the pin's text: `old` (a regex, multiline) becomes `new`.""" + return lambda text: re.sub(old, new, text, count=1, + flags=re.MULTILINE).encode("utf-8") + + +#: The pin with a merge conflict left in it where its `commit:` line was: the +#: two sides of the realistic two-shape-update merge, markers and all. +conflicted_pin = pin_with( + r'^commit: .*$', + '<<<<<<< HEAD\ncommit: "' + "1" * 40 + '"\n=======\n' + 'commit: "' + "2" * 40 + '"\n>>>>>>> edit-a') + +#: The one fault `validate-pins.py` PASSES, so the shape-currency row is the +#: only red one under it. +other_digest_definition = pin_with(r'^digest_definition: .*$', + 'digest_definition: other-v9') + +#: `(what the pin becomes, the refusal `read_pin` raises for it)`. A mutation +#: returns the file's new bytes, or None to delete it. The code is asserted +#: on every case so the case under test is the case reached. +ROOT_PIN_FAULTS = [ + pytest.param(conflicted_pin, "yaml-unparsable", + id="a-merge-conflict-in-it"), + pytest.param(lambda text: b"\t: : [\n", "yaml-unparsable", + id="not-yaml-at-all"), + pytest.param(lambda text: text.encode("utf-8") + b"\xff\xfe\n", + "yaml-unreadable", id="a-byte-that-is-not-utf-8"), + pytest.param(lambda text: b"- one\n- two\n", "update-pin-unreadable", + id="a-list-not-a-mapping"), + pytest.param(pin_with(r'^commit: .*$', 'commit: "zzzz"'), + "update-pin-commit", id="a-commit-that-is-not-40-hex"), + pytest.param(other_digest_definition, "update-pin-digest-definition", + id="another-digest-definition"), + pytest.param(pin_with(r'(?s)^files:.*\Z', 'files: []\n'), + "update-pin-no-files", id="no-files-rows"), + pytest.param(pin_with(r'^ - path: .*$', ' - nopath: x'), + "update-pin-row", id="a-row-with-no-path"), + pytest.param(lambda text: None, "update-pin-missing", + id="a-deleted-pin"), +] + + +def break_the_pin(root: Path, mutation) -> None: + """Rewrite `root`'s `contracts/shape-pin.yaml` as `mutation` says. + + BYTES, never `write_text`: a Windows `write_text` would turn the `\\n`s + into `\\r\\n` and make one more thing differ between the OSes than the + case is about. + """ + pin = root / SHAPE_PIN + mutated = mutation(pin.read_text(encoding="utf-8")) + if mutated is None: + pin.unlink() + else: + pin.write_bytes(mutated) + + +@pytest.mark.parametrize("mutation, code", ROOT_PIN_FAULTS) +def test_a_root_pin_that_does_not_read_is_invalid_not_cannot_answer( + standard, project, mutation, code): + """Each way the ROOT's pin can fail `read_pin`: a finding, exit 1. + + Asserted in both outputs, because the two are read by different people: + `--json` by a scheduled job (the verdict, the row's status and its + `detail`), the text by the person who follows the NEXT. Neither says + `environment`, and stderr carries no `REFUSED shape-doctor-cannot-answer` + and no `fetch --all` for it. + """ + break_the_pin(project, mutation) + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + payload = json.loads(result.stdout) + assert payload["exit"] == 1 + assert payload["verdict"].startswith("INVALID ("), payload["verdict"] + assert "CANNOT ANSWER" not in payload["verdict"] + row = rows_of(result)["shape-currency"] + assert row["status"] == "FINDING", row + assert row["detail"]["error"] == code, row["detail"] + assert row["detail"]["refusal"]["code"] == code + assert row["detail"]["pin"] == SHAPE_PIN + assert "environment" not in row["detail"], row["detail"] + # The loader's own words, whole -- and not its `Remediation:` trailer, + # which for a pin that does not parse is about submodules. + assert f"REFUSED {code}: " in row["reason"], row["reason"] + assert "Remediation" not in row["reason"] + assert "submodule" not in row["reason"] + # The NEXT is the tool that refused, with the file at fault named; it is + # not a fetch in the standard, which cannot change the pin. + assert "update-shape.py" in row["next"] and " check " in row["next"] + assert "shape-pin.yaml" in row["next"] + assert "fetch" not in row["next"] + plain = doctor(standard, project) + assert plain.returncode == 1, plain.stdout + plain.stderr + assert "CANNOT ANSWER" not in plain.stdout + assert "shape-doctor-cannot-answer" not in plain.stderr + assert "fetch --all" not in plain.stdout + plain.stderr + assert verdict_line(plain).startswith("INVALID (") + + +def test_a_merge_conflict_in_the_pin_still_names_the_projects_own_validator( + standard, project): + """What the issue's reader saw and misread: the `pins` row was right. + + The validator the project carries refuses the same file, in its own + words, and its row names it as the next command. The verdict names the + gates that are red -- it used to be one row that was wrong outranking the + two that were right -- and the shape-currency row is the third finding + about the one file, not the verdict's reason for being. + """ + break_the_pin(project, conflicted_pin) + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + rows = rows_of(result) + assert rows["pins"]["status"] == "FINDING" + assert "validate-pins.py" in rows["pins"]["next"] + assert "yaml-unparsable" in rows["pins"]["reason"] + assert json.loads(result.stdout)["verdict"] == \ + "INVALID (pins, manifest-kinds)" + + +def test_a_pin_only_update_shape_refuses_is_the_one_red_row(standard, + project): + """`digest_definition: other-v9` is what `validate-pins.py` PASSES. + + So the shape-currency row is the only finding, and it is not on the + ladder's named list: the catch-all names it, `INVALID (shape-currency)`, + exit 1 -- which is why its NEXT is `update-shape.py check` and not the + validator, whose exit 0 would send this reader nowhere. + """ + break_the_pin(project, other_digest_definition) + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + rows = rows_of(result) + assert rows["pins"]["status"] == "ok", rows["pins"] + assert rows["manifest-kinds"]["status"] == "ok" + assert [name for name, row in rows.items() + if row["status"] == "FINDING"] == ["shape-currency"] + assert json.loads(result.stdout)["verdict"] == "INVALID (shape-currency)" + + +def stranger_standard(standard: Path, tmp_path: Path) -> Path: + """A standard whose history is not the one a project was scaffolded from, + so a pin naming the scaffold's commit is a commit it does not carry. + Built the way `test_a_pin_this_standard_cannot_resolve_is_exit_three` + builds it; that test is left exactly as it was.""" + stranger = tmp_path / "stranger" + stranger.mkdir() + for name in ("contracts", "scripts", "templates"): + shutil.copytree(standard / name, stranger / name, symlinks=True) + for name in ("update-shape.py", "shape-doctor.py", "scaffold-project.py", + "setup-project.py", "setup.sh", "openRepoShape"): + shutil.copy2(standard / name, stranger / name) + git("init", "-q", "-b", "main", ".", cwd=stranger) + commit_all(stranger, "A standard with a history of its own") + return stranger + + +def test_what_decides_is_the_pin_and_not_the_standard(standard, project, + tmp_path): + """The control, and the pair that separates the two kinds of refusal. + + One standard that lacks the scaffold's commit. Asked about a root whose + pin is good it cannot answer -- CANNOT ANSWER, exit 3, `fetch --all`, as + on main, with `environment` in the row's `detail` and the standard's own + refusal as its error. Asked about the same root with a conflicted pin it + never gets as far as the commit: the file is wrong, so INVALID, exit 1. + """ + stranger = stranger_standard(standard, tmp_path) + result = doctor(stranger, project, "--json") + assert result.returncode == 3, result.stdout + result.stderr + payload = json.loads(result.stdout) + assert payload["exit"] == 3 + assert payload["verdict"].startswith("CANNOT ANSWER (shape currency:") + detail = rows_of(result)["shape-currency"]["detail"] + assert detail["environment"] is True + assert detail["error"] == "update-revision-unresolvable" + plain = doctor(stranger, project) + assert plain.returncode == 3 + assert "REFUSED shape-doctor-cannot-answer" in plain.stderr + assert "fetch --all" in plain.stderr + break_the_pin(project, conflicted_pin) + broken = doctor(stranger, project, "--json") + assert broken.returncode == 1, broken.stdout + broken.stderr + assert json.loads(broken.stdout)["verdict"].startswith("INVALID (") + row = rows_of(broken)["shape-currency"] + assert row["detail"]["error"] == "yaml-unparsable" + assert "environment" not in row["detail"] + assert "shape-doctor-cannot-answer" not in broken.stderr + + +def test_a_family_holders_conflicted_pin_is_invalid_too(standard, holder, + tmp_path): + """The row is registered for both kinds of root, and answers alike.""" + root = tmp_path / FAMILY_NAME + shutil.copytree(holder["root"], root, symlinks=True) + break_the_pin(root, conflicted_pin) + result = doctor(standard, root, "--json") + assert result.returncode == 1, result.stdout + result.stderr + payload = json.loads(result.stdout) + assert payload["kind"] == "family" + assert payload["verdict"].startswith("INVALID (") + row = rows_of(result)["shape-currency"] + assert row["status"] == "FINDING" and "environment" not in row["detail"] + assert row["detail"]["error"] == "yaml-unparsable" + + def test_the_doctor_writes_no_bytecode_into_the_standard(standard, project, tmp_path): """"It writes nothing" has to be true of the STANDARD too. From 092a5f914d5741c6a4161d65897be670c381abc1 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sun, 11 Oct 2026 22:46:25 +0000 Subject: [PATCH 02/11] README.md: take back the shape-pin sentence added to the exit-3 paragraph (#237) `tests/test_repo_hygiene.py` caps the README at 1416 lines and pins `docs/handbook.html` to the README's blob; the sentence took it to 1420 and moved the blob, and the doctor's README paragraph already says what stays true (exit 3 is a pin naming a commit THIS checkout does not carry). The argument lives in `unreadable_pin_row`'s and `check_shape_currency`'s docstrings instead. The previous commit's body says the README gained a sentence; this one removes it, and the branch has no README change against its base. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- README.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/README.md b/README.md index c906f2eb..095a1b9e 100644 --- a/README.md +++ b/README.md @@ -1160,11 +1160,7 @@ way, with its own next command. `MISPLACED` sits between the two: a path in the wrong leg is a fact about the shape that no validator, pin or manifest here can see, while a red validator names itself in the table whether or not it also names the verdict line. `--json` is the same report as one object, -each row carrying its `id`. A root's own `contracts/shape-pin.yaml` that does -not read as a pin — a merge conflict in it, a `commit:` that is not 40 hex, a -deleted file — is not exit 3, which is for a standard that cannot answer: it is -a statement about the root, a `FINDING` on **shape currency** whose next -command is `update-shape.py check`, and the verdict is `INVALID`, exit 1. +each row carrying its `id`. **`placement` is the row that reads what is *inside* the legs.** Brett Heap, 2026-09-10: *"we have to look for code in spec and spec in code"*. It runs the From 7c11c62b8320187a5f894ec3f96cefd2f73d944d Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:07:27 +0000 Subject: [PATCH 03/11] shape-doctor.py: a root's own `contracts/shape-pin.yaml` that does not read names the git command that puts back the last pin a tool wrote (`checkout --ours`, `checkout HEAD` or `checkout ~1`, chosen from the pin's own git state), so the row's NEXT fixes it instead of refusing it again (#237) The review of PR #267 at 092a5f9 found the new row's NEXT a dead end (F1). `update-shape.py check` reads the pin with the same `read_pin`, so run as printed it exited 2 with the same refusal every time, and the remediation it then printed was wrong for the case: `git submodule update` for a conflict, "adopt it" for a deleted pin, "re-save the file" (a hand edit of a pinned file) for a stray byte. An operator who follows AGENTS.md ("Run the one the row names ... never edit a pinned file") was stuck, and README's "names the exact command that fixes it" was false for this row. `pin_restore` now picks the command from the pin's state, read-only: - unmerged (porcelain UU, AA, ...): `checkout --ours` (or `--theirs` where only that side holds a pin, read off the index stages, not the letters) of every conflicted path the shape writes: the pin, the manifest, README.md and the copies, never a file of the project's own; - uncommitted, where HEAD holds a pin: `checkout HEAD -- `; - otherwise, where the last commit to change the pin has a parent that holds one: `checkout ~1 -- `; - none of these: the old `update-shape.py check` NEXT, saying that whether the root was ever scaffolded or adopted is a person's call. `--json` records which, as `detail["restore"]`. Status is read as bytes with `--no-optional-locks`, so the doctor still writes nothing. N2: no note carries a `;`, an apostrophe or a backtick, so a pasted line is one command (#277). N1: the docstring names #232's `unloaded_manifest_row`, which is not on main or this branch. Tests run each printed command and ask the doctor again: the nine uncommitted faults and two committed ones (a `git rm` and a `commit: "zzzz"`, each followed by an unrelated commit) end COMPLIANT, exit 0; the issue's own merge, with `Makefile` and a file of the project's own also conflicted, names only `Makefile` and the pin and then reads the pin; a squashed root commit keeps the check NEXT. At 092a5f9 each of these fails, and head's NEXT run as printed exits 2. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 154 +++++++++++++++++++++++++----- tests/test_shape_doctor.py | 191 +++++++++++++++++++++++++++++++++++-- 2 files changed, 313 insertions(+), 32 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index a073a805..294f733a 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -1085,10 +1085,113 @@ def what_a_re_pin_would_move(ctx: Context, us, repository: str, #: sentence, its `--json` detail and its next command (#237). SHAPE_PIN = "contracts/shape-pin.yaml" +#: `git status --porcelain=v1`'s codes for an UNMERGED path, as git's own +#: documentation tabulates them. Which side holds the file is NOT read off +#: the letters (`AU` is this side alone, `UA` the other alone): it is read +#: off the index stages `ls-files -u` lists, 2 for `--ours`, 3 for +#: `--theirs`. +UNMERGED_STATUS = ("DD", "AU", "UD", "UA", "DU", "AA", "UU") + + +def git_answer(args: list, cwd: Path, binary: bool = False): + """`git_out`, or None where git does not answer: a non-zero exit, no + repository, no `git` on the path, output that is not UTF-8.""" + try: + return git_out(args, cwd=cwd, binary=binary) + except (Refusal, OSError, ValueError): + return None + + +def pin_restore(ctx: Context) -> tuple[str, str | None]: + """`(restore, command)`: the git command that puts back a pin a tool + wrote, chosen from the pin's own git state, read-only (#237). + + A PIN IS NEVER MENDED BY HAND, so the fix for one that does not read is + the last one a tool wrote, out of the root's own history. Which command + reaches it depends only on where the bad bytes are, and the first of + these that applies decides, as `detail["restore"]`: + + unmerged a merge left the pin conflicted. `checkout --ours`, or + `--theirs` where only that side holds a pin, of every + conflicted path the shape writes -- the pin, the manifest + that mirrors it, the README's `Shape:` line, the copies -- + because a merge of two `update-shape.py apply` runs + conflicts in them together, and one side taken whole is one + tool's output. A file of the project's own is never named: + its merge is theirs. + uncommitted HEAD holds a pin and the tree or the index differs from it. + `checkout HEAD`, which drops a change no tool of this + standard leaves uncommitted. + committed the last commit to change the pin broke or removed it. + `checkout ~1`, the pin as it was before it -- + named only where that commit has a parent holding a pin. + none nothing above: no history, a root commit, no git. Nothing + git can name fixes it, and the caller says so. + + THE STATUS IS TWO FIXED-WIDTH LETTERS, READ AS BYTES: `git_out`'s text + form strips its output, and the leading space of ` M` with it. And + `--no-optional-locks` (git 2.15) keeps `status` from refreshing the + index, which would be this command writing into their repository. Every + path is read relative to the root (`ls-files` and pathspecs are, and + `:./` is), so each one names the root's own file even where + git answers from a checkout the root sits inside. + """ + root = ctx.root + git_c = f"git -C {quote_arg(root)}" + raw = git_answer(["--no-optional-locks", "status", "--porcelain=v1", + "--", SHAPE_PIN], root, binary=True) + xy = raw.decode("utf-8", "replace")[:2] if raw else "" + if xy in UNMERGED_STATUS: + us = ctx.update_shape() + kind = us.root_kind(root) + shaped = {SHAPE_PIN, kind.manifest, us.README} + for rel in kind.copy_source: + shaped.update((rel, f"{us.COLLISION_DIR}/{rel}")) + # ` SP SP TAB `, one record per stage. + stages: dict[str, set] = {} + listing = git_answer(["ls-files", "-u", "-z"], root, binary=True) + for record in (listing or b"").split(b"\0"): + meta, _tab, path = record.partition(b"\t") + name = path.decode("utf-8", "replace") + if name in shaped: + stages.setdefault(name, set()).add(meta.rsplit(b" ", 1)[-1]) + pin_stages = stages.get(SHAPE_PIN, set()) + side, stage = (("--ours", b"2") if b"2" in pin_stages else + ("--theirs", b"3") if b"3" in pin_stages else + (None, None)) + if side: + note = ("--theirs takes the other side. Take one side whole" + if b"3" in pin_stages and side == "--ours" + else "only that side of the merge holds a pin. Take it " + "whole") + named = sorted(name for name, held in stages.items() + if stage in held) + return "unmerged", ( + f"{git_c} checkout {side} -- " + + " ".join(quote_arg(name) for name in named) + + f" # {note} for every path named and never a " + "hand-merged pin, then finish the merge as git says and run " + "this doctor again") + elif xy and git_answer(["cat-file", "-e", f"HEAD:./{SHAPE_PIN}"], + root) is not None: + return "uncommitted", ( + f"{git_c} checkout HEAD -- {quote_arg(SHAPE_PIN)} # puts back " + "the pin as last committed and discards the uncommitted change " + "that broke it, which no tool of this standard writes") + last = git_answer(["rev-list", "-n", "1", "HEAD", "--", SHAPE_PIN], root) + if last and git_answer(["cat-file", "-e", f"{last}~1:./{SHAPE_PIN}"], + root) is not None: + return "committed", ( + f"{git_c} checkout {quote_arg(last + '~1')} -- " + f"{quote_arg(SHAPE_PIN)} # the pin as it was before " + f"{last[:12]}, the last commit to change it. Commit that rather " + "than editing the pin") + return "none", None + def unreadable_pin_row(ctx: Context, exc: Refusal) -> Row: """The `shape currency` row for a root whose OWN pin does not read as one - (#237): a FINDING, the refusal quoted whole, and the file to put right. + (#237): a FINDING, the refusal quoted whole, and the pin put back. A STATEMENT ABOUT THEIR REPOSITORY, NOT A FAILURE TO ASK. The pin is a file in the root: a merge conflict left in it, a byte that is not UTF-8, @@ -1108,35 +1211,34 @@ def unreadable_pin_row(ctx: Context, exc: Refusal) -> Row: Its `Remediation:` trailer is left off on purpose: `yaml-unparsable` carries `repo_shape`'s default one, about submodules and the lockstep invariant, which would send a person resolving a merge conflict to `git - submodule update`. This is the wording `unloaded_manifest_row` gives a - `project.yaml` that does not load, so the two files are answered alike. - - THE NEXT IS THE TOOL THAT REFUSED, NOT THE VALIDATOR AND NEVER A FETCH. - `update-shape.py check` asks exactly the question this row could not - answer, so it refuses the same way until the file reads as a pin and - compares the copies the moment it does -- including for the refusals - `validate-pins.py` does not make, which is why it is not that script - named here (the `pins` row names it whenever it is red). The comment - names the file, because the file is the fix, and says what a fix is not: - which side of a merge conflict, what a hand edit meant, whether a deleted - pin is restored or the project was never pinned -- all of it a person's - call, and the doctor writes nothing. - """ - pin = ctx.root / SHAPE_PIN + submodule update`. This is the wording #232's `unloaded_manifest_row` + gives a `project.yaml` that does not load, so the two files are answered + alike. + + THE NEXT PUTS BACK THE PIN A TOOL WROTE, NEVER A FETCH. `pin_restore` + names the git command from the pin's own state, and run as printed it + ends at `COMPLIANT` or at a row that reads the pin. Naming the tool that + refused, `update-shape.py check`, was a dead end (review of PR #267): it + reads the pin with the same `read_pin`, refuses it the same way, and its + remediation is about submodules, adoption or a hand edit, none of which + fits. That command stays only where git can name nothing, and its note + says the rest is a person's call. Each note holds no `;`, apostrophe or + backtick, so a pasted line is one command in any shell (#277). + """ + restore, command = pin_restore(ctx) detail = {"error": exc.code, "pin": SHAPE_PIN, - "refusal": {"code": exc.code, "detail": exc.detail}} + "refusal": {"code": exc.code, "detail": exc.detail}, + "restore": restore} + if command is None: + command = (f"{PYTHON} {quote_arg(ctx.shape / 'update-shape.py')} " + f"check --root {quote_arg(ctx.root)} --upstream " + f"{quote_arg(ctx.shape)} # no commit in this root holds " + "a pin to put back, so whether it was ever scaffolded or " + "adopted is for a person to decide") return Row("shape-currency", LABEL_SHAPE_CURRENCY, FINDING, f"the copies cannot be compared, because {SHAPE_PIN} does not " f"read as a pin: REFUSED {exc.code}: {exc.detail}", - f"{PYTHON} {quote_arg(ctx.shape / 'update-shape.py')} check " - f"--root {quote_arg(ctx.root)} --upstream " - f"{quote_arg(ctx.shape)} # the file at fault is " - f"{quote_arg(pin)}, and the line above says what is wrong " - "with it; this refuses it the same way until it reads as a " - "pin, and compares the copies the moment it does. What it " - "should say -- a merge conflict resolved, a deleted file " - "restored, a hand edit put right -- is the person's call", - detail) + command, detail) def check_shape_currency(ctx: Context) -> Row: diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index fbac0b6b..ba0740bd 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2279,6 +2279,60 @@ def break_the_pin(root: Path, mutation) -> None: pin.write_bytes(mutated) +#: Every code `read_pin` raises for a root's own pin, read off the faults +#: above rather than retyped: a row that still says one of these has not +#: read the pin. +READ_PIN_CODES = frozenset(param.values[1] for param in ROOT_PIN_FAULTS) + + +def pasted(line: str) -> list[str]: + """The argv the reader's own shell makes of a printed NEXT. + + `shlex` on POSIX, over the command before the row's ` #` note, the way + `test_a_red_validators_next_command_is_runnable` reads one; on Windows + PowerShell's reading, through `powershell_tokens`, which drops the note + itself and unquotes `quote_arg`'s verbatim strings. + """ + if os.name == "nt": + return powershell_tokens(line) + return shlex.split(line.split(" #", 1)[0]) + + +def run_pasted(line: str) -> None: + """Run a printed NEXT exactly as a reader would, and require exit 0.""" + proc = subprocess.run(pasted(line), capture_output=True, text=True, + check=False, env={**os.environ, **GIT_IDENTITY}) + assert proc.returncode == 0, line + "\n" + proc.stdout + proc.stderr + + +def assert_a_plain_note(line: str) -> None: + """The NEXT's ` #` note holds no `;`, apostrophe or backtick (#277). + + Pasted into an interactive zsh that does not take `#` as a comment, a + `;` starts a second command, an apostrophe opens a quotation nothing + closes and a backtick runs whatever follows it. + """ + note = line.split(" #", 1)[1] + for char in (";", "'", "`"): + assert char not in note, (char, line) + + +def commit_in(root: Path, message: str, *args: str) -> str: + """`git commit` in `root` with an identity that is never global; the new + HEAD.""" + proc = subprocess.run(["git", "commit", "-q", *args, "-m", message], + cwd=str(root), capture_output=True, text=True, + check=False, env={**os.environ, **GIT_IDENTITY}) + assert proc.returncode == 0, proc.stderr + proc.stdout + return git("rev-parse", "HEAD", cwd=root).stdout.strip() + + +def compliant_again(standard: Path, root: Path) -> None: + result = doctor(standard, root, "--json") + assert result.returncode == 0, result.stdout + result.stderr + assert json.loads(result.stdout)["verdict"] == "COMPLIANT" + + @pytest.mark.parametrize("mutation, code", ROOT_PIN_FAULTS) def test_a_root_pin_that_does_not_read_is_invalid_not_cannot_answer( standard, project, mutation, code): @@ -2308,17 +2362,25 @@ def test_a_root_pin_that_does_not_read_is_invalid_not_cannot_answer( assert f"REFUSED {code}: " in row["reason"], row["reason"] assert "Remediation" not in row["reason"] assert "submodule" not in row["reason"] - # The NEXT is the tool that refused, with the file at fault named; it is - # not a fetch in the standard, which cannot change the pin. - assert "update-shape.py" in row["next"] and " check " in row["next"] - assert "shape-pin.yaml" in row["next"] + # The NEXT puts back the pin a tool wrote, from the root's own history: + # the change that broke it is not committed, so that is `checkout HEAD`. + # It is not a fetch in the standard, which cannot change the pin, nor the + # `update-shape.py check` that refuses the same file the same way. + assert row["detail"]["restore"] == "uncommitted", row["detail"] + assert f"checkout HEAD -- {SHAPE_PIN}" in row["next"], row["next"] + assert pasted(row["next"])[:2] == ["git", "-C"], row["next"] assert "fetch" not in row["next"] + assert "update-shape.py" not in row["next"] + assert_a_plain_note(row["next"]) plain = doctor(standard, project) assert plain.returncode == 1, plain.stdout + plain.stderr assert "CANNOT ANSWER" not in plain.stdout assert "shape-doctor-cannot-answer" not in plain.stderr assert "fetch --all" not in plain.stdout + plain.stderr assert verdict_line(plain).startswith("INVALID (") + # And run as printed, it fixes it. + run_pasted(row["next"]) + compliant_again(standard, project) def test_a_merge_conflict_in_the_pin_still_names_the_projects_own_validator( @@ -2348,8 +2410,8 @@ def test_a_pin_only_update_shape_refuses_is_the_one_red_row(standard, So the shape-currency row is the only finding, and it is not on the ladder's named list: the catch-all names it, `INVALID (shape-currency)`, - exit 1 -- which is why its NEXT is `update-shape.py check` and not the - validator, whose exit 0 would send this reader nowhere. + exit 1 -- which is why its NEXT is not the validator, whose exit 0 would + send this reader nowhere, but the pin put back from the root's history. """ break_the_pin(project, other_digest_definition) result = doctor(standard, project, "--json") @@ -2426,6 +2488,123 @@ def test_a_family_holders_conflicted_pin_is_invalid_too(standard, holder, row = rows_of(result)["shape-currency"] assert row["status"] == "FINDING" and "environment" not in row["detail"] assert row["detail"]["error"] == "yaml-unparsable" + assert row["detail"]["restore"] == "uncommitted", row["detail"] + run_pasted(row["next"]) + again = rows_of(doctor(standard, root, "--json"))["shape-currency"] + assert again["detail"].get("error") not in READ_PIN_CODES, again + + +# --- the pin put back: the NEXT for each state git can be in (#237) --------- +# +# Each test below runs the printed NEXT and asks the doctor again. The NEXT +# is chosen from the pin's own git state, read-only: unmerged, then +# uncommitted (the parametrised test above), then committed, then none. + +#: `commit: "zzzz"`, the committed fault the review probed beside a deletion. +not_forty_hex = pin_with(r'^commit: .*$', 'commit: "zzzz"') + + +@pytest.mark.parametrize("mutation, code", [ + pytest.param(lambda text: None, "update-pin-missing", + id="the-pin-deleted-in-a-commit"), + pytest.param(not_forty_hex, "update-pin-commit", + id="a-commit-line-not-40-hex-committed"), +]) +def test_a_pin_broken_in_a_commit_is_put_back_from_before_that_commit( + standard, project, mutation, code): + """Clean status, so `checkout HEAD` would put back the broken pin. + + The NEXT names the last commit to change the pin, `~1`: the pin as it + was before it. An unrelated commit after the break is what tells that + apart from `HEAD~1`. + """ + break_the_pin(project, mutation) + git("add", "--", SHAPE_PIN, cwd=project) + breaking = commit_in(project, "break the pin") + commit_in(project, "an unrelated commit after it", "--allow-empty") + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + row = rows_of(result)["shape-currency"] + assert row["detail"]["error"] == code, row["detail"] + assert row["detail"]["restore"] == "committed", row["detail"] + argv = pasted(row["next"]) + assert argv[:2] == ["git", "-C"], row["next"] + assert argv[3:] == ["checkout", f"{breaking}~1", "--", SHAPE_PIN], argv + assert breaking[:12] in row["next"].split(" #", 1)[1], row["next"] + assert_a_plain_note(row["next"]) + run_pasted(row["next"]) + compliant_again(standard, project) + + +def test_the_issues_own_merge_takes_one_side_of_the_shapes_files_whole( + standard, project): + """The issue's reproduction: two branches rewrite `commit:`, then merge. + + The pin is `UU`, and so are `Makefile`, a shape copy, and a file of the + project's own. The NEXT takes one side whole of every conflicted file the + shape writes and of no other, so the project's own file is left to its + owner. Run, the pin reads again: the row then answers about the commit + it names, which in this fixture is no commit at all. + """ + own = project / "notes.md" + own.write_bytes(b"base\n") + git("add", "--", "notes.md", cwd=project) + base = commit_in(project, "a file of the project's own") + for branch, digit in (("edit-a", "1"), ("edit-b", "2")): + git("checkout", "-q", "-b", branch, base, cwd=project) + break_the_pin(project, pin_with(r'^commit: .*$', + f'commit: "{digit * 40}"')) + for name in ("Makefile", "notes.md"): + path = project / name + path.write_bytes(path.read_bytes() + f"# {branch}\n".encode()) + git("add", "--", SHAPE_PIN, "Makefile", "notes.md", cwd=project) + commit_in(project, branch) + merged = subprocess.run(["git", "merge", "-q", "edit-a"], + cwd=str(project), capture_output=True, text=True, + check=False, env={**os.environ, **GIT_IDENTITY}) + assert merged.returncode != 0, merged.stdout + merged.stderr + status = git("status", "--porcelain=v1", cwd=project).stdout + for name in (SHAPE_PIN, "Makefile", "notes.md"): + assert f"UU {name}" in status, status + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + row = rows_of(result)["shape-currency"] + assert row["detail"]["error"] == "yaml-unparsable", row["detail"] + assert row["detail"]["restore"] == "unmerged", row["detail"] + assert row["next"].startswith("git -C "), row["next"] + assert "checkout --ours -- " in row["next"], row["next"] + argv = pasted(row["next"]) + assert argv[3:] == ["checkout", "--ours", "--", "Makefile", SHAPE_PIN], \ + argv + assert_a_plain_note(row["next"]) + run_pasted(row["next"]) + again = rows_of(doctor(standard, project, "--json"))["shape-currency"] + assert again["detail"]["error"] not in READ_PIN_CODES, again + assert again["detail"]["error"] == "update-revision-unresolvable", again + assert b"<<<<<<<" in own.read_bytes() + + +def test_a_pin_with_no_good_copy_in_its_history_keeps_the_check_next( + standard, project): + """No history to put a pin back from: one root commit holds a bad pin. + + Nothing git can name fixes it, so the NEXT stays the tool that refused, + and says that whether this root was ever scaffolded or adopted is a + person's decision. + """ + break_the_pin(project, not_forty_hex) + git("checkout", "-q", "--orphan", "squashed", cwd=project) + git("add", "--", SHAPE_PIN, cwd=project) + commit_in(project, "the whole history, squashed into one commit") + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + row = rows_of(result)["shape-currency"] + assert row["detail"]["error"] == "update-pin-commit", row["detail"] + assert row["detail"]["restore"] == "none", row["detail"] + assert "update-shape.py" in row["next"] and " check " in row["next"] + assert "checkout" not in row["next"], row["next"] + assert "fetch" not in row["next"] + assert_a_plain_note(row["next"]) def test_the_doctor_writes_no_bytecode_into_the_standard(standard, project, From 9f64bd0b520e48431f573b22bb19bc2f7bc456d2 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:04:47 +0000 Subject: [PATCH 04/11] tests/test_shape_doctor.py: a bad pin staged where HEAD holds none is put back from before the commit that deleted it, and a merge where only the other side holds a pin takes `--theirs`, so the two `pin_restore` branches no earlier test reached are pinned (#237) `pin_restore` (7c11c62) has two choices that none of the round's first tests exercised: - `A ` in the status: the pin was deleted in a commit and a bad one staged at its path. HEAD holds no pin, so `checkout HEAD` names a path git refuses. The code asks `cat-file -e HEAD:./` before it says "uncommitted", and falls through to the last commit that changed the pin, `~1`. The test asserts `committed`, the argv, and that running it ends COMPLIANT. It kills the mutant that drops the `cat-file` check (`elif xy:`). - `DU`: this branch deleted the pin and the other changed it. Only index stage 3 holds a pin, so `--ours` would refuse the path. The test asserts `checkout --theirs -- `, a note that says only that side holds a pin, and that the pin reads once it has run. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_shape_doctor.py | 62 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index ba0740bd..3902b397 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2584,6 +2584,68 @@ def test_the_issues_own_merge_takes_one_side_of_the_shapes_files_whole( assert b"<<<<<<<" in own.read_bytes() +def test_a_bad_pin_staged_where_head_holds_none_is_put_back_from_history( + standard, project): + """`A ` in the status: the pin was deleted in a commit, then a bad one + staged at its path. HEAD holds no pin, so `checkout HEAD` would name a + path HEAD does not have; the NEXT reaches back past the deletion.""" + pin = project / SHAPE_PIN + good = pin.read_text(encoding="utf-8") + git("rm", "-q", "--", SHAPE_PIN, cwd=project) + deleting = commit_in(project, "drop the pin") + pin.write_bytes(not_forty_hex(good)) + git("add", "--", SHAPE_PIN, cwd=project) + status = git("status", "--porcelain=v1", "--", SHAPE_PIN, + cwd=project).stdout + assert status.startswith("A "), status + row = rows_of(doctor(standard, project, "--json"))["shape-currency"] + assert row["detail"]["error"] == "update-pin-commit", row["detail"] + assert row["detail"]["restore"] == "committed", row["detail"] + assert pasted(row["next"])[3:] == [ + "checkout", f"{deleting}~1", "--", SHAPE_PIN], row["next"] + run_pasted(row["next"]) + compliant_again(standard, project) + + +def test_a_merge_where_only_the_other_side_holds_a_pin_takes_theirs( + standard, project): + """`DU`: this branch deleted the pin and the other changed it, so only + `--theirs` holds one -- `--ours` would refuse the path by name -- and the + note says that one side is all there is. + + Git leaves the other side's pin in the tree, and that one reads; the + row is reached once somebody keeping this branch's deletion removes it + too, with the merge still unfinished. + """ + base = git("rev-parse", "HEAD", cwd=project).stdout.strip() + git("checkout", "-q", "-b", "edit", base, cwd=project) + break_the_pin(project, pin_with(r'^commit: .*$', + 'commit: "' + "1" * 40 + '"')) + git("add", "--", SHAPE_PIN, cwd=project) + commit_in(project, "edit the pin") + git("checkout", "-q", "-b", "drop", base, cwd=project) + git("rm", "-q", "--", SHAPE_PIN, cwd=project) + commit_in(project, "drop the pin") + merged = subprocess.run(["git", "merge", "-q", "edit"], + cwd=str(project), capture_output=True, text=True, + check=False, env={**os.environ, **GIT_IDENTITY}) + assert merged.returncode != 0, merged.stdout + merged.stderr + (project / SHAPE_PIN).unlink() + status = git("status", "--porcelain=v1", "--", SHAPE_PIN, + cwd=project).stdout + assert status.startswith("DU "), status + row = rows_of(doctor(standard, project, "--json"))["shape-currency"] + assert row["detail"]["error"] == "update-pin-missing", row["detail"] + assert row["detail"]["restore"] == "unmerged", row["detail"] + assert pasted(row["next"])[3:] == [ + "checkout", "--theirs", "--", SHAPE_PIN], row["next"] + assert "only that side" in row["next"].split(" #", 1)[1] + assert_a_plain_note(row["next"]) + run_pasted(row["next"]) + again = rows_of(doctor(standard, project, "--json"))["shape-currency"] + assert again["detail"]["error"] == "update-revision-unresolvable", again + + def test_a_pin_with_no_good_copy_in_its_history_keeps_the_check_next( standard, project): """No history to put a pin back from: one root commit holds a bad pin. From 462baf1d8fd4e119e5a1cf976aeda86fbcfd31a6 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:41:08 +0000 Subject: [PATCH 05/11] shape-doctor.py: the `manifest kinds` row puts back a file a tool wrote under `contracts/` that does not load (a conflicted or non-UTF-8 shape pin, a leg pin, the naming copy) with the git command `pin_restore` picks from its own state, and leaves a file of the project's own to a person, so its NEXT no longer tells the reader to delete the pin (#291) The row collected every file under `contracts/` that does not load as a mapping into `unreadable`, and gave it the unregistered kind's sentence: "register the kind in shape-doctor.py's MANIFEST_VALIDATORS, beside the check that validates it, or remove the file from the root". A file that does not load has no `kind:` to register, so the one half a reader could follow was to delete the file -- for a conflicted `contracts/shape-pin.yaml`, the pin. AGENTS.md tells an agent to run the NEXT a FINDING names, and the sentence is not even a command: the apostrophe in "shape-doctor.py's" leaves `shlex.split` with no closing quotation. `pin_restore` (#237, PR #267) takes the file to put back, defaulting to the shape pin, so its four states, commands and notes are unchanged for the `shape currency` row. `tool_written` admits every `contracts/-pin.yaml` (the shape pin, a leg's, a neutral product's) and the shape copies the root kind lists, which under `contracts/` is the naming policy; each of those is put back from git, and only the shape pin takes the rest of the shape with it in a merge. A leg pin is named alone: `checkout --ours` on a conflicted gitlink exits 0 and changes nothing. A file of the project's own is not put back from git, because each of the three commands throws away somebody's edit. Its case is `project`, and where no file has a git command the NEXT is this doctor again, with a note that what the file should say is for a person to decide. The row quotes the loader's refusal for each file, less the path, and `detail` gains `restore` and `refusals` only when a file does not load. Where an unregistered kind sits beside a file that does not load, the row names the restore first; the next run names the sentence, which stays the unregistered kind's alone. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 165 +++++++++++++++++---- tests/test_shape_doctor.py | 290 +++++++++++++++++++++++++++++++++++++ 2 files changed, 428 insertions(+), 27 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 294f733a..12e48dcf 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -967,19 +967,44 @@ def check_manifest_kinds(ctx: Context) -> Row: reporting on somebody else's data. What is reported is the other case -- a file that DOES declare a kind that nothing here validates, which is a manifest travelling in a shape root with no gate behind it. + + A FILE THAT DOES NOT LOAD IS NOT AN UNREGISTERED KIND (#291). It used to + share that case's next command -- register the kind, or remove the file + from the root -- which no file without a readable `kind:` can follow, and + which told a reader with a conflicted `contracts/shape-pin.yaml` to + delete the pin. The row now quotes what the loader said about each one, + and `unreadable_next` picks the command. That sentence stays the + unregistered kind's alone, and where a row has both it names the + restore first, because the row cannot say what kind a file declares + until it loads. The next run names the other. `restore` and `refusals` + join `detail` only when a file does not load, so every other root + reports the row it always did. """ known: list[str] = [] unknown: list[str] = [] unreadable: list[str] = [] + refusals: dict[str, dict] = {} for path in manifest_candidates(ctx.root): - data = Context._read(path) + rel = path.relative_to(ctx.root).as_posix() + # `load_yaml` and not `Context._read`, which says None for both a + # refusal and an empty file: the row quotes the refusal, less the + # path it opens with, which the row has just named. + try: + data = load_yaml(path) + except Refusal as exc: + unreadable.append(rel) + refusals[rel] = {"code": exc.code, + "detail": exc.detail.removeprefix(f"{path}: ")} + continue if not isinstance(data, dict): - unreadable.append(path.relative_to(ctx.root).as_posix()) + unreadable.append(rel) + refusals[rel] = {"code": None, "detail": ( + "it holds no YAML document" if data is None else + f"it loads as a {type(data).__name__}, not a mapping")} continue kind = data.get("kind") if not isinstance(kind, str) or not kind: continue - rel = path.relative_to(ctx.root).as_posix() entry = MANIFEST_VALIDATORS.get(kind) (known if entry else unknown).append(f"{rel} ({kind})") detail = {"validated": known, "unregistered": unknown, @@ -990,13 +1015,18 @@ def check_manifest_kinds(ctx: Context) -> Row: if unknown: parts.append("no validator registered for kind " + ", ".join(unknown)) + command = ("register the kind in shape-doctor.py's " + "MANIFEST_VALIDATORS, beside the check that validates it, " + "or remove the file from the root") if unreadable: - parts.append("unreadable as YAML: " + ", ".join(unreadable)) + said = {rel: ": ".join(filter(None, (why["code"], why["detail"]))) + for rel, why in refusals.items()} + parts.append("unreadable as YAML: " + ", ".join( + f"{rel} ({said[rel]})" for rel in unreadable)) + restore, command = unreadable_next(ctx, unreadable) + detail.update({"restore": restore, "refusals": refusals}) return Row("manifest-kinds", LABEL_MANIFEST_KINDS, FINDING, - "; ".join(parts), - "register the kind in shape-doctor.py's " - "MANIFEST_VALIDATORS, beside the check that validates it, " - "or remove the file from the root", detail) + "; ".join(parts), command, detail) return Row("manifest-kinds", LABEL_MANIFEST_KINDS, OK, f"{len(known)} manifest(s), every declared kind has a " "registered validator", None, detail) @@ -1102,7 +1132,7 @@ def git_answer(args: list, cwd: Path, binary: bool = False): return None -def pin_restore(ctx: Context) -> tuple[str, str | None]: +def pin_restore(ctx: Context, pin: str = SHAPE_PIN) -> tuple[str, str | None]: """`(restore, command)`: the git command that puts back a pin a tool wrote, chosen from the pin's own git state, read-only (#237). @@ -1135,18 +1165,35 @@ def pin_restore(ctx: Context) -> tuple[str, str | None]: path is read relative to the root (`ls-files` and pathspecs are, and `:./` is), so each one names the root's own file even where git answers from a checkout the root sits inside. + + ANY FILE UNDER `contracts/` A TOOL WROTE, NOT THE SHAPE PIN ALONE (#291). + `pin` is the root's own `contracts/shape-pin.yaml` unless the caller + names another file `tool_written` admits -- a leg's pin, a neutral + product's, the naming policy's copy -- which is how the `manifest kinds` + row puts back each one that does not load. The four states, their + commands and their notes are the same for every one, with the note + saying `copy` where the file is not a pin. Only the shape pin takes + more paths with it in a merge: `apply` writes it, the manifest, the + README and the copies in one commit. Any other file is named alone. A + leg pin moves with its gitlink, but `checkout --ours` on a conflicted + gitlink exits 0 and leaves all three stages where they were, so naming + it would be a command that changes nothing. Finishing the merge takes + the gitlink, and the `pins` row then says whether the two agree. """ root = ctx.root git_c = f"git -C {quote_arg(root)}" + what = "pin" if pin.endswith("-pin.yaml") else "copy" raw = git_answer(["--no-optional-locks", "status", "--porcelain=v1", - "--", SHAPE_PIN], root, binary=True) + "--", pin], root, binary=True) xy = raw.decode("utf-8", "replace")[:2] if raw else "" if xy in UNMERGED_STATUS: - us = ctx.update_shape() - kind = us.root_kind(root) - shaped = {SHAPE_PIN, kind.manifest, us.README} - for rel in kind.copy_source: - shaped.update((rel, f"{us.COLLISION_DIR}/{rel}")) + shaped = {pin} + if pin == SHAPE_PIN: + us = ctx.update_shape() + kind = us.root_kind(root) + shaped.update((kind.manifest, us.README)) + for rel in kind.copy_source: + shaped.update((rel, f"{us.COLLISION_DIR}/{rel}")) # ` SP SP TAB `, one record per stage. stages: dict[str, set] = {} listing = git_answer(["ls-files", "-u", "-z"], root, binary=True) @@ -1155,40 +1202,104 @@ def pin_restore(ctx: Context) -> tuple[str, str | None]: name = path.decode("utf-8", "replace") if name in shaped: stages.setdefault(name, set()).add(meta.rsplit(b" ", 1)[-1]) - pin_stages = stages.get(SHAPE_PIN, set()) + pin_stages = stages.get(pin, set()) side, stage = (("--ours", b"2") if b"2" in pin_stages else ("--theirs", b"3") if b"3" in pin_stages else (None, None)) if side: note = ("--theirs takes the other side. Take one side whole" if b"3" in pin_stages and side == "--ours" - else "only that side of the merge holds a pin. Take it " - "whole") + else f"only that side of the merge holds a {what}. Take " + "it whole") named = sorted(name for name, held in stages.items() if stage in held) return "unmerged", ( f"{git_c} checkout {side} -- " + " ".join(quote_arg(name) for name in named) + f" # {note} for every path named and never a " - "hand-merged pin, then finish the merge as git says and run " - "this doctor again") - elif xy and git_answer(["cat-file", "-e", f"HEAD:./{SHAPE_PIN}"], + f"hand-merged {what}, then finish the merge as git says and " + "run this doctor again") + elif xy and git_answer(["cat-file", "-e", f"HEAD:./{pin}"], root) is not None: return "uncommitted", ( - f"{git_c} checkout HEAD -- {quote_arg(SHAPE_PIN)} # puts back " - "the pin as last committed and discards the uncommitted change " + f"{git_c} checkout HEAD -- {quote_arg(pin)} # puts back the " + f"{what} as last committed and discards the uncommitted change " "that broke it, which no tool of this standard writes") - last = git_answer(["rev-list", "-n", "1", "HEAD", "--", SHAPE_PIN], root) - if last and git_answer(["cat-file", "-e", f"{last}~1:./{SHAPE_PIN}"], + last = git_answer(["rev-list", "-n", "1", "HEAD", "--", pin], root) + if last and git_answer(["cat-file", "-e", f"{last}~1:./{pin}"], root) is not None: return "committed", ( f"{git_c} checkout {quote_arg(last + '~1')} -- " - f"{quote_arg(SHAPE_PIN)} # the pin as it was before " + f"{quote_arg(pin)} # the {what} as it was before " f"{last[:12]}, the last commit to change it. Commit that rather " - "than editing the pin") + f"than editing the {what}") return "none", None +def tool_written(ctx: Context, rel: str) -> bool: + """Whether `rel`, a root path under `contracts/`, is a file a tool of + this standard writes, so that git holds the last good bytes (#291). + + Every `contracts/-pin.yaml` is a pin. The shape pin is written by + `scaffold-project.py`, `adopt-project.py` and `update-shape.py apply`, a + leg pin by those and by `bump-leg.py`, and a neutral product pin by + `scaffold-project.py --pin`. Every shape copy the root kind lists is + such a file too, and under `contracts/` that is the naming policy. No + one edits any of them by hand, so the bytes a tool last wrote are the + fix. Anything else under `contracts/`, and `project.yaml` or + `family.yaml`, is the project's own. + """ + if rel.startswith("contracts/") and rel.endswith("-pin.yaml"): + return True + return rel in ctx.update_shape().root_kind(ctx.root).copy_source + + +#: The note on the `manifest kinds` row's next command where no file that +#: does not load has a copy git can put back (#291). No `;`, apostrophe or +#: backtick, so a pasted line is one command in any shell (#277). +UNREADABLE_BY_HAND = ( + "the reason names each file that does not load and why, and no commit " + "in this root holds a copy a tool wrote to put back, so what each should " + "say -- a merge conflict resolved, a hand edit put right -- is for a " + "person to decide. This row reads them again once they load") + + +def unreadable_next(ctx: Context, unreadable: list) -> tuple[dict, str]: + """`(restore, command)` for the `manifest kinds` row's files that do not + load (#291): each file's case, and the one command the row prints. + + A FILE A TOOL WROTE IS PUT BACK FROM GIT by `pin_restore`. That is the + helper the `shape currency` row uses for the shape pin, so both rows + print the same command for the same file. The shape pin goes first + where it has a command, then each other file in the row's order. A row + prints one command, and the next run names the next file. + + A FILE OF THE PROJECT'S OWN IS NOT, and its case is `project`, with git + not asked. The git-state choice does not fit it: each of the three + commands discards somebody's edit -- the other side of their merge, + their uncommitted work, their last commit -- and nothing in this + standard says which bytes are right. Where no file has a command to + name, the command is this doctor again, to run once a person has mended + the file, and its note says that what the file should say is theirs to + decide. #205's `manifest` row answers a `project.yaml` that does not + load the same way: the gate that refuses it, and that note. + """ + restore: dict[str, str] = {} + commands: dict[str, str] = {} + for rel in unreadable: + if tool_written(ctx, rel): + restore[rel], found = pin_restore(ctx, rel) + if found: + commands[rel] = found + else: + restore[rel] = "project" + command = commands.get(SHAPE_PIN) or next(iter(commands.values()), None) + if command is None: + command = (f"{PYTHON} {quote_arg(ctx.shape / 'shape-doctor.py')} " + f"--root {quote_arg(ctx.root)} # {UNREADABLE_BY_HAND}") + return restore, command + + def unreadable_pin_row(ctx: Context, exc: Refusal) -> Row: """The `shape currency` row for a root whose OWN pin does not read as one (#237): a FINDING, the refusal quoted whole, and the pin put back. diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index 3902b397..cd17f3a0 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2687,6 +2687,296 @@ def test_the_doctor_writes_no_bytecode_into_the_standard(standard, project, assert left == [], left +# --- the `manifest kinds` row for a file that does not load (#291) ---------- +# +# The row lists each file under `contracts/` that does not load as a mapping, +# and its NEXT was the unregistered kind's sentence: register the kind in +# shape-doctor.py's MANIFEST_VALIDATORS, or remove the file from the root. +# For a conflicted `contracts/shape-pin.yaml` that told the reader to delete +# the pin, and it was not a command a shell could even split. A file a tool +# wrote is now put back from the root's own history by `pin_restore`, the +# helper the `shape currency` row uses (#237). A file of the project's own +# is left to a person. Each test runs the printed NEXT, then asks again. + +SPEC_PIN = "contracts/spec-pin.yaml" +CODE_PIN = "contracts/code-pin.yaml" +#: The one shape copy a root keeps under `contracts/`. +NAMING_COPY = "contracts/repository-naming.yaml" +OWN_CONTRACT = "contracts/notes.yaml" + +#: How each restore's note names the file it puts back, `{what}` being +#: `pin` or `copy`. +NOTE_SAYS = {"unmerged": "never a hand-merged {what}", + "uncommitted": "puts back the {what} as last committed", + "committed": "the {what} as it was before"} + + +def conflict_in(root: Path, rel: str, key: str, *also: str) -> None: + """Leave `rel` conflicted, `UU`, the way two branches leave it. + + Two branches from HEAD each give the `key:` line of `rel` their own + 40-digit value and append a line to each of `also`, and the second + merges the first. Bytes throughout, as `break_the_pin` writes them. + """ + base = git("rev-parse", "HEAD", cwd=root).stdout.strip() + for branch, digit in (("edit-a", "1"), ("edit-b", "2")): + git("checkout", "-q", "-b", branch, base, cwd=root) + path = root / rel + path.write_bytes(pin_with(rf"^{key}: .*$", f'{key}: "{digit * 40}"')( + path.read_text(encoding="utf-8"))) + for name in also: + other = root / name + other.write_bytes(other.read_bytes() + f"# {branch}\n".encode()) + git("add", "--", rel, *also, cwd=root) + commit_in(root, branch) + merged = subprocess.run(["git", "merge", "-q", "edit-a"], cwd=str(root), + capture_output=True, text=True, check=False, + env={**os.environ, **GIT_IDENTITY}) + assert merged.returncode != 0, merged.stdout + merged.stderr + status = git("status", "--porcelain=v1", cwd=root).stdout + for name in (rel, *also): + assert f"UU {name}" in status, status + + +def own_contract(root: Path) -> Path: + """A committed `contracts/notes.yaml` with no `kind:`, the project's own + data, which loads and which the row passes over in silence.""" + own = root / OWN_CONTRACT + own.write_bytes(b"colour: blue\n") + git("add", "--", OWN_CONTRACT, cwd=root) + commit_in(root, "a contract of the project's own") + return own + + +def kinds_and_currency(standard: Path, root: Path) -> tuple[dict, dict]: + rows = rows_of(doctor(standard, root, "--json")) + return rows["manifest-kinds"], rows["shape-currency"] + + +def assert_no_registration(row: dict) -> None: + """The issue's (a) and (b): the NEXT is a command and never the + unregistered kind's sentence, whose second half deletes the file.""" + assert row["next"], row + for words in ("register the kind", "remove the file", + "MANIFEST_VALIDATORS"): + assert words not in row["next"], (words, row["next"]) + assert_a_plain_note(row["next"]) + + +def shape_pin_conflicted(root: Path) -> list: + conflict_in(root, SHAPE_PIN, "commit") + return ["checkout", "--ours", "--", SHAPE_PIN] + + +def shape_pin_not_utf8_committed(root: Path) -> list: + pin = root / SHAPE_PIN + pin.write_bytes(pin.read_bytes() + b"\xff\xfe\n") + git("add", "--", SHAPE_PIN, cwd=root) + breaking = commit_in(root, "a byte that is not UTF-8") + commit_in(root, "an unrelated commit after it", "--allow-empty") + return ["checkout", f"{breaking}~1", "--", SHAPE_PIN] + + +def shape_pin_a_list(root: Path) -> list: + (root / SHAPE_PIN).write_bytes(b"- one\n- two\n") + return ["checkout", "HEAD", "--", SHAPE_PIN] + + +def leg_pin_conflicted(root: Path) -> list: + # `Makefile` conflicts beside it and is a file the shape writes, but + # only the shape pin takes the shape with it: a leg pin is named alone. + conflict_in(root, SPEC_PIN, "commit", "Makefile") + return ["checkout", "--ours", "--", SPEC_PIN] + + +def leg_pin_not_utf8(root: Path) -> list: + pin = root / SPEC_PIN + pin.write_bytes(pin.read_bytes() + b"\xff\n") + return ["checkout", "HEAD", "--", SPEC_PIN] + + +def naming_copy_hand_broken(root: Path) -> list: + copy = root / NAMING_COPY + copy.write_bytes(copy.read_bytes() + b"\t: : [\n") + return ["checkout", "HEAD", "--", NAMING_COPY] + + +@pytest.mark.parametrize("breaks, rel, code, restore, what, compliant", [ + pytest.param(shape_pin_conflicted, SHAPE_PIN, "yaml-unparsable", + "unmerged", "pin", False, id="the-shape-pin-conflicted"), + pytest.param(shape_pin_not_utf8_committed, SHAPE_PIN, "yaml-unreadable", + "committed", "pin", True, + id="the-shape-pin-not-utf-8-committed"), + pytest.param(shape_pin_a_list, SHAPE_PIN, None, "uncommitted", "pin", + True, id="the-shape-pin-a-list"), + pytest.param(leg_pin_conflicted, SPEC_PIN, "yaml-unparsable", + "unmerged", "pin", False, id="a-leg-pin-conflicted"), + pytest.param(leg_pin_not_utf8, SPEC_PIN, "yaml-unreadable", + "uncommitted", "pin", True, id="a-leg-pin-not-utf-8"), + pytest.param(naming_copy_hand_broken, NAMING_COPY, "yaml-unparsable", + "uncommitted", "copy", True, id="the-naming-copy-broken"), +]) +def test_a_file_a_tool_wrote_that_does_not_load_is_put_back_from_git( + standard, project, breaks, rel, code, restore, what, compliant): + """The row names the git command for the file's own state, the loader's + words for it, and never the unregistered kind's sentence. + + Run as printed, the command puts back the bytes a tool wrote, and the + row reads every file again. Where the break was the only fault the + doctor is COMPLIANT. A merge is left for its owner to finish. + """ + expected = breaks(project) + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + rows = rows_of(result) + row = rows["manifest-kinds"] + assert row["status"] == "FINDING", row + assert row["detail"]["unreadable"] == [rel], row["detail"] + assert_no_registration(row) + assert row["detail"]["restore"] == {rel: restore}, row["detail"] + said = row["detail"]["refusals"][rel] + assert said["code"] == code, said + # The loader's own words, less the path it opens with. + words = f"{code}: {said['detail']}" if code else said["detail"] + assert f"unreadable as YAML: {rel} ({words})" in row["reason"], \ + row["reason"] + assert Path(rel).name not in said["detail"], said + argv = pasted(row["next"]) + assert argv[:2] == ["git", "-C"], row["next"] + assert argv[3:] == expected, argv + note = row["next"].split(" #", 1)[1] + assert NOTE_SAYS[restore].format(what=what) in note, note + if rel == SHAPE_PIN: + # One helper, so the two rows about the one file name one command. + assert row["next"] == rows["shape-currency"]["next"], rows + run_pasted(row["next"]) + again = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert again["status"] == "ok", again + assert again["detail"]["unreadable"] == [], again + if compliant: + compliant_again(standard, project) + + +def project_contract_conflicted(root: Path) -> tuple[str, str]: + own_contract(root) + conflict_in(root, OWN_CONTRACT, "colour") + return OWN_CONTRACT, "project" + + +def leg_pin_in_a_squashed_root_commit(root: Path) -> tuple[str, str]: + leg_pin_not_utf8(root) + git("checkout", "-q", "--orphan", "squashed", cwd=root) + git("add", "--", SPEC_PIN, cwd=root) + commit_in(root, "the whole history, squashed into one commit") + return SPEC_PIN, "none" + + +@pytest.mark.parametrize("breaks", [ + pytest.param(project_contract_conflicted, + id="a-contract-of-the-projects-own-conflicted"), + pytest.param(leg_pin_in_a_squashed_root_commit, + id="a-leg-pin-with-no-good-copy-in-its-history"), +]) +def test_a_file_git_cannot_put_back_is_left_to_a_person(standard, project, + breaks): + """No git command to name: the NEXT is this doctor again, and its note + says what the file should say is for a person to decide. + + A file of the project's own is never put back from git, because each + of the three commands throws away an edit somebody made, so git is not + asked (`project`). A pin with no good copy in its history has nothing + to put back (`none`). Run as printed, the doctor finds the same file + and touches none of it. Once a person mends the file, the row reads it. + """ + rel, restore = breaks(project) + row = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert row["status"] == "FINDING", row + assert row["detail"]["unreadable"] == [rel], row["detail"] + assert_no_registration(row) + assert row["detail"]["restore"] == {rel: restore}, row["detail"] + assert "checkout" not in row["next"], row["next"] + argv = pasted(row["next"]) + assert argv[0] == repo_shape.PYTHON, argv + assert Path(argv[1]).name == DOCTOR and argv[2] == "--root", argv + assert Path(argv[3]).resolve() == project.resolve(), argv + assert "for a person to decide" in row["next"].split(" #", 1)[1] + before = (project / rel).read_bytes() + proc = subprocess.run(argv, capture_output=True, text=True, check=False, + stdin=subprocess.DEVNULL, + env={**os.environ, **GIT_IDENTITY}) + assert proc.returncode == 1, proc.stdout + proc.stderr + assert f"unreadable as YAML: {rel} (" in proc.stdout, proc.stdout + assert (project / rel).read_bytes() == before + if restore == "project": + (project / rel).write_bytes(b"colour: green\n") + again = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert again["status"] == "ok", again + + +def test_an_unregistered_kind_beside_a_file_that_does_not_load(standard, + project): + """Both in the reason, and the NEXT is the restore: the row cannot say + what kind a file declares until it loads. Run, the row is the one the + unregistered kind alone got, sentence, keys and all.""" + (project / "contracts" / "inventory.yaml").write_text( + "schema_version: 1\nkind: service-inventory\nservices: []\n", + encoding="utf-8") + alone = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert alone["next"].startswith( + "register the kind in shape-doctor.py's MANIFEST_VALIDATORS"), alone + assert set(alone["detail"]) == {"validated", "unregistered", "unreadable", + "registered_kinds"}, alone["detail"] + break_the_pin(project, conflicted_pin) + row = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert row["detail"]["unreadable"] == [SHAPE_PIN], row["detail"] + assert_no_registration(row) + assert row["reason"].startswith( + "no validator registered for kind contracts/inventory.yaml " + "(service-inventory); unreadable as YAML: contracts/shape-pin.yaml " + "(yaml-unparsable: line "), row["reason"] + assert row["detail"]["unregistered"] == alone["detail"]["unregistered"] + assert row["detail"]["restore"] == {SHAPE_PIN: "uncommitted"} + assert pasted(row["next"])[3:] == ["checkout", "HEAD", "--", SHAPE_PIN] + run_pasted(row["next"]) + after = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert after == alone, (after, alone) + + +def test_the_shape_pin_goes_first_and_each_run_names_the_next_file( + standard, project): + """Three files that do not load: two pins and one of the project's own. + + The row prints one command. The shape pin's comes first, the command + the `shape currency` row prints too, then the other pin's, then this + doctor for the file a person mends. The project's file is never touched. + """ + own = own_contract(project) + for rel in (CODE_PIN, OWN_CONTRACT, SHAPE_PIN): + path = project / rel + path.write_bytes(path.read_bytes() + b"\xff\n") + kinds, currency = kinds_and_currency(standard, project) + assert kinds["detail"]["unreadable"] == [CODE_PIN, OWN_CONTRACT, + SHAPE_PIN], kinds["detail"] + assert_no_registration(kinds) + assert kinds["detail"]["restore"] == { + CODE_PIN: "uncommitted", OWN_CONTRACT: "project", + SHAPE_PIN: "uncommitted"}, kinds["detail"] + assert kinds["next"] == currency["next"], (kinds, currency) + assert pasted(kinds["next"])[3:] == ["checkout", "HEAD", "--", SHAPE_PIN] + run_pasted(kinds["next"]) + kinds, _ = kinds_and_currency(standard, project) + assert kinds["detail"]["unreadable"] == [CODE_PIN, OWN_CONTRACT], kinds + assert pasted(kinds["next"])[3:] == ["checkout", "HEAD", "--", CODE_PIN] + assert_no_registration(kinds) + run_pasted(kinds["next"]) + kinds, _ = kinds_and_currency(standard, project) + assert kinds["detail"]["unreadable"] == [OWN_CONTRACT], kinds + assert kinds["detail"]["restore"] == {OWN_CONTRACT: "project"}, kinds + assert Path(pasted(kinds["next"])[1]).name == DOCTOR, kinds["next"] + assert_no_registration(kinds) + assert own.read_bytes() == b"colour: blue\n\xff\n" + + # --- placement: code in spec, spec in code (#99) ---------------------------- # # Brett Heap, 2026-09-10: "we have to look for code in spec and spec in code". From f80179cb36d6df54dda8e79de9d2165cd3f2bc6c Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:58:15 +0000 Subject: [PATCH 06/11] shape-doctor.py: a merge's NEXT takes whole only the pin and the copies and leaves a conflicted `project.yaml` or `README.md` to a person, and every pin it puts back out of history is read first, so the other side's own edits survive it and two breaking commits in a row, a sparse checkout or assume-unchanged no longer name a wrong rollback (#237) Fix round 2 on PR #267, answering lane openRepoShape-2's re-verify of 9f64bd0 (V1, V2). V1 (P2). The unmerged NEXT named the manifest and README.md beside the pin and the copies, and `checkout --ours` writes stage 2 whole. `apply` writes only two lines of the manifest's `shape:` block and README's `Shape:` line; every other line is the project's own, so the other side's edits to those files went, hunks git had merged cleanly included, and the doctor then said COMPLIANT. Now only the files the shape writes whole are taken: the pin, each `copy_source` key and its collision copy. A conflicted manifest or README.md is named in the note as a person's merge and listed in `detail["left_for_a_person"]`, as #232's manifest row already says of the same file. The review's optional refinement (naming either file where the dropped side changed only tool-written lines) is not taken. V2 (P3). The committed class named `~1` whenever it held a pin. It never asked whether HEAD's pin was the broken one, or whether the pin it put back read. A sparse checkout or assume-unchanged over a good re-pin got a rollback of that re-pin, and two commits in a row that each broke the pin cycled between `checkout ~1` and `checkout HEAD`. - `update-shape.py`: `read_pin`'s checks move into `check_pin(pin, pin_path)`, which `read_pin` calls, so the doctor checks a pin out of git with the same code. - `pin_reads_at` reads `:contracts/shape-pin.yaml` with `cat-file blob` in memory (no temporary file: the doctor writes nothing), decodes and parses it as `load_yaml` does, and runs `check_pin`. - `uncommitted` needs HEAD's pin to read (the review's optional step 4, so a dirty copy over a broken HEAD goes straight to history). - New class `hidden`: HEAD's pin reads and status is clean. The NEXT is `git ls-files -v`, which shows the S or h, and no rollback is named. - `committed` walks `rev-list -n 50 HEAD -- ` newest first and names the first commit whose parent's pin reads. `pin_restore(ctx)` still answers `(restore, command)`; it gains an optional `detail` it writes `left_for_a_person` into, so existing callers are unchanged. Tests: the README case and the issue's own two-`apply` merge (real re-pins from a copy of the standard with two more commits; shape/a also renames the project and adds a README paragraph), two breaking commits, a dirty copy over a committed break, assume-unchanged and a sparse checkout. All six fail at 9f64bd0 for the review's reasons. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 174 ++++++++++++++++++------- tests/test_shape_doctor.py | 251 ++++++++++++++++++++++++++++++++++++- update-shape.py | 14 ++- 3 files changed, 393 insertions(+), 46 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 294f733a..f50d9c06 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -1102,7 +1102,45 @@ def git_answer(args: list, cwd: Path, binary: bool = False): return None -def pin_restore(ctx: Context) -> tuple[str, str | None]: +#: How far back `pin_restore` looks for a pin that reads before it says +#: that git can name nothing. A pin breaks in one commit, or in a few in a +#: row; fifty is far past any history a NEXT should reach into. +PIN_HISTORY_DEPTH = 50 + + +def pin_reads_at(ctx: Context, rev: str) -> bool: + """Whether the pin `rev` holds reads as one, by `read_pin`'s own checks + (#237). + + READ BEFORE IT IS NAMED: a NEXT that puts back a pin out of history + must name one that reads. Rolling back to a pin that is broken too + sends the reader round in a circle (two commits in a row that each broke + it), and where HEAD's own pin reads, the bytes at fault never reached a + commit at all, so no rollback is the fix. + + IN MEMORY, NEVER A TEMPORARY FILE, because this command writes nothing + anywhere. The blob is read the way `load_yaml` reads the file -- strict + UTF-8, its newlines translated as `read_text` translates them, the same + `parse_yaml` -- and `check_pin` makes the checks `read_pin` makes, so + the two cannot disagree about a pin. Where git gives no blob (no such + path, no such commit) there is no pin that reads, and `files: 5`, which + `read_pin` itself fails with a `TypeError`, does not read either. + """ + blob = git_answer(["cat-file", "blob", f"{rev}:./{SHAPE_PIN}"], + ctx.root, binary=True) + if blob is None: + return False + us = ctx.update_shape() + try: + text = blob.decode("utf-8").replace("\r\n", "\n").replace("\r", "\n") + us.check_pin(us.parse_yaml(text), f"{rev}:{SHAPE_PIN}") + except (Refusal, us.YamlError, ValueError, TypeError): + return False + return True + + +def pin_restore(ctx: Context, + detail: dict | None = None) -> tuple[str, str | None]: """`(restore, command)`: the git command that puts back a pin a tool wrote, chosen from the pin's own git state, read-only (#237). @@ -1113,20 +1151,39 @@ def pin_restore(ctx: Context) -> tuple[str, str | None]: unmerged a merge left the pin conflicted. `checkout --ours`, or `--theirs` where only that side holds a pin, of every - conflicted path the shape writes -- the pin, the manifest - that mirrors it, the README's `Shape:` line, the copies -- - because a merge of two `update-shape.py apply` runs - conflicts in them together, and one side taken whole is one - tool's output. A file of the project's own is never named: - its merge is theirs. - uncommitted HEAD holds a pin and the tree or the index differs from it. - `checkout HEAD`, which drops a change no tool of this - standard leaves uncommitted. - committed the last commit to change the pin broke or removed it. - `checkout ~1`, the pin as it was before it -- - named only where that commit has a parent holding a pin. - none nothing above: no history, a root commit, no git. Nothing - git can name fixes it, and the caller says so. + conflicted path the shape writes WHOLE -- the pin, the + copies and their collision copies -- because a merge of + two `update-shape.py apply` runs conflicts in them + together, and one side taken whole is one tool's output. + The manifest and README.md are never named, though `apply` + writes lines of them: every other line is the project's + own, and one side taken whole drops the other side's + edits, the hunks git merged cleanly included (review of + PR #267). Where either is conflicted the note says a + person merges it, and `detail["left_for_a_person"]` lists + it. A file of the project's own is never named: its merge + is theirs. + uncommitted HEAD holds a pin that reads, and the tree or the index + differs from it. `checkout HEAD`, which drops a change no + tool of this standard leaves uncommitted. + hidden HEAD holds a pin that reads, and `git status` shows no + change to it: an index flag hides the copy here (a sparse + checkout, assume-unchanged), or the file cannot be read. + No commit broke it, so nothing is put back: the NEXT is + the `ls-files -v` that shows which, and clearing it is a + person's call. + committed a commit broke or removed the pin. `checkout ~1`, the pin as it was before it. The history is + walked newest first, and the commit named is the first + whose parent holds a pin that reads, so two commits in a + row that each broke it are reached past. + none nothing above: no history, a root commit, no git, a walk + that meets a parent holding no pin, or goes + `PIN_HISTORY_DEPTH` commits back, before a pin that reads. + Nothing git can name fixes it, and the caller says so. + + EVERY PIN THE COMMAND PUTS BACK IS READ FIRST, by `pin_reads_at`, except + the side of a merge, which is one tool's output either way. THE STATUS IS TWO FIXED-WIDTH LETTERS, READ AS BYTES: `git_out`'s text form strips its output, and the leading space of ` M` with it. And @@ -1144,17 +1201,17 @@ def pin_restore(ctx: Context) -> tuple[str, str | None]: if xy in UNMERGED_STATUS: us = ctx.update_shape() kind = us.root_kind(root) - shaped = {SHAPE_PIN, kind.manifest, us.README} + whole = {SHAPE_PIN} for rel in kind.copy_source: - shaped.update((rel, f"{us.COLLISION_DIR}/{rel}")) + whole.update((rel, f"{us.COLLISION_DIR}/{rel}")) # ` SP SP TAB `, one record per stage. stages: dict[str, set] = {} listing = git_answer(["ls-files", "-u", "-z"], root, binary=True) for record in (listing or b"").split(b"\0"): meta, _tab, path = record.partition(b"\t") - name = path.decode("utf-8", "replace") - if name in shaped: - stages.setdefault(name, set()).add(meta.rsplit(b" ", 1)[-1]) + if path: + stages.setdefault(path.decode("utf-8", "replace"), + set()).add(meta.rsplit(b" ", 1)[-1]) pin_stages = stages.get(SHAPE_PIN, set()) side, stage = (("--ours", b"2") if b"2" in pin_stages else ("--theirs", b"3") if b"3" in pin_stages else @@ -1165,27 +1222,55 @@ def pin_restore(ctx: Context) -> tuple[str, str | None]: else "only that side of the merge holds a pin. Take it " "whole") named = sorted(name for name, held in stages.items() - if stage in held) + if name in whole and stage in held) + left = sorted(name for name in (kind.manifest, us.README) + if name in stages) + then = ", then finish" + if left: + if detail is not None: + detail["left_for_a_person"] = left + says = ("are conflicted too and hold lines of the project, " + "so a person merges them" if len(left) > 1 else + "is conflicted too and holds lines of the project, " + "so a person merges it") + then = (f". {' and '.join(left)} {says}, keeping the lines " + "update-shape.py writes from the side taken here. " + "Then finish") return "unmerged", ( f"{git_c} checkout {side} -- " + " ".join(quote_arg(name) for name in named) + f" # {note} for every path named and never a " - "hand-merged pin, then finish the merge as git says and run " + f"hand-merged pin{then} the merge as git says and run " "this doctor again") - elif xy and git_answer(["cat-file", "-e", f"HEAD:./{SHAPE_PIN}"], - root) is not None: - return "uncommitted", ( - f"{git_c} checkout HEAD -- {quote_arg(SHAPE_PIN)} # puts back " - "the pin as last committed and discards the uncommitted change " - "that broke it, which no tool of this standard writes") - last = git_answer(["rev-list", "-n", "1", "HEAD", "--", SHAPE_PIN], root) - if last and git_answer(["cat-file", "-e", f"{last}~1:./{SHAPE_PIN}"], - root) is not None: - return "committed", ( - f"{git_c} checkout {quote_arg(last + '~1')} -- " - f"{quote_arg(SHAPE_PIN)} # the pin as it was before " - f"{last[:12]}, the last commit to change it. Commit that rather " - "than editing the pin") + elif pin_reads_at(ctx, "HEAD"): + if xy: + return "uncommitted", ( + f"{git_c} checkout HEAD -- {quote_arg(SHAPE_PIN)} # puts " + "back the pin as last committed and discards the uncommitted " + "change that broke it, which no tool of this standard writes") + return "hidden", ( + f"{git_c} ls-files -v -- {quote_arg(SHAPE_PIN)} # HEAD holds a " + "pin that reads and git status shows no change to the copy here, " + "so no commit broke it and nothing in history is put back. S in " + "the first column is skip-worktree, as a sparse checkout sets " + "it, and a lowercase h is assume-unchanged: either hides the " + "copy here from git. Any other letter means the file itself " + "cannot be read, as the refusal says. Clearing that is for a " + "person, then run this doctor again") + history = git_answer(["rev-list", "-n", str(PIN_HISTORY_DEPTH), "HEAD", + "--", SHAPE_PIN], root) + for breaking in (history or "").split(): + before = f"{breaking}~1" + if git_answer(["cat-file", "-e", f"{before}:./{SHAPE_PIN}"], + root) is None: + break + if pin_reads_at(ctx, before): + return "committed", ( + f"{git_c} checkout {quote_arg(before)} -- " + f"{quote_arg(SHAPE_PIN)} # the pin as it was before " + f"{breaking[:12]}, the commit that broke it, and the newest " + "pin in its history that reads. Commit that rather than " + "editing the pin") return "none", None @@ -1217,18 +1302,19 @@ def unreadable_pin_row(ctx: Context, exc: Refusal) -> Row: THE NEXT PUTS BACK THE PIN A TOOL WROTE, NEVER A FETCH. `pin_restore` names the git command from the pin's own state, and run as printed it - ends at `COMPLIANT` or at a row that reads the pin. Naming the tool that - refused, `update-shape.py check`, was a dead end (review of PR #267): it - reads the pin with the same `read_pin`, refuses it the same way, and its - remediation is about submodules, adoption or a hand edit, none of which - fits. That command stays only where git can name nothing, and its note + ends at `COMPLIANT` or at a row that reads the pin, but for what its + note leaves to a person: a conflicted manifest or README.md, or an index + flag that hides the pin. Naming the tool that refused, `update-shape.py + check`, was a dead end (review of PR #267): it reads the pin with the + same `read_pin`, refuses it the same way, and its remediation is about + submodules, adoption or a hand edit, none of which fits. That command stays only where git can name nothing, and its note says the rest is a person's call. Each note holds no `;`, apostrophe or backtick, so a pasted line is one command in any shell (#277). """ - restore, command = pin_restore(ctx) detail = {"error": exc.code, "pin": SHAPE_PIN, - "refusal": {"code": exc.code, "detail": exc.detail}, - "restore": restore} + "refusal": {"code": exc.code, "detail": exc.detail}} + restore, command = pin_restore(ctx, detail) + detail["restore"] = restore if command is None: command = (f"{PYTHON} {quote_arg(ctx.shape / 'update-shape.py')} " f"check --root {quote_arg(ctx.root)} --upstream " diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index 3902b397..302b8a17 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2498,7 +2498,8 @@ def test_a_family_holders_conflicted_pin_is_invalid_too(standard, holder, # # Each test below runs the printed NEXT and asks the doctor again. The NEXT # is chosen from the pin's own git state, read-only: unmerged, then -# uncommitted (the parametrised test above), then committed, then none. +# uncommitted (the parametrised test above), then hidden, then committed, +# then none. #: `commit: "zzzz"`, the committed fault the review probed beside a deletion. not_forty_hex = pin_with(r'^commit: .*$', 'commit: "zzzz"') @@ -2576,6 +2577,7 @@ def test_the_issues_own_merge_takes_one_side_of_the_shapes_files_whole( argv = pasted(row["next"]) assert argv[3:] == ["checkout", "--ours", "--", "Makefile", SHAPE_PIN], \ argv + assert "left_for_a_person" not in row["detail"], row["detail"] assert_a_plain_note(row["next"]) run_pasted(row["next"]) again = rows_of(doctor(standard, project, "--json"))["shape-currency"] @@ -2584,6 +2586,151 @@ def test_the_issues_own_merge_takes_one_side_of_the_shapes_files_whole( assert b"<<<<<<<" in own.read_bytes() +def test_a_readme_conflicted_over_the_projects_own_lines_is_left_to_a_person( + standard, project): + """Both branches rewrite the pin's `commit:` and README.md's first line, + and edit-a also adds a paragraph at the end, which git merges cleanly. + + README.md holds no `Shape:` line here, so nothing in it is the shape's, + and where it holds one every other line is still the project's own. + Taken whole, edit-a's paragraph went with it (review of PR #267). The + NEXT takes the pin alone, and its note and `left_for_a_person` say that + README.md is a person's to merge. + """ + readme = project / "README.md" + assert b"Shape:" not in readme.read_bytes() + base = git("rev-parse", "HEAD", cwd=project).stdout.strip() + for branch, digit in (("edit-a", "1"), ("edit-b", "2")): + git("checkout", "-q", "-b", branch, base, cwd=project) + break_the_pin(project, pin_with(r'^commit: .*$', + f'commit: "{digit * 40}"')) + lines = readme.read_bytes().split(b"\n") + lines[0] = f"# Atlas, as {branch} writes it".encode() + if branch == "edit-a": + lines.append(b"A paragraph only edit-a wrote.\n") + readme.write_bytes(b"\n".join(lines)) + git("add", "--", SHAPE_PIN, "README.md", cwd=project) + commit_in(project, branch) + merged = subprocess.run(["git", "merge", "-q", "edit-a"], + cwd=str(project), capture_output=True, text=True, + check=False, env={**os.environ, **GIT_IDENTITY}) + assert merged.returncode != 0, merged.stdout + merged.stderr + status = git("status", "--porcelain=v1", cwd=project).stdout + assert f"UU {SHAPE_PIN}" in status and "UU README.md" in status, status + assert b"only edit-a wrote" in readme.read_bytes() + row = rows_of(doctor(standard, project, "--json"))["shape-currency"] + assert row["detail"]["restore"] == "unmerged", row["detail"] + assert pasted(row["next"])[3:] == [ + "checkout", "--ours", "--", SHAPE_PIN], row["next"] + assert row["detail"]["left_for_a_person"] == ["README.md"], row["detail"] + note = row["next"].split(" #", 1)[1] + assert "README.md is conflicted too" in note, note + assert "a person merges it" in note, note + assert_a_plain_note(row["next"]) + run_pasted(row["next"]) + assert b"only edit-a wrote" in readme.read_bytes(), \ + "edit-a's paragraph was dropped" + again = rows_of(doctor(standard, project, "--json"))["shape-currency"] + assert again["detail"]["error"] not in READ_PIN_CODES, again + + +def standard_moved_on(standard: Path, tmp_path: Path) -> tuple: + """`(a copy of standard, C2, C3)`: two more commits, each moving + `UPSTREAM_TARGET`, so two real `apply` runs re-pin one project to two + different commits. A copy, byte for byte with its `.git`, so the + session's `standard` is left as every other test finds it.""" + moved = tmp_path / "moved" / "openRepoShape" + shutil.copytree(standard, moved, symlinks=True) + commits = [] + for label in ("c2", "c3"): + target = moved / UPSTREAM_TARGET + target.write_bytes(target.read_bytes() + f"# {label}\n".encode()) + commit_all(moved, label) + commits.append(git("rev-parse", "HEAD", cwd=moved).stdout.strip()) + return moved, commits[0], commits[1] + + +def test_the_issues_two_apply_merge_leaves_the_projects_manifest_to_a_person( + standard, project, tmp_path): + """The issue's own merge, with real re-pins: `apply --at C2` on shape/a, + `apply --at C3` on shape/b, and shape/a merged into shape/b. + + Shape/a also renamed the project in `project.yaml` and added a README + paragraph, lines of its own far from any `apply` writes. Git merges both + cleanly, and `project.yaml` is still `UU` over its `shape:` block. + Taking `project.yaml` whole dropped the rename, and the doctor then said + COMPLIANT (review of PR #267). The NEXT takes the files the shape writes + whole, `project.yaml` is left to a person, and the rename survives the + NEXT, that person's merge and the merge commit. + + The row is `check_shape_currency`'s own, asked in process: without + #232 a `project.yaml` that does not load is NOT A SHAPE ROOT and the + command runs no row, and with it the command prints this row. + """ + moved, c2, c3 = standard_moved_on(standard, tmp_path) + manifest, readme = project / "project.yaml", project / "README.md" + for commit, branch in ((c2, "shape/a"), (c3, "shape/b")): + git("checkout", "-q", "main", cwd=project) + applied = run_script(moved / "update-shape.py", "apply", "--root", + str(project), "--upstream", str(moved), "--at", + commit, "--yes", "--branch", branch) + assert applied.returncode == 0, applied.stdout + applied.stderr + if branch == "shape/a": + text = manifest.read_bytes() + renamed = re.sub(rb'(?m)^name: .*$', b'name: "Atlas Renamed"', + text, count=1) + assert renamed != text + manifest.write_bytes(renamed) + readme.write_bytes(readme.read_bytes() + + b"\nA paragraph only shape/a wrote.\n") + git("add", "--", "project.yaml", "README.md", cwd=project) + commit_in(project, "the project's own edits") + merged = subprocess.run(["git", "merge", "-q", "shape/a"], + cwd=str(project), capture_output=True, text=True, + check=False, env={**os.environ, **GIT_IDENTITY}) + assert merged.returncode != 0, merged.stdout + merged.stderr + status = git("status", "--porcelain=v1", cwd=project).stdout + for name in ("Makefile", SHAPE_PIN, "project.yaml"): + assert f"UU {name}" in status, status + assert "UU README.md" not in status, status + assert b'name: "Atlas Renamed"' in manifest.read_bytes() + module = doctor_module(standard) + row = module.check_shape_currency(module.Context(project)).as_dict() + assert row["detail"]["error"] == "yaml-unparsable", row["detail"] + assert row["detail"]["restore"] == "unmerged", row["detail"] + assert pasted(row["next"])[3:] == [ + "checkout", "--ours", "--", "Makefile", SHAPE_PIN], row["next"] + assert row["detail"]["left_for_a_person"] == ["project.yaml"], \ + row["detail"] + note = row["next"].split(" #", 1)[1] + assert "project.yaml is conflicted too" in note, note + assert_a_plain_note(row["next"]) + run_pasted(row["next"]) + assert b'name: "Atlas Renamed"' in manifest.read_bytes(), \ + "shape/a's rename was dropped" + assert b"only shape/a wrote" in readme.read_bytes() + again = module.check_shape_currency(module.Context(project)).as_dict() + assert again["detail"].get("error") not in READ_PIN_CODES, again + # A person's merge of `project.yaml`: each conflicting hunk to the side + # the NEXT took, each clean one kept, over git's own three stages. + stages = [] + for number in (1, 2, 3): + held = subprocess.run(["git", "show", f":{number}:project.yaml"], + cwd=str(project), capture_output=True, + check=True).stdout + stages.append(tmp_path / f"project.yaml.{number}") + stages[-1].write_bytes(held) + subprocess.run(["git", "merge-file", "--ours", str(stages[1]), + str(stages[0]), str(stages[2])], + capture_output=True, check=True) + manifest.write_bytes(stages[1].read_bytes()) + git("add", "--", "project.yaml", "Makefile", SHAPE_PIN, cwd=project) + commit_in(project, "merge shape/a") + kept = git("show", "HEAD:project.yaml", cwd=project).stdout + assert 'name: "Atlas Renamed"' in kept, kept + compliant_again(moved, project) + + def test_a_bad_pin_staged_where_head_holds_none_is_put_back_from_history( standard, project): """`A ` in the status: the pin was deleted in a commit, then a bad one @@ -2646,6 +2793,108 @@ def test_a_merge_where_only_the_other_side_holds_a_pin_takes_theirs( assert again["detail"]["error"] == "update-revision-unresolvable", again +def test_two_commits_that_each_broke_the_pin_are_reached_past(standard, + project): + """`commit: "zzzz"` committed, then `commit: "yyyy"` committed over it. + + Rolling back the last commit put back the first break's pin, and the + NEXT after it was `checkout HEAD`, which put back the second: a circle + (review of PR #267). Each pin is read before it is named, so the NEXT + reaches past both, to the pin before the first. + """ + break_the_pin(project, not_forty_hex) + git("add", "--", SHAPE_PIN, cwd=project) + first = commit_in(project, "break the pin") + break_the_pin(project, pin_with(r'^commit: .*$', 'commit: "yyyy"')) + git("add", "--", SHAPE_PIN, cwd=project) + commit_in(project, "break it again") + row = rows_of(doctor(standard, project, "--json"))["shape-currency"] + assert row["detail"]["restore"] == "committed", row["detail"] + assert pasted(row["next"])[3:] == [ + "checkout", f"{first}~1", "--", SHAPE_PIN], row["next"] + assert first[:12] in row["next"].split(" #", 1)[1], row["next"] + assert_a_plain_note(row["next"]) + run_pasted(row["next"]) + compliant_again(standard, project) + + +def test_an_uncommitted_change_over_a_committed_break_goes_to_history( + standard, project): + """HEAD's own pin does not read either, so `checkout HEAD` would put + back a broken pin: the NEXT goes straight to the pin before the commit + that broke it.""" + break_the_pin(project, not_forty_hex) + git("add", "--", SHAPE_PIN, cwd=project) + breaking = commit_in(project, "break the pin") + break_the_pin(project, lambda text: b"\t: : [\n") + status = git("status", "--porcelain=v1", "--", SHAPE_PIN, + cwd=project).stdout + assert status.startswith(" M"), status + row = rows_of(doctor(standard, project, "--json"))["shape-currency"] + assert row["detail"]["error"] == "yaml-unparsable", row["detail"] + assert row["detail"]["restore"] == "committed", row["detail"] + assert pasted(row["next"])[3:] == [ + "checkout", f"{breaking}~1", "--", SHAPE_PIN], row["next"] + run_pasted(row["next"]) + compliant_again(standard, project) + + +def recommit_a_good_pin(root: Path) -> str: + """A second commit to the pin that leaves it reading: a comment added. + + So a NEXT that rolled back the last commit to the pin has one to name, + and the pin it would roll back is a good one. The new HEAD.""" + pin = root / SHAPE_PIN + pin.write_bytes(pin.read_bytes() + b"# a comment a later commit added\n") + git("add", "--", SHAPE_PIN, cwd=root) + return commit_in(root, "a comment in the pin") + + +def assert_hidden(standard: Path, root: Path, code: str, letter: str) -> None: + """The row says `hidden`, names no rollback, and its NEXT, run as + printed, shows the index flag that hides the pin.""" + assert git("status", "--porcelain=v1", "--", SHAPE_PIN, + cwd=root).stdout == "" + row = rows_of(doctor(standard, root, "--json"))["shape-currency"] + assert row["detail"]["error"] == code, row["detail"] + assert row["detail"]["restore"] == "hidden", row["detail"] + argv = pasted(row["next"]) + assert argv[3:] == ["ls-files", "-v", "--", SHAPE_PIN], row["next"] + assert "checkout" not in argv, row["next"] + assert_a_plain_note(row["next"]) + shown = subprocess.run(argv, capture_output=True, text=True, check=False) + assert shown.returncode == 0, shown.stderr + assert shown.stdout == f"{letter} {SHAPE_PIN}\n", shown.stdout + + +def test_a_broken_copy_assume_unchanged_hides_is_not_rolled_back(standard, + project): + """HEAD's pin reads, and `git status` shows no change to the copy here + because assume-unchanged hides it. No commit broke the pin, so naming + the commit before the last good re-pin rolled back a good pin (review + of PR #267): the NEXT shows the flag instead.""" + recommit_a_good_pin(project) + git("update-index", "--assume-unchanged", "--", SHAPE_PIN, cwd=project) + break_the_pin(project, not_forty_hex) + assert_hidden(standard, project, "update-pin-commit", "h") + + +def test_a_pin_a_sparse_checkout_leaves_out_is_not_rolled_back(standard, + project): + """A sparse checkout leaves the pin out of the tree: `read_pin` finds no + file, `git status` shows nothing, and HEAD's pin reads. The NEXT shows + the `S` that says why, and names no rollback.""" + recommit_a_good_pin(project) + sparse = project / git("rev-parse", "--git-path", "info/sparse-checkout", + cwd=project).stdout.strip() + sparse.parent.mkdir(parents=True, exist_ok=True) + sparse.write_bytes(b"/*\n!/" + SHAPE_PIN.encode() + b"\n") + git("config", "core.sparseCheckout", "true", cwd=project) + git("read-tree", "-mu", "HEAD", cwd=project) + assert not (project / SHAPE_PIN).exists() + assert_hidden(standard, project, "update-pin-missing", "S") + + def test_a_pin_with_no_good_copy_in_its_history_keeps_the_check_next( standard, project): """No history to put a pin back from: one root commit holds a bad pin. diff --git a/update-shape.py b/update-shape.py index 08f4f8c7..3f83eb1c 100755 --- a/update-shape.py +++ b/update-shape.py @@ -381,7 +381,19 @@ def read_pin(root: Path) -> tuple[dict, list[Row]]: "Remediation: this command updates a project that was scaffolded " "or adopted with this shape. A project without a shape pin is " "adopted with `adopt-project.py`, not updated.") - pin = load_yaml(pin_path) + return check_pin(load_yaml(pin_path), pin_path) + + +def check_pin(pin, pin_path) -> tuple[dict, list[Row]]: + """`read_pin`'s checks, over a pin already parsed: the pin and its rows, + or the refusal `read_pin` raises for it. + + APART FROM THE READ so a pin can be checked where it is not a file on + disk: `shape-doctor.py` reads the pins in a root's history out of git, + in memory, before it names one to put back (#237), and a second copy of + these checks there could pass a pin this tool then refuses. `pin_path` + only names the pin in a refusal's words. + """ if not isinstance(pin, dict): raise Refusal("update-pin-unreadable", f"{pin_path}: not a mapping") commit = str(pin.get("commit") or "").lower() From 8badae0f87ac6242ab195908e2b90171a29f4af5 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:58:23 +0000 Subject: [PATCH 07/11] tests/test_shape_doctor.py: the doctor leaves a stat-dirty index byte for byte as it found it, so dropping `--no-optional-locks` from the pin's `git status` fails a test (#237) Fix round 2 on PR #267, V3 of lane openRepoShape-2's re-verify of 9f64bd0. The doctor's first promise is that it never writes into the repository it is pointed at. `--no-optional-locks` is what keeps `pin_restore`'s `git status` from refreshing the root's index, and the review's mutant m7, which drops it, passed all 20 #237 tests. After a committed break, every tracked file is made stat-dirty; the doctor runs; `.git/index` must be byte-identical. A plain `git status` must then rewrite it, as the control. The test is the review's scratch test, renamed. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_shape_doctor.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index 302b8a17..5be67afc 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2895,6 +2895,30 @@ def test_a_pin_a_sparse_checkout_leaves_out_is_not_rolled_back(standard, assert_hidden(standard, project, "update-pin-missing", "S") +def test_reading_the_pins_git_state_leaves_a_stat_dirty_index_alone( + standard, project): + """The pin's `git status` must not refresh the root's index: that is the + doctor writing into it. `--no-optional-locks` is what stops it, and a + plain `git status`, as the control, does rewrite it.""" + break_the_pin(project, not_forty_hex) + git("add", "--", SHAPE_PIN, cwd=project) + commit_in(project, "break the pin") + later = os.stat(project / SHAPE_PIN).st_mtime + 120 + for path in project.rglob("*"): + if (".git" not in path.relative_to(project).parts and path.is_file() + and not path.is_symlink()): + os.utime(path, (later, later)) + index = project / ".git" / "index" + before = index.read_bytes() + row = rows_of(doctor(standard, project, "--json"))["shape-currency"] + assert row["detail"]["restore"] == "committed", row["detail"] + assert index.read_bytes() == before, "the doctor rewrote the root's index" + subprocess.run(["git", "status", "--porcelain=v1", "--", SHAPE_PIN], + cwd=str(project), capture_output=True, check=False) + assert index.read_bytes() != before, \ + "control: a plain status must refresh it" + + def test_a_pin_with_no_good_copy_in_its_history_keeps_the_check_next( standard, project): """No history to put a pin back from: one root commit holds a bad pin. From ebffb08280208954b954e937ee92f5595d9c5524 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:32:22 +0000 Subject: [PATCH 08/11] tests/test_shape_doctor.py: a leg pin conflicted beside `README.md` is named alone and its note says nothing of README.md, so a `pin_restore` that gave every file the shape pin's "a person merges README.md" clause fails a test (#291) #267's round 2 has the unmerged NEXT name a conflicted manifest or README.md for a person, because `apply` writes a line of each beside the shape pin. No tool writes either beside a leg pin, so the merge keeps that clause for the shape pin alone. The leg-pin case now conflicts README.md too, and the test asserts the note does not mention it. With the guard removed the case fails (README.md is named in the note). Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_shape_doctor.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index e4985923..af77e45b 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -3056,9 +3056,11 @@ def shape_pin_a_list(root: Path) -> list: def leg_pin_conflicted(root: Path) -> list: - # `Makefile` conflicts beside it and is a file the shape writes, but - # only the shape pin takes the shape with it: a leg pin is named alone. - conflict_in(root, SPEC_PIN, "commit", "Makefile") + # `Makefile`, a copy the shape writes, and README.md, which holds the + # line `apply` writes, conflict beside it. Only the shape pin takes the + # copies with it and names README.md for a person: a leg pin is named + # alone, and its note says nothing of either. + conflict_in(root, SPEC_PIN, "commit", "Makefile", "README.md") return ["checkout", "--ours", "--", SPEC_PIN] @@ -3119,6 +3121,7 @@ def test_a_file_a_tool_wrote_that_does_not_load_is_put_back_from_git( assert argv[3:] == expected, argv note = row["next"].split(" #", 1)[1] assert NOTE_SAYS[restore].format(what=what) in note, note + assert "README.md" not in note and "a person merges" not in note, note if rel == SHAPE_PIN: # One helper, so the two rows about the one file name one command. assert row["next"] == rows["shape-currency"]["next"], rows From 27458906d876c8566b1dc1b6ace99edce950dccd Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:38:26 +0000 Subject: [PATCH 09/11] shape-doctor.py: the `manifest kinds` row puts back from git only a pin the root declares or a shape copy, and shows a copy's uncommitted change instead of discarding it, so a project's own `contracts/release-pin.yaml` that does not load keeps the edit, the merged hunk or the commit a person made (#291) The review of #358 at ebffb08 (F1, codex P1 discussion_r4248225898) found `tool_written` admitting every `contracts/*-pin.yaml` by its suffix. A committed, undeclared `contracts/release-pin.yaml` that stopped loading was then put back from git, and the NEXT, run as AGENTS.md says to run it, discarded `schedule: daily` (an uncommitted edit, `checkout HEAD`), a cleanly merged `- nightly` (a conflict, `checkout --ours`) or `owner: platform-team` (a commit, `checkout ~1`). The same bytes as `contracts/release.yaml` were left alone. The suffix is the template's naming convention, not a declaration. Ownership now comes from what the root declares, all of it read already: the shape pin, `contracts/-pin.yaml` for each leg `project.yaml` declares (the spelling `check_legs`, validate-pins.py and bump-leg.py use), `contracts/-pin.yaml` for each `neutral_product_pins:` entry (`neutral_pin_path`), and the root kind's copies. Anything else is `project`, and its NEXT is this doctor again. Where `project.yaml` does not load it declares nothing, so a leg or neutral pin that does not load either is the project's for that run: a file wrongly called a tool's loses a person's edit, and one wrongly called the project's costs a run. `tool_written` says so. A copy is not a pin: `update-shape.py`'s `both` exit asks a person to merge one by hand and commit it, and git holds no other copy of that merge until it is committed. So a copy's `uncommitted` NEXT is now the read-only `git diff HEAD -- `, and nothing is put back (F1's optional point). Its `committed` and `unmerged` cases are unchanged: git keeps the person's bytes in the breaking commit or the other branch. Tests: the three release-pin states left to a person; a declared neutral pin put back beside an undeclared one left alone (in process, because the `pins` row would recheck a declared neutral pin through `gh`); a leg pin left alone while `project.yaml` does not load and put back once it loads; a copy's uncommitted change shown and kept. F2: a leg pin broken by a commit, a leg pin whose newest parent holds a list, the naming copy broken by a commit, and a leg pin added back after its deletion (`none`), which kill the review's surviving mutants mu1, mu2 and mu3. N1: the words for a pin that loads as a list, which kill mu5. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 120 +++++++++++++----- tests/test_shape_doctor.py | 242 +++++++++++++++++++++++++++++++++++-- 2 files changed, 323 insertions(+), 39 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 88487632..28ebf3c4 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -1203,7 +1203,13 @@ def pin_restore(ctx: Context, detail: dict | None = None, is theirs. uncommitted HEAD holds a pin that reads, and the tree or the index differs from it. `checkout HEAD`, which drops a change no - tool of this standard leaves uncommitted. + tool of this standard leaves uncommitted. NOT FOR A COPY, + which can hold the project's own edit -- the merge by + hand `update-shape.py` asks for where a copy changed on + both sides above all -- and git keeps no other copy of + that edit until it is committed. A copy's NEXT is the + `diff HEAD` that shows the change, and mending it is a + person's call (review of PR #358). hidden HEAD holds a pin that reads, and `git status` shows no change to it: an index flag hides the copy here (a sparse checkout, assume-unchanged), or the file cannot be read. @@ -1233,19 +1239,20 @@ def pin_restore(ctx: Context, detail: dict | None = None, ANY FILE UNDER `contracts/` A TOOL WROTE, NOT THE SHAPE PIN ALONE (#291). `pin` is the root's own `contracts/shape-pin.yaml` unless the caller - names another file `tool_written` admits -- a leg's pin, a neutral - product's, the naming policy's copy -- which is how the `manifest kinds` - row puts back each one that does not load. The five states, their - commands and their notes are the same for every one, with the note - saying `copy` where the file is not a pin, and `pin_reads_at` says what - reads. Only the shape pin takes more paths with it in a merge -- the - copies `apply` writes in the same commit -- and only its note names a - conflicted manifest or README.md for a person. Any other file is named - alone. A leg pin moves with its gitlink, but `checkout --ours` on a - conflicted gitlink exits 0 and leaves all three stages where they were, - so naming it would be a command that changes nothing. Finishing the - merge takes the gitlink, and the `pins` row then says whether the two - agree. + names another file `tool_written` admits -- a pin `project.yaml` + declares, for a leg or a neutral product, or the naming policy's copy -- + which is how the `manifest kinds` row puts back each one that does not + load. The five states, their commands and their notes are the same for + every one, but for a copy's uncommitted change, which is shown and not + put back. The note says `copy` where the file is not a pin, and + `pin_reads_at` says what reads. Only the shape pin takes more paths with + it in a merge -- the copies `apply` writes in the same commit -- and + only its note names a conflicted manifest or README.md for a person. + Any other file is named alone. A leg pin moves with its gitlink, but + `checkout --ours` on a conflicted gitlink exits 0 and leaves all three + stages where they were, so naming it would be a command that changes + nothing. Finishing the merge takes the gitlink, and the `pins` row then + says whether the two agree. """ root = ctx.root git_c = f"git -C {quote_arg(root)}" @@ -1299,6 +1306,16 @@ def pin_restore(ctx: Context, detail: dict | None = None, f"hand-merged {what}{then} the merge as git says and run " "this doctor again") elif pin_reads_at(ctx, "HEAD", pin): + if xy and what == "copy": + return "uncommitted", ( + f"{git_c} diff HEAD -- {quote_arg(pin)} # HEAD holds a " + "copy that reads, and this shows the uncommitted change that " + "breaks the copy here. A copy can hold an edit of the " + "project, such as the merge by hand update-shape.py asks " + "for, and git keeps no other copy of it until it is " + "committed, so nothing is put back. Mend the change, or take " + "the copy as last committed once nothing in it is worth " + "keeping, then run this doctor again") if xy: return "uncommitted", ( f"{git_c} checkout HEAD -- {quote_arg(pin)} # puts back " @@ -1330,32 +1347,72 @@ def pin_restore(ctx: Context, detail: dict | None = None, return "none", None +def declared_pins(ctx: Context) -> set: + """Every pin this root DECLARES, at the path the tool that writes it + writes it to (#291). + + The shape pin, which every root carries. `contracts/-pin.yaml` + for each leg `project.yaml` declares, the spelling `check_legs` reads, + `validate-pins.py` checks and `bump-leg.py` writes. And + `contracts/-pin.yaml` for each name its + `neutral_product_pins:` lists, which is `shape_materialize`'s + `neutral_pin_path` and the spelling `validate-pins.py` checks, spelled + here as `check_legs` spells a leg's. A role or a product that is not + text names no pin. + """ + pins = {SHAPE_PIN} + pins.update(f"contracts/{leg['role']}-pin.yaml" for leg in ctx.legs() + if isinstance(leg.get("role"), str) and leg["role"]) + products = (ctx.manifest or {}).get("neutral_product_pins") + if isinstance(products, list): + pins.update(f"contracts/{product.lower()}-pin.yaml" + for product in products + if isinstance(product, str) and product) + return pins + + def tool_written(ctx: Context, rel: str) -> bool: """Whether `rel`, a root path under `contracts/`, is a file a tool of this standard writes, so that git holds the last good bytes (#291). - Every `contracts/-pin.yaml` is a pin. The shape pin is written by + WHAT THE ROOT DECLARES DECIDES, NOT THE NAME. A file is a tool's where + it is one of `declared_pins` or a shape copy the root kind lists, which + under `contracts/` is the naming policy. The shape pin is written by `scaffold-project.py`, `adopt-project.py` and `update-shape.py apply`, a leg pin by those and by `bump-leg.py`, and a neutral product pin by - `scaffold-project.py --pin`. Every shape copy the root kind lists is - such a file too, and under `contracts/` that is the naming policy. No - one edits any of them by hand, so the bytes a tool last wrote are the - fix. Anything else under `contracts/`, and `project.yaml` or - `family.yaml`, is the project's own. + `scaffold-project.py --pin`. No one edits a pin by hand, so the bytes a + tool last wrote are the fix. A copy can hold an edit of the project's, + which is why `pin_restore` shows a copy's uncommitted change rather + than putting it back. Anything else under `contracts/`, and + `project.yaml` or `family.yaml`, is the project's own. A + `contracts/release-pin.yaml` that no leg and no product names is a file + the project wrote, whatever it is called, and putting it back from git + would discard the edit, the merged hunk or the commit somebody made + (codex, review of PR #358). + + WHERE `project.yaml` DOES NOT LOAD, IT DECLARES NOTHING. A leg pin or a + neutral product pin that does not load either is then the project's for + that run, and is left alone. That is the safe side. Reading the roles + from HEAD's `project.yaml` instead would decide what git puts back from + a declaration the tree may disagree with, as the other side of a merge + that adds or renames a leg does. A file wrongly called a tool's loses a + person's edit, and one wrongly called the project's costs a run: once + `project.yaml` loads, the next run names the pin's restore. The shape + pin and the copies hang on no manifest, and stay a tool's. """ - if rel.startswith("contracts/") and rel.endswith("-pin.yaml"): - return True - return rel in ctx.update_shape().root_kind(ctx.root).copy_source + return (rel in declared_pins(ctx) + or rel in ctx.update_shape().root_kind(ctx.root).copy_source) #: The note on the `manifest kinds` row's next command where no file that #: does not load has a copy git can put back (#291). No `;`, apostrophe or #: backtick, so a pasted line is one command in any shell (#277). UNREADABLE_BY_HAND = ( - "the reason names each file that does not load and why, and no commit " - "in this root holds a copy a tool wrote to put back, so what each should " - "say -- a merge conflict resolved, a hand edit put right -- is for a " - "person to decide. This row reads them again once they load") + "the reason names each file that does not load and why, and none is a " + "file this root declares a tool writes with a copy that reads in its " + "history, so what each should say -- a merge conflict resolved, a hand " + "edit put right -- is for a person to decide. This row reads them again " + "once they load") def unreadable_next(ctx: Context, unreadable: list) -> tuple[dict, str]: @@ -1368,10 +1425,11 @@ def unreadable_next(ctx: Context, unreadable: list) -> tuple[dict, str]: where it has a command, then each other file in the row's order. A row prints one command, and the next run names the next file. - A FILE OF THE PROJECT'S OWN IS NOT, and its case is `project`, with git - not asked. The git-state choice does not fit it: each of the three - commands discards somebody's edit -- the other side of their merge, - their uncommitted work, their last commit -- and nothing in this + A FILE OF THE PROJECT'S OWN IS NOT, whatever it is called, and its case + is `project`, with git not asked. `tool_written` reads what the root + declares, not the name. The git-state choice does not fit it: each of + the three commands discards somebody's edit -- the other side of their + merge, their uncommitted work, their last commit -- and nothing in this standard says which bytes are right. Where no file has a command to name, the command is this doctor again, to run once a person has mended the file, and its note says that what the file should say is theirs to diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index af77e45b..1d6ed265 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2976,6 +2976,17 @@ def test_the_doctor_writes_no_bytecode_into_the_standard(standard, project, #: The one shape copy a root keeps under `contracts/`. NAMING_COPY = "contracts/repository-naming.yaml" OWN_CONTRACT = "contracts/notes.yaml" +#: A file of the project's own that is NAMED like a pin. No leg has the role +#: `release`, `neutral_product_pins:` does not name it, and it is not a shape +#: copy, so it is the project's whatever it is called (codex, PR #358). +OWN_PIN_NAMED = "contracts/release-pin.yaml" +RELEASE = (b"schedule: weekly\nowner: release-team\nchannels:\n" + b" - stable\nnotes: first\n") +#: The pin `neutral_product_pins:` declares for `openGlass`, at the path +#: `scaffold-project.py --pin` writes it to. +NEUTRAL_PIN = "contracts/openglass-pin.yaml" +#: A slip that stops a file loading, appended to what it held. +SLIP = b"\t: : [\n" #: How each restore's note names the file it puts back, `{what}` being #: `pin` or `copy`. @@ -3070,10 +3081,39 @@ def leg_pin_not_utf8(root: Path) -> list: return ["checkout", "HEAD", "--", SPEC_PIN] -def naming_copy_hand_broken(root: Path) -> list: +def leg_pin_not_utf8_committed(root: Path) -> list: + # The leg pin's own history, not the shape pin's: the walk lists the + # commits that touched THIS file, and asks each parent for THIS file. + pin = root / SPEC_PIN + pin.write_bytes(pin.read_bytes() + b"\xff\n") + git("add", "--", SPEC_PIN, cwd=root) + breaking = commit_in(root, "a byte that is not UTF-8 in the spec pin") + commit_in(root, "an unrelated commit after it", "--allow-empty") + return ["checkout", f"{breaking}~1", "--", SPEC_PIN] + + +def leg_pin_past_a_list_in_its_history(root: Path) -> list: + # Two commits in a row broke it, the first into a list, which loads and + # is not a mapping. The walk steps past that parent to the pin before + # both, because putting back the list would leave the row where it is. + pin = root / SPEC_PIN + good = pin.read_bytes() + pin.write_bytes(b"- a\n") + git("add", "--", SPEC_PIN, cwd=root) + first = commit_in(root, "the spec pin made a list") + pin.write_bytes(good + b"\xff\n") + git("add", "--", SPEC_PIN, cwd=root) + commit_in(root, "then a byte that is not UTF-8 in it") + commit_in(root, "an unrelated commit after it", "--allow-empty") + return ["checkout", f"{first}~1", "--", SPEC_PIN] + + +def naming_copy_broken_by_a_commit(root: Path) -> list: copy = root / NAMING_COPY - copy.write_bytes(copy.read_bytes() + b"\t: : [\n") - return ["checkout", "HEAD", "--", NAMING_COPY] + copy.write_bytes(copy.read_bytes() + SLIP) + git("add", "--", NAMING_COPY, cwd=root) + breaking = commit_in(root, "a slip committed in the naming copy") + return ["checkout", f"{breaking}~1", "--", NAMING_COPY] @pytest.mark.parametrize("breaks, rel, code, restore, what, compliant", [ @@ -3088,8 +3128,15 @@ def naming_copy_hand_broken(root: Path) -> list: "unmerged", "pin", False, id="a-leg-pin-conflicted"), pytest.param(leg_pin_not_utf8, SPEC_PIN, "yaml-unreadable", "uncommitted", "pin", True, id="a-leg-pin-not-utf-8"), - pytest.param(naming_copy_hand_broken, NAMING_COPY, "yaml-unparsable", - "uncommitted", "copy", True, id="the-naming-copy-broken"), + pytest.param(leg_pin_not_utf8_committed, SPEC_PIN, "yaml-unreadable", + "committed", "pin", True, + id="a-leg-pin-not-utf-8-committed"), + pytest.param(leg_pin_past_a_list_in_its_history, SPEC_PIN, + "yaml-unreadable", "committed", "pin", True, + id="a-leg-pin-past-a-list-in-its-history"), + pytest.param(naming_copy_broken_by_a_commit, NAMING_COPY, + "yaml-unparsable", "committed", "copy", True, + id="the-naming-copy-broken-by-a-commit"), ]) def test_a_file_a_tool_wrote_that_does_not_load_is_put_back_from_git( standard, project, breaks, rel, code, restore, what, compliant): @@ -3111,6 +3158,9 @@ def test_a_file_a_tool_wrote_that_does_not_load_is_put_back_from_git( assert row["detail"]["restore"] == {rel: restore}, row["detail"] said = row["detail"]["refusals"][rel] assert said["code"] == code, said + if code is None: + # What a file that loads as something else is said to hold. + assert said["detail"] == "it loads as a list, not a mapping", said # The loader's own words, less the path it opens with. words = f"{code}: {said['detail']}" if code else said["detail"] assert f"unreadable as YAML: {rel} ({words})" in row["reason"], \ @@ -3147,11 +3197,87 @@ def leg_pin_in_a_squashed_root_commit(root: Path) -> tuple[str, str]: return SPEC_PIN, "none" +def leg_pin_added_back_after_its_deletion(root: Path) -> tuple[str, str]: + # The walk stops at the commit that added the file back: its parent + # holds none, so the pin from before the deletion is not this file's + # history to put back, though the parent holds a shape pin. + pin = root / SPEC_PIN + good = pin.read_bytes() + git("rm", "-q", "--", SPEC_PIN, cwd=root) + commit_in(root, "the spec pin deleted") + pin.write_bytes(good + b"\xff\n") + git("add", "--", SPEC_PIN, cwd=root) + commit_in(root, "the spec pin added back, with a byte that is not UTF-8") + return SPEC_PIN, "none" + + +def own_pin_named(root: Path) -> Path: + """`contracts/release-pin.yaml`, committed: the project's own file, + named like a pin, which nothing in the root declares.""" + path = root / OWN_PIN_NAMED + path.write_bytes(RELEASE) + git("add", "--", OWN_PIN_NAMED, cwd=root) + commit_in(root, "the project's own release train") + return path + + +def own_pin_named_edited_with_a_slip(root: Path) -> tuple[str, str]: + # A person's edit, not yet committed, with a slip in it. `checkout + # HEAD` would drop `schedule: daily` along with the slip. + own_pin_named(root).write_bytes(RELEASE.replace(b"weekly", b"daily") + + SLIP) + return OWN_PIN_NAMED, "project" + + +def own_pin_named_conflicted(root: Path) -> tuple[str, str]: + # Both sides change `schedule:`, and the other side also adds a channel + # that git merges cleanly. `checkout --ours` would take this side whole + # and drop `- nightly`. + own_pin_named(root) + base = git("rev-parse", "HEAD", cwd=root).stdout.strip() + for branch, body in ( + ("edit-a", RELEASE.replace(b"weekly", b"daily").replace( + b" - stable\n", b" - stable\n - nightly\n")), + ("edit-b", RELEASE.replace(b"weekly", b"hourly"))): + git("checkout", "-q", "-b", branch, base, cwd=root) + (root / OWN_PIN_NAMED).write_bytes(body) + git("add", "--", OWN_PIN_NAMED, cwd=root) + commit_in(root, branch) + merged = subprocess.run(["git", "merge", "-q", "edit-a"], cwd=str(root), + capture_output=True, text=True, check=False, + env={**os.environ, **GIT_IDENTITY}) + assert merged.returncode != 0, merged.stdout + merged.stderr + status = git("status", "--porcelain=v1", cwd=root).stdout + assert f"UU {OWN_PIN_NAMED}" in status, status + assert b" - nightly\n" in (root / OWN_PIN_NAMED).read_bytes() + return OWN_PIN_NAMED, "project" + + +def own_pin_named_broken_by_a_commit(root: Path) -> tuple[str, str]: + # A commit that hands the train to another team, with a slip in it. + # `checkout ~1` would drop `owner: platform-team` with the slip. + path = own_pin_named(root) + path.write_bytes(RELEASE.replace(b"release-team", b"platform-team") + + SLIP) + git("add", "--", OWN_PIN_NAMED, cwd=root) + commit_in(root, "the release train moves to the platform team") + commit_in(root, "an unrelated commit after it", "--allow-empty") + return OWN_PIN_NAMED, "project" + + @pytest.mark.parametrize("breaks", [ pytest.param(project_contract_conflicted, id="a-contract-of-the-projects-own-conflicted"), + pytest.param(own_pin_named_edited_with_a_slip, + id="a-file-of-the-projects-own-named-like-a-pin-edited"), + pytest.param(own_pin_named_conflicted, + id="a-file-of-the-projects-own-named-like-a-pin-conflicted"), + pytest.param(own_pin_named_broken_by_a_commit, + id="a-file-of-the-projects-own-named-like-a-pin-committed"), pytest.param(leg_pin_in_a_squashed_root_commit, id="a-leg-pin-with-no-good-copy-in-its-history"), + pytest.param(leg_pin_added_back_after_its_deletion, + id="a-leg-pin-added-back-after-its-deletion"), ]) def test_a_file_git_cannot_put_back_is_left_to_a_person(standard, project, breaks): @@ -3160,9 +3286,11 @@ def test_a_file_git_cannot_put_back_is_left_to_a_person(standard, project, A file of the project's own is never put back from git, because each of the three commands throws away an edit somebody made, so git is not - asked (`project`). A pin with no good copy in its history has nothing - to put back (`none`). Run as printed, the doctor finds the same file - and touches none of it. Once a person mends the file, the row reads it. + asked (`project`). That holds for one named like a pin, in each of the + three states: what the root declares decides, not the name. A pin with + no good copy in its history has nothing to put back (`none`). Run as + printed, the doctor finds the same file and touches none of it. Once a + person mends the file, the row reads it. """ rel, restore = breaks(project) row = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] @@ -3253,6 +3381,104 @@ def test_the_shape_pin_goes_first_and_each_run_names_the_next_file( assert own.read_bytes() == b"colour: blue\n\xff\n" +def declare_a_neutral_pin(root: Path) -> None: + """`neutral_product_pins:` naming `openGlass` in `project.yaml`, and the + pin it declares committed beside it, as `scaffold-project.py --pin` + leaves the two.""" + manifest = root / "project.yaml" + text = manifest.read_bytes() + assert text.count(b"neutral_product_pins: []\n") == 1, text + manifest.write_bytes(text.replace(b"neutral_product_pins: []\n", + b"neutral_product_pins:\n" + b" - openGlass\n")) + (root / NEUTRAL_PIN).write_bytes( + b"pin_role: neutral-product\nproduct: openGlass\n" + b'commit: "' + b"1" * 40 + b'"\n') + git("add", "--", "project.yaml", NEUTRAL_PIN, cwd=root) + commit_in(root, "a pin on the neutral product openGlass") + + +def test_a_pin_is_what_the_root_declares_not_what_it_is_called(standard, + project): + """A neutral product pin `project.yaml` declares is put back from git, + and a file of the project's own named like a pin is not. + + IN PROCESS, through the row alone: a declared neutral pin makes the + `pins` row recheck its digest, and with no checkout of the product + beside the root that asks `gh`, which no test here may reach. + """ + declare_a_neutral_pin(project) + own = own_pin_named(project) + for rel in (NEUTRAL_PIN, OWN_PIN_NAMED): + path = project / rel + path.write_bytes(path.read_bytes() + b"\xff\n") + module = doctor_module(standard) + row = module.check_manifest_kinds(module.Context(project)).as_dict() + assert row["detail"]["unreadable"] == [NEUTRAL_PIN, OWN_PIN_NAMED], row + assert row["detail"]["restore"] == {NEUTRAL_PIN: "uncommitted", + OWN_PIN_NAMED: "project"}, row + assert_no_registration(row) + assert pasted(row["next"])[3:] == ["checkout", "HEAD", "--", + NEUTRAL_PIN], row["next"] + run_pasted(row["next"]) + row = module.check_manifest_kinds(module.Context(project)).as_dict() + assert row["detail"]["restore"] == {OWN_PIN_NAMED: "project"}, row + assert Path(pasted(row["next"])[1]).name == DOCTOR, row["next"] + assert own.read_bytes() == RELEASE + b"\xff\n" + + +def test_a_leg_pin_is_left_to_a_person_while_project_yaml_does_not_load( + standard, project): + """`project.yaml` declares the leg pins, so where it does not load it + declares none, and a leg pin that does not load either is left alone + for that run. The shape pin and the copies are declared by no manifest, + and stay a tool's. Once `project.yaml` loads, the row puts the leg pin + back. + + In process, because at this branch a root whose `project.yaml` does not + load is not a shape root, and the doctor asks no row of it. + """ + for rel in ("project.yaml", SPEC_PIN): + path = project / rel + path.write_bytes(path.read_bytes() + b"\xff\n") + module = doctor_module(standard) + ctx = module.Context(project) + assert ctx.manifest is None + row = module.check_manifest_kinds(ctx).as_dict() + assert row["detail"]["restore"][SPEC_PIN] == "project", row["detail"] + assert Path(pasted(row["next"])[1]).name == DOCTOR, row["next"] + assert module.tool_written(ctx, SHAPE_PIN) + assert module.tool_written(ctx, NAMING_COPY) + git("checkout", "-q", "HEAD", "--", "project.yaml", cwd=project) + row = module.check_manifest_kinds(module.Context(project)).as_dict() + assert row["detail"]["restore"] == {SPEC_PIN: "uncommitted"}, row + assert pasted(row["next"])[3:] == ["checkout", "HEAD", "--", SPEC_PIN] + + +def test_an_uncommitted_change_to_a_copy_is_shown_and_kept(standard, + project): + """A copy can hold the project's own edit -- the merge by hand that + `update-shape.py` asks for where a copy changed on both sides -- and + git keeps no other copy of it until it is committed. So where HEAD's + copy reads and the change here breaks it, the NEXT shows the change and + puts nothing back. Run as printed, the bytes are still the person's. + """ + copy = project / NAMING_COPY + copy.write_bytes(copy.read_bytes() + SLIP) + before = copy.read_bytes() + row = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert row["detail"]["restore"] == {NAMING_COPY: "uncommitted"}, row + assert_no_registration(row) + assert pasted(row["next"])[3:] == ["diff", "HEAD", "--", NAMING_COPY], \ + row["next"] + assert "checkout" not in row["next"], row["next"] + assert "nothing is put back" in row["next"].split(" #", 1)[1] + run_pasted(row["next"]) + assert copy.read_bytes() == before + again = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert again["detail"]["restore"] == {NAMING_COPY: "uncommitted"}, again + + # --- placement: code in spec, spec in code (#99) ---------------------------- # # Brett Heap, 2026-09-10: "we have to look for code in spec and spec in code". From 30c963cb624fbd3365fa1043cb23e6c9c871136f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 12 Oct 2026 21:03:42 +0000 Subject: [PATCH 10/11] shape-doctor.py: a copy the shape pin has a row for, and a neutral product pin, get a read-only `git diff` in every git state, and a copy is what the pin's `files:` rows name, so the `manifest kinds` NEXT no longer drops a person's merge, re-pin or edit with `checkout --ours`, `checkout HEAD` or a `~1` rollback (#291) Codex re-reviewed 2745890 with two P1s, and rv358 reproduced both (probes P10 to P13). r4250578605: a copy can hold the project's own edit -- above all the merge by hand `update-shape.py` asks for where a copy changed on both sides -- yet only its uncommitted state was shown. Conflicted on its own, the NEXT took one side whole with `checkout --ours`, and the hunk git had merged cleanly went with the other side; committed, it rolled the hand merge back with `checkout ~1`. Now `pin_restore` gives a file `edited_by_hand` a read-only command in every state: `diff` for `unmerged`, `diff HEAD` for `uncommitted`, `diff ~1` for `committed`. The note asks for the merge by hand, which is what the `shape currency` row's `--accept-local` and AGENTS.md's "`both` is a merge" already say. r4250578619: `tool_written` took copies from this standard's static copy lists, so a `contracts/repository-naming.yaml` an adopted project made its own (its row dropped) or kept beside a collision copy under `shape/` was put back from git. A copy is now what the root's own shape pin has a `files:` row for (`pinned_copies`), and where the pin does not read nothing is a copy, the safe side. The shape pin's own merge, which takes the copies whole with the pin (#267), now names the copies the taken side's pin has rows for (`pin_at` on the index stage), not the lists. rv358's P3: no tool writes a neutral product pin after the scaffold, so its change is a person's and gets the same read-only commands. Tests: one per probe, red at ce2ce75 (10 failed of 23 selected). Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 208 ++++++++++++----- tests/test_shape_doctor.py | 467 +++++++++++++++++++++++++++++++++++-- 2 files changed, 600 insertions(+), 75 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 7d3805c7..5644a7d7 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -2208,8 +2208,14 @@ def git_answer(args: list, cwd: Path, binary: bool = False): def pin_reads_at(ctx: Context, rev: str, pin: str = SHAPE_PIN) -> bool: - """Whether the pin `rev` holds reads as one, by `read_pin`'s own checks - (#237). + """Whether the pin `rev` holds reads as one: `pin_at` names something.""" + return pin_at(ctx, rev, pin) is not None + + +def pin_at(ctx: Context, rev: str, pin: str = SHAPE_PIN): + """What the pin `rev` holds reads as, by `read_pin`'s own checks (#237), + or None where it does not read: for the shape pin the paths its + `files:` rows name, never empty, and for any other file the mapping. READ BEFORE IT IS NAMED: a NEXT that puts back a pin out of history must name one that reads. Rolling back to a pin that is broken too @@ -2229,21 +2235,26 @@ def pin_reads_at(ctx: Context, rev: str, pin: str = SHAPE_PIN) -> bool: (#291). That is the question the `manifest kinds` row asks of it, and the one reader this file holds for a leg pin or a copy, so what the row puts back is what the row then reads. + + THE ROWS, NOT A YES, for the shape pin (review of PR #358): `rev` may + be an index stage, `:2` or `:3`, and the rows of the side of a merge + `pin_restore` takes whole are the copies that side's tool wrote, so + they are what it names with the pin. """ blob = git_answer(["cat-file", "blob", f"{rev}:./{pin}"], ctx.root, binary=True) if blob is None: - return False + return None us = ctx.update_shape() try: text = blob.decode("utf-8").replace("\r\n", "\n").replace("\r", "\n") data = us.parse_yaml(text) if pin != SHAPE_PIN: - return isinstance(data, dict) - us.check_pin(data, f"{rev}:{SHAPE_PIN}") + return data if isinstance(data, dict) else None + _pin, rows = us.check_pin(data, f"{rev}:{SHAPE_PIN}") except (Refusal, us.YamlError, ValueError, TypeError): - return False - return True + return None + return [row.path for row in rows] def pin_restore(ctx: Context, detail: dict | None = None, @@ -2258,10 +2269,14 @@ def pin_restore(ctx: Context, detail: dict | None = None, unmerged a merge left the pin conflicted. `checkout --ours`, or `--theirs` where only that side holds a pin, of every - conflicted path the shape writes WHOLE -- the pin, the - copies and their collision copies -- because a merge of - two `update-shape.py apply` runs conflicts in them - together, and one side taken whole is one tool's output. + conflicted path the shape writes WHOLE -- the pin, and + the copies that side's pin has `files:` rows for, their + collision copies included -- because a merge of two + `update-shape.py apply` runs conflicts in them together, + and one side taken whole is one tool's output. Those rows + and not this standard's copy lists: a file the pin has no + row for is not a shape copy, though the lists name its + path (review of PR #358). The manifest and README.md are never named, though `apply` writes lines of them: every other line is the project's own, and one side taken whole drops the other side's @@ -2272,13 +2287,7 @@ def pin_restore(ctx: Context, detail: dict | None = None, is theirs. uncommitted HEAD holds a pin that reads, and the tree or the index differs from it. `checkout HEAD`, which drops a change no - tool of this standard leaves uncommitted. NOT FOR A COPY, - which can hold the project's own edit -- the merge by - hand `update-shape.py` asks for where a copy changed on - both sides above all -- and git keeps no other copy of - that edit until it is committed. A copy's NEXT is the - `diff HEAD` that shows the change, and mending it is a - person's call (review of PR #358). + tool of this standard leaves uncommitted. hidden HEAD holds a pin that reads, and `git status` shows no change to it: an index flag hides the copy here (a sparse checkout, assume-unchanged), or the file cannot be read. @@ -2309,11 +2318,9 @@ def pin_restore(ctx: Context, detail: dict | None = None, ANY FILE UNDER `contracts/` A TOOL WROTE, NOT THE SHAPE PIN ALONE (#291). `pin` is the root's own `contracts/shape-pin.yaml` unless the caller names another file `tool_written` admits -- a pin `project.yaml` - declares, for a leg or a neutral product, or the naming policy's copy -- - which is how the `manifest kinds` row puts back each one that does not - load. The five states, their commands and their notes are the same for - every one, but for a copy's uncommitted change, which is shown and not - put back. The note says `copy` where the file is not a pin, and + declares, for a leg or a neutral product, or a copy the shape pin has a + row for -- which is how the `manifest kinds` row answers each one that + does not load. The note says `copy` where the file is not a pin, and `pin_reads_at` says what reads. Only the shape pin takes more paths with it in a merge -- the copies `apply` writes in the same commit -- and only its note names a conflicted manifest or README.md for a person. @@ -2322,20 +2329,42 @@ def pin_restore(ctx: Context, detail: dict | None = None, stages where they were, so naming it would be a command that changes nothing. Finishing the merge takes the gitlink, and the `pins` row then says whether the two agree. + + A FILE A PERSON MAY HAVE EDITED IS SHOWN, NEVER PUT BACK, in every + state (review of PR #358). That is a copy, which can hold an edit of + the project's -- the merge by hand `update-shape.py` asks for where a + copy changed on both sides above all -- and a neutral product pin, + which no tool writes after the scaffold, so every change to one is a + person's. `edited_by_hand` says which. Each of the three commands above + drops that edit: the other side of their merge and the hunks git merged + cleanly, their uncommitted work, their commit. So the state picks a + read-only command instead: `diff` for `unmerged`, which shows the hunks + git could not merge, `diff HEAD` for `uncommitted`, and `diff + ~1` for `committed`, every change since the newest one + that reads. Mending the file, or merging it, is the person's, as the + `both` exit of `update-shape.py` says of a copy changed on both sides: + a merge by hand, then `--accept-local` on it, which is what the `shape + currency` row names for the same file. """ root = ctx.root git_c = f"git -C {quote_arg(root)}" what = "pin" if pin.endswith("-pin.yaml") else "copy" + shown = edited_by_hand(ctx, pin) + why = BY_HAND_WHY[what] raw = git_answer(["--no-optional-locks", "status", "--porcelain=v1", "--", pin], root, binary=True) xy = raw.decode("utf-8", "replace")[:2] if raw else "" + if xy in UNMERGED_STATUS and shown: + return "unmerged", ( + f"{git_c} diff -- {quote_arg(pin)} # a merge left the {what} " + "conflicted, and this shows the hunks git could not merge. " + f"{why}, so neither side is taken whole: merge it by hand, " + "keeping the hunks git merged, then finish the merge as git says " + "and run this doctor again") if xy in UNMERGED_STATUS: us = ctx.update_shape() kind = us.root_kind(root) whole = {pin} - if pin == SHAPE_PIN: - for rel in kind.copy_source: - whole.update((rel, f"{us.COLLISION_DIR}/{rel}")) # ` SP SP TAB `, one record per stage. stages: dict[str, set] = {} listing = git_answer(["ls-files", "-u", "-z"], root, binary=True) @@ -2349,6 +2378,8 @@ def pin_restore(ctx: Context, detail: dict | None = None, ("--theirs", b"3") if b"3" in pin_stages else (None, None)) if side: + if pin == SHAPE_PIN: + whole.update(pin_at(ctx, f":{stage.decode()}") or ()) note = ("--theirs takes the other side. Take one side whole" if b"3" in pin_stages and side == "--ours" else f"only that side of the merge holds a {what}. Take " @@ -2375,16 +2406,14 @@ def pin_restore(ctx: Context, detail: dict | None = None, f"hand-merged {what}{then} the merge as git says and run " "this doctor again") elif pin_reads_at(ctx, "HEAD", pin): - if xy and what == "copy": + if xy and shown: return "uncommitted", ( f"{git_c} diff HEAD -- {quote_arg(pin)} # HEAD holds a " - "copy that reads, and this shows the uncommitted change that " - "breaks the copy here. A copy can hold an edit of the " - "project, such as the merge by hand update-shape.py asks " - "for, and git keeps no other copy of it until it is " - "committed, so nothing is put back. Mend the change, or take " - "the copy as last committed once nothing in it is worth " - "keeping, then run this doctor again") + f"{what} that reads, and this shows the uncommitted change " + f"that breaks the {what} here. {why}. Git keeps no other copy " + "of the change until it is committed, so nothing is put back. " + f"Mend the change, or take the {what} as last committed once " + "nothing in it is worth keeping, then run this doctor again") if xy: return "uncommitted", ( f"{git_c} checkout HEAD -- {quote_arg(pin)} # puts back " @@ -2406,13 +2435,22 @@ def pin_restore(ctx: Context, detail: dict | None = None, if git_answer(["cat-file", "-e", f"{before}:./{pin}"], root) is None: break - if pin_reads_at(ctx, before, pin): + if not pin_reads_at(ctx, before, pin): + continue + if shown: return "committed", ( - f"{git_c} checkout {quote_arg(before)} -- " - f"{quote_arg(pin)} # the {what} as it was before " - f"{breaking[:12]}, the commit that broke it, and the newest " - f"{what} in its history that reads. Commit that rather than " - f"editing the {what}") + f"{git_c} diff {quote_arg(before)} -- {quote_arg(pin)} # " + f"the newest {what} in its history that reads is the one " + f"before {breaking[:12]}, the commit that broke it, and this " + f"shows every change since. {why}, so nothing is rolled " + f"back: mend the {what}, commit that and run this doctor " + "again") + return "committed", ( + f"{git_c} checkout {quote_arg(before)} -- " + f"{quote_arg(pin)} # the {what} as it was before " + f"{breaking[:12]}, the commit that broke it, and the newest " + f"{what} in its history that reads. Commit that rather than " + f"editing the {what}") return "none", None @@ -2432,12 +2470,66 @@ def declared_pins(ctx: Context) -> set: pins = {SHAPE_PIN} pins.update(f"contracts/{leg['role']}-pin.yaml" for leg in ctx.legs() if isinstance(leg.get("role"), str) and leg["role"]) + return pins | neutral_pins(ctx) + + +def neutral_pins(ctx: Context) -> set: + """`contracts/-pin.yaml` for each name the root's + `neutral_product_pins:` lists, as `declared_pins` spells them (#291).""" products = (ctx.manifest or {}).get("neutral_product_pins") - if isinstance(products, list): - pins.update(f"contracts/{product.lower()}-pin.yaml" - for product in products - if isinstance(product, str) and product) - return pins + if not isinstance(products, list): + return set() + return {f"contracts/{product.lower()}-pin.yaml" for product in products + if isinstance(product, str) and product} + + +def pinned_copies(ctx: Context) -> set: + """The root paths the root's own shape pin has `files:` rows for: its + shape copies, read as `update-shape.py` reads them (review of PR #358). + + THE ROWS, NOT THIS STANDARD'S COPY LISTS. A file with no row is not a + shape copy (`update-shape.py`'s own rule, and AGENTS.md's): an adopted + project that merged a copy away drops its row, and the file left at its + path is the project's own, as is the source's own file at a path an + adoption collided on, whose copy went under `shape/` with a row naming + that path. The lists name both paths all the same. Where the pin does + not read, no row says anything, so nothing is a copy for that run: the + safe side, as for a `project.yaml` that does not load. + """ + us = ctx.update_shape() + try: + _pin, rows = us.read_pin(ctx.root) + except (Refusal, us.YamlError, OSError, ValueError, TypeError): + return set() + return {row.path for row in rows} + + +def edited_by_hand(ctx: Context, rel: str) -> bool: + """Whether `rel` is a file a tool wrote that a person may have edited + since, so that `pin_restore` shows it and puts nothing back (review of + PR #358): a copy the shape pin has a row for, or a neutral product pin. + + A copy can hold the project's own edit: `update-shape.py` calls one + `locally-modified` and leaves it, and where a copy changed on both + sides it asks for a merge by hand. A neutral product pin is written by + `scaffold-project.py --pin` and by no tool after it, and the manifest + template says to add one by hand, so every change to one is a + person's. The shape pin and a leg pin are a tool's alone: a person + re-pins a leg with `bump-leg.py`, and the shape with `update-shape.py`. + """ + return rel != SHAPE_PIN and (rel in pinned_copies(ctx) + or rel in neutral_pins(ctx)) + + +#: Why `pin_restore` puts nothing back for a file `edited_by_hand`, by what +#: its note calls the file. No `;`, apostrophe or backtick (#277). +BY_HAND_WHY = { + "copy": ("A copy can hold an edit of the project, such as the merge by " + "hand update-shape.py asks for where a copy changed on both " + "sides"), + "pin": ("No tool of this standard writes a neutral product pin after the " + "scaffold, so every change to one was made by a person"), +} def tool_written(ctx: Context, rel: str) -> bool: @@ -2445,19 +2537,20 @@ def tool_written(ctx: Context, rel: str) -> bool: this standard writes, so that git holds the last good bytes (#291). WHAT THE ROOT DECLARES DECIDES, NOT THE NAME. A file is a tool's where - it is one of `declared_pins` or a shape copy the root kind lists, which - under `contracts/` is the naming policy. The shape pin is written by + it is one of `declared_pins` or one of `pinned_copies`, the rows of the + root's own shape pin, which under `contracts/` is the naming policy + where the root keeps it. The shape pin is written by `scaffold-project.py`, `adopt-project.py` and `update-shape.py apply`, a leg pin by those and by `bump-leg.py`, and a neutral product pin by - `scaffold-project.py --pin`. No one edits a pin by hand, so the bytes a - tool last wrote are the fix. A copy can hold an edit of the project's, - which is why `pin_restore` shows a copy's uncommitted change rather - than putting it back. Anything else under `contracts/`, and + `scaffold-project.py --pin`. A copy or a neutral product pin can hold a + person's edit, which is why `pin_restore` shows one and puts nothing + back (`edited_by_hand`). Anything else under `contracts/`, and `project.yaml` or `family.yaml`, is the project's own. A `contracts/release-pin.yaml` that no leg and no product names is a file the project wrote, whatever it is called, and putting it back from git would discard the edit, the merged hunk or the commit somebody made - (codex, review of PR #358). + (codex, review of PR #358). So is a `contracts/repository-naming.yaml` + the shape pin has no row for, though this standard copies a file there. WHERE `project.yaml` DOES NOT LOAD, IT DECLARES NOTHING. A leg pin or a neutral product pin that does not load either is then the project's for @@ -2467,10 +2560,10 @@ def tool_written(ctx: Context, rel: str) -> bool: that adds or renames a leg does. A file wrongly called a tool's loses a person's edit, and one wrongly called the project's costs a run: once `project.yaml` loads, the next run names the pin's restore. The shape - pin and the copies hang on no manifest, and stay a tool's. + pin and its copies hang on no manifest: the shape pin stays a tool's, + and the copies are what its rows say, whatever `project.yaml` holds. """ - return (rel in declared_pins(ctx) - or rel in ctx.update_shape().root_kind(ctx.root).copy_source) + return rel in declared_pins(ctx) or rel in pinned_copies(ctx) #: The note on the `manifest kinds` row's next command where no file that @@ -2492,7 +2585,10 @@ def unreadable_next(ctx: Context, unreadable: list) -> tuple[dict, str]: helper the `shape currency` row uses for the shape pin, so both rows print the same command for the same file. The shape pin goes first where it has a command, then each other file in the row's order. A row - prints one command, and the next run names the next file. + prints one command, and the next run names the next file. A copy or a + neutral product pin, which a person may have edited, gets the + read-only command `pin_restore` picks for it instead, a `git diff` + that shows what breaks it, and nothing is put back. A FILE OF THE PROJECT'S OWN IS NOT, whatever it is called, and its case is `project`, with git not asked. `tool_written` reads what the root diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index 42b2b78f..787ca4db 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -27,6 +27,7 @@ from __future__ import annotations import ast +import hashlib import importlib.util import json import os @@ -4322,7 +4323,10 @@ def test_the_doctor_writes_no_bytecode_into_the_standard(standard, project, # the pin, and it was not a command a shell could even split. A file a tool # wrote is now put back from the root's own history by `pin_restore`, the # helper the `shape currency` row uses (#237). A file of the project's own -# is left to a person. Each test runs the printed NEXT, then asks again. +# is left to a person. A copy the shape pin has a row for, and a neutral +# product pin, can hold a person's edit, so the NEXT shows it and puts +# nothing back (review of PR #358). Each test runs the printed NEXT, then +# asks again. SPEC_PIN = "contracts/spec-pin.yaml" CODE_PIN = "contracts/code-pin.yaml" @@ -4461,14 +4465,6 @@ def leg_pin_past_a_list_in_its_history(root: Path) -> list: return ["checkout", f"{first}~1", "--", SPEC_PIN] -def naming_copy_broken_by_a_commit(root: Path) -> list: - copy = root / NAMING_COPY - copy.write_bytes(copy.read_bytes() + SLIP) - git("add", "--", NAMING_COPY, cwd=root) - breaking = commit_in(root, "a slip committed in the naming copy") - return ["checkout", f"{breaking}~1", "--", NAMING_COPY] - - @pytest.mark.parametrize("breaks, rel, code, restore, what, compliant", [ pytest.param(shape_pin_conflicted, SHAPE_PIN, "yaml-unparsable", "unmerged", "pin", False, id="the-shape-pin-conflicted"), @@ -4487,9 +4483,6 @@ def naming_copy_broken_by_a_commit(root: Path) -> list: pytest.param(leg_pin_past_a_list_in_its_history, SPEC_PIN, "yaml-unreadable", "committed", "pin", True, id="a-leg-pin-past-a-list-in-its-history"), - pytest.param(naming_copy_broken_by_a_commit, NAMING_COPY, - "yaml-unparsable", "committed", "copy", True, - id="the-naming-copy-broken-by-a-commit"), ]) def test_a_file_a_tool_wrote_that_does_not_load_is_put_back_from_git( standard, project, breaks, rel, code, restore, what, compliant): @@ -4753,8 +4746,14 @@ def declare_a_neutral_pin(root: Path) -> None: def test_a_pin_is_what_the_root_declares_not_what_it_is_called(standard, project): - """A neutral product pin `project.yaml` declares is put back from git, - and a file of the project's own named like a pin is not. + """A neutral product pin `project.yaml` declares is asked of git, and a + file of the project's own named like a pin is not. + + The declared pin's case is its git state, and its NEXT the `diff HEAD` + that shows the change: no tool writes a neutral product pin after the + scaffold, so the change is a person's and nothing is put back (review + of PR #358). The file named like a pin is `project`, git not asked. + Run as printed, neither file's bytes move. IN PROCESS, through the row alone: a declared neutral pin makes the `pins` row recheck its digest, and with no checkout of the product @@ -4765,18 +4764,20 @@ def test_a_pin_is_what_the_root_declares_not_what_it_is_called(standard, for rel in (NEUTRAL_PIN, OWN_PIN_NAMED): path = project / rel path.write_bytes(path.read_bytes() + b"\xff\n") + neutral = (project / NEUTRAL_PIN).read_bytes() module = doctor_module(standard) row = module.check_manifest_kinds(module.Context(project)).as_dict() assert row["detail"]["unreadable"] == [NEUTRAL_PIN, OWN_PIN_NAMED], row assert row["detail"]["restore"] == {NEUTRAL_PIN: "uncommitted", OWN_PIN_NAMED: "project"}, row assert_no_registration(row) - assert pasted(row["next"])[3:] == ["checkout", "HEAD", "--", + assert pasted(row["next"])[3:] == ["diff", "HEAD", "--", NEUTRAL_PIN], row["next"] - run_pasted(row["next"]) - row = module.check_manifest_kinds(module.Context(project)).as_dict() - assert row["detail"]["restore"] == {OWN_PIN_NAMED: "project"}, row - assert Path(pasted(row["next"])[1]).name == DOCTOR, row["next"] + # As bytes: the diff shows the byte that is not UTF-8. + run_read_only(standard, project, row) + again = module.check_manifest_kinds(module.Context(project)).as_dict() + assert again == row, (again, row) + assert (project / NEUTRAL_PIN).read_bytes() == neutral assert own.read_bytes() == RELEASE + b"\xff\n" @@ -4887,6 +4888,434 @@ def test_a_contracts_family_yaml_beside_a_file_that_does_not_load_names_the_rest assert json.loads(final.stdout)["verdict"] == "COMPLIANT" +# --- a copy or a neutral pin is a person's to mend (review of PR #358) ------ +# +# Codex on 2745890, and rv358's probes P10 to P13. A copy can hold the +# project's own edit, above all the merge by hand `update-shape.py` asks for +# where a copy changed on both sides, so its `unmerged` and `committed` +# states get what its `uncommitted` state got: a read-only `git diff` and a +# person's merge, never one side taken whole and never a rollback (codex +# r4250578605). A copy is what the shape pin's `files:` rows name, not what +# this standard's copy lists name (codex r4250578619). And no tool writes a +# neutral product pin after the scaffold, so a change to one is a person's. + +#: Lines a person writes into the naming copy. Comments, so that any copy +#: of the policy stays YAML, and the end of the file is where both sides of +#: a merge write, so git cannot merge the two tails. +PROJECT_LINE = b"# Atlas_docs: a line the project keeps in its copy\n" +PROJECT_TAIL = b"# a closing line the project wrote\n" +SHAPE_TAIL = b"# a closing line the shape wrote\n" +#: A conflict hunk, as git leaves one in the working file. +CONFLICT_HUNK = re.compile( + rb"(?ms)^<<<<<<< [^\n]*\n(.*?)^=======\n(.*?)^>>>>>>> [^\n]*\n") + + +def insert_after_first_line(path: Path, line: bytes) -> None: + first, rest = path.read_bytes().split(b"\n", 1) + path.write_bytes(first + b"\n" + line + rest) + + +def append_to(path: Path, line: bytes) -> None: + path.write_bytes(path.read_bytes() + line) + + +def repin(root: Path, rel: str) -> None: + """The shape pin's `sha256:` for `rel` set to the file's bytes, as + `update-shape.py apply` sets it once it has re-copied the file.""" + pin = root / SHAPE_PIN + digest = hashlib.sha256((root / rel).read_bytes()).hexdigest() + row = (rb'(?m)^( - path: ' + re.escape(rel.encode()) + + rb'\n sha256: )"[0-9a-f]{64}"$') + text, count = re.subn(row, rb'\1"' + digest.encode() + rb'"', + pin.read_bytes()) + assert count == 1, rel + pin.write_bytes(text) + + +def merge_two_edits(root: Path, edit_a, edit_b, *paths: str) -> str: + """`edit-a` and `edit-b` from HEAD, each making its own edit of `paths`, + and `edit-b` merging `edit-a`, so `--ours` is edit-b's side and git + merges edit-a's hunks where it can. The status the merge leaves.""" + base = git("rev-parse", "HEAD", cwd=root).stdout.strip() + for branch, edit in (("edit-a", edit_a), ("edit-b", edit_b)): + git("checkout", "-q", "-b", branch, base, cwd=root) + edit(root) + git("add", "--", *paths, cwd=root) + commit_in(root, branch) + merged = subprocess.run(["git", "merge", "-q", "edit-a"], cwd=str(root), + capture_output=True, text=True, check=False, + env={**os.environ, **GIT_IDENTITY}) + assert merged.returncode != 0, merged.stdout + merged.stderr + return git("status", "--porcelain=v1", cwd=root).stdout + + +def a_copy_conflicted_on_its_own(root: Path) -> None: + """P10: edit-a is the project's own edit of the naming copy, a line + near the top and a line at the end; edit-b is an `apply`-like re-copy + that writes the end its own way and re-pins the copy. Git merges the + project's line and conflicts on the end. Only the copy is `UU`: the + shape pin changed on one side alone.""" + def project_edit(r: Path) -> None: + insert_after_first_line(r / NAMING_COPY, PROJECT_LINE) + append_to(r / NAMING_COPY, PROJECT_TAIL) + + def apply_like(r: Path) -> None: + append_to(r / NAMING_COPY, SHAPE_TAIL) + repin(r, NAMING_COPY) + + status = merge_two_edits(root, project_edit, apply_like, NAMING_COPY, + SHAPE_PIN) + assert f"UU {NAMING_COPY}" in status and SHAPE_PIN not in status, status + held = (root / NAMING_COPY).read_bytes() + for line in (PROJECT_LINE, PROJECT_TAIL, SHAPE_TAIL): + assert line in held, line + + +def merge_by_hand(path: Path) -> None: + """A person's merge: both sides of every conflict hunk kept, and the + hunks git merged left as they are.""" + held = path.read_bytes() + merged, count = CONFLICT_HUNK.subn(rb"\1\2", held) + assert count == 1 and b"<<<<<<<" not in merged, held[-400:] + path.write_bytes(merged) + + +def run_read_only(standard: Path, root: Path, row: dict) -> bytes: + """Run the printed NEXT, which must be `git -C diff ...`, and + require that it changed nothing: the file's bytes, the status, HEAD. + Its output.""" + argv = pasted(row["next"]) + assert argv[:2] == ["git", "-C"] and argv[3] == "diff", row["next"] + assert "checkout" not in row["next"], row["next"] + rel = argv[-1] + before = ((root / rel).read_bytes(), + git("status", "--porcelain=v1", cwd=root).stdout, + git("rev-parse", "HEAD", cwd=root).stdout) + proc = subprocess.run(argv, capture_output=True, check=False, + stdin=subprocess.DEVNULL, + env={**os.environ, **GIT_IDENTITY}) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert ((root / rel).read_bytes(), + git("status", "--porcelain=v1", cwd=root).stdout, + git("rev-parse", "HEAD", cwd=root).stdout) == before + return proc.stdout + + +def test_a_copy_conflicted_on_its_own_is_shown_and_merged_by_a_person( + standard, project): + """P10 (codex r4250578605). The NEXT was `checkout --ours`, the + apply-like side whole: run, and the merge finished as its note said, + the root was COMPLIANT and HEAD's copy had lost the project's line and + its tail, which git had merged or held for a person. + + Now the NEXT is the `diff` that shows the hunk git could not merge, + its note says a person merges it, and it changes nothing. Merged by + hand, both sides kept, the row reads the copy again. + """ + a_copy_conflicted_on_its_own(project) + row = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert row["detail"]["unreadable"] == [NAMING_COPY], row["detail"] + assert row["detail"]["restore"] == {NAMING_COPY: "unmerged"}, row + assert_no_registration(row) + assert pasted(row["next"])[3:] == ["diff", "--", NAMING_COPY], \ + row["next"] + note = row["next"].split(" #", 1)[1] + assert "neither side is taken whole" in note, note + assert "merge it by hand, keeping the hunks git merged" in note, note + shown = run_read_only(standard, project, row) + assert SHAPE_TAIL.strip() in shown and PROJECT_TAIL.strip() in shown, \ + shown + merge_by_hand(project / NAMING_COPY) + git("add", "--", NAMING_COPY, cwd=project) + commit_in(project, "the naming copy merged by hand") + again = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert again["status"] == "ok", again + kept = git("show", f"HEAD:{NAMING_COPY}", cwd=project).stdout + for line in (PROJECT_LINE, PROJECT_TAIL, SHAPE_TAIL): + assert line.decode() in kept, line + + +def test_a_conflicted_copys_next_agrees_with_shape_currency_and_agents_md( + standard, project): + """P10b. In the same report the `shape currency` row names `apply + --accept-local` on the copy, which keeps the local bytes, while the + `manifest kinds` row took one side whole. AGENTS.md says `both` is a + merge: by hand, commit, then `--accept-local` on the file merged. + + Now the `manifest kinds` row asks for that merge, and the two rows + are one path: the merge by hand, then the `shape currency` row's own + NEXT, run as its note spells it, ends COMPLIANT with every line of + both sides in the copy and its pin. + """ + a_copy_conflicted_on_its_own(project) + kinds, currency = kinds_and_currency(standard, project) + assert "merge it by hand" in kinds["next"].split(" #", 1)[1], kinds + assert "checkout" not in kinds["next"], kinds["next"] + assert f"--accept-local {NAMING_COPY}" in currency["next"], currency + merge_by_hand(project / NAMING_COPY) + git("add", "--", NAMING_COPY, cwd=project) + commit_in(project, "the naming copy merged by hand") + kinds, currency = kinds_and_currency(standard, project) + assert kinds["status"] == "ok", kinds + argv = pasted(currency["next"]) + assert Path(argv[1]).name == "update-shape.py" and argv[2] == "check", \ + currency["next"] + then = currency["next"].split(" #", 1)[1].split() + assert then[:2] == ["then", "apply"], currency["next"] + assert then[then.index("--accept-local") + 1] == NAMING_COPY, then + applied = run_script(standard / "update-shape.py", "apply", "--root", + str(project), "--upstream", str(standard), *then[2:]) + assert applied.returncode == 0, applied.stdout + applied.stderr + compliant_again(standard, project) + kept = git("show", f"HEAD:{NAMING_COPY}", cwd=project).stdout + for line in (PROJECT_LINE, PROJECT_TAIL, SHAPE_TAIL): + assert line.decode() in kept, line + + +def drop_the_copys_row(root: Path) -> None: + """The naming copy's `files:` row taken out of the shape pin and + committed, as an adopted project that made the file its own does.""" + pin = root / SHAPE_PIN + row = (rb'(?m)^ - path: ' + re.escape(NAMING_COPY.encode()) + + rb'\n sha256: "[0-9a-f]{64}"\n') + text, count = re.subn(row, b"", pin.read_bytes()) + assert count == 1 + pin.write_bytes(text) + git("add", "--", SHAPE_PIN, cwd=root) + commit_in(root, "the naming policy is the project's own now") + + +def test_a_file_the_shape_pin_has_no_row_for_is_the_projects(standard, + project): + """P11a (codex r4250578619). The pin has no row for + `contracts/repository-naming.yaml`, so the file there is the + project's, and the root is COMPLIANT. A commit of the project's own + breaks it: the NEXT was `checkout ~1`, the copy this + standard's lists name, and it dropped the project's line. + + Now the copy is what the rows say, so the file is `project`, git is not + asked, and the NEXT is this doctor again, which touches nothing. + """ + drop_the_copys_row(project) + compliant_again(standard, project) + own = project / NAMING_COPY + insert_after_first_line(own, PROJECT_LINE) + append_to(own, SLIP) + git("add", "--", NAMING_COPY, cwd=project) + commit_in(project, "the project's own naming examples, with a slip") + commit_in(project, "an unrelated commit after it", "--allow-empty") + row = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert row["detail"]["restore"] == {NAMING_COPY: "project"}, row + assert "checkout" not in row["next"], row["next"] + argv = pasted(row["next"]) + assert Path(argv[1]).name == DOCTOR, row["next"] + before = own.read_bytes() + proc = subprocess.run(argv, capture_output=True, text=True, check=False, + stdin=subprocess.DEVNULL, + env={**os.environ, **GIT_IDENTITY}) + assert proc.returncode == 1, proc.stdout + proc.stderr + assert own.read_bytes() == before and PROJECT_LINE in before + module = doctor_module(standard) + ctx = module.Context(project) + assert NAMING_COPY not in module.pinned_copies(ctx) + assert "Makefile" in module.pinned_copies(ctx) + assert not module.tool_written(ctx, NAMING_COPY) + + +def collision_layout(root: Path) -> None: + """An in-place adoption that collided on the naming policy: the shape's + copy under `shape/`, the pin's row naming it there, as + `shape_materialize` writes the two, and the source's own file at + `contracts/repository-naming.yaml`, which no row names.""" + copy = (root / NAMING_COPY).read_bytes() + (root / "shape" / "contracts").mkdir(parents=True) + (root / "shape" / NAMING_COPY).write_bytes(copy) + pin = root / SHAPE_PIN + text = pin.read_bytes() + row = f" - path: {NAMING_COPY}\n".encode() + assert text.count(row) == 1, text + pin.write_bytes(text.replace(row, f" - path: shape/{NAMING_COPY}\n" + .encode())) + git("add", "--", SHAPE_PIN, f"shape/{NAMING_COPY}", cwd=root) + commit_in(root, "the shape's copy under shape/, the source's own file " + "at contracts/") + + +def test_the_sources_own_file_at_a_collided_copy_path_is_the_projects( + standard, project): + """P11b (codex r4250578619). In the collision layout the source's own + `contracts/repository-naming.yaml` is conflicted, the other side's line + merged cleanly. The NEXT was `checkout --ours`, which dropped that + line. Now the file is `project`, and the NEXT is this doctor again, + which leaves the merge as git left it.""" + collision_layout(project) + compliant_again(standard, project) + + def ours(r: Path) -> None: + insert_after_first_line(r / NAMING_COPY, PROJECT_LINE) + append_to(r / NAMING_COPY, PROJECT_TAIL) + + status = merge_two_edits( + project, ours, lambda r: append_to(r / NAMING_COPY, SHAPE_TAIL), + NAMING_COPY) + assert f"UU {NAMING_COPY}" in status, status + own = project / NAMING_COPY + before = own.read_bytes() + assert PROJECT_LINE in before + row = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert row["detail"]["restore"] == {NAMING_COPY: "project"}, row + assert "checkout" not in row["next"], row["next"] + assert Path(pasted(row["next"])[1]).name == DOCTOR, row["next"] + run = subprocess.run(pasted(row["next"]), capture_output=True, + text=True, check=False, stdin=subprocess.DEVNULL, + env={**os.environ, **GIT_IDENTITY}) + assert run.returncode == 1, run.stdout + run.stderr + assert own.read_bytes() == before + assert f"UU {NAMING_COPY}" in git("status", "--porcelain=v1", + cwd=project).stdout + + +def test_a_conflicted_shape_pin_takes_only_the_copies_its_side_has_rows_for( + standard, project): + """Codex r4250578619 in the shape pin's own merge (#267): the pin and + the source's own `contracts/repository-naming.yaml` conflicted + together, in the collision layout. The NEXT took that file whole with + the pin, because this standard's copy lists name its path. + + Now the side taken is read for its rows, which name the copy under + `shape/` and not the source's file, so the pin is named alone, by both + rows, and the source's file keeps both sides for its owner. + """ + collision_layout(project) + + def side(digit: str, tail: bytes): + def edit(r: Path) -> None: + break_the_pin(r, pin_with(r'^commit: .*$', + f'commit: "{digit * 40}"')) + append_to(r / NAMING_COPY, tail) + return edit + + status = merge_two_edits(project, side("1", PROJECT_TAIL), + side("2", SHAPE_TAIL), SHAPE_PIN, NAMING_COPY) + assert f"UU {SHAPE_PIN}" in status and f"UU {NAMING_COPY}" in status, \ + status + own = project / NAMING_COPY + before = own.read_bytes() + kinds, currency = kinds_and_currency(standard, project) + assert currency["detail"]["restore"] == "unmerged", currency["detail"] + assert pasted(currency["next"])[3:] == [ + "checkout", "--ours", "--", SHAPE_PIN], currency["next"] + assert kinds["next"] == currency["next"], (kinds, currency) + assert kinds["detail"]["restore"] == {NAMING_COPY: "project", + SHAPE_PIN: "unmerged"}, kinds + run_pasted(currency["next"]) + assert own.read_bytes() == before + again = rows_of(doctor(standard, project, "--json"))["shape-currency"] + assert again["detail"].get("error") not in READ_PIN_CODES, again + + +def test_a_committed_hand_merge_of_a_copy_is_shown_not_rolled_back( + standard, project): + """P12 (codex r4250578605, the committed state). A hand merge of the + copy, as `update-shape.py`'s `both` exit asks for, committed with a + slip in it. The NEXT was `checkout ~1`, which dropped the + merge from the tree. + + Now the NEXT is the `diff` from the newest copy that reads, which shows + the merge and the slip and changes nothing. Mended and committed, the + row reads the copy again, with the merge in it. + """ + copy = project / NAMING_COPY + insert_after_first_line(copy, PROJECT_LINE) + append_to(copy, SLIP) + git("add", "--", NAMING_COPY, cwd=project) + breaking = commit_in(project, "the hand merge, with a slip") + commit_in(project, "an unrelated commit after it", "--allow-empty") + row = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert row["detail"]["restore"] == {NAMING_COPY: "committed"}, row + assert_no_registration(row) + assert pasted(row["next"])[3:] == ["diff", f"{breaking}~1", "--", + NAMING_COPY], row["next"] + note = row["next"].split(" #", 1)[1] + assert breaking[:12] in note and "nothing is rolled back" in note, note + shown = run_read_only(standard, project, row) + assert PROJECT_LINE.strip() in shown and SLIP.strip() in shown, shown + copy.write_bytes(copy.read_bytes()[:-len(SLIP)]) + git("add", "--", NAMING_COPY, cwd=project) + commit_in(project, "the slip mended") + again = rows_of(doctor(standard, project, "--json"))["manifest-kinds"] + assert again["status"] == "ok", again + assert PROJECT_LINE.decode() in git("show", f"HEAD:{NAMING_COPY}", + cwd=project).stdout + + +def neutral_pin_body(digit: str) -> bytes: + return (b"pin_role: neutral-product\nproduct: openGlass\n" + b'commit: "' + digit.encode() * 40 + b'"\n') + + +def neutral_pin_re_pinned_by_hand(root: Path) -> tuple[str, list]: + # P13: re-pinned by hand to another commit, with a slip, uncommitted. + # `checkout HEAD` discarded the re-pin. + (root / NEUTRAL_PIN).write_bytes(neutral_pin_body("2") + SLIP) + return "uncommitted", ["diff", "HEAD", "--", NEUTRAL_PIN] + + +def neutral_pin_re_pin_committed(root: Path) -> tuple[str, list]: + # The same re-pin committed. `checkout ~1` rolled it back. + (root / NEUTRAL_PIN).write_bytes(neutral_pin_body("2") + SLIP) + git("add", "--", NEUTRAL_PIN, cwd=root) + breaking = commit_in(root, "openGlass re-pinned by hand, with a slip") + commit_in(root, "an unrelated commit after it", "--allow-empty") + return "committed", ["diff", f"{breaking}~1", "--", NEUTRAL_PIN] + + +def neutral_pin_re_pins_conflicted(root: Path) -> tuple[str, list]: + # Two people re-pin it, each to their own commit. `checkout --ours` + # took one of them whole. + status = merge_two_edits( + root, + lambda r: (r / NEUTRAL_PIN).write_bytes(neutral_pin_body("2")), + lambda r: (r / NEUTRAL_PIN).write_bytes(neutral_pin_body("3")), + NEUTRAL_PIN) + assert f"UU {NEUTRAL_PIN}" in status, status + return "unmerged", ["diff", "--", NEUTRAL_PIN] + + +@pytest.mark.parametrize("breaks", [ + pytest.param(neutral_pin_re_pinned_by_hand, + id="re-pinned-by-hand-uncommitted"), + pytest.param(neutral_pin_re_pin_committed, id="a-hand-re-pin-committed"), + pytest.param(neutral_pin_re_pins_conflicted, + id="two-hand-re-pins-conflicted"), +]) +def test_a_neutral_product_pin_is_shown_never_put_back(standard, project, + breaks): + """P13. No tool writes a neutral product pin after the scaffold, and the + manifest template says to add one by hand, so every change to one is a + person's. In each git state the NEXT is a read-only `diff`, run as + printed it changes nothing, and the person's re-pin is still there. + + In process, for the reason `test_a_pin_is_what_the_root_declares_not_ + what_it_is_called` gives: a declared neutral pin sends the `pins` row + to `gh`. + """ + declare_a_neutral_pin(project) + restore, expected = breaks(project) + before = (project / NEUTRAL_PIN).read_bytes() + module = doctor_module(standard) + row = module.check_manifest_kinds(module.Context(project)).as_dict() + assert row["detail"]["unreadable"] == [NEUTRAL_PIN], row["detail"] + assert row["detail"]["restore"] == {NEUTRAL_PIN: restore}, row + assert_no_registration(row) + assert pasted(row["next"])[3:] == expected, row["next"] + assert "No tool of this standard writes a neutral product pin" in \ + row["next"].split(" #", 1)[1], row["next"] + run_read_only(standard, project, row) + assert (project / NEUTRAL_PIN).read_bytes() == before + assert b"2" * 40 in before + + # --- placement: code in spec, spec in code (#99) ---------------------------- # # Brett Heap, 2026-09-10: "we have to look for code in spec and spec in code". From 65def406f791b469937623b02b4856742ccb4a6e Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 12 Oct 2026 22:36:52 +0000 Subject: [PATCH 11/11] shape-doctor.py: a neutral product pin's reason no longer ends in its own "so", and `pinned_copies` no longer catches `TypeError`, so the pin's `unmerged` and `committed` notes read as one sentence and a caller out of step with `check_pin` fails loudly here as it does in `pin_at` (#291) Two nits from the re-verify of a5563c3 (rv358c, N1 and N2). N1. `BY_HAND_WHY["pin"]` ended "after the scaffold, so every change to one was made by a person", and `pin_restore` appends ", so neither side is taken whole" to the `unmerged` note and ", so nothing is rolled back" to the `committed` note, so both printed two `so` clauses in one sentence. The reason now joins its two facts with "and" ("..., and every change to one since was made by a person"), which leaves each note one conclusion; the copy's reason, which never ended in "so", and the `uncommitted` note are unchanged. The P13 test now also asserts that no sentence of the note holds two ", so " clauses; it opens on the same words it asserted before. N2. #343 (437a360) stopped `pin_at` catching `TypeError`: `check_pin` now refuses a `files:` that is not a list, or a row that is not a mapping, as `update-pin-row`, so nothing in a pin's bytes raises one, and one means a caller out of step with `check_pin`. `pinned_copies` calls `read_pin`, which runs the same `check_pin`, so it drops `TypeError` from its `except` by the same reasoning. A pin that does not read still makes the empty set, and nothing is a copy for that run. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 4 ++-- tests/test_shape_doctor.py | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 3aa257f4..51d67b80 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -2504,7 +2504,7 @@ def pinned_copies(ctx: Context) -> set: us = ctx.update_shape() try: _pin, rows = us.read_pin(ctx.root) - except (Refusal, us.YamlError, OSError, ValueError, TypeError): + except (Refusal, us.YamlError, OSError, ValueError): return set() return {row.path for row in rows} @@ -2533,7 +2533,7 @@ def edited_by_hand(ctx: Context, rel: str) -> bool: "hand update-shape.py asks for where a copy changed on both " "sides"), "pin": ("No tool of this standard writes a neutral product pin after the " - "scaffold, so every change to one was made by a person"), + "scaffold, and every change to one since was made by a person"), } diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index a6706f66..2001b1ec 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -5467,6 +5467,11 @@ def test_a_neutral_product_pin_is_shown_never_put_back(standard, project, assert pasted(row["next"])[3:] == expected, row["next"] assert "No tool of this standard writes a neutral product pin" in \ row["next"].split(" #", 1)[1], row["next"] + # One `so` to a sentence (review of PR #358): the reason ended in its own + # ", so every change to one was made by a person", and the note's ", so + # neither side is taken whole" or ", so nothing is rolled back" followed. + assert not any(sentence.count(", so ") > 1 for sentence in + row["next"].split(" #", 1)[1].split(". ")), row["next"] run_read_only(standard, project, row) assert (project / NEUTRAL_PIN).read_bytes() == before assert b"2" * 40 in before