Skip to content

fix: stop 409-rebased stale buffers from silently clobbering newer note bodies (0804 wipe RCA) - #73

Merged
chasehuh merged 1 commit into
mainfrom
task/body-wipe-rca
Aug 4, 2026
Merged

fix: stop 409-rebased stale buffers from silently clobbering newer note bodies (0804 wipe RCA)#73
chasehuh merged 1 commit into
mainfrom
task/body-wipe-rca

Conversation

@chasehuh

@chasehuh chasehuh commented Aug 4, 2026

Copy link
Copy Markdown
Member

Incident

2026-08-04 evening (KST), note dsb-wbhi-aqa (0804.md): the operator lost the note body twice while actively editing. At 19:09:31 KST, revision 555 captured the 1680-char body as the live note dropped to a stale 503-char prefix. note_revisions shows the body ping-ponging beforehand: 1060 → 1006 → 503 → 1006 → 503 → 534 → 1680 → 503 (18:46–19:09 KST). The note has zero note_doc_snapshots/note_doc_updates rows (the whole DB has zero) — prod runs entirely on the legacy whole-document PUT path; the CRDT flag has never been on.

Root cause

The server-side optimistic-concurrency CAS (lib/notes.ts updateNote, #58) is sound. The clobber came from the client conflict protocol laundering stale buffers into valid tokens, via four compounding flaws:

  1. 409 token laundering (primary). On conflict, persist rebased baseUpdatedAtRef to the server's fresh updated_at while keeping the stale buffer (noteAfterConflictKeepLocalBuffer). The code comment claimed "only a user action re-sends this buffer" — false, see (2). One poll cycle after the real editor paused, the stale tab held a valid ticket; its next PUT silently won. This is the 1006↔503 oscillation: each tab 409s, rebases, overwrites the other.
  2. Poll-driven auto-PUT. The autosave effect treated any activeNote list-row change as a local edit signal (activeNote.body !== body → dirty → PUT). When the 1.5s poll refreshed the row while the buffer correctly refused the remote body, an idle tab automatically PUT its stale buffer — no user input at all.
  3. Self-conflict seeding. The autosave timer never awaited the in-flight PUT, so a slow save + continued typing produced two same-token PUTs; the superseded ack was dropped including its token advance, so the second PUT 409'd against the tab's own write. A single tab on a flaky network was enough to enter state (1)/(2).
  4. Poisoned list row. The 409 handler stored {server updated_at, local stale body} into the list; isRemoteNoteNewer (strict >) then never refetched the true body, and reselecting the note loaded the stale body as "saved" with a live ticket.

Wipe #2 (after the operator's SQL restore) was the same class re-triggering; the 1680-char pre-image left no recovery row because the 60s revision coalesce swallowed it — the operator had to paste the content back from chat.

Fix

Client (components/agentnote-app.tsx):

  • 409 never advances the base token and never poisons the list row — the row shows the honest server note; the buffer keeps local text.
  • Conflict banner now has explicit Use server / Overwrite actions. These are the only paths that may hand a diverged buffer a fresh token, and Overwrite fetches the current generation at click time.
  • Autosave arms only on buffer changes (lastArmedBodyRef guard); a poll/broadcast row refresh can never turn an idle tab into a writer.
  • Persists are serialized (each awaits the prior in-flight persist), so overlapping same-token PUTs can no longer self-409.

Server:

  • PUT /api/notes/[id] rejects a body-less payload (400) instead of body ?? "" blanking the note — a live hole, though not this incident's cause.
  • lib/notes.ts: destructive overwrites (new body < half the previous) bypass the 60s revision coalesce, so a wipe's pre-image is always recoverable regardless of burst timing.

Removed noteAfterConflictKeepLocalBuffer (the laundering helper) and its tests.

Test plan

  • pnpm test: 22 files, 222 tests green (14 new/updated), including:
    • incident-shaped regression: destructive 1680→503 overwrite always records a revision (coalesce window forced to 0);
    • isDestructiveBodyOverwrite boundary tests;
    • body-less PUT → 400, updateNote not called;
    • source-level guards (repo's established pattern): 409 branch must not rebase onto conflictNote.updated_at; autosave must carry the lastArmedBodyRef guard; persists must serialize; conflict resolution must go through the explicit actions.
  • tsc --noEmit and pnpm build green. pnpm lint has pre-existing errors identical on origin/main (react-hooks plugin), none introduced here.
  • Read-only prod SQL was used for forensics only; no data mutated.

Rollout notes / residual risks

  • CRDT flag: prod should eventually converge on NEXT_PUBLIC_AGENTNOTE_CRDT=1 (+ deployed NEXT_PUBLIC_AGENTNOTE_COLLAB_URL, optional — HTTP sync works without it). It structurally eliminates this bug class for the body. Recommended as a separate, verified rollout: flipping the flag makes every opened note permanently CRDT-managed (seed-on-open), and legacy tabs still open during rollout will fail closed with 409 crdt_managed_body (safe but visible). Notes like 0804.md need no manual migration — first open seeds from notes.body. Land this hardening first; it is what prod runs today.
  • Tabs running the old bundle until refresh can still launder tokens; the serialization+banner changes only protect updated clients. ReloadToUpdate limits this window.
  • A stale tab with unsaved divergence now parks in a visible conflict state instead of silently winning; leaving the note still prompts "discard?" via the existing flush path.
  • Known pre-existing gaps, out of scope here: archive/delete broadcasts overwrite a dirty buffer without a flush; manual SQL restores must bump updated_at (README's snippet already does).

🤖 Generated with Claude Code

…te bodies

RCA of the 2026-08-04 note wipe (dsb-wbhi-aqa, 1680 -> 503 chars, twice):
the conflict handler rebased baseUpdatedAtRef to the server's updated_at
while keeping the stale buffer, so the next re-send (keystroke, poll-driven
auto-PUT, flush, Retry) carried a valid token and silently overwrote the
newer body. Overlapping in-flight PUTs self-409'd to seed the state, and
the poisoned list row hid the divergence from the poll.

- 409 no longer advances the base token or poisons the list row; the row
  shows the honest server note and the buffer keeps local text.
- Conflicts resolve only through explicit "Use server" / "Overwrite"
  banner actions; only these may take a fresh token for a diverged buffer.
- Autosave arms only on buffer changes; a poll/broadcast row refresh can
  never turn an idle tab into a writer.
- Persists are serialized so overlapping same-token PUTs cannot self-409.
- PUT rejects a body-less payload instead of blanking the note.
- Destructive overwrites (body shrinks below half) bypass the 60s revision
  coalesce so the pre-image is always recoverable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
memo Ready Ready Preview Aug 4, 2026 11:19am

Request Review

@chasehuh
chasehuh merged commit 7b981a2 into main Aug 4, 2026
2 checks passed
@chasehuh
chasehuh deleted the task/body-wipe-rca branch August 4, 2026 11:19
chasehuh added a commit that referenced this pull request Aug 4, 2026
…a valid save token (#74)

Post-#73, multi-tab clobber survived through BroadcastChannel drafts:
broadcastDraft stamped outgoing drafts with the LIST ROW's updated_at,
which a poll/upsert advances past a stale dirty buffer (exactly the state
#57/#73 enforce). A clean peer at that generation passed isDraftBaseCurrent,
adopted the stale body as 'saved', took the current token, and its next
keystroke PUT the stale lineage with a valid token — silent overwrite,
no 409 anywhere on the winning path (0804.md rev 556 divergence).

- broadcastDraft now stamps the buffer's pinned base (baseUpdatedAtRef)
  plus a fingerprint of the base body; never the list row.
- Receivers refuse editor apply unless the sender proves matching base
  CONTENT (fail closed on missing fingerprint, so old bundles cannot keep
  laundering after this ships).
- New baseBodyRef tracks the raw server body at the base generation,
  advancing only alongside the base token — never from unacked peer text.
- PUT carries base_fingerprint; the server 409s a valid-token write whose
  base content is not the current body (guards against paths client-side
  gates miss and against victims of still-open pre-fix bundles).

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant