From 4fd2b211ccc97188b26663de9d8fb4b7f212c96d Mon Sep 17 00:00:00 2001
From: N <15332201130@163.com>
Date: Thu, 29 Oct 2026 23:23:08 +0800
Subject: [PATCH] fix(artifacts): keep saved revisions ahead of stale reads
 (#7650)

* fix(artifacts): keep saved revisions ahead of stale reads

* fix(artifacts): abort cancelled content requests

Signed-off-by: Willem Jiang <willem.jiang@gmail.com>

---------

Signed-off-by: Willem Jiang <willem.jiang@gmail.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
---
 README.md                                     |   2 +-
 frontend/src/AGENTS.md                        |   3 +-
 .../artifacts/artifact-file-detail.tsx        |   5 +
 frontend/src/core/artifacts/hooks.ts          |   6 +-
 frontend/src/core/artifacts/loader.ts         |   3 +
 .../artifact-file-save-race.dom.test.tsx      | 275 ++++++++++++++++++
 .../artifacts/artifact-viewer.dom.test.tsx    |   3 +
 .../content-cancellation.dom.test.tsx         | 112 +++++++
 .../unit/core/artifacts/hooks.dom.test.tsx    |   3 +
 9 files changed, 408 insertions(+), 4 deletions(-)
 create mode 100644 frontend/tests/unit/components/workspace/artifacts/artifact-file-save-race.dom.test.tsx
 create mode 100644 frontend/tests/unit/core/artifacts/content-cancellation.dom.test.tsx

diff --git a/README.md b/README.md
index 66cab51a9d..40cbab8363 100644
--- a/README.md
+++ b/README.md
@@ -3053,7 +3053,7 @@ recording the others. A concurrent scratch-file removal therefore does not
 discard the whole snapshot or suppress detection of other produced outputs.
 This remains a best-effort scan, not an atomic filesystem snapshot.
 
-Files presented through `present_files` remain part of the thread's artifact state, and the Web UI restores the artifact panel and selected document after a page refresh. Long artifact lists scroll within the panel on desktop and in the mobile drawer, keeping every file reachable. When a completed response successfully presents between 2 and 50 files, its final file card also offers one ZIP download. Archive membership comes from the terminal delivery receipt rather than browser-supplied paths, and the ZIP contains the current file versions, which may have changed since the response. The currently selected formal artifact is refreshed once when the run finishes so edits become visible without a manual reload. Existing UTF-8 text artifacts under `/mnt/user-data/outputs` can also be edited and explicitly saved from the panel on Unix and Windows while the thread is idle; saves use content revisions to prevent overwriting agent changes. Source previews also recognize extensionless `Dockerfile` and `Makefile` artifacts by their file names. Unknown file types, including names such as `constructor` and `__proto__`, retain the download fallback.
+Files presented through `present_files` remain part of the thread's artifact state, and the Web UI restores the artifact panel and selected document after a page refresh. Long artifact lists scroll within the panel on desktop and in the mobile drawer, keeping every file reachable. When a completed response successfully presents between 2 and 50 files, its final file card also offers one ZIP download. Archive membership comes from the terminal delivery receipt rather than browser-supplied paths, and the ZIP contains the current file versions, which may have changed since the response. The currently selected formal artifact is refreshed once when the run finishes so edits become visible without a manual reload. Existing UTF-8 text artifacts under `/mnt/user-data/outputs` can also be edited and explicitly saved from the panel on Unix and Windows while the thread is idle; saves use content revisions to prevent overwriting agent changes. A delayed background read cannot replace a successfully saved revision in the editor; cancelled preview and full-file reads also abort their HTTP requests. Source previews also recognize extensionless `Dockerfile` and `Makefile` artifacts by their file names. Unknown file types, including names such as `constructor` and `__proto__`, retain the download fallback.
 
 Unsaved artifact edits survive switching chats, workspace pages, or visiting the
 home page and returning in the same browser tab. Drafts stay in memory for the
diff --git a/frontend/src/AGENTS.md b/frontend/src/AGENTS.md
index 9a198a3a63..143e29eee4 100644
--- a/frontend/src/AGENTS.md
+++ b/frontend/src/AGENTS.md
@@ -29,7 +29,8 @@ Tests: `html-assets.dom.test.ts`, `artifact-file-preview.dom.test.tsx`,
 2. Stream events update thread state (messages, artifacts, todos, goal). The main thread stream uses the LangGraph SDK's `throttle: true` mode so updates received in the same macrotask coalesce before React is notified; do not replace it with a numeric delay without validating the SDK's trailing-debounce behavior on a continuous stream.
    File-tool artifact auto-open work must run in an effect with timer cleanup; never schedule timers while rendering streamed `write_file` or `str_replace` updates.
    `ThreadState.artifacts` remains the authoritative artifact list. The artifacts provider persists only thread-scoped panel UI state (`open`, selected path, and a refresh bootstrap cache) in session storage; an initial empty stream value must not overwrite that restored state before history finishes loading.
-   Formal artifact content is refreshed once when the run finishes; transient `write-file:` previews remain message-driven.
+   Refetch formal artifacts when runs finish; `write-file:` previews stay message-driven.
+   On save, cancel reads before cache/draft writes; pass both hooks' signals via loader to fetch.
    UTF-8 outputs under `/mnt/user-data/outputs` are editable while idle. Root drafts survive SPA/home/showcase routes and unknown auth; confirmed account change/logout clears them. Owner/generation fences reject stale updates. Auth binds through a stable setter context, outside the typing state subscription. Drafts preview in Markdown/HTML and warn on unload off-route. SHA-256 conflicts preserve edits. Layout, draft-context and editor DOM tests cover this.
    Regular artifact text loads request at most the first 1 MiB through an HTTP
    byte range. A truncated preview must stay lightweight and expose an explicit
diff --git a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx
index cbf7af2ae2..977479a541 100644
--- a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx
+++ b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx
@@ -330,6 +330,11 @@ export function ArtifactFileDetail({
         content: submittedDraftContent,
         expectedSha256: submittedBaselineSha256,
       });
+      // A refetch started before or during the save can still return old bytes.
+      // Cancel both preview and full reads before publishing the saved revision.
+      await queryClient.cancelQueries({
+        queryKey: ["artifact", filepathFromProps, threadId, isMock],
+      });
       setDrafts((current) => ({
         ...current,
         [filepath]: (() => {
diff --git a/frontend/src/core/artifacts/hooks.ts b/frontend/src/core/artifacts/hooks.ts
index 1b1ae68ca1..8b384c27ea 100644
--- a/frontend/src/core/artifacts/hooks.ts
+++ b/frontend/src/core/artifacts/hooks.ts
@@ -34,12 +34,13 @@ export function useArtifactContent({
 
   const { data, isLoading, error, refetch } = useQuery({
     queryKey: ["artifact", filepath, threadId, isMock, fullContentRequested],
-    queryFn: () => {
+    queryFn: ({ signal }) => {
       return loadArtifactContent({
         filepath,
         threadId,
         isMock,
         full: fullContentRequested,
+        signal,
       });
     },
     enabled,
@@ -103,12 +104,13 @@ export function useStandaloneArtifactContent({
 
   const { data, isLoading, error } = useQuery({
     queryKey: ["artifact", filepath, threadId, isMock, fullContentRequested],
-    queryFn: () =>
+    queryFn: ({ signal }) =>
       loadArtifactContent({
         filepath,
         threadId,
         isMock,
         full: fullContentRequested,
+        signal,
       }),
     staleTime: 0,
     refetchOnWindowFocus: true,
diff --git a/frontend/src/core/artifacts/loader.ts b/frontend/src/core/artifacts/loader.ts
index 6192ce92f0..54c41e2a20 100644
--- a/frontend/src/core/artifacts/loader.ts
+++ b/frontend/src/core/artifacts/loader.ts
@@ -52,11 +52,13 @@ export async function loadArtifactContent({
   threadId,
   isMock,
   full = false,
+  signal,
 }: {
   filepath: string;
   threadId: string;
   isMock?: boolean;
   full?: boolean;
+  signal?: AbortSignal;
 }) {
   let enhancedFilepath = filepath;
   if (filepath.endsWith(".skill")) {
@@ -65,6 +67,7 @@ export async function loadArtifactContent({
   const url = urlOfArtifact({ filepath: enhancedFilepath, threadId, isMock });
   const response = await fetch(url, {
     cache: "no-store",
+    signal,
     headers: full
       ? undefined
       : { Range: `bytes=0-${ARTIFACT_PREVIEW_MAX_BYTES - 1}` },
diff --git a/frontend/tests/unit/components/workspace/artifacts/artifact-file-save-race.dom.test.tsx b/frontend/tests/unit/components/workspace/artifacts/artifact-file-save-race.dom.test.tsx
new file mode 100644
index 0000000000..974a8027cf
--- /dev/null
+++ b/frontend/tests/unit/components/workspace/artifacts/artifact-file-save-race.dom.test.tsx
@@ -0,0 +1,275 @@
+import { afterEach, beforeEach, describe, expect, it, rs } from "@rstest/core";
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import {
+  act,
+  cleanup,
+  fireEvent,
+  render,
+  screen,
+  waitFor,
+} from "@testing-library/react";
+import type { PropsWithChildren } from "react";
+import { toast } from "sonner";
+
+rs.mock("@/components/workspace/messages/context", () => ({
+  useThread: () => ({
+    thread: { isLoading: false, messages: [] },
+    isMock: false,
+  }),
+}));
+rs.mock("@/components/ui/sidebar", () => ({
+  useSidebar: () => ({ setOpen: rs.fn() }),
+}));
+rs.mock("next/navigation", () => ({
+  usePathname: () => "/workspace/chats/artifact-save-race",
+}));
+rs.mock("@/core/auth/AuthProvider", () => ({
+  useAuth: () => ({ user: null }),
+}));
+rs.mock("@/core/config", () => ({
+  getBackendBaseURL: () => "/backend",
+}));
+rs.mock("@/env", () => ({
+  env: { NEXT_PUBLIC_STATIC_WEBSITE_ONLY: "false" },
+}));
+rs.mock("@/core/api/fetcher", () => ({ fetch: rs.fn() }));
+rs.mock("sonner", () => ({ toast: { success: rs.fn(), error: rs.fn() } }));
+
+// Keep the production content hook, query client and draft providers. Only the
+// editor surface is simplified so the test can exercise actual edit/save events.
+rs.mock("@/components/workspace/code-editor", () => ({
+  CodeEditor: ({
+    value,
+    readonly,
+    disabled,
+    onChange,
+  }: {
+    value: string;
+    readonly?: boolean;
+    disabled?: boolean;
+    onChange?: (value: string) => void;
+  }) => (
+    <textarea
+      aria-label="Artifact source"
+      value={value}
+      readOnly={readonly}
+      disabled={disabled}
+      onChange={(event) => onChange?.(event.target.value)}
+    />
+  ),
+}));
+
+import { ArtifactFileDetail } from "@/components/workspace/artifacts/artifact-file-detail";
+import { ArtifactsProvider } from "@/components/workspace/artifacts/context";
+import { fetch as apiFetch } from "@/core/api/fetcher";
+import { ArtifactDraftsProvider } from "@/core/artifacts/draft-context";
+import { I18nProvider } from "@/core/i18n/context";
+
+const filepath = "/mnt/user-data/outputs/report.txt";
+const threadId = "artifact-save-race";
+const oldRevision = "a".repeat(64);
+const savedRevision = "b".repeat(64);
+const nextRevision = "c".repeat(64);
+const mockedFetch = rs.mocked(apiFetch);
+let queryClient: QueryClient;
+
+function deferred<T>() {
+  let resolve!: (value: T) => void;
+  const promise = new Promise<T>((done) => {
+    resolve = done;
+  });
+  return { promise, resolve };
+}
+
+function textResponse(content: string, revision: string) {
+  return new Response(content, {
+    headers: { ETag: `"${revision}"` },
+  });
+}
+
+function saveResponse(content: string, revision: string) {
+  return new Response(
+    JSON.stringify({ path: filepath, sha256: revision, size: content.length }),
+    { headers: { "Content-Type": "application/json" } },
+  );
+}
+
+function Wrapper({ children }: PropsWithChildren) {
+  return (
+    <QueryClientProvider client={queryClient}>
+      <I18nProvider initialLocale="en-US">
+        <ArtifactDraftsProvider>
+          <ArtifactsProvider>{children}</ArtifactsProvider>
+        </ArtifactDraftsProvider>
+      </I18nProvider>
+    </QueryClientProvider>
+  );
+}
+
+function source() {
+  return screen.getByRole<HTMLTextAreaElement>("textbox", {
+    name: "Artifact source",
+  });
+}
+
+beforeEach(() => {
+  window.sessionStorage.clear();
+  queryClient = new QueryClient({
+    defaultOptions: { queries: { retry: false, gcTime: Infinity } },
+  });
+});
+
+afterEach(() => {
+  cleanup();
+  queryClient.clear();
+  rs.clearAllMocks();
+});
+
+describe("artifact saves racing content refetches", () => {
+  it.each([
+    ["before the save", false, false, false],
+    ["during the save", true, false, false],
+    ["during a full-file save", true, true, false],
+    ["during a save followed by more editing", true, false, true],
+  ])(
+    "keeps the saved content and revision when a read begun %s finishes late",
+    async (
+      _scenario,
+      refetchDuringSave,
+      fullContentRequested,
+      editWhileSaving,
+    ) => {
+      const pendingRead = deferred<Response>();
+      const pendingSave = deferred<Response>();
+      const savedBodies: unknown[] = [];
+      let delayReads = false;
+
+      mockedFetch.mockImplementation(async (_url, options) => {
+        if (options?.method === "PUT") {
+          savedBodies.push(JSON.parse(options.body as string));
+          return savedBodies.length === 1
+            ? pendingSave.promise
+            : saveResponse("next edit", nextRevision);
+        }
+        if (delayReads) return pendingRead.promise;
+        if (
+          fullContentRequested &&
+          new Headers(options?.headers).has("Range")
+        ) {
+          return new Response("old", {
+            status: 206,
+            headers: {
+              ETag: `"${oldRevision}"`,
+              "Content-Range": "bytes 0-2/8",
+            },
+          });
+        }
+        return textResponse("old file", oldRevision);
+      });
+
+      // Restore the provider before mounting the detail, as the real async
+      // thread/artifact load does.
+      const view = render(<div />, { wrapper: Wrapper });
+      view.rerender(
+        <ArtifactFileDetail filepath={filepath} threadId={threadId} />,
+      );
+      if (fullContentRequested) {
+        fireEvent.click(
+          await screen.findByRole("button", { name: "Load full file" }),
+        );
+      }
+      await waitFor(() => expect(source().value).toBe("old file"));
+      fireEvent.click(screen.getByRole("button", { name: "Edit" }));
+      fireEvent.change(source(), { target: { value: "saved edit" } });
+
+      const queryKey = [
+        "artifact",
+        filepath,
+        threadId,
+        false,
+        fullContentRequested,
+      ];
+      let refetch: Promise<void> | undefined;
+      const startRead = async () => {
+        delayReads = true;
+        await act(async () => {
+          refetch = queryClient.refetchQueries({ queryKey, exact: true });
+        });
+        expect(queryClient.isFetching({ queryKey, exact: true })).toBe(1);
+      };
+
+      if (!refetchDuringSave) await startRead();
+      fireEvent.click(screen.getByRole("button", { name: "Save" }));
+      await waitFor(() => expect(savedBodies).toHaveLength(1));
+      if (refetchDuringSave) await startRead();
+
+      if (editWhileSaving) {
+        // Reopening the panel restores its draft while the original save is
+        // still pending, so subsequent typing must survive that completion.
+        view.rerender(<div />);
+        view.rerender(
+          <ArtifactFileDetail filepath={filepath} threadId={threadId} />,
+        );
+        expect(source().disabled).toBe(false);
+        fireEvent.change(source(), { target: { value: "newer draft" } });
+      }
+      const expectedContent = editWhileSaving ? "newer draft" : "saved edit";
+      const unrelatedRead = deferred<string>();
+      const unrelatedKeys = [
+        [
+          "artifact",
+          "/mnt/user-data/outputs/other.txt",
+          threadId,
+          false,
+          false,
+        ],
+        ["artifact", filepath, "other-thread", false, false],
+      ];
+      const unrelatedQueries = unrelatedKeys.map((key) =>
+        queryClient
+          .fetchQuery({ queryKey: key, queryFn: () => unrelatedRead.promise })
+          .catch(() => null),
+      );
+
+      await act(async () => {
+        pendingSave.resolve(saveResponse("saved edit", savedRevision));
+      });
+      await waitFor(() => expect(toast.success).toHaveBeenCalledOnce());
+      expect(source().value).toBe(expectedContent);
+      for (const key of unrelatedKeys) {
+        expect(queryClient.isFetching({ queryKey: key, exact: true })).toBe(1);
+      }
+      unrelatedRead.resolve("unrelated content");
+      expect(await Promise.all(unrelatedQueries)).toEqual([
+        "unrelated content",
+        "unrelated content",
+      ]);
+
+      await act(async () => {
+        pendingRead.resolve(textResponse("old file", oldRevision));
+        await refetch;
+        // Query observer notifications are batched onto the next task.
+        await new Promise((resolve) => setTimeout(resolve, 0));
+      });
+
+      expect(source().value).toBe(expectedContent);
+      expect(screen.queryByText("Changed remotely")).toBeNull();
+      expect(queryClient.getQueryData(queryKey)).toMatchObject({
+        content: "saved edit",
+        sha256: savedRevision,
+      });
+
+      // A later edit must compare against the committed revision, otherwise
+      // the Gateway rejects it as a false optimistic-concurrency conflict.
+      fireEvent.change(source(), { target: { value: "next edit" } });
+      fireEvent.click(screen.getByRole("button", { name: "Save" }));
+      await waitFor(() => expect(savedBodies).toHaveLength(2));
+      expect(savedBodies[1]).toEqual({
+        content: "next edit",
+        expected_sha256: savedRevision,
+      });
+      await waitFor(() => expect(toast.success).toHaveBeenCalledTimes(2));
+      expect(toast.error).not.toHaveBeenCalled();
+    },
+  );
+});
diff --git a/frontend/tests/unit/components/workspace/artifacts/artifact-viewer.dom.test.tsx b/frontend/tests/unit/components/workspace/artifacts/artifact-viewer.dom.test.tsx
index d7277aa236..92063f0ed4 100644
--- a/frontend/tests/unit/components/workspace/artifacts/artifact-viewer.dom.test.tsx
+++ b/frontend/tests/unit/components/workspace/artifacts/artifact-viewer.dom.test.tsx
@@ -67,6 +67,7 @@ describe("ArtifactViewer", () => {
       threadId,
       isMock: false,
       full: false,
+      signal: expect.any(AbortSignal),
     });
   });
 
@@ -79,6 +80,7 @@ describe("ArtifactViewer", () => {
         threadId,
         isMock: true,
         full: false,
+        signal: expect.any(AbortSignal),
       });
     });
   });
@@ -139,6 +141,7 @@ describe("ArtifactViewer", () => {
       threadId,
       isMock: false,
       full: true,
+      signal: expect.any(AbortSignal),
     });
     expect(screen.queryByRole("button", { name: "Load full file" })).toBe(null);
   });
diff --git a/frontend/tests/unit/core/artifacts/content-cancellation.dom.test.tsx b/frontend/tests/unit/core/artifacts/content-cancellation.dom.test.tsx
new file mode 100644
index 0000000000..3aaaa51aa7
--- /dev/null
+++ b/frontend/tests/unit/core/artifacts/content-cancellation.dom.test.tsx
@@ -0,0 +1,112 @@
+import { afterEach, describe, expect, it, rs } from "@rstest/core";
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import { act, cleanup, renderHook, waitFor } from "@testing-library/react";
+import type { PropsWithChildren } from "react";
+
+rs.mock("@/components/workspace/messages/context", () => ({
+  useThread: () => ({
+    thread: { isLoading: false, messages: [] },
+    isMock: false,
+  }),
+}));
+rs.mock("@/core/config", () => ({
+  getBackendBaseURL: () => "/backend",
+}));
+rs.mock("@/env", () => ({
+  env: { NEXT_PUBLIC_STATIC_WEBSITE_ONLY: "false" },
+}));
+
+import {
+  useArtifactContent,
+  useStandaloneArtifactContent,
+} from "@/core/artifacts/hooks";
+
+const filepath = "/mnt/user-data/outputs/report.txt";
+const threadId = "content-cancellation";
+
+function usePanelContent() {
+  return useArtifactContent({ filepath, threadId, enabled: true });
+}
+
+function useStandaloneContent() {
+  return useStandaloneArtifactContent({ filepath, threadId });
+}
+
+afterEach(() => {
+  cleanup();
+  rs.restoreAllMocks();
+});
+
+describe.each([
+  ["panel", usePanelContent],
+  ["standalone", useStandaloneContent],
+] as const)("%s artifact content cancellation", (_viewer, useContent) => {
+  it.each([false, true])(
+    "aborts the HTTP request before reading its body (full=%s)",
+    async (full) => {
+      const client = new QueryClient({
+        defaultOptions: { queries: { retry: false, gcTime: Infinity } },
+      });
+      const response = new Response("obsolete file");
+      const readBody = rs.spyOn(response, "arrayBuffer");
+      const aborted = rs.fn();
+      let finishRead!: (response: Response) => void;
+      let failRead!: (reason: unknown) => void;
+      const pendingRead = new Promise<Response>((resolve, reject) => {
+        finishRead = resolve;
+        failRead = reject;
+      });
+      // Model fetch's native abort behavior at the HTTP boundary; the hook,
+      // loader and authenticated fetch wrapper all remain production code.
+      rs.spyOn(globalThis, "fetch").mockImplementation(async (_url, init) => {
+        if (full && new Headers(init?.headers).has("Range")) {
+          return new Response("preview", {
+            status: 206,
+            headers: { "Content-Range": "bytes 0-6/100" },
+          });
+        }
+        init?.signal?.addEventListener(
+          "abort",
+          () => {
+            aborted();
+            failRead(new DOMException("Aborted", "AbortError"));
+          },
+          { once: true },
+        );
+        return pendingRead;
+      });
+
+      function Wrapper({ children }: PropsWithChildren) {
+        return (
+          <QueryClientProvider client={client}>{children}</QueryClientProvider>
+        );
+      }
+
+      const view = renderHook(() => useContent(), { wrapper: Wrapper });
+      try {
+        if (full) {
+          await waitFor(() => expect(view.result.current.truncated).toBe(true));
+          act(() => view.result.current.loadFullContent());
+        }
+        await waitFor(() =>
+          expect(globalThis.fetch).toHaveBeenCalledTimes(full ? 2 : 1),
+        );
+
+        await act(async () => {
+          await client.cancelQueries({
+            queryKey: ["artifact", filepath, threadId, false],
+          });
+        });
+
+        expect(aborted).toHaveBeenCalledOnce();
+        expect(readBody).not.toHaveBeenCalled();
+        expect(view.result.current.error).toBeNull();
+        expect(client.isFetching()).toBe(0);
+      } finally {
+        finishRead(response);
+        view.unmount();
+        client.clear();
+      }
+    },
+  );
+});
diff --git a/frontend/tests/unit/core/artifacts/hooks.dom.test.tsx b/frontend/tests/unit/core/artifacts/hooks.dom.test.tsx
index c7b37eae24..3588d13b78 100644
--- a/frontend/tests/unit/core/artifacts/hooks.dom.test.tsx
+++ b/frontend/tests/unit/core/artifacts/hooks.dom.test.tsx
@@ -72,6 +72,7 @@ describe("useArtifactContent", () => {
         threadId: "thread-a",
         isMock: false,
         full: false,
+        signal: expect.any(AbortSignal),
       });
     });
 
@@ -85,6 +86,7 @@ describe("useArtifactContent", () => {
         threadId: "thread-a",
         isMock: false,
         full: true,
+        signal: expect.any(AbortSignal),
       });
     });
 
@@ -96,6 +98,7 @@ describe("useArtifactContent", () => {
         threadId: "thread-b",
         isMock: false,
         full: false,
+        signal: expect.any(AbortSignal),
       });
     });
   });