fix(clients): preserve review drafts on dismissal

apps/mobile/src/features/review/ReviewCommentComposerSheet.tsx

@@ -22,7 +22,7 @@ import {
   formatReviewCommentContext,
   getReviewUnifiedLineNumber,
   getSelectedReviewCommentLines,
-  useReviewCommentTarget,
+  getReviewCommentTarget,
 } from "./reviewCommentSelection";
 import { useAppearanceCodeSurface } from "../settings/appearance/useAppearanceCodeSurface";
 import { useAppearancePreferences } from "../settings/appearance/AppearancePreferencesProvider";
@@ -32,6 +32,9 @@ import {
   type ReviewHighlightedToken,
 } from "./shikiReviewHighlighter";
 
+import { useReviewCommentDismissal } from "./useReviewCommentDismissal";
+import { useReviewCommentSubmission } from "./useReviewCommentSubmission";
+
 const REVIEW_COMMENT_PREVIEW_MAX_LINES = 5;
 
 type ReviewCommentComposerSheetProps = StaticScreenProps<{
@@ -45,14 +48,18 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp
   const insets = useSafeAreaInsets();
   const { width } = useWindowDimensions();
   const { themeAppearance: selectedTheme } = useAppearancePreferences();
-  const target = useReviewCommentTarget();
+  // Freeze the target and destination together for this editor instance.
+  const [{ target, environmentId, threadId }] = useState(() => ({
+    target: getReviewCommentTarget(),
+    ...props.route.params,
+  }));
   const { codeSurface } = useAppearanceCodeSurface();
-  const { environmentId, threadId } = props.route.params;
   const [commentText, setCommentText] = useState("");
   const [highlightedLinesById, setHighlightedLinesById] = useState<
     Record<string, ReadonlyArray<ReviewHighlightedToken>>
   >({});
   const [attachments, setAttachments] = useState<ReadonlyArray<DraftComposerImageAttachment>>([]);
+  const [pendingImages, setPendingImages] = useState(0);
   const [previewFile, setPreviewFile] = useState<FilePreviewSource | null>(null);
 
   const selectedLines = useMemo(
@@ -78,11 +85,25 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp
     codeSurface.rowHeight,
   );
   const previewViewportWidth = Math.max(width - 40, 280);
-  const dismissComposer = useCallback(() => {
-    clearReviewCommentTarget();
-    navigation.goBack();
-  }, [navigation]);
+  const { submitted, submit, accepted } = useReviewCommentSubmission();
+  useReviewCommentDismissal({
+    commentText,
+    attachmentCount: attachments.length,
+    pendingImages,
+    submitted,
+    accepted,
+  });
+  useEffect(
+    () => () => {
+      // A newer selection belongs to another editor, even if this route is removed later.
+      if (getReviewCommentTarget() === target) clearReviewCommentTarget();
+    },
+    [target],
+  );
+  const dismissComposer = useCallback(() => navigation.goBack(), [navigation]);
   const handleNativePaste = useNativePaste((uris) => {
+    if (submitted) return;
+    setPendingImages((count) => count + 1);
     void (async () => {
       try {
         const images = await convertPastedImagesToAttachments({
@@ -94,6 +115,8 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp
         }
       } catch (error) {
         console.error("[review comment] error converting pasted images", error);
+      } finally {
+        setPendingImages((count) => count - 1);
       }
     })();
   });
@@ -127,12 +150,18 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp
   }, [selectedLines, selectedTheme, target]);
 
   async function handlePickImages(): Promise<void> {
-    const result = await pickComposerImages({ existingCount: attachments.length });
-    if (result.images.length > 0) {
-      setAttachments((current) => [...current, ...result.images]);
-    }
-    if (result.error) {
-      setPendingConnectionError(result.error);
+    if (submitted) return;
+    setPendingImages((count) => count + 1);
+    try {
+      const result = await pickComposerImages({ existingCount: attachments.length });
+      if (result.images.length > 0) {
+        setAttachments((current) => [...current, ...result.images]);
+      }
+      if (result.error) {
+        setPendingConnectionError(result.error);
+      }
+    } finally {
+      setPendingImages((count) => count - 1);
     }
   }
 
@@ -141,15 +170,20 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp
       return;
     }
 
-    appendReviewCommentToDraft({
-      environmentId,
-      threadId,
-      text: formatReviewCommentContext(target, commentText),
-      attachments,
-    });
-    setAttachments([]);
-    dismissComposer();
-  }, [attachments, commentText, dismissComposer, environmentId, target, threadId]);
+    if (submitted || pendingImages > 0) return;
+    try {
+      submit(() =>
+        appendReviewCommentToDraft({
+          environmentId,
+          threadId,
+          text: formatReviewCommentContext(target, commentText),
+          attachments,
+        }),
+      );
+    } catch {
+      setPendingConnectionError("Could not add the comment. Your input has been kept; try again.");
+    }
+  }, [attachments, commentText, environmentId, pendingImages, submit, submitted, target, threadId]);
 
   return (
     <View className="flex-1 bg-sheet">
@@ -256,6 +290,7 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp
                     <TextInputWrapper onPaste={handleNativePaste} style={{ flex: 1, minHeight: 0 }}>
                       <TextInput
                         autoFocus
+                        editable={!submitted}
                         multiline
                         scrollEnabled
                         placeholder="Leave a comment..."
@@ -291,6 +326,7 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp
           <View className="flex-row items-center gap-3 bg-sheet px-5 py-2">
             <ControlPill
               accessibilityLabel="Add image"
+              disabled={submitted || pendingImages > 0}
               icon="plus"
               onPress={() => void handlePickImages()}
             />
@@ -300,7 +336,7 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp
               icon="arrow.up"
               label="Comment"
               variant="primary"
-              disabled={!canSubmit}
+              disabled={!canSubmit || submitted || pendingImages > 0}
               onPress={handleSubmit}
             />
           </View>
@@ -317,6 +353,7 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp
           >
             <ControlPill
               accessibilityLabel="Add image"
+              disabled={submitted || pendingImages > 0}
               icon="plus"
               onPress={() => void handlePickImages()}
             />
@@ -326,7 +363,7 @@ export function ReviewCommentComposerSheet(props: ReviewCommentComposerSheetProp
               icon="arrow.up"
               label="Comment"
               variant="primary"
-              disabled={!canSubmit}
+              disabled={!canSubmit || submitted || pendingImages > 0}
               onPress={handleSubmit}
             />
           </View>

apps/mobile/src/features/review/useReviewCommentDismissal.test.ts

@@ -0,0 +1,117 @@
+import { beforeEach, describe, expect, it, vi } from "vite-plus/test";
+import type { NavigationAction } from "@react-navigation/native";
+import type { AlertButton } from "react-native";
+
+const harness = vi.hoisted(() => ({
+  prevented: false,
+  onRemove: (_event: { data: { action: NavigationAction } }) => {},
+  confirmation: { current: false },
+  effects: [] as Array<() => unknown>,
+  buttons: [] as AlertButton[],
+  dispatch: vi.fn(),
+  goBack: vi.fn(),
+  alert: vi.fn(),
+}));
+vi.mock("react", () => ({
+  useRef: () => harness.confirmation,
+  useEffect: (effect: () => unknown) => harness.effects.push(effect),
+}));
+vi.mock("@react-navigation/native", () => ({
+  useNavigation: () => ({ dispatch: harness.dispatch, goBack: harness.goBack }),
+  usePreventRemove: (prevented: boolean, callback: typeof harness.onRemove) => {
+    harness.prevented = prevented;
+    harness.onRemove = callback;
+  },
+}));
+vi.mock("react-native", () => ({
+  Alert: {
+    alert: (...args: [string, string, AlertButton[]]) => {
+      harness.alert(...args);
+      harness.buttons = args[2];
+    },
+  },
+}));
+
+import { useReviewCommentDismissal } from "./useReviewCommentDismissal";
+
+function render(overrides: Partial<Parameters<typeof useReviewCommentDismissal>[0]> = {}) {
+  useReviewCommentDismissal({
+    commentText: "",
+    attachmentCount: 0,
+    pendingImages: 0,
+    submitted: false,
+    accepted: { current: false },
+    ...overrides,
+  });
+}
+const action: NavigationAction = { type: "GO_BACK", source: "review-editor" };
+function remove() {
+  if (harness.prevented) harness.onRemove({ data: { action } });
+}
+function choose(text: string) {
+  harness.buttons.find((button) => button.text === text)?.onPress?.();
+}
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  harness.confirmation.current = false;
+  harness.buttons = [];
+  harness.effects = [];
+});
+
+describe("review comment removal boundary", () => {
+  it("allows an empty editor to leave without confirmation", () => {
+    render();
+    remove();
+    expect(harness.prevented).toBe(false);
+    expect(harness.alert).not.toHaveBeenCalled();
+  });
+  it.each([{ commentText: "Unsent" }, { attachmentCount: 1 }])(
+    "protects invested input %j",
+    (input) => {
+      render(input);
+      remove();
+      remove();
+      expect(harness.prevented).toBe(true);
+      expect(harness.alert).toHaveBeenCalledTimes(1);
+      choose("Keep editing");
+      expect(harness.dispatch).not.toHaveBeenCalled();
+      remove();
+      expect(harness.alert).toHaveBeenCalledTimes(2);
+      choose("Discard");
+      expect(harness.dispatch).toHaveBeenCalledExactlyOnceWith(action);
+    },
+  );
+  it("keeps the sheet protected after a failed transfer", () => {
+    render({ commentText: "Unsent", attachmentCount: 1, submitted: false });
+    remove();
+    expect(harness.prevented).toBe(true);
+    expect(harness.alert).toHaveBeenCalledOnce();
+    expect(harness.goBack).not.toHaveBeenCalled();
+  });
+  it("blocks removal while selected images are still being prepared", () => {
+    render({ pendingImages: 1 });
+    remove();
+    expect(harness.prevented).toBe(true);
+    expect(harness.alert).not.toHaveBeenCalled();
+    expect(harness.dispatch).not.toHaveBeenCalled();
+  });
+  it("unlocks successful transfer without a discard prompt", () => {
+    render({ commentText: "Transferred", attachmentCount: 1, submitted: true });
+    harness.effects.forEach((effect) => effect());
+    expect(harness.goBack).toHaveBeenCalledOnce();
+    remove();
+    expect(harness.prevented).toBe(false);
+    expect(harness.alert).not.toHaveBeenCalled();
+  });
+});
+
+it("allows removal immediately after transfer, before the guard's render catches up", () => {
+  const accepted = { current: false };
+  render({ commentText: "Transferred", accepted });
+  expect(harness.prevented).toBe(true);
+  accepted.current = true;
+  remove();
+  expect(harness.dispatch).toHaveBeenCalledExactlyOnceWith(action);
+  expect(harness.alert).not.toHaveBeenCalled();
+});

apps/mobile/src/features/review/useReviewCommentDismissal.ts

@@ -0,0 +1,57 @@
+import { useNavigation, usePreventRemove } from "@react-navigation/native";
+import { useEffect, useRef, type RefObject } from "react";
+import { Alert } from "react-native";
+
+export function useReviewCommentDismissal({
+  commentText,
+  attachmentCount,
+  pendingImages,
+  submitted,
+  accepted,
+}: {
+  readonly commentText: string;
+  readonly attachmentCount: number;
+  readonly pendingImages: number;
+  readonly submitted: boolean;
+  readonly accepted: RefObject<boolean>;
+}) {
+  const navigation = useNavigation();
+  const confirmingDiscard = useRef(false);
+  usePreventRemove(
+    !submitted && (pendingImages > 0 || commentText.length > 0 || attachmentCount > 0),
+    ({ data }) => {
+      // The native removal guard can lag the successful synchronous transfer.
+      if (accepted.current) {
+        navigation.dispatch(data.action);
+        return;
+      }
+      if (pendingImages > 0 || confirmingDiscard.current) return;
+      confirmingDiscard.current = true;
+      Alert.alert(
+        "Discard comment?",
+        "Your comment and attachments have not been added to the draft.",
+        [
+          {
+            text: "Keep editing",
+            style: "cancel",
+            onPress: () => {
+              confirmingDiscard.current = false;
+            },
+          },
+          {
+            text: "Discard",
+            style: "destructive",
+            onPress: () => {
+              confirmingDiscard.current = false;
+              navigation.dispatch(data.action);
+            },
+          },
+        ],
+      );
+    },
+  );
+  useEffect(() => {
+    if (!submitted) return;
+    navigation.goBack();
+  }, [navigation, submitted]);
+}

apps/mobile/src/features/review/useReviewCommentSubmission.test.ts

@@ -0,0 +1,44 @@
+import { beforeEach, describe, expect, it, vi } from "vite-plus/test";
+
+const state = vi.hoisted(() => ({ accepted: { current: false }, setSubmitted: vi.fn() }));
+vi.mock("react", () => ({
+  useRef: () => state.accepted,
+  useState: () => [false, state.setSubmitted],
+  useCallback: (callback: () => unknown) => callback,
+}));
+import { useReviewCommentSubmission } from "./useReviewCommentSubmission";
+
+beforeEach(() => {
+  state.accepted.current = false;
+  vi.clearAllMocks();
+});
+describe("review comment submission", () => {
+  it("transfers exactly once when pressed twice before a rerender", () => {
+    const { submit } = useReviewCommentSubmission();
+    const transfer = vi.fn(() => true);
+    submit(transfer);
+    submit(transfer);
+    expect(transfer).toHaveBeenCalledOnce();
+    expect(state.setSubmitted).toHaveBeenCalledExactlyOnceWith(true);
+  });
+  it("keeps failure editable and permits a successful retry", () => {
+    const { submit } = useReviewCommentSubmission();
+    const transfer = vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true);
+    submit(transfer);
+    expect(state.setSubmitted).not.toHaveBeenCalled();
+    submit(transfer);
+    expect(transfer).toHaveBeenCalledTimes(2);
+    expect(state.setSubmitted).toHaveBeenCalledExactlyOnceWith(true);
+  });
+  it("unlocks after a thrown transfer without marking the input submitted", () => {
+    const { submit } = useReviewCommentSubmission();
+    expect(() =>
+      submit(() => {
+        throw new Error("Transfer failed");
+      }),
+    ).toThrow("Transfer failed");
+    expect(state.setSubmitted).not.toHaveBeenCalled();
+    submit(() => true);
+    expect(state.setSubmitted).toHaveBeenCalledExactlyOnceWith(true);
+  });
+});