diff --git a/src/vouch/storage.py b/src/vouch/storage.py index 653ea806..bff01be7 100644 --- a/src/vouch/storage.py +++ b/src/vouch/storage.py @@ -497,7 +497,14 @@ def update_claim(self, claim: Claim) -> Claim: # --- pages ------------------------------------------------------------- - def put_page(self, page: Page) -> Page: + def _validate_page_refs(self, page: Page) -> None: + """Reject dangling graph references on a Page before it lands. + + Mirrors `_validate_claim_refs` and the write-time gate on + `put_relation`: page edits (vault-sync approve path via + `update_page`) must not reintroduce the gap that `put_page` already + closed for new pages. + """ for cid in page.claims: if not self._claim_path(cid).exists(): raise ValueError(f"page {page.id} references unknown claim {cid}") @@ -507,6 +514,9 @@ def put_page(self, page: Page) -> Page: for sid in page.sources: if not (self._source_dir(sid) / "meta.yaml").exists(): raise ValueError(f"page {page.id} references unknown source {sid}") + + def put_page(self, page: Page) -> Page: + self._validate_page_refs(page) try: # Explicit UTF-8: page bodies are user / agent prose and routinely # contain non-ASCII (em-dashes, smart quotes, unicode in claims). @@ -536,6 +546,14 @@ def update_page(self, page: Page) -> Page: """ if not self._page_path(page.id).exists(): raise ArtifactNotFoundError(f"page {page.id}") + # Re-validate the in-memory Page before persisting so model + # invariants hold even when a caller mutated fields in place after + # get_page(). Field validators only run at construction time. + Page.model_validate(page.model_dump(mode="json")) + # Re-check graph references too: an in-place mutation (or a claim + # deleted between propose and approve on the vault-edit path) can + # introduce dangling links that the model validator can't catch. + self._validate_page_refs(page) self._page_path(page.id).write_text( _serialize_page(page), encoding="utf-8" ) diff --git a/tests/test_storage.py b/tests/test_storage.py index 03ce0fba..1d17536e 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -548,6 +548,37 @@ def test_check_approvable_clean_for_well_formed_proposal( assert check_approvable(store, pr.id, approved_by="reviewer") is None +def test_update_page_rejects_dangling_claim_ref(store: KBStore) -> None: + """update_page must mirror put_page's ref guard — vault edits land here.""" + src = store.put_source(b"e") + store.put_claim(Claim(id="c1", text="t", evidence=[src.id])) + store.put_page( + Page(id="p1", title="T", body="body", type=PageType.CONCEPT, claims=["c1"]), + ) + bad = Page( + id="p1", title="T", body="updated", type=PageType.CONCEPT, claims=["ghost"], + ) + with pytest.raises(ValueError, match="unknown claim"): + store.update_page(bad) + + +def test_approve_page_update_rejects_stale_claim_ref(store: KBStore) -> None: + """A claim deleted between propose and approve must not become a dangling + page reference — the vault-edit path routes through update_page.""" + src = store.put_source(b"e") + store.put_claim(Claim(id="c1", text="t", evidence=[src.id])) + store.put_page( + Page(id="p1", title="T", body="body", type=PageType.CONCEPT, claims=["c1"]), + ) + pr = propose_page( + store, title="T", body="updated", claim_ids=["c1"], + proposed_by="vault-sync", slug_hint="p1", + ) + (store.kb_dir / "claims" / "c1.yaml").unlink() + with pytest.raises(ValueError, match="unknown claim"): + approve(store, pr.id, approved_by="reviewer") + + # --- evidence -------------------------------------------------------------