From b2090ec351d1e3c0c72308e61fc5fb326acc15e8 Mon Sep 17 00:00:00 2001
From: Dotta <bippadotta@protonmail.com>
Date: Tue, 16 Jun 2026 14:11:40 +0000
Subject: [PATCH] Show document annotation save failures

---
 ui/src/components/DocumentAnnotationPanel.tsx |  44 ++++--
 .../IssueDocumentAnnotations.test.tsx         | 126 +++++++++++++++++-
 2 files changed, 151 insertions(+), 19 deletions(-)

diff --git a/ui/src/components/DocumentAnnotationPanel.tsx b/ui/src/components/DocumentAnnotationPanel.tsx
index 4ffa4d4bef5..25903d11756 100644
--- a/ui/src/components/DocumentAnnotationPanel.tsx
+++ b/ui/src/components/DocumentAnnotationPanel.tsx
@@ -105,6 +105,7 @@ function AnnotationPanelBody(props: AnnotationPanelProps) {
   const queryClient = useQueryClient();
   const [composerValue, setComposerValue] = useState("");
   const [replyDrafts, setReplyDrafts] = useState<Record<string, string>>({});
+  const [mutationError, setMutationError] = useState<string | null>(null);
   const composerRef = useRef<HTMLTextAreaElement | null>(null);
   const bodyTestId = props.isMobile ? "document-annotation-panel" : undefined;
   const annotationTarget = useMemo<DocumentAnnotationTarget>(() => {
@@ -182,6 +183,7 @@ function AnnotationPanelBody(props: AnnotationPanelProps) {
     onMutate: async (body: string) => {
       const anchor = props.pendingAnchor;
       if (!anchor || !props.baseRevisionId) return undefined;
+      setMutationError(null);
       await queryClient.cancelQueries({ queryKey: annotationsQueryKey });
       const previous = queryClient.getQueryData<DocumentAnnotationThreadWithComments[]>(annotationsQueryKey);
       const optimisticThread = buildOptimisticThread({
@@ -199,16 +201,16 @@ function AnnotationPanelBody(props: AnnotationPanelProps) {
         annotationsQueryKey,
         (current) => [...(current ?? []), optimisticThread],
       );
-      // Clear the composer immediately — the optimistic thread now carries the text.
-      props.onClearPendingAnchor();
-      setComposerValue("");
       props.onFocusThread(optimisticThread.id);
       return { previous, optimisticId: optimisticThread.id };
     },
-    onError: (_error, _body, context) => {
+    onError: (error, _body, context) => {
       if (context?.previous) {
         queryClient.setQueryData(annotationsQueryKey, context.previous);
       }
+      setMutationError(error instanceof Error && error.message
+        ? error.message
+        : "Failed to create comment.");
     },
     onSuccess: (thread, _body, context) => {
       // Swap the optimistic placeholder for the real thread before refetch settles.
@@ -218,6 +220,9 @@ function AnnotationPanelBody(props: AnnotationPanelProps) {
           entry.id === context?.optimisticId ? thread : entry,
         ),
       );
+      props.onClearPendingAnchor();
+      setComposerValue("");
+      setMutationError(null);
       props.onFocusThread(thread.id);
     },
     onSettled: () => invalidateAll(),
@@ -228,6 +233,7 @@ function AnnotationPanelBody(props: AnnotationPanelProps) {
       documentAnnotationsApi.addCommentForTarget(annotationTarget, threadId, { body }),
     // Optimistically append the reply so it stays on screen through the round-trip.
     onMutate: async ({ threadId, body }) => {
+      setMutationError(null);
       await queryClient.cancelQueries({ queryKey: annotationsQueryKey });
       const previous = queryClient.getQueryData<DocumentAnnotationThreadWithComments[]>(annotationsQueryKey);
       const optimisticComment = buildOptimisticComment({
@@ -244,13 +250,19 @@ function AnnotationPanelBody(props: AnnotationPanelProps) {
             : thread,
         ),
       );
-      setReplyDrafts((current) => ({ ...current, [threadId]: "" }));
       return { previous };
     },
-    onError: (_error, _variables, context) => {
+    onError: (error, _variables, context) => {
       if (context?.previous) {
         queryClient.setQueryData(annotationsQueryKey, context.previous);
       }
+      setMutationError(error instanceof Error && error.message
+        ? error.message
+        : "Failed to add reply.");
+    },
+    onSuccess: (_comment, variables) => {
+      setReplyDrafts((current) => ({ ...current, [variables.threadId]: "" }));
+      setMutationError(null);
     },
     onSettled: () => invalidateAll(),
   });
@@ -259,6 +271,7 @@ function AnnotationPanelBody(props: AnnotationPanelProps) {
     mutationFn: ({ threadId, status }: { threadId: string; status: DocumentAnnotationThreadStatus }) =>
       documentAnnotationsApi.updateStatusForTarget(annotationTarget, threadId, status),
     onMutate: async ({ threadId, status }) => {
+      setMutationError(null);
       await queryClient.cancelQueries({ queryKey: annotationsQueryKey });
       const previous = queryClient.getQueryData<DocumentAnnotationThreadWithComments[]>(annotationsQueryKey);
       queryClient.setQueryData<DocumentAnnotationThreadWithComments[]>(
@@ -269,11 +282,15 @@ function AnnotationPanelBody(props: AnnotationPanelProps) {
       );
       return { previous };
     },
-    onError: (_error, _variables, context) => {
+    onError: (error, _variables, context) => {
       if (context?.previous) {
         queryClient.setQueryData(annotationsQueryKey, context.previous);
       }
+      setMutationError(error instanceof Error && error.message
+        ? error.message
+        : "Failed to update comment status.");
     },
+    onSuccess: () => setMutationError(null),
     onSettled: () => invalidateAll(),
   });
 
@@ -336,6 +353,14 @@ function AnnotationPanelBody(props: AnnotationPanelProps) {
           {props.newCommentDisabledReason}
         </p>
       ) : null}
+      {mutationError ? (
+        <p
+          data-testid="document-annotation-error"
+          className="border-b border-border bg-destructive/10 px-3 py-1.5 text-[11px] text-destructive"
+        >
+          {mutationError}
+        </p>
+      ) : null}
       <div ref={listScrollRef} className="min-h-0 flex-1 overflow-y-auto bg-popover px-3 py-2">
         {visibleThreads.length === 0 ? null : (
           <ul className="space-y-2">
@@ -409,11 +434,6 @@ function AnnotationPanelBody(props: AnnotationPanelProps) {
             disabled={props.newCommentDisabled}
             className="resize-y rounded-none text-sm"
           />
-          {createThread.isError ? (
-            <p className="mt-1 text-xs text-destructive">
-              {(createThread.error as Error).message || "Failed to create comment"}
-            </p>
-          ) : null}
           <div className="mt-2 flex items-center justify-end gap-2">
             <Button
               type="button"
diff --git a/ui/src/components/IssueDocumentAnnotations.test.tsx b/ui/src/components/IssueDocumentAnnotations.test.tsx
index e22d1d4d258..2df463d0aef 100644
--- a/ui/src/components/IssueDocumentAnnotations.test.tsx
+++ b/ui/src/components/IssueDocumentAnnotations.test.tsx
@@ -13,13 +13,31 @@ import {
   IssueDocumentAnnotations,
 } from "./IssueDocumentAnnotations";
 
-const mockAnnotationsApi = vi.hoisted(() => ({
-  list: vi.fn(),
-  get: vi.fn(),
-  create: vi.fn(),
-  addComment: vi.fn(),
-  updateStatus: vi.fn(),
-}));
+const mockAnnotationsApi = vi.hoisted(() => {
+  const api = {
+    list: vi.fn(),
+    listForTarget: vi.fn(),
+    get: vi.fn(),
+    getForTarget: vi.fn(),
+    create: vi.fn(),
+    createForTarget: vi.fn(),
+    addComment: vi.fn(),
+    addCommentForTarget: vi.fn(),
+    updateStatus: vi.fn(),
+    updateStatusForTarget: vi.fn(),
+  };
+  api.listForTarget.mockImplementation((target, options) =>
+    target.kind === "issue" ? api.list(target.issueId, target.documentKey, options) : api.list(target.routineId, target.documentKey, options));
+  api.getForTarget.mockImplementation((target, threadId) =>
+    target.kind === "issue" ? api.get(target.issueId, target.documentKey, threadId) : api.get(target.routineId, target.documentKey, threadId));
+  api.createForTarget.mockImplementation((target, data) =>
+    target.kind === "issue" ? api.create(target.issueId, target.documentKey, data) : api.create(target.routineId, target.documentKey, data));
+  api.addCommentForTarget.mockImplementation((target, threadId, data) =>
+    target.kind === "issue" ? api.addComment(target.issueId, target.documentKey, threadId, data) : api.addComment(target.routineId, target.documentKey, threadId, data));
+  api.updateStatusForTarget.mockImplementation((target, threadId, status) =>
+    target.kind === "issue" ? api.updateStatus(target.issueId, target.documentKey, threadId, status) : api.updateStatus(target.routineId, target.documentKey, threadId, status));
+  return api;
+});
 
 const mockPendingAnchor = vi.hoisted(() => ({
   selector: {
@@ -714,6 +732,54 @@ describe("IssueDocumentAnnotations", () => {
     expect(mockAnnotationsApi.list.mock.calls.length).toBeGreaterThan(1);
   });
 
+  it("keeps the composer visible with the draft when creating a thread fails", async () => {
+    mockAnnotationsApi.list.mockResolvedValue([]);
+    mockAnnotationsApi.create.mockRejectedValue(new Error("Annotation anchor does not match the current document revision"));
+    const root = createRoot(container);
+    const queryClient = makeQueryClient();
+    const doc = makeDoc();
+
+    await act(async () => {
+      root.render(
+        <QueryClientProvider client={queryClient}>
+          <Harness doc={doc} initialPanelOpen />
+        </QueryClientProvider>,
+      );
+    });
+    await flush();
+    await flush();
+
+    const selectButton = container.querySelector('[data-testid="mock-annotation-selection"]') as HTMLButtonElement | null;
+    expect(selectButton).not.toBeNull();
+    await act(async () => {
+      selectButton!.click();
+    });
+    await flush();
+
+    const composer = container.querySelector('[data-testid="document-annotation-composer"]') as HTMLTextAreaElement | null;
+    expect(composer).not.toBeNull();
+    await act(async () => {
+      setTextareaValue(composer!, "New anchored comment");
+    });
+    await flush();
+
+    const submit = Array.from(container.querySelectorAll("button")).find(
+      (button) => button.textContent === "Comment",
+    );
+    expect(submit).not.toBeUndefined();
+    await act(async () => {
+      submit!.click();
+    });
+    await flush();
+    await flush();
+
+    const composerAfterFailure = container.querySelector('[data-testid="document-annotation-composer"]') as HTMLTextAreaElement | null;
+    expect(composerAfterFailure).not.toBeNull();
+    expect(composerAfterFailure!.value).toBe("New anchored comment");
+    expect(container.querySelector('[data-testid="document-annotation-error"]')?.textContent)
+      .toContain("Annotation anchor does not match the current document revision");
+  });
+
   it("submits a new anchored comment with ⌘↵", async () => {
     mockAnnotationsApi.list.mockResolvedValue([]);
     mockAnnotationsApi.create.mockResolvedValue(makeThread({ id: "created-1" }));
@@ -787,6 +853,52 @@ describe("IssueDocumentAnnotations", () => {
     });
   });
 
+  it("keeps a reply draft visible when submitting the reply fails", async () => {
+    mockAnnotationsApi.list.mockResolvedValue([makeThread({ id: "open-1" })]);
+    mockAnnotationsApi.addComment.mockRejectedValue(new Error("Failed to add reply"));
+    const root = createRoot(container);
+    const queryClient = makeQueryClient();
+    const doc = makeDoc();
+
+    await act(async () => {
+      root.render(
+        <QueryClientProvider client={queryClient}>
+          <Harness doc={doc} initialPanelOpen />
+        </QueryClientProvider>,
+      );
+    });
+    await flush();
+    await flush();
+
+    const openThread = container.querySelector('[data-thread-id="open-1"]') as HTMLElement | null;
+    expect(openThread).not.toBeNull();
+    await act(async () => openThread!.click());
+    await flush();
+
+    const reply = container.querySelector(
+      '[data-testid="document-annotation-reply-open-1"]',
+    ) as HTMLTextAreaElement | null;
+    expect(reply).not.toBeNull();
+    await act(async () => setTextareaValue(reply!, "Reply should stay visible"));
+    await flush();
+
+    const replyButton = Array.from(container.querySelectorAll("button")).find(
+      (button) => button.textContent === "Reply",
+    );
+    expect(replyButton).not.toBeUndefined();
+    await act(async () => replyButton!.click());
+    await flush();
+    await flush();
+
+    const replyAfterFailure = container.querySelector(
+      '[data-testid="document-annotation-reply-open-1"]',
+    ) as HTMLTextAreaElement | null;
+    expect(replyAfterFailure).not.toBeNull();
+    expect(replyAfterFailure!.value).toBe("Reply should stay visible");
+    expect(container.querySelector('[data-testid="document-annotation-error"]')?.textContent)
+      .toContain("Failed to add reply");
+  });
+
   it("shows resolve and reopen actions and updates thread status", async () => {
     mockAnnotationsApi.list.mockResolvedValue([
       makeThread({ id: "open-1" }),