fix: reject dangling Relation/Page references at every write path

[galuis116]

What changed

Every write path that lands a Relation or a Page now rejects dangling foreign-id references instead of silently persisting them. relation.source / relation.target must resolve to an existing claim, page, entity, or source; relation.evidence must resolve to a source or evidence; page.entities and page.sources must resolve (the page.claims check already existed). The gate is enforced at five layers so no entry point is left open:

• store.put_relation, store.put_relation_idempotent, store.put_page (src/vouch/storage.py) — the durable write path used by proposals.approve and lifecycle.supersede / contradict.
• proposals.propose_relation, proposals.propose_page (src/vouch/proposals.py) — surface the same checks at proposal time as a friendly ProposalError, mirroring the existing propose_claim evidence loop.
• bundle.import_check (src/vouch/bundle.py) — bundle import writes member bytes directly to disk, bypassing the storage layer, so a new _check_graph_integrity cross-artifact pass resolves every incoming relation / page / claim reference against the post-merge id set (destination KB plus the bundle's own members). import_apply already raises on the first issue, so it inherits the fix.
• sync.sync_check (src/vouch/sync.py) — runs the same cross-artifact pass against the destination KB.

Why

Fixes #123. relation.source/target, relation.evidence, page.entities, and page.sources could all reference ids that resolve to nothing. health.lint / doctor already reports the resulting dangling_relation as an error-severity finding (src/vouch/health.py:135-145), but no write path enforced it — so the corrupt state was reachable through propose → approve, the lifecycle ops, bundle.import_apply, and sync.sync_apply, and a manifest-consistent bundle could ship a fully dangling knowledge graph with import_check.ok == True. This is the same structural shape as #81 (uncited claims enforced only in propose_claim): the invariant lived in one observer but was enforced by zero writers. This PR makes the writers enforce what the observer already reports — the prevention counterpart to the after-the-fact vouch fsck detection in #112.

The endpoint surface is intentionally permissive about Source ids: sources are first-class graph nodes (they back claims and crystallize into pages), so a relation may point at a source id. put_relation_idempotent deliberately skips re-validation when the relation file already exists on disk, so a supersede / contradict retry doesn't spuriously fail if a linked claim was later archived or retracted.

What might break

No on-disk-layout, schema, kb.* method, bundle-format, or audit-log-shape change. Existing .vouch/ directories are unaffected — nothing moves and no field changes shape. The only behavioural change is that data the model + health.lint already say should never exist now raises at write time instead of landing silently:

• propose_relation / propose_page raise ProposalError on an unresolved reference (previously deferred to approve time, where it landed).
• put_relation / put_page raise ValueError on an unresolved reference.
• import_check / sync_check report a dangling reference: … issue and import_apply / sync_apply refuse, the same way they already refuse a hash mismatch (bug: import_check / import_apply never verify member sha256 against manifest — bundle integrity gate is bypassable #74) or an uncited claim (bug: Claim model has no min-evidence validator — uncited claims land via bundle import, put_claim, update_claim #81).

A KB that already contains a dangling relation on disk (written by the old code) is unaffected on read; it still surfaces as a health.lint finding. Legitimate self-contained bundles (shipping an entity alongside the relation that points at it) import cleanly because references resolve against the post-merge id set.

VEP

Not required. No change to the object model, kb.* method surface, on-disk layout, bundle format, or audit-log shape — this only tightens write-time validation to match an invariant the schema and health.lint already document.

Tests

• make check passes locally (lint + mypy + pytest) — ruff clean, mypy Success: no issues found in 31 source files, full pytest green on Windows / Python 3.13 except the pre-existing test_read_under_root_rejects_symlink_at_resolved_path which needs symlink-create privilege (unrelated to this change).
• New / changed behaviour has a test — 21 regressions across tests/test_storage.py (put_relation / put_relation_idempotent / put_page + propose_relation / propose_page, positive and negative), tests/test_bundle.py (dangling relation / page rejection, self-contained bundle accepted, refs resolved against destination KB), and tests/test_sync.py (sync_check reports + sync_apply refuses). tests/test_health.py::test_lint_dangling_relation was migrated to hand-write its dangling YAML, since put_relation no longer lets the old setup construct one.
• CHANGELOG.md updated under ## [Unreleased].

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 35bc7eb1-e115-4f45-9cdb-d592183826d0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[plind-junior]

Fix the conflict

[galuis116]

Fix the conflict

done

[plind-junior]

Review

Summary: Closes #123 by enforcing referential integrity at every write path — put_relation, put_relation_idempotent, put_page, propose_relation, propose_page, import_check, and sync_check. The shape mirrors the #81 fix for uncited claims; the invariant that health.lint already reported as dangling_relation is now prevented at write time across all five entry points described in the issue.

What works

• src/vouch/storage.py:314-336 — _node_exists and _evidence_ref_exists are tight filesystem checks (no model load, no index query), and _validate_relation_refs composes them cleanly. The helpers match the definitions used at proposal time (proposals._node_exists), so both layers reject on the same criteria.
• src/vouch/storage.py:523-531 — The idempotent path intentionally skips re-validation when the file already exists. The comment is explicit about why: a supersede/contradict retry must not fail spuriously if the linked claim was later retracted. tests/test_storage.py:742-757 covers this branch directly with the "unlink after write" pattern.
• src/vouch/bundle.py:46-77 — _existing_ids snapshots the destination KB before the merge, then the incoming bundle's ids are unioned in during the loop (bundle.py:106-108), so a self-contained bundle (entity shipped alongside the relation pointing at it) resolves cleanly. tests/test_bundle.py:572-604 verifies both the self-contained case and the destination-already-has-it case.
• src/vouch/bundle.py:165-169 — Swallowing parse exceptions in _check_graph_integrity is correct: _validate_content already recorded structural issues for the same file, and a second cryptic traceback would mask it.
• tests/test_health.py:646-664 — Migrating the test_lint_dangling_relation fixture to hand-write YAML directly is the right call. The comment ("simulate a relation that landed before put_relation enforced endpoint existence") makes the regression intent clear.
• src/vouch/proposals.py:270-290 — The module-level _node_exists helper re-implements the same four-getter probe as KBStore._node_exists. Both are correct; duplication is acceptable given proposals operate on KBStore via its public API rather than internal paths.

Suggestions

• [non-blocking] src/vouch/bundle.py:111-113 — failed_schema is built by splitting on ": " with maxsplit=2 and taking index [1]. The _validate_content message format is "schema validation failed: {path}: {e}", so split(": ", 2)[1] yields path only when the path itself contains no ": ". A path like relations/a: b.yaml would give a truncated key and the guard would miss it, leaving a confusing second parse error. Low-probability in practice (paths are slugified), but the safer form is i[len("schema validation failed: "):].split(": ", 1)[0] or just matching with i.removeprefix("schema validation failed: ").split(": ")[0].

• [non-blocking] src/vouch/bundle.py:95-108 — incoming_meta accumulates (path, kind, body) for every artifact including relation entries, but relation is not a key in ids. The if kind in ids guard on line 106 correctly gates the ids[kind].add(aid) call, so relations are appended to incoming_meta but never added to ids. This is intentional and the comment explains it, but the comment says "The incoming relation itself is still appended below" while the append actually happens unconditionally inside the same if kind_id is not None block. A one-line clarification — "relation entries skip the ids update but are still appended for ref-checking" — would make the logic easier to audit on a future read.

• [non-blocking] src/vouch/sync.py:423-424 — _validation_issues_for_source now accepts kb_dir: Path | None = None and guards the graph-integrity call behind if kb_dir is not None. The only existing caller that omits kb_dir is inside _validation_issues_for_source itself (the export_check sub-call at line 409 is on a different object). A quick grep confirms sync_check passes kb_dir and sync_apply reaches graph validation through sync_check. The None default is safe but could mask a future caller forgetting to pass kb_dir; a Path (non-optional) parameter with sync_apply constructing the call explicitly would be slightly safer. Not blocking since the current callers are correct.

Verdict

approve — The fix is complete and correctly scoped. All five entry points from the issue are addressed; the test coverage (storage positive/negative, bundle self-contained + destination-resident + dangling, sync check + apply, proposal rejection) is thorough. The put_relation_idempotent skip-on-existing design is well-reasoned and tested. The suggestions above are quality-of-life items, none of them a correctness risk.