From d148a6612cdbe3021a1304b75fdb8c7c597696ef Mon Sep 17 00:00:00 2001 From: bigbes Date: Wed, 22 Jul 2026 11:18:12 +0300 Subject: [PATCH] docs: fix an unimplementable crash repair The repair table claimed the reconciler recreates a proposal row from an orphan ref. It cannot: the id is a Postgres serial, and title, rationale, base_rev, agent and agent_session exist nowhere in a ref, since trailers live on commits and title lives nowhere in git. Splits the truth rule instead. Refs stay authoritative for merged-ness; the row is authoritative for existence and metadata. Row-first ordering is already forced by deriving the branch name from the serial, and it is also the safe order, so the unrecoverable case is designed out rather than repaired. Orphan refs and contentless rows are deleted, which is safe because an agent still holds the content and can re-propose. Also records the schema-enforced invariants, notably that merged implies a non-null approval so policy merges cannot be laundered as human ones. --- docs/DESIGN.md | 53 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/docs/DESIGN.md b/docs/DESIGN.md index f55701d2d3d7d9d2705843aa7451e5a56772254d..eaa9fc81a8da49f37a7f50446d77cac780586dd8 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -447,23 +447,54 @@ whole-document `PUT`, which gives an agent no way to express "delete this" or Three systems are touched by a merge — git refs, the bleve index, and Postgres — and **none of it is transactional**. The rule that makes this tractable: -> **Git refs are the source of truth for whether a proposal exists and whether it -> merged. Postgres holds metadata that is reconstructable from git. The index -> and the render cache are pure caches.** - -That gives every crash a defined repair, rather than a bespoke recovery per -failure point: +> **Git refs are the source of truth for whether a proposal has merged. The +> Postgres row is the source of truth for that a proposal exists and what it is. +> The index and the render cache are pure caches.** + +An earlier draft made git refs authoritative for *existence* too, and paired that +with a repair reading "reconciler recreates the row from the ref". **That repair +is not implementable**, and the split above is the fix. + +A bare `proposals/42` ref cannot reconstruct its row: the `42` is a Postgres +serial, and `title`, `rationale`, `base_rev`, `agent` and `agent_session` live +nowhere in a ref — trailers are on *commits*, not refs, and title and rationale +are nowhere in git at all. Inventing placeholder values would be worse than the +divergence. + +**Write ordering removes the unrecoverable case rather than repairing it.** The +branch name is derived from the proposal's serial id, so the row *must* be +inserted before the branch can be named. Row-first is therefore forced by the +schema, and it is also the safe order: a crash can leave a row with no branch — +fully recoverable, since the row holds every field — but never a branch whose +metadata is lost. | Crash between | Symptom | Repair | |---|---|---| -| branch write and row insert | orphan `proposals/*` ref | reconciler recreates the row from the ref | -| merge commit and row update | merged ref, row still `open` | reconciler marks merged (ref is truth) | +| row insert and branch write | `open` row, no branch | reconciler deletes the row; the agent re-proposes (it holds the content) | +| — | orphan `proposals/*` ref with no row | delete the ref: it is unreferenced and its content is unrecoverable anyway | +| merge commit and row update | merged ref, row still `open` | reconciler marks merged (the ref is truth for *merged-ness*) | | merge and reindex | stale index | per-space index rev stamp ≠ approved head → reindex | A **reconciler runs at startup and periodically**: scan `proposals/*` refs and -each space's approved head, compare against rows and the index rev stamps, repair -divergence. It is perhaps a hundred lines and it is what lets every other -component crash without ceremony. +each space's approved head, compare against rows and index rev stamps, repair +divergence. Deleting an unreferenced ref or a contentless row is safe precisely +because an agent's proposal is cheap to reproduce — it still holds the document +it wanted to write. + +**Schema-enforced invariants** (added during implementation, worth stating in the +design because they encode decisions rather than mechanics): + +- `state = 'merged'` **iff** `approval IS NOT NULL` **iff** `merged_rev IS NOT + NULL`; `state = 'open'` iff `resolved IS NULL`. A merged row with a null + approval would silently launder policy-merged content as human-approved — the + exact distinction the bimodal decision exists to preserve. +- `agent` and `agent_session` are non-empty, not merely `NOT NULL`. Provenance is + described as mandatory in prose; `''` would satisfy `NOT NULL` and gut it. +- Illegal transitions fail in SQL (`UPDATE ... WHERE state = 'open'`), not only + in Go, so a second writer cannot merge an already-merged proposal. +- Deleting a document **unregisters its ID**, so the global registry does not + reserve names forever. Without this, IDs would be permanently burned by + deletion, which is human-push-only and therefore easy to do by accident. ### Concurrency and ownership