From 9cd4ab895c74a61f3459314aed5129866eec4b43 Mon Sep 17 00:00:00 2001 From: basseyriman Date: Mon, 25 May 2026 23:36:53 +0100 Subject: [PATCH] fix(web): address PR review feedback for Nexus AI stop Guard stream cleanup against Stop-then-Send races, remove dead cancelled handler, tighten abort error detection, add stopped tool-call status, and extend abort unit tests. Fixes #1615. --- gitnexus-web/src/components/ToolCallCard.tsx | 8 +++ gitnexus-web/src/core/llm/agent.ts | 8 +-- gitnexus-web/src/core/llm/types.ts | 2 +- gitnexus-web/src/hooks/useAppState.tsx | 62 ++++---------------- gitnexus-web/src/locales/en/graph.json | 3 +- gitnexus-web/src/locales/zh-CN/graph.json | 3 +- gitnexus-web/test/unit/agent-abort.test.ts | 38 +++++++++++- 7 files changed, 67 insertions(+), 57 deletions(-) diff --git a/gitnexus-web/src/components/ToolCallCard.tsx b/gitnexus-web/src/components/ToolCallCard.tsx index da27b789a..b94239781 100644 --- a/gitnexus-web/src/components/ToolCallCard.tsx +++ b/gitnexus-web/src/components/ToolCallCard.tsx @@ -13,6 +13,7 @@ import { Check, Loader2, AlertCircle, + Square, } from '@/lib/lucide-icons'; import type { ToolCallInfo } from '../core/llm/types'; import type { TFunction } from 'i18next'; @@ -77,6 +78,13 @@ const getStatusDisplay = (status: ToolCallInfo['status']) => { bgColor: 'bg-rose-500/10', borderColor: 'border-rose-500/30', }; + case 'stopped': + return { + icon: , + color: 'text-amber-300', + bgColor: 'bg-amber-500/10', + borderColor: 'border-amber-500/30', + }; default: return { icon: , diff --git a/gitnexus-web/src/core/llm/agent.ts b/gitnexus-web/src/core/llm/agent.ts index 7ed0af378..d49915212 100644 --- a/gitnexus-web/src/core/llm/agent.ts +++ b/gitnexus-web/src/core/llm/agent.ts @@ -367,11 +367,11 @@ export interface AgentRuntimeOptions { signal?: AbortSignal; } -const isAbortError = (error: unknown): boolean => { +const isAbortError = (error: unknown, signal?: AbortSignal): boolean => { + if (signal?.aborted) return true; if (error instanceof DOMException && error.name === 'AbortError') return true; if (error instanceof Error && error.name === 'AbortError') return true; - const message = error instanceof Error ? error.message : String(error); - return /aborted|abort/i.test(message); + return false; }; export const buildLangChainMessages = (messages: AgentMessage[]): BaseMessage[] => @@ -662,7 +662,7 @@ export async function* streamAgentResponse( : undefined, }; } catch (error) { - if (options.signal?.aborted || isAbortError(error)) { + if (isAbortError(error, options.signal)) { yield { type: 'cancelled' }; return; } diff --git a/gitnexus-web/src/core/llm/types.ts b/gitnexus-web/src/core/llm/types.ts index 4a8aa3fea..fc661a50e 100644 --- a/gitnexus-web/src/core/llm/types.ts +++ b/gitnexus-web/src/core/llm/types.ts @@ -254,7 +254,7 @@ export interface ToolCallInfo { name: string; args: Record; result?: string; - status: 'pending' | 'running' | 'completed' | 'error'; + status: 'pending' | 'running' | 'completed' | 'error' | 'stopped'; } /** diff --git a/gitnexus-web/src/hooks/useAppState.tsx b/gitnexus-web/src/hooks/useAppState.tsx index 5da52eedc..453b01c80 100644 --- a/gitnexus-web/src/hooks/useAppState.tsx +++ b/gitnexus-web/src/hooks/useAppState.tsx @@ -1017,39 +1017,6 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { // Finalize the assistant message - just call updateMessage one more time scheduleMessageUpdate(); break; - - case 'cancelled': { - const stoppedLabel = i18n.t('chat:stopped'); - for (let i = 0; i < toolCallsForMessage.length; i++) { - const tc = toolCallsForMessage[i]; - if (tc.status === 'running' || tc.status === 'pending') { - toolCallsForMessage[i] = { - ...tc, - status: 'error', - result: stoppedLabel, - }; - } - } - for (let i = 0; i < stepsForMessage.length; i++) { - const step = stepsForMessage[i]; - if ( - step.type === 'tool_call' && - step.toolCall && - (step.toolCall.status === 'running' || step.toolCall.status === 'pending') - ) { - stepsForMessage[i] = { - ...step, - toolCall: { - ...step.toolCall, - status: 'error', - result: stoppedLabel, - }, - }; - } - } - scheduleMessageUpdate(); - break; - } } }; @@ -1062,9 +1029,6 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { captureHistory: providerCapabilities.preserveAssistantTranscript, signal: chatAbortController.signal, })) { - if (chatAbortController.signal.aborted) { - break; - } onChunk(chunk); if (chunk.type === 'cancelled') { break; @@ -1078,9 +1042,13 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { } finally { if (chatAbortRef.current === chatAbortController) { chatAbortRef.current = null; + setIsChatLoading(false); + setCurrentToolCalls([]); + } else if (!chatAbortRef.current) { + // Stopped with no new turn started — safe to clear loading UI + setIsChatLoading(false); + setCurrentToolCalls([]); } - setIsChatLoading(false); - setCurrentToolCalls([]); } }, [ @@ -1105,13 +1073,12 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { chatAbortRef.current = null; const stoppedLabel = i18n.t('chat:stopped'); - setCurrentToolCalls((prev) => - prev.map((tc) => - tc.status === 'running' || tc.status === 'pending' - ? { ...tc, status: 'error', result: stoppedLabel } - : tc, - ), - ); + const markStoppedToolCall = (tc: ToolCallInfo): ToolCallInfo => + tc.status === 'running' || tc.status === 'pending' + ? { ...tc, status: 'stopped', result: stoppedLabel } + : tc; + + setCurrentToolCalls((prev) => prev.map(markStoppedToolCall)); setChatMessages((prev) => { const lastAssistantIdx = [...prev] @@ -1121,10 +1088,7 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { if (lastAssistantIdx === undefined) return prev; const message = prev[lastAssistantIdx]; - const markStopped = (tc: ToolCallInfo): ToolCallInfo => - tc.status === 'running' || tc.status === 'pending' - ? { ...tc, status: 'error', result: stoppedLabel } - : tc; + const markStopped = markStoppedToolCall; const updated: ChatMessage = { ...message, diff --git a/gitnexus-web/src/locales/en/graph.json b/gitnexus-web/src/locales/en/graph.json index 6f4bdb66f..78e917eb0 100644 --- a/gitnexus-web/src/locales/en/graph.json +++ b/gitnexus-web/src/locales/en/graph.json @@ -10,7 +10,8 @@ "status": { "running": "running", "completed": "completed", - "error": "error" + "error": "error", + "stopped": "stopped" }, "tools": { "search": "🔍 Search Code", diff --git a/gitnexus-web/src/locales/zh-CN/graph.json b/gitnexus-web/src/locales/zh-CN/graph.json index d65689f8a..2ee0255c4 100644 --- a/gitnexus-web/src/locales/zh-CN/graph.json +++ b/gitnexus-web/src/locales/zh-CN/graph.json @@ -10,7 +10,8 @@ "status": { "running": "运行中", "completed": "已完成", - "error": "错误" + "error": "错误", + "stopped": "已停止" }, "tools": { "search": "🔍 搜索代码", diff --git a/gitnexus-web/test/unit/agent-abort.test.ts b/gitnexus-web/test/unit/agent-abort.test.ts index e8ac9fede..405be6ad7 100644 --- a/gitnexus-web/test/unit/agent-abort.test.ts +++ b/gitnexus-web/test/unit/agent-abort.test.ts @@ -27,7 +27,6 @@ describe('streamAgentResponse abort', () => { stream: async function* () { yield ['values', { messages: [] }]; controller.abort(); - // Simulate a long-running graph step after abort for (let i = 0; i < 100; i++) { yield ['messages', [{ _getType: () => 'ai', content: 'still going' }]]; } @@ -45,4 +44,41 @@ describe('streamAgentResponse abort', () => { expect(chunks.some((c) => c.type === 'cancelled')).toBe(true); expect(chunks.some((c) => c.type === 'error')).toBe(false); }); + + it('passes AbortSignal to agent.stream config', async () => { + const controller = new AbortController(); + let capturedConfig: Record | undefined; + + const agent = { + stream: async (_input: unknown, config: Record) => { + capturedConfig = config; + throw new DOMException('aborted', 'AbortError'); + }, + }; + + for await (const _chunk of streamAgentResponse(agent as any, userMessage, { + signal: controller.signal, + })) { + // drain + } + + expect(capturedConfig?.signal).toBe(controller.signal); + }); + + it('does not treat unrelated errors mentioning abort as cancellation', async () => { + const agent = { + stream: async () => { + throw new Error('Cannot abort the current transaction'); + }, + }; + + const chunks = []; + for await (const chunk of streamAgentResponse(agent as any, userMessage)) { + chunks.push(chunk); + } + + expect(chunks).toEqual([ + { type: 'error', error: 'Cannot abort the current transaction' }, + ]); + }); });