fix(dashboard): discard superseded workspace reads (RACE-1)

dashboard/src/pages/Agent/Workspace/components/WorkspaceDrawer.tsx

@@ -4,7 +4,7 @@
  * without modifying the global active-agent selection.
  */
 
-import { useCallback, useEffect, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
 import {
   App,
   Tree,
@@ -56,6 +56,7 @@ import { saveBlobAsFile } from "../../../../utils/saveBlobAsFile";
 import AgentNotReadyScreen from "../../../Chat/components/AgentNotReadyScreen";
 import { fileTreeIcon } from "../../../../utils/fileTreeIcon";
 import { dedupeFileTreeInfos } from "../../../../utils/fileTreeNodes";
+import { createReadGuard } from "./readGuard";
 import { workspaceEntryPath } from "../../../../utils/workspacePath";
 import FileViewer from "./FileViewer";
 import {
@@ -266,6 +267,12 @@ export default function WorkspaceDrawer({
   const [content, setContent] = useState<string>("");
   const [editMode, setEditMode] = useState(false);
   const [fileLoading, setFileLoading] = useState(false);
+  /**
+   * Monotonic ticket for content reads. See `readGuard.ts` — without it a slow
+   * read for file A can land after a fast read for file B and overwrite the
+   * viewer, so the user sees the wrong file's contents.
+   */
+  const contentReadSeq = useRef(0);
   const [saving, setSaving] = useState(false);
   const [previewMode, setPreviewMode] = useState(true);
   const [archiveExporting, setArchiveExporting] = useState(false);
@@ -533,6 +540,8 @@ export default function WorkspaceDrawer({
         setContent("");
         if (isMobile) setMobilePane("viewer");
         if (!getMediaKind(dest) && isProbablyText(dest)) {
+          const guard = createReadGuard(contentReadSeq);
+          const token = guard.begin();
           setFileLoading(true);
           try {
             const r = await request<{ content: string }>(
@@ -542,9 +551,10 @@ export default function WorkspaceDrawer({
                 )}`,
               ),
             );
+            if (!guard.isCurrent(token)) return;
             setContent(r.content);
           } finally {
-            setFileLoading(false);
+            if (guard.isCurrent(token)) setFileLoading(false);
           }
         }
       }
@@ -750,6 +760,11 @@ export default function WorkspaceDrawer({
     }
     setDirEntries([]);
     if (isMobile) setMobilePane("viewer");
+    // Take the ticket BEFORE the early returns: selecting an image or a binary
+    // must still invalidate an in-flight text read, or that read lands later and
+    // paints text over the image viewer.
+    const guard = createReadGuard(contentReadSeq);
+    const token = guard.begin();
     if (getMediaKind(path)) return;
     if (!isProbablyText(path)) return;
     setFileLoading(true);
@@ -759,14 +774,18 @@ export default function WorkspaceDrawer({
           `/agents/${agentId}/workspace/file?path=${encodeURIComponent(path)}`,
         ),
       );
+      // A newer selection superseded this read; applying it would show the
+      // wrong file's contents under the newly selected name.
+      if (!guard.isCurrent(token)) return;
       setContent(r.content);
     } catch (err: unknown) {
+      if (!guard.isCurrent(token)) return;
       message.error(
         (err instanceof Error ? err.message : String(err)) ||
           t("workspace.readFailed", "读取失败"),
       );
     } finally {
-      setFileLoading(false);
+      if (guard.isCurrent(token)) setFileLoading(false);
     }
   };
 
@@ -883,22 +902,27 @@ export default function WorkspaceDrawer({
       setEditMode(true);
       return;
     }
+    const guard = createReadGuard(contentReadSeq);
+    const token = guard.begin();
     setFileLoading(true);
     try {
       const r = await request<{ content: string }>(
         withFromWorkspace(
           `/agents/${agentId}/workspace/doc?path=${encodeURIComponent(path)}`,
         ),
       );
+      // Entering edit mode on a superseded read would edit the wrong file.
+      if (!guard.isCurrent(token)) return;
       setContent(r.content);
       setEditMode(true);
     } catch (err: unknown) {
+      if (!guard.isCurrent(token)) return;
       message.error(
         (err instanceof Error ? err.message : String(err)) ||
           t("workspace.readFailed", "读取失败"),
       );
     } finally {
-      setFileLoading(false);
+      if (guard.isCurrent(token)) setFileLoading(false);
     }
   };
 

dashboard/src/pages/Agent/Workspace/components/readGuard.test.ts

@@ -0,0 +1,82 @@
+import { describe, expect, it } from "vitest";
+import { createReadGuard, type MutableRef } from "./readGuard";
+
+/**
+ * RACE-1 regression guard.
+ *
+ * WorkspaceDrawer awaits a file read and then applies it unconditionally, so a
+ * slow read for file A can land after a fast read for file B and overwrite the
+ * viewer. The user then sees A's contents under B's name.
+ *
+ * These cases pin the ordering contract directly. The drawer is 1500 lines with
+ * pdfjs at module scope (which throws DOMMatrix in jsdom), so drive the guard
+ * rather than mounting it - the same reasoning behind extracting runExclusive.
+ */
+
+function ref(): MutableRef<number> {
+  return { current: 0 };
+}
+
+describe("createReadGuard (RACE-1)", () => {
+  it("treats the newest ticket as current", () => {
+    const guard = createReadGuard(ref());
+    const token = guard.begin();
+    expect(guard.isCurrent(token)).toBe(true);
+  });
+
+  it("invalidates the earlier ticket when a newer one is taken", () => {
+    // The core of the bug: B begins after A, so A must no longer apply.
+    const guard = createReadGuard(ref());
+    const slowA = guard.begin();
+    const fastB = guard.begin();
+    expect(guard.isCurrent(slowA)).toBe(false);
+    expect(guard.isCurrent(fastB)).toBe(true);
+  });
+
+  it("keeps only the LAST of many concurrent reads current", () => {
+    const guard = createReadGuard(ref());
+    const tokens = [guard.begin(), guard.begin(), guard.begin()];
+    expect(tokens.map((t) => guard.isCurrent(t))).toEqual([false, false, true]);
+  });
+
+  it("never revalidates an old token as new ones arrive", () => {
+    const guard = createReadGuard(ref());
+    const first = guard.begin();
+    for (let i = 0; i < 5; i += 1) guard.begin();
+    expect(guard.isCurrent(first)).toBe(false);
+  });
+
+  it("survives across separate guard instances sharing one ref", () => {
+    // The drawer recreates the guard on every render, so the counter - not the
+    // guard object - is what must carry the ordering.
+    const counter = ref();
+    const a = createReadGuard(counter).begin();
+    const b = createReadGuard(counter).begin();
+    expect(createReadGuard(counter).isCurrent(a)).toBe(false);
+    expect(createReadGuard(counter).isCurrent(b)).toBe(true);
+  });
+
+  it("simulates the out-of-order landing the bug produces", async () => {
+    // Direct model of the failure: A resolves after B.
+    const guard = createReadGuard(ref());
+    let viewer = "";
+
+    const readA = (async () => {
+      const token = guard.begin();
+      await new Promise((r) => setTimeout(r, 30)); // slow
+      if (!guard.isCurrent(token)) return; // the fix
+      viewer = "A";
+    })();
+
+    const readB = (async () => {
+      const token = guard.begin();
+      await new Promise((r) => setTimeout(r, 5)); // fast
+      if (!guard.isCurrent(token)) return;
+      viewer = "B";
+    })();
+
+    await Promise.all([readA, readB]);
+    // Without the guard this is "A" - the stale read wins and the viewer lies.
+    expect(viewer).toBe("B");
+  });
+});

dashboard/src/pages/Agent/Workspace/components/readGuard.ts

@@ -0,0 +1,54 @@
+/**
+ * Latest-write-wins guard for async reads.
+ *
+ * A drawer that awaits a file read and then applies it unconditionally has no
+ * way to tell a fresh response from a stale one. Click slow file A, then fast
+ * file B: B renders, then A's response lands last and overwrites it. The user
+ * sees the wrong file's contents under the right file's name.
+ *
+ * `runExclusive` does not help here — it *blocks* concurrent calls, which is
+ * right for double-submit and wrong for reads. Reads should all proceed, and
+ * only the newest one may apply.
+ *
+ * This is deliberately not a React hook: the caller holds the counter in a ref
+ * so it survives re-renders, and keeping it a plain function makes the ordering
+ * testable without mounting a 1500-line drawer.
+ */
+
+/** Minimal shape of a React `useRef` result — avoids importing React here. */
+export interface MutableRef<T> {
+  current: T;
+}
+
+/** A ticket issued by `beginRead`. */
+export interface ReadTicket {
+  /** Issues a ticket and marks it as the newest request. */
+  begin: () => number;
+  /** True only if `token` is still the newest ticket issued. */
+  isCurrent: (token: number) => boolean;
+}
+
+/**
+ * Create a guard whose `isCurrent` is true only for the most recent `begin`.
+ *
+ * ```ts
+ * const guard = createReadGuard(seqRef);
+ * const token = guard.begin();
+ * const r = await request(...);
+ * if (!guard.isCurrent(token)) return;   // a newer read superseded this one
+ * setContent(r.content);
+ * ```
+ *
+ * The counter only ever increases, so a token can never be revalidated by
+ * wrapping around, and a caller that forgets to check simply keeps the old
+ * (buggy) behaviour rather than introducing a new one.
+ */
+export function createReadGuard(ref: MutableRef<number>): ReadTicket {
+  return {
+    begin: () => {
+      ref.current += 1;
+      return ref.current;
+    },
+    isCurrent: (token: number) => ref.current === token,
+  };
+}