fix(health): fsck survives approved DELETE and GOAL proposals

CHANGELOG.md

@@ -155,6 +155,19 @@ All notable changes to vouch are documented here. Format follows
   artifact the caller could not already retrieve, and it touches no write path.
 
 ### Fixed
+- **`vouch fsck` no longer crashes on approved DELETE or GOAL proposals**
+  (#682; revisits #538/#683): `_check_decided_proposals` indexed a
+  `presence` dict by every approved proposal's own kind, but only the
+  four create kinds (claim/page/entity/relation) were keys — so any KB
+  that had approved a delete or a goal crashed `fsck()` with an uncaught
+  `KeyError`. DELETE proposals are checked in a first pass against
+  `target_kind` (reporting `decided_delete_invalid_target_kind` /
+  `decided_delete_artifact_present` as appropriate), and ids they
+  legitimately removed are excluded from the second pass so the original
+  creating proposal does not false-positive as `decided_missing_artifact`.
+  GOAL is a create kind and is checked against `list_goals()`. An
+  exhaustiveness test ties the artifact-kind set to `ProposalKind` so a
+  seventh member fails the suite instead of users.
 - **`extract` no longer fractures file paths/URLs into auto-approved
   garbage claims** (#702): the sentence segmenter only skipped a `.` as a
   boundary when it was flanked by digits on both sides (decimals/versions

src/vouch/health.py

@@ -462,6 +462,21 @@ def _check_claim_graph_refs(
                 )
 
 
+# Artifact kinds whose approve path writes a durable file. DELETE is the
+# inverse (approve removes the file) and is handled in a dedicated pass.
+# Kept exhaustive over ``ProposalKind`` minus DELETE so a seventh kind
+# fails the suite instead of crashing ``vouch fsck`` for users (#682/#683).
+_ARTIFACT_PROPOSAL_KINDS: frozenset[ProposalKind] = frozenset(
+    {
+        ProposalKind.CLAIM,
+        ProposalKind.PAGE,
+        ProposalKind.ENTITY,
+        ProposalKind.RELATION,
+        ProposalKind.GOAL,
+    }
+)
+
+
 def _check_decided_proposals(
     store: KBStore,
     claims: dict[str, Claim],
@@ -473,16 +488,79 @@ def _check_decided_proposals(
 
     A crash between `put_<kind>()` and `move_proposal_to_decided()` would
     leave a `decided/` entry without a matching artifact (or vice versa);
-    surface the artifact-missing case so an operator can investigate.
+    surface the artifact-missing case so an operator can investigate. A
+    ``DELETE`` proposal is the inverse: approving it removes the artifact,
+    so its target must be *absent*, checked against ``target_kind`` (the
+    kind of what was deleted) rather than ``pr.kind`` (always ``DELETE``).
     """
     relations = {r.id for r in store.list_relations()}
     presence: dict[ProposalKind, set[str]] = {
         ProposalKind.CLAIM: set(claims),
         ProposalKind.PAGE: set(pages),
         ProposalKind.ENTITY: set(entities),
         ProposalKind.RELATION: relations,
+        ProposalKind.GOAL: {g.id for g in store.list_goals()},
     }
-    for pr in store.list_proposals(ProposalStatus.APPROVED):
+    # Exhaustiveness over ProposalKind is enforced by
+    # test_artifact_proposal_kinds_cover_enum — a new member must land in
+    # _ARTIFACT_PROPOSAL_KINDS (or DELETE) before it can KeyError fsck.
+
+    approved = list(store.list_proposals(ProposalStatus.APPROVED))
+
+    # First pass: an approved DELETE proposal's target is expected to be
+    # absent. Collect what it legitimately removed so the second pass
+    # (which checks that a create/edit proposal's artifact still exists)
+    # doesn't flag the artifact its own delete proposal correctly removed.
+    deleted: dict[ProposalKind, set[str]] = {k: set() for k in presence}
+    for pr in approved:
+        if pr.kind is not ProposalKind.DELETE:
+            continue
+        artifact_id = pr.payload.get("id") if isinstance(pr.payload, dict) else None
+        if not artifact_id:
+            findings.append(
+                Finding(
+                    "error",
+                    "decided_no_artifact_id",
+                    f"approved proposal {pr.id} has no payload id",
+                    [pr.id],
+                )
+            )
+            continue
+        target_kind_str = (
+            pr.payload.get("target_kind") if isinstance(pr.payload, dict) else None
+        )
+        try:
+            target_kind = ProposalKind(target_kind_str) if target_kind_str else None
+        except ValueError:
+            target_kind = None
+        if target_kind is None or target_kind not in presence:
+            findings.append(
+                Finding(
+                    "error",
+                    "decided_delete_invalid_target_kind",
+                    f"approved delete proposal {pr.id} has an invalid or "
+                    f"missing target_kind {target_kind_str!r}",
+                    [pr.id],
+                )
+            )
+            continue
+        if artifact_id in presence[target_kind]:
+            findings.append(
+                Finding(
+                    "error",
+                    "decided_delete_artifact_present",
+                    f"approved delete proposal {pr.id} targeted "
+                    f"{target_kind.value} {artifact_id}, but the artifact "
+                    f"still exists on disk",
+                    [pr.id, artifact_id],
+                )
+            )
+        else:
+            deleted[target_kind].add(artifact_id)
+
+    for pr in approved:
+        if pr.kind is ProposalKind.DELETE:
+            continue
         artifact_id = pr.payload.get("id") if isinstance(pr.payload, dict) else None
         if not artifact_id:
             findings.append(
@@ -494,6 +572,8 @@ def _check_decided_proposals(
                 )
             )
             continue
+        if artifact_id in deleted[pr.kind]:
+            continue  # removed by a later, separately-verified delete proposal
         if artifact_id not in presence[pr.kind]:
             findings.append(
                 Finding(

tests/test_health.py

@@ -8,6 +8,7 @@
 
 from vouch import health, index_db
 from vouch.models import Claim, ClaimStatus, Proposal, ProposalKind, ProposalStatus
+from vouch.proposals import approve, propose_claim, propose_delete, propose_goal
 from vouch.storage import KBStore, _yaml_dump
 
 
@@ -315,6 +316,108 @@ def test_fsck_decided_missing_artifact(store: KBStore) -> None:
     assert "decided_missing_artifact" in codes
 
 
+def test_artifact_proposal_kinds_cover_enum() -> None:
+    """Every ProposalKind is either DELETE or an artifact kind fsck knows.
+
+    Hand-maintained presence maps over a growing enum are what made #682
+    (DELETE) and the #683 follow-up (GOAL) crash vouch fsck. A new member
+    must extend _ARTIFACT_PROPOSAL_KINDS (or be DELETE) or this fails.
+    """
+    assert health._ARTIFACT_PROPOSAL_KINDS | {ProposalKind.DELETE} == set(
+        ProposalKind
+    )
+    assert ProposalKind.DELETE not in health._ARTIFACT_PROPOSAL_KINDS
+
+
+def test_fsck_survives_approved_delete_proposal(store: KBStore) -> None:
+    """An approved delete must not crash fsck, and must not false-positive
+    the original create proposal as decided_missing_artifact."""
+    src = store.put_source(b"evidence")
+    pr = propose_claim(store, text="a fact", evidence=[src.id], proposed_by="agent-a")
+    claim = approve(store, pr.id, approved_by="human-b")
+
+    del_pr = propose_delete(
+        store, target_kind="claim", target_id=claim.id, proposed_by="agent-a",
+    )
+    approve(store, del_pr.id, approved_by="human-c")
+
+    report = health.fsck(store)
+    assert report.findings == []
+
+
+def test_fsck_survives_approved_goal_proposal(store: KBStore) -> None:
+    """Approved GOAL proposals are live create kinds — must not KeyError."""
+    pr = propose_goal(store, title="ship the thing", proposed_by="agent")
+    approve(store, pr.id, approved_by="reviewer")
+
+    report = health.fsck(store)
+    codes = {f.code for f in report.findings}
+    assert "decided_missing_artifact" not in codes
+    assert report.ok is True
+
+
+def test_fsck_flags_delete_whose_artifact_still_exists(store: KBStore) -> None:
+    """Delete approved but target still on disk — the delete never took."""
+    src = store.put_source(b"evidence")
+    claim = Claim(id="still-here", text="t", evidence=[src.id])
+    store.put_claim(claim)
+    store.put_proposal(Proposal(
+        id="del-1",
+        kind=ProposalKind.DELETE,
+        proposed_by="agent",
+        payload={"target_kind": "claim", "id": "still-here", "snapshot": {}},
+        status=ProposalStatus.APPROVED,
+    ))
+
+    report = health.fsck(store)
+    codes = {f.code for f in report.findings}
+    assert "decided_delete_artifact_present" in codes
+
+
+def test_fsck_flags_delete_with_invalid_target_kind(store: KBStore) -> None:
+    store.put_proposal(Proposal(
+        id="del-2",
+        kind=ProposalKind.DELETE,
+        proposed_by="agent",
+        payload={"target_kind": "not-a-real-kind", "id": "whatever", "snapshot": {}},
+        status=ProposalStatus.APPROVED,
+    ))
+
+    report = health.fsck(store)
+    codes = {f.code for f in report.findings}
+    assert "decided_delete_invalid_target_kind" in codes
+
+
+def test_fsck_flags_delete_proposal_with_no_artifact_id(store: KBStore) -> None:
+    """Malformed DELETE with no payload id is reported, not silently skipped."""
+    store.put_proposal(Proposal(
+        id="del-3",
+        kind=ProposalKind.DELETE,
+        proposed_by="agent",
+        payload={"target_kind": "claim", "snapshot": {}},
+        status=ProposalStatus.APPROVED,
+    ))
+
+    report = health.fsck(store)
+    codes = {f.code for f in report.findings}
+    assert "decided_no_artifact_id" in codes
+
+
+def test_fsck_flags_create_proposal_with_no_artifact_id(store: KBStore) -> None:
+    """Second-pass create/edit proposals also report a missing payload id."""
+    store.put_proposal(Proposal(
+        id="claim-no-id",
+        kind=ProposalKind.CLAIM,
+        proposed_by="agent",
+        payload={"text": "t", "evidence": ["e1"]},
+        status=ProposalStatus.APPROVED,
+    ))
+
+    report = health.fsck(store)
+    codes = {f.code for f in report.findings}
+    assert "decided_no_artifact_id" in codes
+
+
 def test_fsck_index_orphan_row(store: KBStore) -> None:
     """An FTS5 row with no on-disk claim is reported as an index orphan."""
     src = store.put_source(b"e")