Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 5.3k
Split chat send state into worktree prep and turn-send phases#97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -53,6 +53,7 @@ import { truncateTitle } from "../truncateTitle"; | ||
| import { | ||
| DEFAULT_THREAD_TERMINAL_ID, | ||
| MAX_THREAD_TERMINAL_COUNT, | ||
| type ChatMessage, | ||
| type ChatImageAttachment, | ||
| type TurnDiffSummary, | ||
| } from "../types"; | ||
| @@ -188,6 +189,8 @@ type ComposerCommandItem = | ||
| description: string; | ||
| }; | ||
| type SendPhase = "idle" | "preparing-worktree" | "sending-turn"; | ||
| function readFileAsDataUrl(file: File): Promise<string> { | ||
| return new Promise((resolve, reject) => { | ||
| const reader = new FileReader(); | ||
| @@ -342,8 +345,9 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| const [composerImages, setComposerImages] = useState<ComposerImageAttachment[]>([]); | ||
| const [isDragOverComposer, setIsDragOverComposer] = useState(false); | ||
| const [expandedImage, setExpandedImage] = useState<ExpandedImagePreview | null>(null); | ||
| const [isSending, setIsSending] = useState(false); | ||
| const [isConnecting, setIsConnecting] = useState(false); | ||
| const [optimisticUserMessages, setOptimisticUserMessages] = useState<ChatMessage[]>([]); | ||
| const [sendPhase, setSendPhase] = useState<SendPhase>("idle"); | ||
| const [isConnecting, _setIsConnecting] = useState(false); | ||
| const [isRevertingCheckpoint, setIsRevertingCheckpoint] = useState(false); | ||
| const [selectedEffort, setSelectedEffort] = useState(DEFAULT_REASONING); | ||
| const [envMode, setEnvMode] = useState<"local" | "worktree">("local"); | ||
| @@ -397,7 +401,9 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| activeThread?.model ?? activeProject?.model ?? DEFAULT_MODEL, | ||
| ); | ||
| const phase = derivePhase(activeThread?.session ?? null); | ||
| const isWorking = phase === "running" || isSending || isConnecting || isRevertingCheckpoint; | ||
| const isSendBusy = sendPhase !== "idle"; | ||
| const isPreparingWorktree = sendPhase === "preparing-worktree"; | ||
| const isWorking = phase === "running" || isSendBusy || isConnecting || isRevertingCheckpoint; | ||
| const nowIso = new Date(nowTick).toISOString(); | ||
| const threadActivities = activeThread?.activities ?? []; | ||
| const workLogEntries = useMemo( | ||
| @@ -417,9 +423,21 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| () => derivePendingApprovals(threadActivities), | ||
| [threadActivities], | ||
| ); | ||
| const timelineMessages = useMemo(() => { | ||
| const serverMessages = activeThread?.messages ?? []; | ||
| if (optimisticUserMessages.length === 0) { | ||
| return serverMessages; | ||
| } | ||
| const serverIds = new Set(serverMessages.map((message) => message.id)); | ||
| const pendingMessages = optimisticUserMessages.filter((message) => !serverIds.has(message.id)); | ||
| if (pendingMessages.length === 0) { | ||
| return serverMessages; | ||
| } | ||
| return [...serverMessages, ...pendingMessages]; | ||
| }, [activeThread?.messages, optimisticUserMessages]); | ||
| const timelineEntries = useMemo( | ||
| () => deriveTimelineEntries(activeThread?.messages ?? [], workLogEntries), | ||
| [activeThread?.messages, workLogEntries], | ||
| () => deriveTimelineEntries(timelineMessages, workLogEntries), | ||
| [timelineMessages, workLogEntries], | ||
| ); | ||
| const { turnDiffSummaries, inferredCheckpointTurnCountByTurnId } = | ||
| useTurnDiffSummaries(activeThread); | ||
| @@ -960,7 +978,7 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| }, [lastInvokedScriptByProjectId]); | ||
| // Auto-scroll on new messages | ||
| const messageCount = activeThread?.messages.length ?? 0; | ||
| const messageCount = timelineMessages.length; | ||
| const workLogCount = workLogEntries.length; | ||
| const scrollMessagesToBottom = useCallback((behavior: ScrollBehavior = "auto") => { | ||
| const scrollContainer = messagesScrollRef.current; | ||
| @@ -1019,14 +1037,30 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| composerImagesRef.current = composerImages; | ||
| }, [composerImages]); | ||
| useEffect(() => { | ||
| if (!activeThread?.id) { | ||
| setOptimisticUserMessages([]); | ||
| return; | ||
| } | ||
| if (activeThread.messages.length === 0) { | ||
| return; | ||
| } | ||
| const serverIds = new Set(activeThread.messages.map((message) => message.id)); | ||
| setOptimisticUserMessages((existing) => { | ||
| const next = existing.filter((message) => !serverIds.has(message.id)); | ||
| return next.length === existing.length ? existing : next; | ||
| }); | ||
| }, [activeThread?.id, activeThread?.messages]); | ||
| useEffect(() => { | ||
| setComposerImages((existing) => { | ||
| revokePreviewUrls(existing); | ||
| return []; | ||
| }); | ||
| setOptimisticUserMessages([]); | ||
| setPrompt(""); | ||
| promptRef.current = ""; | ||
| setIsSending(false); | ||
| setSendPhase("idle"); | ||
| setComposerCursor(0); | ||
| setComposerHighlightedItemId(null); | ||
| dragDepthRef.current = 0; | ||
| @@ -1315,7 +1349,7 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| const api = readNativeApi(); | ||
| if (!api || !activeThread || isRevertingCheckpoint) return; | ||
| if (phase === "running" || isSending || isConnecting) { | ||
| if (phase === "running" || isSendBusy || isConnecting) { | ||
| setThreadError(activeThread.id, "Interrupt the current turn before reverting checkpoints."); | ||
| return; | ||
| } | ||
| @@ -1349,36 +1383,67 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| setIsRevertingCheckpoint(false); | ||
| } | ||
| }, | ||
| [activeThread, isConnecting, isRevertingCheckpoint, isSending, phase, setThreadError], | ||
| [activeThread, isConnecting, isRevertingCheckpoint, isSendBusy, phase, setThreadError], | ||
| ); | ||
| const onSend = async (e: React.SubmitEvent | React.KeyboardEvent) => { | ||
| e.preventDefault(); | ||
| const api = readNativeApi(); | ||
| if (!api || !activeThread || isSending || isConnecting) return; | ||
| if (!api || !activeThread || isSendBusy || isConnecting) return; | ||
| const trimmed = prompt.trim(); | ||
| if (!trimmed && composerImages.length === 0) return; | ||
| if (!activeProject) return; | ||
| const threadIdForSend = activeThread.id; | ||
| const isFirstMessage = activeThread.messages.length === 0; | ||
| const baseBranchForWorktree = | ||
| isFirstMessage && envMode === "worktree" && !activeThread.worktreePath | ||
| ? activeThread.branch | ||
| : null; | ||
| const composerImagesSnapshot = [...composerImages]; | ||
| const messageIdForSend = newMessageId(); | ||
| const messageCreatedAt = new Date().toISOString(); | ||
| const optimisticAttachments = composerImagesSnapshot.map((image) => ({ | ||
| type: "image" as const, | ||
| id: image.id, | ||
| name: image.name, | ||
| mimeType: image.mimeType, | ||
| sizeBytes: image.sizeBytes, | ||
| previewUrl: image.previewUrl, | ||
| })); | ||
| setOptimisticUserMessages((existing) => [ | ||
| ...existing, | ||
| { | ||
| id: messageIdForSend, | ||
| role: "user", | ||
| text: trimmed, | ||
| ...(optimisticAttachments.length > 0 ? { attachments: optimisticAttachments } : {}), | ||
| createdAt: messageCreatedAt, | ||
| streaming: false, | ||
| }, | ||
| ]); | ||
| // On first message: lock in branch + create worktree if needed. | ||
| if ( | ||
| activeThread.messages.length === 0 && | ||
| activeThread.branch && | ||
| envMode === "worktree" && | ||
| !activeThread.worktreePath | ||
| ) { | ||
| try { | ||
| setThreadError(threadIdForSend, null); | ||
| promptRef.current = ""; | ||
| setPrompt(""); | ||
| setComposerImages([]); | ||
| setComposerCursor(0); | ||
| setComposerHighlightedItemId(null); | ||
| let attemptedTurnStart = false; | ||
| try { | ||
| // On first message: lock in branch + create worktree if needed. | ||
| if (baseBranchForWorktree) { | ||
| setSendPhase("preparing-worktree"); | ||
| const newBranch = `codething/${crypto.randomUUID().slice(0, 8)}`; | ||
| const result = await createWorktreeMutation.mutateAsync({ | ||
| cwd: activeProject.cwd, | ||
| branch: activeThread.branch, | ||
| branch: baseBranchForWorktree, | ||
| newBranch, | ||
| }); | ||
| await api.orchestration.dispatchCommand({ | ||
| type: "thread.meta.update", | ||
| commandId: newCommandId(), | ||
| threadId: activeThread.id, | ||
| threadId: threadIdForSend, | ||
| branch: result.worktree.branch, | ||
| worktreePath: result.worktree.path, | ||
| }); | ||
| @@ -1390,43 +1455,25 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| rememberAsLastInvoked: false, | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| dispatch({ | ||
| type: "SET_ERROR", | ||
| threadId: activeThread.id, | ||
| error: err instanceof Error ? err.message : "Failed to create worktree", | ||
| }); | ||
| return; | ||
| } | ||
| } | ||
| // Auto-title from first message | ||
| if (activeThread.messages.length === 0) { | ||
| const titleSeed = | ||
| trimmed || | ||
| (composerImagesSnapshot.length > 0 | ||
| ? `Image: ${composerImagesSnapshot[0]?.name ?? "attachment"}` | ||
| : "New thread"); | ||
| const title = truncateTitle(titleSeed); | ||
| if (api) { | ||
| // Auto-title from first message | ||
| if (isFirstMessage) { | ||
| const titleSeed = | ||
| trimmed || | ||
| (composerImagesSnapshot.length > 0 | ||
| ? `Image: ${composerImagesSnapshot[0]?.name ?? "attachment"}` | ||
| : "New thread"); | ||
| const title = truncateTitle(titleSeed); | ||
| await api.orchestration.dispatchCommand({ | ||
| type: "thread.meta.update", | ||
| commandId: newCommandId(), | ||
| threadId: activeThread.id, | ||
| threadId: threadIdForSend, | ||
| title, | ||
| }); | ||
| } | ||
| } | ||
| setThreadError(activeThread.id, null); | ||
| promptRef.current = ""; | ||
| setPrompt(""); | ||
| setComposerImages([]); | ||
| setComposerCursor(0); | ||
| setComposerHighlightedItemId(null); | ||
| setIsSending(true); | ||
| try { | ||
| setSendPhase("sending-turn"); | ||
| const turnAttachments = await Promise.all( | ||
| composerImagesSnapshot.map( | ||
| async ( | ||
| @@ -1446,27 +1493,41 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| }), | ||
| ), | ||
| ); | ||
| attemptedTurnStart = true; | ||
| await api.orchestration.dispatchCommand({ | ||
| type: "thread.turn.start", | ||
| commandId: newCommandId(), | ||
| threadId: activeThread.id, | ||
| threadId: threadIdForSend, | ||
| message: { | ||
| messageId: newMessageId(), | ||
| messageId: messageIdForSend, | ||
| role: "user", | ||
| text: trimmed || IMAGE_ONLY_BOOTSTRAP_PROMPT, | ||
| attachments: turnAttachments, | ||
| }, | ||
| model: selectedModel || undefined, | ||
| effort: selectedEffort || undefined, | ||
| createdAt: new Date().toISOString(), | ||
| createdAt: messageCreatedAt, | ||
| }); | ||
| } catch (err) { | ||
| if ( | ||
| !attemptedTurnStart && | ||
| promptRef.current.length === 0 && | ||
| composerImagesRef.current.length === 0 | ||
| ) { | ||
| setOptimisticUserMessages((existing) => | ||
| existing.filter((message) => message.id !== messageIdForSend), | ||
| ); | ||
| promptRef.current = trimmed; | ||
| setPrompt(trimmed); | ||
| setComposerImages(composerImagesSnapshot); | ||
| setComposerCursor(trimmed.length); | ||
| } | ||
juliusmarminge marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optimistic message persists as ghost after failed dispatchMedium Severity When Additional Locations (1) | ||
| setThreadError( | ||
| activeThread.id, | ||
| threadIdForSend, | ||
| err instanceof Error ? err.message : "Failed to send message.", | ||
| ); | ||
| } finally { | ||
| setIsSending(false); | ||
| setSendPhase("idle"); | ||
| } | ||
| }; | ||
| @@ -1733,7 +1794,7 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| onScroll={onMessagesScroll} | ||
| > | ||
| <MessagesTimeline | ||
| hasMessages={activeThread.messages.length > 0} | ||
| hasMessages={timelineMessages.length > 0} | ||
| isWorking={isWorking} | ||
| scrollContainerRef={messagesScrollRef} | ||
| timelineEntries={timelineEntries} | ||
| @@ -1851,7 +1912,7 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| ? "Ask for follow-up changes or attach images" | ||
| : "Ask anything, @tag files/folders, or use /model" | ||
| } | ||
| disabled={isSending || isConnecting} | ||
| disabled={isConnecting} | ||
| /> | ||
| </div> | ||
| @@ -1897,6 +1958,9 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| {/* Right side: send / stop button */} | ||
| <div className="flex shrink-0 items-center gap-2"> | ||
| {isPreparingWorktree ? ( | ||
| <span className="text-muted-foreground/70 text-xs">Preparing worktree...</span> | ||
| ) : null} | ||
| {phase === "running" ? ( | ||
| <button | ||
| type="button" | ||
| @@ -1919,13 +1983,19 @@ export default function ChatView({ threadId }: ChatViewProps) { | ||
| type="submit" | ||
| className="flex h-9 w-9 items-center justify-center rounded-full bg-primary/90 text-primary-foreground transition-all duration-150 hover:bg-primary hover:scale-105 disabled:opacity-30 disabled:hover:scale-100 sm:h-8 sm:w-8" | ||
| disabled={ | ||
| isSending || isConnecting || (!prompt.trim() && composerImages.length === 0) | ||
| isSendBusy || isConnecting || (!prompt.trim() && composerImages.length === 0) | ||
| } | ||
| aria-label={ | ||
| isConnecting ? "Connecting" : isSending ? "Sending" : "Send message" | ||
| isConnecting | ||
| ? "Connecting" | ||
| : isPreparingWorktree | ||
| ? "Preparing worktree" | ||
| : isSendBusy | ||
| ? "Sending" | ||
| : "Send message" | ||
| } | ||
| > | ||
| {isConnecting || isSending ? ( | ||
| {isConnecting || isSendBusy ? ( | ||
| <svg | ||
| width="14" | ||
| height="14" | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Send phase not set before first await
Medium Severity
For non-worktree first messages, the composer is cleared (lines 1350–1355) but
setSendPhaseis not called until line 1401, after the auto-titleawait. During this gap,sendPhaseremains"idle", soisSendBusyisfalse. Because the textarea is now always enabled during sends (disabled={isConnecting}), a user could type a new message and submit it beforesetSendPhase("sending-turn")fires, bypassing theisSendBusyguard and causing concurrent turn dispatches.Additional Locations (1)
apps/web/src/components/ChatView.tsx#L1845-L1846