update-shape.py: a pinned copy, contracts/shape-pin.yaml or the manifest kept as a symlink, or under one, is refused as update-rewrite-symlink before anything is written, and check calls such a copy copy-is-link, so apply no longer writes the standard's bytes at the link's end and reports a commit that does not hold them (#286)

shape-doctor.py

@@ -3024,6 +3024,13 @@ def check_shape_currency(ctx: Context) -> Row:
     without this file being told.
     """
     us = ctx.update_shape()
+    # A PIN OR MANIFEST KEPT AS A LINK IS ASKED ABOUT FIRST, before anything
+    # reads the pin or opens this checkout as the upstream (#286): see
+    # `linked_record_row`.
+    linked = linked_record_row(ctx, us)
+    if linked is not None:
+        return linked
+
     args = argparse.Namespace(root=str(ctx.root), upstream=str(ctx.shape),
                               at=None)
     try:
@@ -3131,6 +3138,52 @@ def check_shape_currency(ctx: Context) -> Row:
         upstream.close()
 
 
+def linked_record_row(ctx: Context, us) -> Row | None:
+    """The `shape currency` row for a root that keeps its pin or its
+    manifest as a symlink, or under one, or None when it keeps neither
+    (#286).
+
+    A STATEMENT ABOUT THEIR REPOSITORY, NOT A FAILURE TO ASK. `update-shape.py
+    apply` rewrites `contracts/shape-pin.yaml` and the manifest where they
+    stand, so it refuses a root that keeps either as a link
+    (`update-rewrite-symlink`): the re-pin would land at the link's end,
+    outside the root if that is where it points, and git, which holds the
+    link, would commit none of it. That is what the root's own tree holds,
+    and no `git fetch` in the standard changes it. Filed with `prepare`'s
+    other refusals, it read `CANNOT ANSWER`, exit 3, with a fetch for its
+    NEXT. So this row carries no `environment`, which is the only thing that
+    makes `verdict_for` answer CANNOT ANSWER; the ladder goes on, and where
+    every gate reads through the link and is green, the verdict is `INVALID
+    (shape-currency)`.
+
+    ASKED FIRST, AND BY THE SAME FUNCTION. `linked_record` is the question
+    `read_pin` and `prepare` ask before they refuse, for the KIND of root
+    `prepare` would see, so this row and `update-shape.py` cannot disagree
+    about which record is linked. Asked before anything reads the pin, so a
+    linked pin is answered as a link, whatever else reads it next.
+
+    THE REFUSAL, QUOTED AS `update-shape.py` PRINTS IT, and its remedy for
+    the NEXT: replace the link with what it points at, commit that, run the
+    doctor again. Said, not spelled as a command: what the link points at
+    may be shared with other trees, and removing a link -- a junction above
+    all -- is a person's call.
+    """
+    linked = us.linked_record(ctx.root, us.root_kind(ctx.root))
+    if linked is None:
+        return None
+    record, (name, points) = linked
+    refusal = us.rewrite_link_refusal([linked])
+    return Row("shape-currency", LABEL_SHAPE_CURRENCY, FINDING,
+               "update-shape.py cannot re-pin this root: "
+               f"REFUSED {refusal.code}: {refusal.detail}",
+               f"replace {name} with the file or directory it points at, "
+               "or with what it replaced where it points at nothing, commit "
+               "that, and run this doctor again -- update-shape.py never "
+               f"rewrites {record} through a link",
+               {"error": refusal.code, "path": record, "link": name,
+                "points": points})
+
+
 def head_of(path: Path) -> str | None:
     try:
         return git_out(["rev-parse", "HEAD"], cwd=path).lower()
@@ -6249,18 +6302,24 @@ def drifted_file_reasons(counts: dict) -> list:
 
     TWO GROUPS AND THEY SAY DIFFERENT THINGS. `locally-modified` and `both`
     are what MADE this repository drifted -- a copy of the standard carries
-    an edit -- so they are named first. `upstream-removed`, `unmapped` and
-    `copy-missing` are states in which the standard and the copy no longer
-    map onto each other at all, which is worth quoting beside the edits
-    because it is the same person's next question.
+    an edit -- so they are named first. `upstream-removed`, `unmapped`,
+    `copy-missing` and `copy-is-link` are states in which the standard and
+    the copy no longer map onto each other at all, which is worth quoting
+    beside the edits because it is the same person's next question.
+    `copy-is-link` (#286) is a pinned copy the tree keeps as a symlink, or
+    under one: git holds the link, not the bytes the pin's digest describes,
+    and `update-shape.py apply` refuses it. It is a conflict there, so the
+    row is `drifted`; unnamed here, that drift had no reason to print, and
+    the verdict fell through to the catch-all `INVALID (shape-currency)`.
 
     A state is named only when its count is non-zero: a verdict line reading
     `0 unmapped` would be a parenthetical contradicting the words above it.
 
     Split out of `verdict_for` for #134.
     """
     edited = ("locally-modified", "both")
-    unmapped = ("upstream-removed", "unmapped", "copy-missing")
+    unmapped = ("upstream-removed", "unmapped", "copy-missing",
+                "copy-is-link")
     return [f"{counts[state]} {state}" for state in edited + unmapped
             if counts.get(state)]
 

tests/test_shape_doctor.py

@@ -2277,6 +2277,122 @@ def test_a_deleted_pinned_agent_file_is_not_an_add(standard, project):
     assert "checkout --" in row["next"], row["next"]
 
 
+@WINDOWS_SKIP
+def test_a_pinned_copy_kept_as_a_link_is_drifted_by_name(standard, project,
+                                                         tmp_path):
+    """`AGENTS-shape.md` moved outside the root, a committed symlink in its
+    place, with the bytes unchanged (#286).
+
+    `update-shape.py` reads that row as `copy-is-link`, a conflict its
+    `apply` refuses: git holds the link, not the bytes the pin's digest
+    describes. The doctor names it the way it names `copy-missing` --
+    `DRIFTED (1 copy-is-link)` -- rather than reaching a verdict whose
+    parenthesis says nothing about the file. Main read through the link and
+    called this root COMPLIANT.
+    """
+    outside = tmp_path / "outside"
+    outside.mkdir()
+    shutil.move(str(project / "AGENTS-shape.md"),
+                str(outside / "AGENTS-shape.md"))
+    (project / "AGENTS-shape.md").symlink_to(outside / "AGENTS-shape.md")
+    commit_all(project, "keep AGENTS-shape.md as a link")
+
+    result = doctor(standard, project, "--json")
+    assert result.returncode == 1, result.stdout + result.stderr
+    payload = json.loads(result.stdout)
+    assert payload["verdict"] == "DRIFTED (1 copy-is-link)", payload["verdict"]
+    row = rows_of(result)["shape-currency"]
+    assert row["status"] == "FINDING", row
+    assert row["detail"]["counts"] == {"copy-is-link": 1,
+                                       "unchanged": 10}, row["detail"]
+    assert row["detail"]["conflicts"] == ["AGENTS-shape.md"], row["detail"]
+    assert "AGENTS-shape.md" in row["reason"], row["reason"]
+
+
+def keep_as_committed_link(root: Path, linked: str, tmp_path: Path) -> Path:
+    """`linked` moved beside the root, a symlink to it left in its place and
+    committed, so the link is what git holds. Returns where it points."""
+    end = tmp_path / "outside" / Path(linked).name
+    end.parent.mkdir(exist_ok=True)
+    shutil.move(str(root / linked), str(end))
+    (root / linked).symlink_to(end, target_is_directory=end.is_dir())
+    commit_all(root, f"keep {linked} as a link")
+    return end
+
+
+def assert_a_linked_record_is_their_finding(result, linked: str,
+                                            record: str) -> None:
+    """The `shape-currency` row for a root that keeps `record`, or the
+    directory above it, as a committed link at `linked` (#286).
+
+    A STATEMENT ABOUT THEIR REPOSITORY. `update-shape.py` refuses to
+    re-pin through the link, and the doctor had filed that refusal with
+    the ones about THIS checkout of the standard: `CANNOT ANSWER`, exit 3,
+    with a `git fetch --all` that changes nothing. It is a finding now:
+    `INVALID (shape-currency)`, exit 1, the refusal quoted, and the NEXT is
+    the refusal's own remedy.
+    """
+    payload = json.loads(result.stdout)
+    assert (payload["verdict"], payload["exit"]) == (
+        "INVALID (shape-currency)", 1), payload["verdict"]
+    assert result.returncode == 1, result.stdout + result.stderr
+    assert "cannot-answer" not in result.stderr, result.stderr
+    row = rows_of(result)["shape-currency"]
+    assert row["status"] == "FINDING", row
+    assert "environment" not in row["detail"], row["detail"]
+    assert row["detail"]["error"] == "update-rewrite-symlink", row["detail"]
+    assert (row["detail"]["path"], row["detail"]["link"]) == (
+        record, linked), row["detail"]
+    assert "REFUSED update-rewrite-symlink: `apply` would rewrite " \
+        f"{record}, but {linked} in this root is a symlink" in row["reason"], \
+        row["reason"]
+    assert "fetch" not in row["next"], row["next"]
+    assert row["next"].startswith(f"replace {linked} with the file or "
+                                  "directory it points at"), row["next"]
+    assert "run this doctor again" in row["next"], row["next"]
+    # Every other row reads THROUGH the link and finds nothing wrong: the
+    # verdict is this row's alone.
+    red = [key for key, other in rows_of(result).items()
+           if other["status"] == "FINDING"]
+    assert red == ["shape-currency"], red
+
+
+@WINDOWS_SKIP
+@pytest.mark.parametrize("linked, record", [
+    pytest.param("contracts/shape-pin.yaml", "contracts/shape-pin.yaml",
+                 id="pin-at-a-link"),
+    pytest.param("contracts", "contracts/shape-pin.yaml",
+                 id="contracts-directory-link"),
+    pytest.param("project.yaml", "project.yaml", id="manifest-at-a-link"),
+])
+def test_a_pin_or_manifest_kept_as_a_link_is_a_finding_not_cannot_answer(
+        standard, project, tmp_path, linked, record):
+    """The pin, the `contracts` directory above it, or `project.yaml` kept
+    as a committed link, with the bytes unchanged and the pin naming this
+    standard's commit. Main read through the link and said COMPLIANT; #286
+    made `update-shape.py` refuse the root, and the doctor then answered
+    CANNOT ANSWER. Neither is the truth: `apply` cannot re-pin this root
+    until the link is replaced, and that is the root's to fix."""
+    keep_as_committed_link(project, linked, tmp_path)
+    result = doctor(standard, project, "--json")
+    assert_a_linked_record_is_their_finding(result, linked, record)
+
+
+@WINDOWS_SKIP
+def test_a_family_manifest_kept_as_a_link_is_a_finding_not_cannot_answer(
+        standard, holder, tmp_path):
+    """A family holder's manifest is `family.yaml`, and `apply` rewrites it
+    as it rewrites an assembly root's `project.yaml`: the doctor asks after
+    the manifest of the KIND of root it was pointed at."""
+    root = tmp_path / FAMILY_NAME
+    shutil.copytree(holder["root"], root, symlinks=True)
+    keep_as_committed_link(root, "family.yaml", tmp_path)
+    result = doctor(standard, root, "--json")
+    assert json.loads(result.stdout)["kind"] == "family", result.stdout
+    assert_a_linked_record_is_their_finding(result, "family.yaml",
+                                            "family.yaml")
+
+
 #: How the `leg shape files` row spells a copy ON THIS HOST. `cp` is not a
 #: program on Windows, it is an alias of `Copy-Item` (Microsoft's own
 #: reference: "PowerShell includes the following aliases for `Copy-Item`:

tests/test_update_shape.py

@@ -179,7 +179,8 @@ def apply(root, upstream_and_project, *extra, env=None):
 
 #: Every verdict `check` can print, in the column it prints them in.
 STATES = ("unchanged", "upstream-changed", "locally-modified", "both",
-          "already-at-target", "upstream-removed", "unmapped", "copy-missing")
+          "already-at-target", "upstream-removed", "unmapped", "copy-missing",
+          "copy-is-link")
 
 
 def verdicts(stdout: str) -> dict:

tests/test_update_shape_add.py

@@ -281,7 +281,7 @@ def pin_rows(root) -> dict:
 #: mistaken for one.
 STATES = ("unchanged", "upstream-changed", "upstream-added",
           "locally-modified", "both", "already-at-target", "upstream-removed",
-          "unmapped", "copy-missing")
+          "unmapped", "copy-missing", "copy-is-link")
 
 
 def verdicts(stdout: str) -> dict:

tests/test_update_shape_add_symlink.py

@@ -383,10 +383,11 @@ def test_add_refuses_a_link_by_name_before_it_writes_anything(
         "the refusal says where the link points, quoted as a literal")
     assert "remove or move" in result.stderr, (
         "and what the operator does about it")
-    # The promise is `--add`'s, not the whole command's: a pinned copy, or
-    # the pin itself, kept as a link is still rewritten through it.
-    assert "`--add` never writes through a link" in result.stderr
-    assert "this command never writes through a link" not in result.stderr
+    # The promise is the whole command's again (#286): a pinned copy, the
+    # pin and the manifest kept as a link are refused too. It says SYMLINK:
+    # a hard-linked copy is still rewritten through its other name.
+    assert "this command never writes through a symlink" in result.stderr
+    assert "never writes through a link" not in result.stderr
     assert "update-validators-red" not in result.stderr
     assert "ROLLED BACK" not in result.stderr, (
         "nothing was written, so nothing is rolled back")