From 0780b4cc1cff73e982bbaefec0e40b6d3b88188a Mon Sep 17 00:00:00 2001 From: bigbes Date: Wed, 22 Jul 2026 14:10:17 +0300 Subject: [PATCH] docs: guard the reconciler against eating live proposals Implementing the corrected repair table showed it destroys data as written, in two ways. Row-first ordering is forced by the schema, so every proposal passes through the exact state the table says to delete. An ungraced reconciler on a timer would delete an agent's work at random and never once in a test. A row younger than the grace window is in flight, not abandoned. A proposal branch is cut at the approved head, so its tip is trivially an ancestor of that head until the agent's first commit. The plain ancestry rule marks a contentless proposal merged, and open to merged is terminal. Comparing the tip against the recorded base distinguishes never-written from actually-merged. Also records the within-pass ordering that makes the grace window sufficient, that marking merged must invent approval and merged_rev and why policy is the safe lie, the two states left unrepaired, and that spaces are created repository-first while proposals are created row-first. --- docs/DESIGN.md | 51 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/docs/DESIGN.md b/docs/DESIGN.md index ab293bb2d2fa4a06f294e2d62daccfc178adf354..7f4b98fb4b908b987a353387a95efe5292b926af 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -540,13 +540,60 @@ 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. +**Spaces order the other way round: repository first, then the row**, with the +directory removed if the insert fails. Git is authoritative and a space's owner +and name are recoverable from its path, whereas a row without a repository is a +phantom that lists correctly and 404s on every read — and, with no delete for it, +is unrepairable. + | Crash between | Symptom | Repair | |---|---|---| -| row insert and branch write | `open` row, no branch | reconciler deletes the row; the agent re-proposes (it holds the content) | +| row insert and branch write | `open` row, no branch, **older than the grace window** | delete 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 commit and row update | row still `open`, branch merged into the approved head **and tip ≠ its recorded base** | mark merged (the ref is truth for *merged-ness*) | | merge and reindex | stale index | per-space index rev stamp ≠ approved head → reindex | +The two emphasised guards are not fussiness. Without them **this table eats live +data**, and both were found by implementing it: + +- **The grace window is mandatory.** Row-first ordering is *forced* by the schema + (the branch name derives from the serial), so **every** proposal passes through + "row exists, branch does not". An ungrace­d reconciler on a timer would delete + an agent's work at random, and never once in a test. A row younger than the + window (5 minutes) is in flight, not abandoned. +- **Merged-ness needs more than ancestry.** A proposal branch is cut *at* the + approved head, so between creating it and the agent's first commit its tip is + trivially an ancestor of that head — and the plain ancestry rule would mark a + contentless proposal merged, permanently, since `open → merged` is terminal. + Comparing the tip against the proposal's recorded `base_rev` distinguishes + "never written to" from "actually merged". + +**Ordering within a pass matters too:** list proposal *refs* for every space +before reading any proposal *row*. The other order makes a concurrently-opened +proposal look like "branch with no row" and deletes a live agent's branch; this +order makes it look like "row with no branch", which the grace window covers. + +**Marking merged cannot fully satisfy the schema, and invents two values.** The +`merged` state requires `approval` and `merged_rev`, and neither is recoverable +from a ref — the same argument that killed rebuilding a row from a ref. So: + +- `approval` is recorded as `policy`. Of the two available lies it is the safe + one: `human` would launder unreviewed content as blessed, whereas `policy` + merely over-reports into the digest, where a human sees it again. A sound + inference exists — a merge touching any path that fails `Policy.AutoMerges` + cannot have been policy-authorized — and is worth adding once auto-merge + evaluation exists in Phase 3. +- `merged_rev` is recorded as the current approved head, not the merge commit. + They coincide in the common crash-right-after-merge case and diverge if the + branch moved on. Finding the true merge commit needs a history walk for "the + commit whose second parent is this tip", which `gitx` does not expose. + +**Two states are deliberately not repaired**, and are reported rather than +touched: a merged or rejected row whose branch still exists (not a half-finished +write — branch cleanup is separate), and a space row whose repository is missing +(the table covers proposals and the index; a missing repo is an operator +problem, not a torn write). + A **reconciler runs at startup and periodically**: scan `proposals/*` refs and 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