diff --git a/CHANGELOG.md b/CHANGELOG.md index 0704c6a4..e6ae8b98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,16 @@ All notable changes to vouch are documented here. Format follows per-prompt block, the session banner, `vouch status` and the opt-in question all say so rather than calling it "this repo's" knowledge. +### Fixed +- `vouch fsck` no longer crashes with a `KeyError` on a KB that has an + approved delete proposal. the decided-proposals check only knew the + create kinds, so the first approved delete broke fsck permanently. + approved deletes are now checked for the inverse drift — a target + artifact still on disk is reported as `decided_delete_artifact_present`, + a missing or unknown target kind as `decided_delete_invalid_target_kind` + — and an artifact removed by an approved delete no longer misreports + its create proposal as `decided_missing_artifact`. + ## [1.5.0] — 2026-07-20 ### Added diff --git a/src/vouch/health.py b/src/vouch/health.py index 6c8df7d6..ca3207b4 100644 --- a/src/vouch/health.py +++ b/src/vouch/health.py @@ -436,6 +436,10 @@ def _check_decided_proposals( A crash between `put_()` 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. + approved delete proposals promise the inverse — the target artifact is + gone — so for those the still-present case is surfaced instead, and an + artifact a later approved delete removed is not missing: the delete + explains its absence. """ relations = {r.id for r in store.list_relations()} presence: dict[ProposalKind, set[str]] = { @@ -444,7 +448,14 @@ def _check_decided_proposals( ProposalKind.ENTITY: set(entities), ProposalKind.RELATION: relations, } - for pr in store.list_proposals(ProposalStatus.APPROVED): + presence_by_target_kind = {kind.value: ids for kind, ids in presence.items()} + approved = store.list_proposals(ProposalStatus.APPROVED) + deleted_by_gate = { + (pr.payload.get("target_kind"), pr.payload.get("id")) + for pr in approved + if pr.kind is ProposalKind.DELETE and isinstance(pr.payload, dict) + } + for pr in approved: artifact_id = pr.payload.get("id") if isinstance(pr.payload, dict) else None if not artifact_id: findings.append( @@ -456,7 +467,35 @@ def _check_decided_proposals( ) ) continue - if artifact_id not in presence[pr.kind]: + if pr.kind is ProposalKind.DELETE: + target_kind = pr.payload.get("target_kind", "") + target_ids = presence_by_target_kind.get(target_kind) + if target_ids is None: + findings.append( + Finding( + "error", + "decided_delete_invalid_target_kind", + f"approved delete proposal {pr.id} has a missing or " + f"unknown target_kind {target_kind!r}", + [pr.id, artifact_id], + ) + ) + elif artifact_id in target_ids: + findings.append( + Finding( + "error", + "decided_delete_artifact_present", + f"approved delete proposal {pr.id} promised " + f"{target_kind} {artifact_id} removed but the " + f"artifact is still on disk", + [pr.id, artifact_id], + ) + ) + continue + if ( + artifact_id not in presence[pr.kind] + and (pr.kind.value, artifact_id) not in deleted_by_gate + ): findings.append( Finding( "error", diff --git a/tests/test_health.py b/tests/test_health.py index 26b15a6b..9840280c 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -6,8 +6,15 @@ import pytest -from vouch import health, index_db -from vouch.models import Claim, ClaimStatus, Proposal, ProposalKind, ProposalStatus +from vouch import health, index_db, proposals +from vouch.models import ( + Claim, + ClaimStatus, + Entity, + Proposal, + ProposalKind, + ProposalStatus, +) from vouch.storage import KBStore, _yaml_dump @@ -296,6 +303,75 @@ def test_fsck_decided_missing_artifact(store: KBStore) -> None: assert "decided_missing_artifact" in codes +def _approve_entity_delete(store: KBStore) -> Entity: + """Create and then delete an entity through the real review gate.""" + created = proposals.propose_entity( + store, name="acme example", entity_type="company", proposed_by="agent", + ) + entity = proposals.approve(store, created.id, approved_by="reviewer") + assert isinstance(entity, Entity) + deletion = proposals.propose_delete( + store, target_kind="entity", target_id=entity.id, proposed_by="agent", + ) + proposals.approve(store, deletion.id, approved_by="reviewer") + return entity + + +def test_fsck_approved_delete_with_artifact_gone_is_clean(store: KBStore) -> None: + """An approved delete whose target is gone is the healthy case. + + Regression: the presence map only covered create kinds, so any KB with + an approved delete proposal crashed fsck with a KeyError. + """ + _approve_entity_delete(store) + + report = health.fsck(store) + codes = {f.code for f in report.findings} + assert "decided_missing_artifact" not in codes + assert "decided_delete_artifact_present" not in codes + assert report.ok is True + + +def test_fsck_flags_approved_delete_artifact_still_present(store: KBStore) -> None: + """An approved delete whose target survived on disk is the drift case — + a crash between the artifact removal and the decided-move leaves the + target in place; simulate it by recreating the entity after approval.""" + entity = _approve_entity_delete(store) + store.put_entity(entity) + + report = health.fsck(store) + codes = {f.code for f in report.findings} + assert "decided_delete_artifact_present" in codes + assert report.ok is False + + +def test_fsck_flags_approved_delete_with_invalid_target_kind( + store: KBStore, +) -> None: + """A decided delete with an unknown target_kind is drift, not clean. + + No current write path produces this (propose_delete validates the + kind), so hand-land the decided YAML the way an older writer or a + hand edit could have left it — fsck is the after-the-fact net. + """ + store.put_proposal(Proposal( + id="prop-del-bad", + kind=ProposalKind.DELETE, + proposed_by="agent", + payload={"target_kind": "wormhole", "id": "x", "snapshot": {}}, + status=ProposalStatus.APPROVED, + )) + src_path = store.kb_dir / "proposed" / "prop-del-bad.yaml" + dst_path = store.kb_dir / "decided" / "prop-del-bad.yaml" + dst_path.write_text(src_path.read_text()) + src_path.unlink() + + report = health.fsck(store) + codes = {f.code for f in report.findings} + assert "decided_delete_invalid_target_kind" in codes + assert report.ok is False + + 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")