Wait for pending edits before a boundary snapshot, and time each request on its own

Boundary snapshots flushed the debounced writes but didn't wait for them,
so the snapshot could read the row before the edit it was meant to keep.
They now await flushAllModelWrites first. A closing window can't await,
so it sends the writes and then the snapshot straight behind them: in
the browser the worker keeps that order, and on the desktop the snapshot
can still win, leaving the edit saved but unversioned until its next
send.

The idle tracker kept one timer for all requests, so a write to any
request, from another window or sync, restarted the active one's
countdown. Each request now has its own, cleared when it fires or when
that request is captured at a boundary.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

apps/yaak-client/lib/editSessionTracker.test.ts

@@ -39,6 +39,53 @@ describe("EditSessionTracker", () => {
     expect(captured).toEqual([["rq_1", "idle"]]);
   });
 
+  test("each request keeps its own countdown when two are edited in turn", () => {
+    const { tracker: t, captured } = tracker();
+    t.noteEdit("rq_a");
+    vi.advanceTimersByTime(600);
+    // An edit to B, from this window, another one, or sync, must not touch A's countdown
+    t.noteEdit("rq_b");
+    vi.advanceTimersByTime(400);
+    expect(captured).toEqual([["rq_a", "idle"]]);
+
+    // B keeps writing; A is done and stays done
+    t.noteEdit("rq_b");
+    vi.advanceTimersByTime(999);
+    expect(captured).toEqual([["rq_a", "idle"]]);
+    vi.advanceTimersByTime(1);
+    expect(captured).toEqual([
+      ["rq_a", "idle"],
+      ["rq_b", "idle"],
+    ]);
+  });
+
+  test("a boundary capture settles that request's countdown, and only that one", () => {
+    const { tracker: t, captured } = tracker();
+    t.noteActiveRequest("rq_a");
+    t.noteEdit("rq_a");
+    t.noteEdit("rq_b");
+    t.noteActiveRequest("rq_c");
+    expect(captured).toEqual([["rq_a", "switch"]]);
+
+    vi.advanceTimersByTime(1000);
+    // A was already captured when it was left; B's countdown still ran
+    expect(captured).toEqual([
+      ["rq_a", "switch"],
+      ["rq_b", "idle"],
+    ]);
+  });
+
+  test("a close can bring its own snapshot", () => {
+    const { tracker: t, captured } = tracker();
+    const onClose: Capture[] = [];
+    t.noteActiveRequest("rq_1");
+    t.noteEdit("rq_1");
+    t.noteBoundary((id, reason) => onClose.push([id, reason]));
+    vi.advanceTimersByTime(1000);
+    expect(onClose).toEqual([["rq_1", "switch"]]);
+    expect(captured).toEqual([]);
+  });
+
   test("captures the request being left, not the one being opened", () => {
     const { tracker: t, captured } = tracker();
     t.noteActiveRequest("rq_1");

apps/yaak-client/lib/editSessionTracker.ts

@@ -18,12 +18,16 @@ type Snapshot = (requestId: string, reason: ModelVersionReason) => void;
  *
  * It deliberately knows nothing about *what* changed. Versions are
  * content-addressed, so a boundary that turns out to have nothing behind it
- * costs one query and creates nothing — which is what lets this stay a timer
- * and two assignments instead of a change-tracking system.
+ * costs one query and creates nothing — which is what lets this stay a few
+ * timers instead of a change-tracking system.
  */
 export class EditSessionTracker {
-  private timer: ReturnType<typeof setTimeout> | null = null;
-  private idleRequestId: string | null = null;
+  /**
+   * One idle countdown per request. Writes arrive for any request — from other
+   * windows and sync too — and an edit to one must never restart or cancel
+   * another's countdown.
+   */
+  private timers = new Map<string, ReturnType<typeof setTimeout>>();
   private activeRequestId: string | null = null;
 
   constructor(
@@ -31,15 +35,16 @@ export class EditSessionTracker {
     private readonly idleMs: number = IDLE_MS,
   ) {}
 
-  /** A request was written. Restarts its idle countdown. */
+  /** A request was written. Restarts that request's idle countdown. */
   noteEdit(requestId: string) {
-    if (this.timer != null) clearTimeout(this.timer);
-    this.idleRequestId = requestId;
-    this.timer = setTimeout(() => {
-      this.timer = null;
-      const requestId = this.idleRequestId;
-      if (requestId != null) this.snapshot(requestId, "idle");
-    }, this.idleMs);
+    this.clearTimer(requestId);
+    this.timers.set(
+      requestId,
+      setTimeout(() => {
+        this.timers.delete(requestId);
+        this.snapshot(requestId, "idle");
+      }, this.idleMs),
+    );
   }
 
   /** The user moved to a different request, so the one they left is finished. */
@@ -47,11 +52,27 @@ export class EditSessionTracker {
     if (requestId === this.activeRequestId) return;
     const left = this.activeRequestId;
     this.activeRequestId = requestId;
-    if (left != null) this.snapshot(left, "switch");
+    if (left != null) this.capture(left, this.snapshot);
   }
 
-  /** The window lost focus or is closing. */
-  noteBoundary() {
-    if (this.activeRequestId != null) this.snapshot(this.activeRequestId, "switch");
+  /**
+   * The window lost focus or is closing. A close can't wait for anything, so it
+   * passes its own snapshot function.
+   */
+  noteBoundary(snapshot: Snapshot = this.snapshot) {
+    if (this.activeRequestId != null) this.capture(this.activeRequestId, snapshot);
+  }
+
+  /** Capture at a boundary, which also settles any idle countdown still pending */
+  private capture(requestId: string, snapshot: Snapshot) {
+    this.clearTimer(requestId);
+    snapshot(requestId, "switch");
+  }
+
+  private clearTimer(requestId: string) {
+    const timer = this.timers.get(requestId);
+    if (timer == null) return;
+    clearTimeout(timer);
+    this.timers.delete(requestId);
   }
 }

apps/yaak-client/lib/requestVersions.test.ts

@@ -0,0 +1,60 @@
+import { beforeEach, describe, expect, test, vi } from "vite-plus/test";
+
+const mocks = vi.hoisted(() => ({
+  calls: [] as string[],
+  finishWrites: null as (() => void) | null,
+  flushAllModelWrites: vi.fn(),
+  rpc: vi.fn(),
+}));
+
+vi.mock("@yaakapp-internal/models", () => ({
+  flushAllModelWrites: mocks.flushAllModelWrites,
+  flushAllPendingPatches: vi.fn(),
+}));
+vi.mock("@yaakapp-internal/platform", () => ({ platform: {} }));
+vi.mock("../hooks/useActiveRequestId", () => ({ activeRequestIdAtom: null }));
+vi.mock("./jotai", () => ({ jotaiStore: {} }));
+vi.mock("./rpc", () => ({ rpc: mocks.rpc }));
+
+describe("snapshotRequestVersion", () => {
+  beforeEach(() => {
+    mocks.calls = [];
+    mocks.flushAllModelWrites.mockImplementation(
+      () =>
+        new Promise<void>((resolve) => {
+          mocks.calls.push("flush");
+          mocks.finishWrites = () => {
+            mocks.calls.push("writes landed");
+            resolve();
+          };
+        }),
+    );
+    mocks.rpc.mockImplementation(async (cmd: string) => {
+      mocks.calls.push(cmd);
+      return { id: "mv_1" };
+    });
+  });
+
+  test("asks for the snapshot only once pending writes have landed", async () => {
+    const { snapshotRequestVersion } = await import("./requestVersions");
+    snapshotRequestVersion("rq_1", "switch");
+    await Promise.resolve();
+    expect(mocks.calls).toEqual(["flush"]);
+
+    mocks.finishWrites?.();
+    await vi.waitFor(() =>
+      expect(mocks.calls).toEqual(["flush", "writes landed", "models_snapshot_request"]),
+    );
+    expect(mocks.rpc).toHaveBeenCalledWith("models_snapshot_request", {
+      requestId: "rq_1",
+      reason: "switch",
+    });
+  });
+
+  test("a failed write still lets the snapshot go ahead", async () => {
+    mocks.flushAllModelWrites.mockRejectedValue(new Error("write failed"));
+    const { snapshotRequestVersion } = await import("./requestVersions");
+    snapshotRequestVersion("rq_1", "idle");
+    await vi.waitFor(() => expect(mocks.rpc).toHaveBeenCalledTimes(1));
+  });
+});

apps/yaak-client/lib/requestVersions.ts

@@ -1,4 +1,4 @@
-import { flushAllPendingPatches } from "@yaakapp-internal/models";
+import { flushAllModelWrites, flushAllPendingPatches } from "@yaakapp-internal/models";
 import type { ModelPayload, ModelVersion, ModelVersionReason } from "@yaakapp-internal/models";
 import { platform } from "@yaakapp-internal/platform";
 import { EditSessionTracker } from "./editSessionTracker";
@@ -16,8 +16,31 @@ const REQUEST_MODELS = ["http_request", "grpc_request", "websocket_request"];
  * anything.
  */
 export function snapshotRequestVersion(requestId: string, reason: ModelVersionReason) {
-  // Edits reach the database on a debounce, so flush before asking for a
-  // version of what is in it
+  captureRequestVersion(requestId, reason).catch((err: unknown) => {
+    console.warn("Failed to snapshot request version", err);
+  });
+}
+
+async function captureRequestVersion(requestId: string, reason: ModelVersionReason) {
+  // Edits reach the database on a debounce. Wait until every pending write has
+  // landed, or the snapshot can read the row before the edit this boundary is
+  // meant to capture. A write that failed leaves nothing newer to wait for.
+  await flushAllModelWrites().catch((err: unknown) => {
+    console.warn("A pending write failed before a request snapshot", err);
+  });
+  await rpc<ModelVersion>("models_snapshot_request", { requestId, reason });
+}
+
+/**
+ * The same, for a window that is closing, where nothing can be awaited.
+ *
+ * The pending writes go out first and the snapshot right behind them. In the
+ * browser that is enough: the worker runs commands in the order they arrive.
+ * On the desktop each command runs on its own, so the snapshot can still reach
+ * the database before the last write does. The edit itself is saved either
+ * way, and the request's next send versions it.
+ */
+function snapshotRequestVersionOnClose(requestId: string, reason: ModelVersionReason) {
   flushAllPendingPatches();
   rpc<ModelVersion>("models_snapshot_request", { requestId, reason }).catch((err: unknown) => {
     console.warn("Failed to snapshot request version", err);
@@ -46,5 +69,7 @@ export function initRequestVersionSnapshots() {
 
   // Closing is the last boundary there is. Nothing can be awaited here, but the
   // write is already on its way and the backend outlives the window.
-  window.addEventListener("beforeunload", () => tracker.noteBoundary());
+  window.addEventListener("beforeunload", () =>
+    tracker.noteBoundary(snapshotRequestVersionOnClose),
+  );
 }