Make rebase conflict resolution message read-only (#871)

2025-09-29 17:44:16 +01:00
parent 6f2d6d4e40
commit bcd6bdbe05
10 changed files with 254 additions and 168 deletions
--- a/frontend/src/components/dialogs/tasks/EditorSelectionDialog.tsx
+++ b/frontend/src/components/dialogs/tasks/EditorSelectionDialog.tsx
@@ -15,77 +15,80 @@ import {
  SelectTrigger,
  SelectValue,
 } from '@/components/ui/select';
-import { EditorType, TaskAttempt } from 'shared/types';
+import { EditorType } from 'shared/types';
 import { useOpenInEditor } from '@/hooks/useOpenInEditor';
 import NiceModal, { useModal } from '@ebay/nice-modal-react';

 export interface EditorSelectionDialogProps {
-  selectedAttempt: TaskAttempt | null;
+  selectedAttemptId?: string;
+  filePath?: string;
 }

 export const EditorSelectionDialog =
-  NiceModal.create<EditorSelectionDialogProps>(({ selectedAttempt }) => {
-    const modal = useModal();
-    const handleOpenInEditor = useOpenInEditor(selectedAttempt, () =>
-      modal.hide()
-    );
-    const [selectedEditor, setSelectedEditor] = useState<EditorType>(
-      EditorType.VS_CODE
-    );
+  NiceModal.create<EditorSelectionDialogProps>(
+    ({ selectedAttemptId, filePath }) => {
+      const modal = useModal();
+      const handleOpenInEditor = useOpenInEditor(selectedAttemptId, () =>
+        modal.hide()
+      );
+      const [selectedEditor, setSelectedEditor] = useState<EditorType>(
+        EditorType.VS_CODE
+      );

-    const handleConfirm = () => {
-      handleOpenInEditor(selectedEditor);
-      modal.resolve(selectedEditor);
-      modal.hide();
-    };
+      const handleConfirm = () => {
+        handleOpenInEditor({ editorType: selectedEditor, filePath });
+        modal.resolve(selectedEditor);
+        modal.hide();
+      };

-    const handleCancel = () => {
-      modal.resolve(null);
-      modal.hide();
-    };
+      const handleCancel = () => {
+        modal.resolve(null);
+        modal.hide();
+      };

-    return (
-      <Dialog
-        open={modal.visible}
-        onOpenChange={(open) => !open && handleCancel()}
-      >
-        <DialogContent className="sm:max-w-[425px]">
-          <DialogHeader>
-            <DialogTitle>Choose Editor</DialogTitle>
-            <DialogDescription>
-              The default editor failed to open. Please select an alternative
-              editor to open the task worktree.
-            </DialogDescription>
-          </DialogHeader>
-          <div className="grid gap-4 py-4">
-            <div className="space-y-2">
-              <label className="text-sm font-medium">Editor</label>
-              <Select
-                value={selectedEditor}
-                onValueChange={(value) =>
-                  setSelectedEditor(value as EditorType)
-                }
-              >
-                <SelectTrigger>
-                  <SelectValue />
-                </SelectTrigger>
-                <SelectContent>
-                  {Object.values(EditorType).map((editor) => (
-                    <SelectItem key={editor} value={editor}>
-                      {editor}
-                    </SelectItem>
-                  ))}
-                </SelectContent>
-              </Select>
+      return (
+        <Dialog
+          open={modal.visible}
+          onOpenChange={(open) => !open && handleCancel()}
+        >
+          <DialogContent className="sm:max-w-[425px]">
+            <DialogHeader>
+              <DialogTitle>Choose Editor</DialogTitle>
+              <DialogDescription>
+                The default editor failed to open. Please select an alternative
+                editor to open the task worktree.
+              </DialogDescription>
+            </DialogHeader>
+            <div className="grid gap-4 py-4">
+              <div className="space-y-2">
+                <label className="text-sm font-medium">Editor</label>
+                <Select
+                  value={selectedEditor}
+                  onValueChange={(value) =>
+                    setSelectedEditor(value as EditorType)
+                  }
+                >
+                  <SelectTrigger>
+                    <SelectValue />
+                  </SelectTrigger>
+                  <SelectContent>
+                    {Object.values(EditorType).map((editor) => (
+                      <SelectItem key={editor} value={editor}>
+                        {editor}
+                      </SelectItem>
+                    ))}
+                  </SelectContent>
+                </Select>
+              </div>
            </div>
-          </div>
-          <DialogFooter>
-            <Button variant="outline" onClick={handleCancel}>
-              Cancel
-            </Button>
-            <Button onClick={handleConfirm}>Open Editor</Button>
-          </DialogFooter>
-        </DialogContent>
-      </Dialog>
-    );
-  });
+            <DialogFooter>
+              <Button variant="outline" onClick={handleCancel}>
+                Cancel
+              </Button>
+              <Button onClick={handleConfirm}>Open Editor</Button>
+            </DialogFooter>
+          </DialogContent>
+        </Dialog>
+      );
+    }
+  );
--- a/frontend/src/components/tasks/AttemptHeaderCard.tsx
+++ b/frontend/src/components/tasks/AttemptHeaderCard.tsx
@@ -44,7 +44,7 @@ export function AttemptHeaderCard({
  } = useDevServer(selectedAttempt?.id);
  const rebaseMutation = useRebase(selectedAttempt?.id, projectId);
  const mergeMutation = useMerge(selectedAttempt?.id);
-  const openInEditor = useOpenInEditor(selectedAttempt);
+  const openInEditor = useOpenInEditor(selectedAttempt?.id);
  const { fileCount, added, deleted } = useDiffSummary(
    selectedAttempt?.id ?? null
  );
--- a/frontend/src/components/tasks/ConflictBanner.tsx
+++ b/frontend/src/components/tasks/ConflictBanner.tsx
@@ -7,11 +7,12 @@ export type Props = Readonly<{
  attemptBranch: string | null;
  baseBranch?: string;
  conflictedFiles: readonly string[];
-  isEditable: boolean;
  onOpenEditor: () => void;
-  onInsertInstructions: () => void;
  onAbort: () => void;
  op?: ConflictOp | null;
+  onResolve?: () => void;
+  enableResolve: boolean;
+  enableAbort: boolean;
 }>;

 const MAX_VISIBLE_FILES = 8;
@@ -40,11 +41,12 @@ export function ConflictBanner({
  attemptBranch,
  baseBranch,
  conflictedFiles,
-  isEditable,
  onOpenEditor,
-  onInsertInstructions,
  onAbort,
  op,
+  onResolve,
+  enableResolve,
+  enableAbort,
 }: Props) {
  const { full: opTitle, lower: opTitleLower } = getOperationTitle(op);
  const {
@@ -59,12 +61,15 @@ export function ConflictBanner({

  return (
    <div
-      className="flex flex-col gap-2 rounded-md border border-yellow-300 bg-yellow-50 p-3 text-yellow-900"
+      className="flex flex-col gap-2 rounded-md border border-warning/40 bg-warning/10 p-3 text-warning-foreground dark:text-warning"
      role="status"
      aria-live="polite"
    >
      <div className="flex items-start gap-2">
-        <AlertCircle className="mt-0.5 h-4 w-4 text-yellow-700" aria-hidden />
+        <AlertCircle
+          className="mt-0.5 h-4 w-4 text-warning dark:text-warning/90"
+          aria-hidden
+        />
        <div className="text-sm leading-relaxed">
          <span>{heading}</span>{' '}
          <span>
@@ -72,7 +77,7 @@ export function ConflictBanner({
            until you resolve the conflicts or abort the {opTitleLower}.
          </span>
          {visibleFiles.length > 0 && (
-            <div className="mt-1 text-xs text-yellow-800">
+            <div className="mt-1 text-xs text-warning-foreground/90 dark:text-warning/80">
              <div className="font-medium">
                Conflicted files ({visibleFiles.length}
                {hasMore ? ` of ${total}` : ''}):
@@ -90,10 +95,20 @@ export function ConflictBanner({
      </div>

      <div className="flex flex-wrap gap-2">
+        {onResolve && (
+          <Button
+            size="sm"
+            onClick={onResolve}
+            disabled={!enableResolve}
+            className="bg-warning text-warning-foreground hover:bg-warning/90"
+          >
+            Resolve conflicts
+          </Button>
+        )}
        <Button
          size="sm"
          variant="outline"
-          className="border-yellow-300 text-yellow-800 hover:bg-yellow-100"
+          className="border-warning/40 text-warning-foreground hover:bg-warning/10 dark:text-warning/90"
          onClick={onOpenEditor}
        >
          Open in Editor
@@ -102,19 +117,10 @@ export function ConflictBanner({
        <Button
          size="sm"
          variant="outline"
-          className="border-yellow-300 text-yellow-800 hover:bg-yellow-100"
-          onClick={onInsertInstructions}
-          disabled={!isEditable}
-          aria-disabled={!isEditable}
-        >
-          Insert Resolve-Conflicts Instructions
-        </Button>
-
-        <Button
-          size="sm"
-          variant="outline"
-          className="border-red-300 text-red-700 hover:bg-red-50"
+          className="border-destructive/40 text-destructive hover:bg-destructive/10"
          onClick={onAbort}
+          disabled={!enableAbort}
+          aria-disabled={!enableAbort}
        >
          Abort {opTitle}
        </Button>
--- a/frontend/src/components/tasks/TaskFollowUpSection.tsx
+++ b/frontend/src/components/tasks/TaskFollowUpSection.tsx
@@ -30,6 +30,7 @@ import { useDraftAutosave } from '@/hooks/follow-up/useDraftAutosave';
 import { useDraftQueue } from '@/hooks/follow-up/useDraftQueue';
 import { useFollowUpSend } from '@/hooks/follow-up/useFollowUpSend';
 import { useDefaultVariant } from '@/hooks/follow-up/useDefaultVariant';
+import { buildResolveConflictsInstructions } from '@/lib/conflicts';

 interface TaskFollowUpSectionProps {
  task: TaskWithAttemptStatus;
@@ -56,6 +57,23 @@ export function TaskFollowUpSection({
    [generateReviewMarkdown, comments]
  );

+  // Non-editable conflict resolution instructions (derived, like review comments)
+  const conflictResolutionInstructions = useMemo(() => {
+    const hasConflicts = (branchStatus?.conflicted_files?.length ?? 0) > 0;
+    if (!hasConflicts) return null;
+    return buildResolveConflictsInstructions(
+      attemptBranch,
+      branchStatus?.base_branch_name,
+      branchStatus?.conflicted_files || [],
+      branchStatus?.conflict_op ?? null
+    );
+  }, [
+    attemptBranch,
+    branchStatus?.base_branch_name,
+    branchStatus?.conflicted_files,
+    branchStatus?.conflict_op,
+  ]);
+
  // Draft stream and synchronization
  const {
    draft,
@@ -135,6 +153,7 @@ export function TaskFollowUpSection({
    useFollowUpSend({
      attemptId: selectedAttemptId,
      message: followUpMessage,
+      conflictMarkdown: conflictResolutionInstructions,
      reviewMarkdown,
      selectedVariant,
      images,
@@ -176,23 +195,22 @@ export function TaskFollowUpSection({
      return false;
    }

-    // Allow sending if either review comments exist OR follow-up message is present
-    return Boolean(reviewMarkdown || followUpMessage.trim());
-  }, [canTypeFollowUp, reviewMarkdown, followUpMessage]);
+    // Allow sending if conflict instructions or review comments exist, or message is present
+    return Boolean(
+      conflictResolutionInstructions || reviewMarkdown || followUpMessage.trim()
+    );
+  }, [
+    canTypeFollowUp,
+    conflictResolutionInstructions,
+    reviewMarkdown,
+    followUpMessage,
+  ]);
  // currentProfile is provided by useDefaultVariant

  const isDraftLocked =
    displayQueued || isQueuing || isUnqueuing || !!draft?.sending;
  const isEditable = isDraftLoaded && !isDraftLocked;

-  const appendToFollowUpMessage = (text: string) => {
-    setFollowUpMessage((prev) => {
-      const sep =
-        prev.trim().length === 0 ? '' : prev.endsWith('\n') ? '\n' : '\n\n';
-      return prev + sep + text;
-    });
-  };
-
  // When a process completes (e.g., agent resolved conflicts), refresh branch status promptly
  const prevRunningRef = useRef<boolean>(isAttemptRunning);
  useEffect(() => {
@@ -284,21 +302,27 @@ export function TaskFollowUpSection({
            )}

            {/* Conflict notice and actions (optional UI) */}
-            <FollowUpConflictSection
-              selectedAttemptId={selectedAttemptId}
-              attemptBranch={attemptBranch}
-              branchStatus={branchStatus}
-              isEditable={isEditable}
-              appendInstructions={appendToFollowUpMessage}
-              refetchBranchStatus={refetchBranchStatus}
-            />
+            {branchStatus && (
+              <FollowUpConflictSection
+                selectedAttemptId={selectedAttemptId}
+                attemptBranch={attemptBranch}
+                branchStatus={branchStatus}
+                isEditable={isEditable}
+                onResolve={onSendFollowUp}
+                enableResolve={
+                  canSendFollowUp && !isAttemptRunning && isEditable
+                }
+                enableAbort={canSendFollowUp && !isAttemptRunning}
+                conflictResolutionInstructions={conflictResolutionInstructions}
+              />
+            )}

            <div className="flex flex-col gap-2">
              <FollowUpEditorCard
                placeholder={
                  isQueued
                    ? 'Type your follow-up… It will auto-send when ready.'
-                    : reviewMarkdown
+                    : reviewMarkdown || conflictResolutionInstructions
                      ? '(Optional) Add additional instructions... Type @ to search files.'
                      : 'Continue working on this task attempt... Type @ to search files.'
                }
@@ -389,7 +413,9 @@ export function TaskFollowUpSection({
                      ) : (
                        <>
                          <Send className="h-4 w-4 mr-2" />
-                          Send
+                          {conflictResolutionInstructions
+                            ? 'Resolve conflicts'
+                            : 'Send'}
                        </>
                      )}
                    </Button>
--- a/frontend/src/components/tasks/Toolbar/CurrentAttempt.tsx
+++ b/frontend/src/components/tasks/Toolbar/CurrentAttempt.tsx
@@ -121,7 +121,7 @@ function CurrentAttempt({
    () => displayConflictOpLabel(branchStatus?.conflict_op),
    [branchStatus?.conflict_op]
  );
-  const handleOpenInEditor = useOpenInEditor(selectedAttempt);
+  const handleOpenInEditor = useOpenInEditor(selectedAttempt?.id);
  const { jumpToProcess } = useProcessSelection();

  // Attempt action hooks
--- a/frontend/src/components/tasks/follow-up/FollowUpConflictSection.tsx
+++ b/frontend/src/components/tasks/follow-up/FollowUpConflictSection.tsx
@@ -1,72 +1,85 @@
-import { useCallback } from 'react';
-import { attemptsApi } from '@/lib/api';
+import { useEffect, useRef, useState } from 'react';
 import { ConflictBanner } from '@/components/tasks/ConflictBanner';
-import { buildResolveConflictsInstructions } from '@/lib/conflicts';
+import { useOpenInEditor } from '@/hooks/useOpenInEditor';
+import { useAttemptConflicts } from '@/hooks/useAttemptConflicts';
 import type { BranchStatus } from 'shared/types';

 type Props = {
  selectedAttemptId?: string;
  attemptBranch: string | null;
-  branchStatus?: BranchStatus;
+  branchStatus: BranchStatus;
  isEditable: boolean;
-  appendInstructions: (text: string) => void;
-  refetchBranchStatus: () => void;
+  onResolve?: () => void;
+  enableResolve: boolean;
+  enableAbort: boolean;
+  conflictResolutionInstructions: string | null;
 };

 export function FollowUpConflictSection({
  selectedAttemptId,
  attemptBranch,
  branchStatus,
-  isEditable,
-  appendInstructions,
-  refetchBranchStatus,
+  onResolve,
+  enableResolve,
+  enableAbort,
+  conflictResolutionInstructions,
 }: Props) {
-  const op = branchStatus?.conflict_op ?? null;
-  const handleInsertInstructions = useCallback(() => {
-    const template = buildResolveConflictsInstructions(
-      attemptBranch,
-      branchStatus?.base_branch_name,
-      branchStatus?.conflicted_files || [],
-      op
-    );
-    appendInstructions(template);
-  }, [
-    attemptBranch,
-    branchStatus?.base_branch_name,
-    branchStatus?.conflicted_files,
-    op,
-    appendInstructions,
-  ]);
+  const op = branchStatus.conflict_op ?? null;
+  const openInEditor = useOpenInEditor(selectedAttemptId);
+  const { abortConflicts } = useAttemptConflicts(selectedAttemptId);

-  const hasConflicts = (branchStatus?.conflicted_files?.length ?? 0) > 0;
-  if (!hasConflicts) return null;
+  // write using setAborting and read through abortingRef in async handlers
+  const [aborting, setAborting] = useState(false);
+  const abortingRef = useRef(false);
+  useEffect(() => {
+    abortingRef.current = aborting;
+  }, [aborting]);
+
+  if (
+    !branchStatus.is_rebase_in_progress &&
+    !branchStatus.conflicted_files?.length
+  )
+    return null;

  return (
-    <ConflictBanner
-      attemptBranch={attemptBranch}
-      baseBranch={branchStatus?.base_branch_name}
-      conflictedFiles={branchStatus?.conflicted_files || []}
-      isEditable={isEditable}
-      op={op}
-      onOpenEditor={async () => {
-        if (!selectedAttemptId) return;
-        try {
-          const first = branchStatus?.conflicted_files?.[0];
-          await attemptsApi.openEditor(selectedAttemptId, undefined, first);
-        } catch (e) {
-          console.error('Failed to open editor', e);
-        }
-      }}
-      onInsertInstructions={handleInsertInstructions}
-      onAbort={async () => {
-        if (!selectedAttemptId) return;
-        try {
-          await attemptsApi.abortConflicts(selectedAttemptId);
-          refetchBranchStatus();
-        } catch (e) {
-          console.error('Failed to abort conflicts', e);
-        }
-      }}
-    />
+    <>
+      <ConflictBanner
+        attemptBranch={attemptBranch}
+        baseBranch={branchStatus.base_branch_name}
+        conflictedFiles={branchStatus.conflicted_files || []}
+        op={op}
+        onResolve={onResolve}
+        enableResolve={enableResolve && !aborting}
+        onOpenEditor={() => {
+          if (!selectedAttemptId) return;
+          const first = branchStatus.conflicted_files?.[0];
+          openInEditor(first ? { filePath: first } : undefined);
+        }}
+        onAbort={async () => {
+          if (!selectedAttemptId) return;
+          if (!enableAbort || abortingRef.current) return;
+          try {
+            setAborting(true);
+            await abortConflicts();
+          } catch (e) {
+            console.error('Failed to abort conflicts', e);
+          } finally {
+            setAborting(false);
+          }
+        }}
+        enableAbort={enableAbort && !aborting}
+      />
+      {/* Conflict instructions preview (non-editable) */}
+      {conflictResolutionInstructions && enableResolve && (
+        <div className="text-sm mb-4">
+          <div className="text-xs font-medium text-warning-foreground dark:text-warning mb-1">
+            Conflict resolution instructions
+          </div>
+          <div className="whitespace-pre-wrap">
+            {conflictResolutionInstructions}
+          </div>
+        </div>
+      )}
+    </>
  );
 }
--- a/frontend/src/hooks/follow-up/useFollowUpSend.ts
+++ b/frontend/src/hooks/follow-up/useFollowUpSend.ts
@@ -5,6 +5,7 @@ import type { ImageResponse } from 'shared/types';
 type Args = {
  attemptId?: string;
  message: string;
+  conflictMarkdown: string | null;
  reviewMarkdown: string;
  selectedVariant: string | null;
  images: ImageResponse[];
@@ -18,6 +19,7 @@ type Args = {
 export function useFollowUpSend({
  attemptId,
  message,
+  conflictMarkdown,
  reviewMarkdown,
  selectedVariant,
  images,
@@ -33,7 +35,7 @@ export function useFollowUpSend({
  const onSendFollowUp = useCallback(async () => {
    if (!attemptId) return;
    const extraMessage = message.trim();
-    const finalPrompt = [reviewMarkdown, extraMessage]
+    const finalPrompt = [conflictMarkdown, reviewMarkdown, extraMessage]
      .filter(Boolean)
      .join('\n\n');
    if (!finalPrompt) return;
@@ -66,6 +68,7 @@ export function useFollowUpSend({
  }, [
    attemptId,
    message,
+    conflictMarkdown,
    reviewMarkdown,
    newlyUploadedImageIds,
    images,
--- a/frontend/src/hooks/index.ts
+++ b/frontend/src/hooks/index.ts
@@ -6,3 +6,4 @@ export { useRebase } from './useRebase';
 export { useMerge } from './useMerge';
 export { usePush } from './usePush';
 export { useKeyboardShortcut } from './useKeyboardShortcut';
+export { useAttemptConflicts } from './useAttemptConflicts';
--- a/frontend/src/hooks/useAttemptConflicts.ts
+++ b/frontend/src/hooks/useAttemptConflicts.ts
@@ -0,0 +1,17 @@
+import { useCallback } from 'react';
+import { useQueryClient } from '@tanstack/react-query';
+import { attemptsApi } from '@/lib/api';
+
+export function useAttemptConflicts(attemptId?: string) {
+  const queryClient = useQueryClient();
+
+  const abortConflicts = useCallback(async () => {
+    if (!attemptId) return;
+    await attemptsApi.abortConflicts(attemptId);
+    await queryClient.invalidateQueries({
+      queryKey: ['branchStatus', attemptId],
+    });
+  }, [attemptId, queryClient]);
+
+  return { abortConflicts } as const;
+}
--- a/frontend/src/hooks/useOpenInEditor.ts
+++ b/frontend/src/hooks/useOpenInEditor.ts
@@ -1,24 +1,38 @@
 import { useCallback } from 'react';
 import { attemptsApi } from '@/lib/api';
 import NiceModal from '@ebay/nice-modal-react';
-import type { EditorType, TaskAttempt } from 'shared/types';
+import type { EditorType } from 'shared/types';
+
+type OpenEditorOptions = {
+  editorType?: EditorType;
+  filePath?: string;
+};

 export function useOpenInEditor(
-  attempt: TaskAttempt | null,
+  attemptId?: string,
  onShowEditorDialog?: () => void
 ) {
  return useCallback(
-    async (editorType?: EditorType) => {
-      if (!attempt) return;
+    async (options?: OpenEditorOptions): Promise<void> => {
+      if (!attemptId) return;
+
+      const { editorType, filePath } = options ?? {};

      try {
-        const result = await attemptsApi.openEditor(attempt.id, editorType);
+        const result = await attemptsApi.openEditor(
+          attemptId,
+          editorType,
+          filePath
+        );

        if (result === undefined && !editorType) {
          if (onShowEditorDialog) {
            onShowEditorDialog();
          } else {
-            NiceModal.show('editor-selection', { selectedAttempt: attempt });
+            NiceModal.show('editor-selection', {
+              selectedAttemptId: attemptId,
+              filePath,
+            });
          }
        }
      } catch (err) {
@@ -27,11 +41,14 @@ export function useOpenInEditor(
          if (onShowEditorDialog) {
            onShowEditorDialog();
          } else {
-            NiceModal.show('editor-selection', { selectedAttempt: attempt });
+            NiceModal.show('editor-selection', {
+              selectedAttemptId: attemptId,
+              filePath,
+            });
          }
        }
      }
    },
-    [attempt, onShowEditorDialog]
+    [attemptId, onShowEditorDialog]
  );
 }