fix(edit-sync): retain buffer on force-post so a stale settlement-ack post survives

src/webview/cm/edit-sync.ts

@@ -114,32 +114,54 @@ export type EditSync = {
   discardBuffer: () => void;
   /** TEARDOWN-precursor signals (`visibilitychange:hidden` / `pagehide` /
    *  `blur` — see shell.ts). Force-posts the latest pending content to the host
-   *  EVEN while an Edit is in flight, then (on a successful post) sets
-   *  editInFlight and NULLs the buffer. Unlike `trySend`, it does NOT
-   *  buffer-and-wait when an Edit is in flight: on a real tab close the iframe
-   *  is destroyed before the next ack, so the host — which stashes the
-   *  in-flight arrival and drains it on settlement (QuollEditorPanel
-   *  `applyEditSettled`) — is the last place the bytes survive; nulling the
-   *  buffer makes the host queue the single authority. Setting editInFlight
-   *  keeps single-flight intact on an ALIVE hide→show / blur→focus (a mere tab
-   *  switch), so the next keystroke buffers rather than double-posting. Still a
-   *  HARD DROP under readonly, a buffer-keeping hold pre-seed / while the
-   *  serialize-error gate is closed, and a buffer-keeping hold when the post
+   *  EVEN while an Edit is in flight (unlike `trySend`, which buffers-and-waits
+   *  under single-flight): on a real tab close the iframe is destroyed before
+   *  the next ack, so the host — which stashes the in-flight arrival and drains
+   *  it on settlement (QuollEditorPanel `applyEditSettled`) — is a place the
+   *  bytes can survive. On a successful post it sets editInFlight (keeping
+   *  single-flight intact on an ALIVE hide→show / blur→focus so the next
+   *  keystroke buffers rather than double-posting).
+   *
+   *  BUFFER RETENTION IS CONDITIONAL on there having been an Edit in flight —
+   *  it mirrors `trySend`, which retains only under single-flight and nulls on
+   *  an idle post:
+   *    - Edit in flight → RETAIN. The host stash covers ONLY the lock-held
+   *      window; if the host has ALREADY settled that in-flight Edit (lock
+   *      released, ack in transit), this force-post carries a now-stale
+   *      docVersion, misses the stash, and is `stale`-rejected → the
+   *      authoritative Document reposts over the typed bytes. Retaining lets the
+   *      next ack replay them at the fresh docVersion (double delivery is
+   *      idempotent via the host `no-op` verdict; `replayIfNeeded` nulls the
+   *      buffer on its own post, so it is exactly ONE replay). On a real close
+   *      the retained buffer is simply never replayed (iframe gone).
+   *    - Nothing in flight → NULL (like trySend's idle post). The force-post
+   *      lands at a matching version and is `accept`ed outright, so the host is
+   *      already the authority for those bytes; retaining them would serve no
+   *      recovery purpose and could later replay already-applied content over a
+   *      racing EXTERNAL edit — the host has no client-side conflict guard for a
+   *      post-settlement replay, so that would silently clobber the external
+   *      change.
+   *
+   *  Still a HARD DROP under readonly, a buffer-keeping hold pre-seed / while
+   *  the serialize-error gate is closed, and a buffer-keeping hold when the post
    *  itself fails. NOT a mid-session call — for a reseed always use
    *  `cancelPendingFlush` (capture-preserving), never `flush`. */
   flush: () => void;
   /** Mid-session flush barrier (context-handoff): clear the debounce timer and,
    *  if a keystroke was typed inside the window, post it NOW — but RESPECT
    *  single-flight (buffer for replay when an Edit is already in flight) rather
    *  than force-posting like `flush`. Use this for barriers where the panel
-   *  STAYS ALIVE afterward (a handoff): `flush` nulls the buffer after a
-   *  force-post that the host can drop as stale (the ack-in-transit window),
-   *  leaving nothing to replay when the subsequent reseed clobbers the unsent
-   *  bytes; `flushIfIdle` never does — trySend buffers the in-flight case, so
-   *  the normal ack→replay path preserves the keystrokes. Reserve `flush` for
-   *  TEARDOWN paths (visibilitychange / pagehide / switch-to-text) where the
-   *  panel disposes and the host queue is the last authority. No-op when nothing
-   *  was typed in the debounce window. */
+   *  STAYS ALIVE afterward (a handoff): `flushIfIdle` never posts a second Edit
+   *  at a stale version — trySend buffers the in-flight case, so the normal
+   *  ack→replay path preserves the keystrokes with no redundant round-trip.
+   *  `flush` force-posts even while in flight (it must, for teardown) and can
+   *  therefore emit a `stale`-rejected Edit in the ack-in-transit window; in
+   *  that in-flight case it RETAINS the buffer so the reseed cannot strand the
+   *  bytes, but that costs an extra idempotent round-trip `flushIfIdle` avoids.
+   *  Reserve `flush`
+   *  for TEARDOWN paths (visibilitychange / pagehide / switch-to-text) where the
+   *  panel may dispose and the host stash / retained buffer are the last
+   *  authorities. No-op when nothing was typed in the debounce window. */
   flushIfIdle: () => void;
 };
 
@@ -315,16 +337,12 @@ export function createEditSync(opts: EditSyncOptions): EditSync {
       buffered = null;
     },
     flush: () => {
-      // TEARDOWN-precursor signal (visibilitychange:hidden / pagehide / blur —
-      // see shell.ts). Force the latest pending bytes to the host EVEN while an
-      // Edit is in flight: on a real close the iframe dies before the next ack,
-      // so the host is the last place the bytes survive. Post directly
-      // (bypassing trySend's in-flight buffer arm), then — on a SUCCESSFUL post
-      // — set editInFlight and null the buffer, so the HOST QUEUE is the single
-      // authority (host-side the in-flight arrival is stashed + drained on
-      // settlement — QuollEditorPanel applyEditSettled). Setting editInFlight
-      // keeps single-flight intact on an ALIVE hide→show (a mere tab switch),
-      // so the next keystroke buffers rather than double-posting. Same gates as
+      // TEARDOWN-precursor signal (visibilitychange:hidden / pagehide / blur).
+      // Force the latest pending bytes to the host even while an Edit is in
+      // flight (bypassing trySend's single-flight buffer arm). Post-success
+      // buffer handling is CONDITIONAL on prior in-flight state — see the flush
+      // JSDoc for the full rationale (settlement→ack stale recovery vs the
+      // external-edit clobber the accept path would cause). Same gates as
       // trySend: readonly is a HARD DROP; pre-seed / serialize-error gate keeps
       // the buffer; a failed post keeps the buffer for the next ack.
       const hadTimer = timer !== null;
@@ -341,10 +359,14 @@ export function createEditSync(opts: EditSyncOptions): EditSync {
         buffered = content; // pre-seed / gate closed: keep for a later drain
         return;
       }
+      const wasInFlight = editInFlight;
       const ok = opts.post(content, docVersion);
       if (ok) {
         editInFlight = true; // maintain single-flight even on an alive hide→show
-        buffered = null;
+        // Retain for ack-replay ONLY under in-flight contention (the sole path
+        // to the stale settlement→ack window); otherwise the host accepted the
+        // post and is the authority, so null it like trySend's idle post (JSDoc).
+        buffered = wasInFlight ? content : null;
       } else {
         buffered = content; // post failed: keep for the next ack
       }
@@ -354,7 +376,9 @@ export function createEditSync(opts: EditSyncOptions): EditSync {
       // the latest bytes are already posted / in-flight / buffered-for-replay,
       // so there is nothing to force (matches flush's no-op-when-nothing-typed).
       // trySend RESPECTS single-flight: posts when idle, buffers when an Edit is
-      // in flight — never the force-post-then-null that flush does.
+      // in flight — never the force-post-even-while-in-flight that flush does
+      // (flush can emit a stale-rejected Edit + one idempotent replay round-trip;
+      // flushIfIdle emits neither).
       const hadTimer = timer !== null;
       clearTimer();
       if (hadTimer) {

test/webview/cm-edit-sync.test.ts

@@ -448,7 +448,7 @@ describe("cm edit-sync — flush (teardown)", () => {
     }
   });
 
-  it("force-posts the latest even while an Edit is in flight, and nulls the buffer", () => {
+  it("force-posts the latest even while an Edit is in flight, and RETAINS the buffer for ack-replay", () => {
     vi.useFakeTimers();
     try {
       let doc = "seed";
@@ -472,10 +472,112 @@ describe("cm edit-sync — flush (teardown)", () => {
         { content: "a", baseDocVersion: 1 },
         { content: "ab", baseDocVersion: 1 },
       ]);
-      // Buffer nulled → host queue is the single authority: a later ack must
-      // NOT replay "ab" a second time.
+      // Buffer is RETAINED (not nulled) because an Edit was in flight: the
+      // force-post can be stale-rejected in the host settlement→ack window
+      // (write lock already released → the lock-held stash path is missed →
+      // `stale` verdict), so the bytes must survive for the next ack to replay.
+      // Double delivery is idempotent at the host (no-op verdict on content
+      // equality).
       sync.onReducerCommit(false);
-      expect(posted.length).toBe(2);
+      expect(posted).toEqual([
+        { content: "a", baseDocVersion: 1 },
+        { content: "ab", baseDocVersion: 1 },
+        { content: "ab", baseDocVersion: 1 }, // replay from the retained buffer
+      ]);
+      // EXACTLY ONE replay, never a loop: replayIfNeeded nulls the buffer on its
+      // own post, so a SECOND commit must NOT post again. Pins the invariant the
+      // comments promise (a mutation of replayIfNeeded's self-null would grow
+      // `posted` here). Revert-check: replayIfNeeded `buffered = null` →
+      // `buffered = content` makes this assertion red.
+      sync.onReducerCommit(false);
+      expect(posted.length).toBe(3);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  it("does NOT retain the buffer when the force-post had no Edit in flight (accepted → host is authority)", () => {
+    // Codex + error-handler review 2026-07-17: retaining unconditionally lets a
+    // force-post that the host ACCEPTS outright (no prior in-flight Edit → base
+    // matches → `accept`) leave a buffer holding already-applied bytes. A later
+    // ack (e.g. after a racing external edit advanced the version) would replay
+    // those stale bytes and clobber the external change — the host has no
+    // client-side conflict guard for a post-settlement replay. So retention is
+    // gated on there having been an Edit in flight; the not-in-flight force-post
+    // nulls the buffer like trySend's idle post.
+    //
+    // Revert-check: change flush's ok arm to `buffered = content` (unconditional)
+    // → this test goes red (the replay reposts "seed+edit" a second time).
+    vi.useFakeTimers();
+    try {
+      let doc = "seed";
+      const posted: Array<{ content: string; baseDocVersion: number }> = [];
+      const sync = createEditSync({
+        getDoc: () => doc,
+        post: (content, baseDocVersion) => {
+          posted.push({ content, baseDocVersion });
+          return true;
+        },
+      });
+      sync.onHostSnapshot(1, true);
+      doc = "seed+edit";
+      sync.onLocalChange(); // timer pending, NO Edit in flight (editInFlight false)
+      sync.flush(); // force-posts once; nothing was in flight → buffer nulled
+      expect(posted).toEqual([{ content: "seed+edit", baseDocVersion: 1 }]);
+      // A later ack must NOT replay the already-posted bytes (buffer was nulled).
+      sync.onReducerCommit(false);
+      expect(posted.length).toBe(1);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  it("retains the buffer on force-post so a stale settlement→ack-window post survives via replay", () => {
+    // The bug (Fable review 2026-07-17): flush() force-posts the pending bytes
+    // with the CURRENT (stale) docVersion. If an Edit was in flight and the
+    // host had ALREADY settled it (write lock released, ack still in transit),
+    // the force-posted Edit misses the host's lock-held stash path
+    // (host-session-core `case "edit"` only stashes while the lock is held) and
+    // hits the `stale` verdict (edit-decision) → the host reposts the
+    // authoritative Document and the typed bytes are visibly erased. Nulling
+    // the buffer on force-post left NOTHING to replay. Fix: retain the buffer;
+    // the normal ack→replay drains it at the fresh docVersion.
+    //
+    // Revert-check: restore `buffered = null` in flush's ok arm → red (the
+    // replay at v2 never fires, "ab" is lost).
+    vi.useFakeTimers();
+    try {
+      let doc = "seed";
+      const posted: Array<{ content: string; baseDocVersion: number }> = [];
+      const sync = createEditSync({
+        getDoc: () => doc,
+        post: (content, baseDocVersion) => {
+          posted.push({ content, baseDocVersion });
+          return true;
+        },
+      });
+      sync.onHostSnapshot(1, true);
+      doc = "a";
+      sync.onLocalChange();
+      vi.advanceTimersByTime(300); // edit #1 posts at v1, editInFlight = true
+      expect(posted.length).toBe(1);
+      // Type one more char inside the debounce window, then hide (alive tab
+      // switch) — flush force-posts it at the STALE v1.
+      doc = "ab";
+      sync.onLocalChange();
+      sync.flush();
+      expect(posted).toEqual([
+        { content: "a", baseDocVersion: 1 },
+        { content: "ab", baseDocVersion: 1 }, // force-posted at stale v1
+      ]);
+      // Host had already settled edit #1 (now at v2) → the force-posted
+      // {ab, v1} is stale → host reposts the authoritative Document at v2. The
+      // webview processes it: snapshot advances the version, the commit clears
+      // in-flight and REPLAYS the retained buffer at the fresh v2 — the bytes
+      // the stale force-post could not deliver.
+      sync.onHostSnapshot(2, true);
+      sync.onReducerCommit(false);
+      expect(posted).toContainEqual({ content: "ab", baseDocVersion: 2 });
     } finally {
       vi.useRealTimers();
     }