Skip to content

Data loss: stale-tab LWW PUT still truncates note bodies after #51/#56 dirty guards (RCA 2026-07-30 uvk-cumd-omo) #57

Description

@chasehuh

Summary

Post-incident RCA for note uvk-cumd-omo ("0730-2.md") on 2026-07-30 ~05:04:33Z: notes.body was truncated ~360 → 142 chars by a successful PUT, while note_revisions correctly captured the prior 360-char body (rev id 6 @ 05:04:34Z; already restored).

This happened after Production deploy of 8d30561 (~04:40Z) which included the #51/#56 dirty-guard fixes and #53/#55 save-failure UX. Those fixes stop a remote body from clobbering a dirty/saving/error editor buffer, but they do not stop a stale peer tab from winning last-writer-wins on the server, nor do they stop a shorter body from replacing a saved local buffer via draft/upsert/poll.

Why This Matters

Silent truncation of a live note is core data-loss. #54 made this recoverable (revision existed); without a write-path concurrency fix, the same LWW race will recur and revisions are only a safety net (and coalesce window can drop rapid thrash).

Conversation Context / Incident Facts

Constraints for follow-up work: no further prod DB writes for recovery; no deploy from this RCA thread; prefer a designed fix issue over a speculative one-line PR.

Root Cause Hypotheses (ranked)

H1 — Stale peer PUT wins unconditional LWW (highest confidence)

Mechanism: PUT /api/notes/:idupdateNote always UPDATEs body / updated_at = NOW() with no expected_updated_at / If-Match / generation check (app/api/notes/[id]/route.ts, lib/notes.ts).

Any tab whose local buffer is still the older 142-char draft can persist and overwrite the 360-char server body. #51/#56 only gates applying remotes into an unsaved editor; it never blocks the peer’s own autosave/retry/Reload-to-Update flush.

Evidence:

  • Revision prev_body length ~360 at the moment short body was written — exactly the shouldRecordBodyRevision path in updateNote.
  • No concurrency token exists anywhere in the PUT payload.
  • Autosave (persist), retry (bodyRefState.current), and Reload-to-Update onFlushSave all issue the same unconditional PUT.

Why guards didn’t stop it: They never claimed to. Dirty-guard protects the victim tab’s React/CM buffer while dirty|saving|error; the aggressor tab is happily dirty with the short body and is allowed to save.

H2 — Shorter body applied while local saveState === "saved" (high — UI truncation path)

Mechanism: canApplyRemoteBody("saved") === true by design (clean-tab sync). Paths that still replace the active buffer when saved:

  • applyRemoteDraft (BroadcastChannel draft, 32ms debounce) — no isRemoteNoteNewer, no length/prefix guard
  • applyRemoteNote via poll (1.5s) or upsert after peer save — allowed when saved if remote updated_at is newer

So Tab B can show the full 360-char body, be saved (matched last ack), then Tab A’s short draft/upsert replaces the UI with 142 without needing forceBody.

Evidence: Explicit #51 acceptance: “Clean-tab sync: Tab B saved still receives Tab A upsert.” That requirement re-opens truncation whenever Tab A’s “clean” or draft content is stale/shorter.

H3 — Mixed client bundles in the ~24min post-deploy window (medium)

Mechanism: Tabs that never reloaded still run pre-#56 JS where upsert used forceBody: true and bypassed the dirty guard (original #51 bug). Deploy banner (ReloadToUpdate) encourages staggered reloads; a stale tab’s flush/save of 142 can land just as another tab reloads onto the new build.

Evidence: Deploy @ 04:40:34Z; incident @ ~05:04Z; ReloadToUpdate flushes unsaved work before reload (components/reload-to-update.tsx).

H4 — Persist success marks saved while buffer already advanced (medium-low race)

Mechanism: persist(id, nextBody) on HTTP success does setSaveStateNow("saved") if isCurrentSaveAttempt still matches, even when bodyRefState.current !== nextBody (user typed during the in-flight PUT; next persist only armed on a 400ms timer). Briefly saveStateRef === "saved" while local body is longer → H2 apply window opens for a peer short body.

Evidence: Success path at components/agentnote-app.tsx persist callback; no bodyRefState.current === nextBody check before marking saved.

H5 — selectNote / navigation drops pending autosave (lower for this incident)

Mechanism: selectNote clears saveTimer and does not flush the previous note’s dirty buffer. Could lose in-flight longer text if the user switched notes; less direct fit for “same note 360→142 with revision of 360” unless combined with a peer short PUT.

Ruled down / not primary

How 360→142 happened at 05:04:33 despite forceBody removal

Most likely combined sequence:

  1. Server (and/or Tab B) held the ~360-char body after normal editing/autosave.
  2. Another client context (second tab, background tab, or pre-reload bundle) still held or re-saved the earlier ~142-char draft.
  3. That client’s PUT succeeded (H1). updateNote wrote 142 and recorded revision of 360 @ ~05:04:34Z.
  4. Optionally, Tab B was saved (or on an old bundle) so draft/upsert/poll applied 142 into the visible editor (H2/H3) — matching “body looked truncated.”

forceBody removal only closes “remote apply while dirty”; it does not make short stale writes safe.

Desired Behavior

  1. Server: Reject stale overwrites. PUT must carry the client’s last-known updated_at (or generation); if the row’s updated_at differs, return 409 Conflict with the current note (no body replace, no revision of the winner).
  2. Client on 409: Keep local buffer; do not mark saved from the failed attempt; surface conflict/retry UX; never blindly re-PUT the stale short body over the newer server body.
  3. Client “clean” apply: Treat “safe to apply remote” as local body === last successfully acked server body (or equivalent generation), not merely saveState === "saved". Applying a remote/draft that shrinks content relative to last-ack should require explicit conflict handling, not silent replace.
  4. Persist completion: If bodyRefState.current !== persistedBody, remain dirty and ensure a follow-up persist is armed (close H4).
  5. Note switch: Flush or preserve dirty buffer when leaving a note (selectNote / clear).
  6. Tests: Multi-tab LWW fixture: Tab B saves long → Tab A PUT short with stale expected_updated_at → 409, server stays long; Tab B saved must not silently shrink on stale draft without ack identity check.

Source Of Truth

Internal

Implementation Notes

Likely files

  • lib/notes.ts / app/api/notes/[id]/route.ts — optimistic concurrency on update
  • components/agentnote-app.tsx — send base timestamp; handle 409; tighten clean-apply predicate; persist dirty-if-diverged; flush on note switch
  • lib/remote-apply-guard.ts (+ tests) — ack-identity / conflict helpers
  • lib/notes-revisions.test.ts or new API tests — 409 does not clobber / does not revise winner incorrectly

Suggested minimal slice (hotfix-shaped)

  1. expected_updated_at on PUT + 409 (server + client).
  2. Persist success: stay dirty if buffer advanced.
  3. Apply remote/draft only when bodyRefState === lastAckedBody (and saveState allows).

Larger follow-ups (separate): CRDT/merge UI, per-field presence, richer conflict banner.

Edge Cases And Risks

  • Intentional large deletions in one tab must still win when that tab is based on the latest updated_at.
  • Clock skew: compare server-authored updated_at only; never client Date.now() for concurrency tokens (already partially addressed for drafts in fix: stop forceBody upsert from overwriting dirty note bodies #56).
  • Old clients without expected_updated_at: decide fail-closed (409/400) vs temporary allow — prefer fail-closed after short grace if any old bundles remain.
  • Revision coalesce (60s) may skip a second revision during conflict storms; concurrency fix reduces need.

Non-Goals

  • Another forceBody-only client tweak without server precondition (insufficient; this incident proves it).
  • Prod DB mutation / further manual restores in the implementation PR.
  • Full OT/CRDT collaborative editing in the hotfix.

Acceptance Criteria

  • PUT with stale expected_updated_at returns 409; notes.body unchanged; no revision row that records the newer body as prev for a rejected write.
  • Repro: Tab A short stale + Tab B long saved/dirty → server remains long after Tab A’s save attempt.
  • Tab B in saved with body === lastAck does not apply a peer draft/upsert that does not share that ack identity (or surfaces conflict instead of silent shrink).
  • In-flight typing during PUT leaves note dirty until the longer buffer is persisted.
  • pnpm test / pnpm build green; manual two-tab checklist documented in PR.
  • No deploy from RCA-only work; ship via normal PR review.

Test Plan

  • Unit: updateNote/API rejects mismatched expected timestamp.
  • Unit: canApply / ack-identity helpers.
  • Unit: persist completion with diverged bodyRefState.
  • Manual: two Chrome tabs, long vs short, including one tab left backgrounded through a Production deploy / Reload-to-Update flush.
  • Manual: Korean IME compose during poll (no spurious shrink).

Open Questions

  • Single issue vs split (server 409 hotfix vs client ack-identity)? Recommend one hotfix PR covering both; split only if review velocity demands.
  • 409 UX: silent keep-local + retry vs visible “Another tab saved — kept your version / show theirs”?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions