From bcd6bdbe0591cbe2b79f3ee67020f0d66312be71 Mon Sep 17 00:00:00 2001 From: Solomon Date: Mon, 29 Sep 2025 17:44:16 +0100 Subject: [PATCH] Make rebase conflict resolution message read-only (#871) --- .../dialogs/tasks/EditorSelectionDialog.tsx | 129 +++++++++--------- .../components/tasks/AttemptHeaderCard.tsx | 2 +- .../src/components/tasks/ConflictBanner.tsx | 46 ++++--- .../components/tasks/TaskFollowUpSection.tsx | 68 ++++++--- .../tasks/Toolbar/CurrentAttempt.tsx | 2 +- .../follow-up/FollowUpConflictSection.tsx | 119 +++++++++------- .../src/hooks/follow-up/useFollowUpSend.ts | 5 +- frontend/src/hooks/index.ts | 1 + frontend/src/hooks/useAttemptConflicts.ts | 17 +++ frontend/src/hooks/useOpenInEditor.ts | 33 +++-- 10 files changed, 254 insertions(+), 168 deletions(-) create mode 100644 frontend/src/hooks/useAttemptConflicts.ts diff --git a/frontend/src/components/dialogs/tasks/EditorSelectionDialog.tsx b/frontend/src/components/dialogs/tasks/EditorSelectionDialog.tsx index a063a142..c384427c 100644 --- 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(({ selectedAttempt }) => { - const modal = useModal(); - const handleOpenInEditor = useOpenInEditor(selectedAttempt, () => - modal.hide() - ); - const [selectedEditor, setSelectedEditor] = useState( - EditorType.VS_CODE - ); + NiceModal.create( + ({ selectedAttemptId, filePath }) => { + const modal = useModal(); + const handleOpenInEditor = useOpenInEditor(selectedAttemptId, () => + modal.hide() + ); + const [selectedEditor, setSelectedEditor] = useState( + 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 ( - !open && handleCancel()} - > - - - Choose Editor - - The default editor failed to open. Please select an alternative - editor to open the task worktree. - - -
-
- - + return ( + !open && handleCancel()} + > + + + Choose Editor + + The default editor failed to open. Please select an alternative + editor to open the task worktree. + + +
+
+ + +
-
- - - - - -
- ); - }); + + + + + + + ); + } + ); diff --git a/frontend/src/components/tasks/AttemptHeaderCard.tsx b/frontend/src/components/tasks/AttemptHeaderCard.tsx index 7a48d00f..33b9c436 100644 --- 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 ); diff --git a/frontend/src/components/tasks/ConflictBanner.tsx b/frontend/src/components/tasks/ConflictBanner.tsx index cd94311b..3d46598a 100644 --- 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 (
- +
{heading}{' '} @@ -72,7 +77,7 @@ export function ConflictBanner({ until you resolve the conflicts or abort the {opTitleLower}. {visibleFiles.length > 0 && ( -
+
Conflicted files ({visibleFiles.length} {hasMore ? ` of ${total}` : ''}): @@ -90,10 +95,20 @@ export function ConflictBanner({
+ {onResolve && ( + + )} - - diff --git a/frontend/src/components/tasks/TaskFollowUpSection.tsx b/frontend/src/components/tasks/TaskFollowUpSection.tsx index f058edd8..43778b1e 100644 --- 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(isAttemptRunning); useEffect(() => { @@ -284,21 +302,27 @@ export function TaskFollowUpSection({ )} {/* Conflict notice and actions (optional UI) */} - + {branchStatus && ( + + )}
- Send + {conflictResolutionInstructions + ? 'Resolve conflicts' + : 'Send'} )} diff --git a/frontend/src/components/tasks/Toolbar/CurrentAttempt.tsx b/frontend/src/components/tasks/Toolbar/CurrentAttempt.tsx index 20681b53..51ffc0d4 100644 --- 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 diff --git a/frontend/src/components/tasks/follow-up/FollowUpConflictSection.tsx b/frontend/src/components/tasks/follow-up/FollowUpConflictSection.tsx index 8fa50739..347bd046 100644 --- 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 ( - { - 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); - } - }} - /> + <> + { + 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 && ( +
+
+ Conflict resolution instructions +
+
+ {conflictResolutionInstructions} +
+
+ )} + ); } diff --git a/frontend/src/hooks/follow-up/useFollowUpSend.ts b/frontend/src/hooks/follow-up/useFollowUpSend.ts index 41b0dd22..d38254db 100644 --- 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, diff --git a/frontend/src/hooks/index.ts b/frontend/src/hooks/index.ts index 3637e49a..b502bcce 100644 --- 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'; diff --git a/frontend/src/hooks/useAttemptConflicts.ts b/frontend/src/hooks/useAttemptConflicts.ts new file mode 100644 index 00000000..ba911506 --- /dev/null +++ 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; +} diff --git a/frontend/src/hooks/useOpenInEditor.ts b/frontend/src/hooks/useOpenInEditor.ts index e163b57f..cb44a291 100644 --- 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 => { + 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] ); }