fix(graph): make edge upserts explicit and observable

[ohdearquant]

AI-assisted contribution: OpenAI Codex

Summary

• retain replacement semantics for live edge natural-key matches while returning an explicit created, updated, or resurrected mutation disposition
• refuse implicit soft-delete resurrection unless the caller supplies resurrect=true, consistently across singleton, bulk, atomic, and coordinator-routed links
• emit LinkCreated/EdgeUpdated lifecycle events with edge referent observations and replacement preimages, and document the wire/storage/ADR contract

Test plan

• cargo fmt --all -- --check (in crates/)
• cargo clippy -p khive-storage -p khive-db -p khive-runtime -p khive-pack-kg -p khive-mcp -p kkernel --all-targets -- -D warnings
• cargo test -p khive-db stores::graph::tests --no-fail-fast
• cargo test -p khive-db stores::event::tests --no-fail-fast
• cargo test -p khive-pack-kg handlers::tests -- --nocapture
• focused atomic-link create/update/resurrection/event tests in khive-runtime
• cargo test -p khive-mcp coordinator::tests --no-fail-fast
• cargo test -p kkernel coordinator::tests --no-fail-fast
• cargo test --workspace (not run; affected suites and strict affected-crate clippy were used proportionally)
• deno fmt --check docs/ (deno is not installed in the local environment)

ADR

Updates ADR-004, ADR-009, ADR-014, and ADR-168 to record edge referent observations, explicit resurrection, mutation disposition, and production lifecycle emitters.

AI-assisted contribution checklist

• Every claim in this PR description matches the actual diff
• Any agent-authored comment / PR body starts with an attribution line
• cargo test output included for behavior-changing code
• Behavior touching typed edges cites the governing ADRs

Out of scope

• changing the accepted live-row metadata replacement policy to a metadata merge
• changing Edge into a fourth substrate

Closes #2087

[ohdearquant]

Reviewed against head dcc1e1412ba750aafdf0d6340cc2a8095cef98af. The resurrect gate itself holds up
well under checking — the substantive issue is a routing divergence, and two of the things that look
alarming on first read turn out to be pre-existing rather than introduced here.

The gate does what it claims

I enumerated the write paths rather than taking the description's list, and the refusal is enforced at
the storage write on all of them — singleton, bulk atomic, bulk non-atomic, atomic plan/apply, and the
coordinator-routed single-UUID link. upsert_edge_guarded_observed reads the prior row and refuses a
tombstone unless resurrect is set (crates/khive-db/src/stores/graph.rs:1371-1378), and the SQL
conflict clause carries the deleted_at predicate as well, so the check is at the write rather than
at a validation layer a different entry point could skip. The missing-field default is false on
every route.

The read-then-write window is closed too: BEGIN IMMEDIATE precedes the natural-key read
(graph.rs:848-865), so another writer cannot soft-delete between the check and the write. Atomic
prepare reads earlier by design, but its apply statement guards on the expected revision and deletion
marker, so a tombstone appearing after the snapshot fails the guard rather than being implicitly
resurrected. Legacy direct GraphStore wrappers hard-code resurrect=false. I did not find a
production caller that can clear a tombstone implicitly.

The coordinator route silently coerces malformed arguments — and it is not only resurrect

crates/khive-mcp/src/server.rs:2194-2197:

let resurrect = args_value.get("resurrect").and_then(Value::as_bool).unwrap_or(false);

The canonical handler deserializes into LinkParams, where resurrect is Option<bool> under
#[serde(deny_unknown_fields)] (crates/khive-pack-kg/src/handlers/params.rs:195-208). So
resurrect: "true" is a deserialization error on the normal path and a silent false on the
coordinator path. Same request, two routings, two behaviours — an explicit opt-in is discarded rather
than rejected. The direction is fail-safe, so this is a wire-contract defect rather than a tombstone
hazard, but it is the sort of divergence that gets discovered by a caller wondering why their flag did
nothing.

Worth widening before fixing: the adjacent line does the same thing to weight.

let weight = args_value.get("weight").and_then(Value::as_f64).unwrap_or(1.0);

LinkParams::weight is Option<f64>, so weight: "0.5" is likewise rejected canonically and
silently becomes 1.0 here — and that one is not fail-safe, since it writes a wrong weight rather
than declining an option. relation is handled correctly by contrast (.parse().ok()? falls
through). Parsing the intercepted arguments through the same LinkParams validation, or falling
through on any invalid field, fixes the class rather than the instance.

The Created disposition is wrong on an ID conflict, but no production path can reach it

The upsert has two conflict arms, and the first rewrites the natural key
(graph.rs:120-131):

ON CONFLICT(namespace, id) DO UPDATE SET source_id = excluded.source_id,
    target_id = excluded.target_id, relation = excluded.relation, ...
ON CONFLICT(namespace, source_id, target_id, relation) DO UPDATE SET ...

previous is read only by natural key, and the disposition is derived from that read alone
(graph.rs:1392-1403). So an ID that already belongs to a live row with a different natural key
takes the ID arm, relocates that row, and still reports Created with previous = None — which would
pair a replacement with LinkCreated and lose the preimage.

That is a genuine inconsistency in the storage API and worth fixing, but I checked reachability before
weighting it: all three production call sites mint the id fresh — LinkId::from(Uuid::new_v4()) at
operations.rs:2561, :2658, and inside build_edge for the bulk path. No production caller supplies
an edge id, so the two conflict arms cannot disagree in practice. This is a latent property of the
storage contract, not a live defect in this change, and I would not hold the PR for it.

One related error path is worth folding into the same fix: when affected == 0, the code re-reads by
natural key and does .ok_or(rusqlite::Error::QueryReturnedNoRows)?. In the ID-collision case that
lookup is empty, so the caller gets a raw rusqlite error rather than a typed refusal.

Two items that are pre-existing, not introduced here

Atomic link events carry an empty actor. event_append_statements passes "" where the normal
path passes format!("{}:{}", token.actor().kind, token.actor().id)
(atomic_prepare.rs:384-395 vs operations.rs:2699), and the helper takes no actor parameter at all.
Real defect against ADR-014's attribution requirement — but it has six call sites and only one is new
in this diff. The other five already emit unattributed events on main. This wants its own fix that
adds the parameter and updates all six, not a hold on this PR.

The edge write and the event append are not one transaction on the singleton, non-atomic bulk, and
coordinator paths, so a committed edge can lack its event and a bulk run can persist a prefix. This is
also pre-existing, and the codebase already says so — the doc comment directly above
event_append_statements states that "the non-atomic handlers write the event in a separate
transaction, ordered but not atomic with the row mutation."

What is this PR's to fix is the documentation it adds on top of that. ADR-014's "every curation
operation emits" and the API reference's "every successful mutation emits" describe a durability
property the non-atomic paths do not provide. Either soften the wording to the ordered-but-not-atomic
guarantee the code actually gives, or state the committed-but-unobserved case explicitly, so the ADR
does not promise more than the implementation.

Documentation

ADR-004's observation vocabulary (entity | note | edge) matches the code exactly, and edge correctly
remains a referent kind rather than a fourth substrate. The three disposition spellings are closed and
consistent across EdgeUpsertDisposition::name, handler output, atomic output, and event payload.
ADR-168's retention rows are right, but its EdgeUpdated emitter inventory does not name this link
replacement/restoration path — worth updating while the change is fresh.

ADR-014's statement that dispositions are determined in the storage write transaction is also slightly
too strong: atomic prepare derives its disposition from the prepare-time snapshot
(atomic_prepare.rs:1548-1569) and protects it with a first-statement guard, which is sound but is not
the same claim.

Tests

The added tests are real — the storage test covers live replacement, tombstone refusal, explicit
restoration, disposition, row id and preimage, and the handler test covers the response fields and a
resurrection preimage. What is not covered: tombstone refusal on the bulk atomic, bulk non-atomic and
coordinator routes (only singleton is directly exercised), a malformed resurrect on the coordinator
route, and a second resurrection's preimage. The coordinator coercion above is exactly the gap the
missing route-level test would have caught.

No builds or tests were run for this pass; every claim above was checked against source at the head
named, and the checked boxes in the description were not relied on.

[ohdearquant]

The red Secret scan (gitleaks) is not from this change

main now resolves the scan scope before running gitleaks, so a pull request run judges only the
commits under review. Branches whose merge base predates that change still run the unscoped form,
which walks every ref in the checkout and reports a credential that has been in history
independently of this work — which is why the finding is identical on several unrelated branches
and why the log shows several thousand commits scanned rather than the handful in this diff.

This branch currently conflicts with main, so it cannot pick that up automatically. Resolving the
conflict and merging main in brings the scoped scan with it, and this check should go green with
no change to the code here.

Full-history coverage is unaffected either way: the nightly scheduled run takes no range and still
scans every ref, so a credential reachable from any branch is still found within a day.

[ohdearquant]

Automated review. Posted by this repository's automated pull-request review pipeline; this is not a human read and does not gate the merge by itself.

Verdict on head 70a6a93: REQUEST-CHANGES, 2 blocking findings. Finding details are delivered to the review's recipients rather than posted here. Do not merge this head while blocking findings are outstanding; a pipeline comment on a newer head supersedes this one.