From 9345a864f436d9a5effb6aec12c2fbae3563527d Mon Sep 17 00:00:00 2001 From: William Valentin Date: Tue, 17 Feb 2026 23:05:21 -0800 Subject: [PATCH] fix(agent): add model request timeouts and empty-response fallback --- docs/plans/state.json | 16 ++++- src/backends/native/agent.test.ts | 98 +++++++++++++++++++++++++++++++ src/backends/native/agent.ts | 56 ++++++++++++++---- 3 files changed, 158 insertions(+), 12 deletions(-) diff --git a/docs/plans/state.json b/docs/plans/state.json index 4b627ab..13962b8 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -1,6 +1,6 @@ { "version": "1.0", - "updated_at": "2026-02-17", + "updated_at": "2026-02-18", "description": "Tracks the status of all Flynn plans and implementation phases", "plans": { "makefile-skills-convenience-targets": { @@ -5076,10 +5076,22 @@ "docs/plans/state.json" ], "test_status": "pnpm test:run src/session/manager.test.ts src/gateway/handlers/handlers.test.ts + pnpm typecheck passing" + }, + "native-agent-model-timeout-hardening": { + "status": "completed", + "date": "2026-02-18", + "updated": "2026-02-18", + "summary": "Added hard per-request model timeouts in `NativeAgent` (`modelTimeoutMs`, default 120000ms) to fail fast when providers hang, covering both single-turn and tool-loop model calls. Also added empty-final-response normalization to avoid silent blank replies and added regression tests for timeout and empty-response cases.", + "files_modified": [ + "src/backends/native/agent.ts", + "src/backends/native/agent.test.ts", + "docs/plans/state.json" + ], + "test_status": "pnpm test:run src/backends/native/agent.test.ts passing" } }, "overall_progress": { - "total_test_count": 1887, + "total_test_count": 1889, "all_tests_passing": true, "p0_completion": "3/3 (100%)", "p1_completion": "4/4 (100%)", diff --git a/src/backends/native/agent.test.ts b/src/backends/native/agent.test.ts index 077defb..ab7bc10 100644 --- a/src/backends/native/agent.test.ts +++ b/src/backends/native/agent.test.ts @@ -110,6 +110,43 @@ describe('NativeAgent', () => { const history = agent.getHistory(); expect(history[history.length - 1]).toEqual({ role: 'assistant', content: 'Operation cancelled by user.' }); }); + + it('returns fallback text when model response is empty', async () => { + const mockClient: ModelClient = { + chat: vi.fn().mockResolvedValue({ + content: '', + stopReason: 'end_turn', + usage: { inputTokens: 10, outputTokens: 5 }, + }), + }; + + const agent = new NativeAgent({ + modelClient: mockClient, + systemPrompt: 'You are helpful.', + }); + + const response = await agent.process('Hi'); + expect(response).toBe('I could not generate a response for that. Please try again.'); + const history = agent.getHistory(); + expect(history[history.length - 1]).toEqual({ + role: 'assistant', + content: 'I could not generate a response for that. Please try again.', + }); + }); + + it('times out single-turn model calls', async () => { + const mockClient: ModelClient = { + chat: vi.fn().mockImplementation(() => new Promise(() => {})), + }; + + const agent = new NativeAgent({ + modelClient: mockClient, + systemPrompt: 'You are helpful.', + modelTimeoutMs: 10, + }); + + await expect(agent.process('Hi')).rejects.toThrow('Model request timed out after 10ms'); + }); }); // Simple test tool @@ -611,4 +648,65 @@ describe('NativeAgent tool loop', () => { expect(response).toBe('Got both results'); expect(mockClient.chat).toHaveBeenCalledTimes(2); }); + + it('returns fallback text when tool loop final response is empty', async () => { + const mockClient: ModelClient = { + chat: vi + .fn() + .mockResolvedValueOnce({ + content: '', + stopReason: 'tool_use', + usage: { inputTokens: 10, outputTokens: 5 }, + toolCalls: [{ id: 'call_1', name: 'test.echo', args: { text: 'hello' } }], + }) + .mockResolvedValueOnce({ + content: '', + stopReason: 'end_turn', + usage: { inputTokens: 12, outputTokens: 4 }, + }), + }; + + const registry = new ToolRegistry(); + registry.register(echoTool); + const hooks = new HookEngine({ confirm: [], log: [], silent: [] }); + const executor = new ToolExecutor(registry, hooks); + + const agent = new NativeAgent({ + modelClient: mockClient, + systemPrompt: 'You are helpful.', + toolRegistry: registry, + toolExecutor: executor, + }); + + const response = await agent.process('echo hello'); + expect(response).toBe('I could not generate a response for that. Please try again.'); + const history = agent.getHistory(); + expect(history[history.length - 1]).toEqual({ + role: 'assistant', + content: 'I could not generate a response for that. Please try again.', + }); + }); + + it('times out tool-loop model calls and returns an error message', async () => { + const mockClient: ModelClient = { + chat: vi.fn().mockImplementation(() => new Promise(() => {})), + }; + + const registry = new ToolRegistry(); + registry.register(echoTool); + const hooks = new HookEngine({ confirm: [], log: [], silent: [] }); + const executor = new ToolExecutor(registry, hooks); + + const agent = new NativeAgent({ + modelClient: mockClient, + systemPrompt: 'You are helpful.', + toolRegistry: registry, + toolExecutor: executor, + modelTimeoutMs: 10, + }); + + const response = await agent.process('echo hello'); + expect(response).toContain('Error in tool loop'); + expect(response).toContain('Model request timed out after 10ms'); + }); }); diff --git a/src/backends/native/agent.ts b/src/backends/native/agent.ts index bcf1766..f5f0137 100644 --- a/src/backends/native/agent.ts +++ b/src/backends/native/agent.ts @@ -40,6 +40,8 @@ export interface NativeAgentConfig { toolPolicyContext?: ToolPolicyContext; /** Collector for outbound attachments queued by tools (e.g. media.send). */ attachmentCollector?: OutboundAttachmentCollector; + /** Hard timeout for each model request in milliseconds. */ + modelTimeoutMs?: number; } // Internal message type for the tool loop — supports both text and structured content blocks. @@ -55,6 +57,9 @@ interface PseudoToolUse { } export class NativeAgent { + private static readonly EMPTY_RESPONSE_FALLBACK = + 'I could not generate a response for that. Please try again.'; + private static readonly DEFAULT_MODEL_TIMEOUT_MS = 120_000; private modelClient: ModelClient | ModelRouter; private systemPrompt: string; private session?: Session; @@ -72,6 +77,7 @@ export class NativeAgent { private _lastToolFingerprint?: string; private _cancelRequested = false; private _runInProgress = false; + private modelTimeoutMs: number; constructor(config: NativeAgentConfig) { this.modelClient = config.modelClient; @@ -83,6 +89,9 @@ export class NativeAgent { this.onToolUse = config.onToolUse; this._toolPolicyContext = config.toolPolicyContext; this._attachmentCollector = config.attachmentCollector; + this.modelTimeoutMs = Number.isFinite(config.modelTimeoutMs) + ? Math.max(1, Math.floor(config.modelTimeoutMs as number)) + : NativeAgent.DEFAULT_MODEL_TIMEOUT_MS; } private get history(): Message[] { @@ -145,13 +154,15 @@ export class NativeAgent { this._totalUsage.outputTokens += response.usage.outputTokens; this._callCount++; + const normalizedContent = this.normalizeAssistantContent(response.content); + // Prepend thinking content if present - let finalContent = response.content; + let finalContent = normalizedContent; if (response.thinkingContent) { - finalContent = `\n${response.thinkingContent}\n\n\n${response.content}`; + finalContent = `\n${response.thinkingContent}\n\n\n${normalizedContent}`; } - const assistantMsg: Message = { role: 'assistant', content: response.content }; + const assistantMsg: Message = { role: 'assistant', content: normalizedContent }; this.addToHistory(assistantMsg); return finalContent; @@ -230,13 +241,14 @@ export class NativeAgent { && response.toolCalls && response.toolCalls.length > 0; if (!wantsToolUse) { const pseudoToolUse = this.extractPseudoToolUse(response.content); - let finalContent = pseudoToolUse + const baseContent = pseudoToolUse ? this.buildPseudoToolUseWarning(response.content, pseudoToolUse) - : response.content; + : this.normalizeAssistantContent(response.content); + let finalContent = baseContent; if (response.thinkingContent) { - finalContent = `\n${response.thinkingContent}\n\n\n${finalContent}`; + finalContent = `\n${response.thinkingContent}\n\n\n${baseContent}`; } - const assistantMsg: Message = { role: 'assistant', content: finalContent }; + const assistantMsg: Message = { role: 'assistant', content: baseContent }; this.addToHistory(assistantMsg); return finalContent; } @@ -411,10 +423,27 @@ export class NativeAgent { } private async chatWithRouter(request: ChatRequest): Promise { - if ('getClient' in this.modelClient) { - return (this.modelClient as ModelRouter).chat(request, this.currentTier); + const requestPromise = 'getClient' in this.modelClient + ? (this.modelClient as ModelRouter).chat(request, this.currentTier) + : this.modelClient.chat(request); + + let timer: NodeJS.Timeout | undefined; + const timeoutPromise = new Promise((_, reject) => { + timer = setTimeout(() => { + const error = new Error(`Model request timed out after ${this.modelTimeoutMs}ms`); + error.name = 'TimeoutError'; + reject(error); + }, this.modelTimeoutMs); + timer.unref?.(); + }); + + try { + return await Promise.race([requestPromise, timeoutPromise]); + } finally { + if (timer) { + clearTimeout(timer); + } } - return this.modelClient.chat(request); } private addToHistory(msg: Message): void { @@ -560,4 +589,11 @@ export class NativeAgent { rawContent, ].join('\n'); } + + private normalizeAssistantContent(content: string): string { + if (content.trim().length > 0) { + return content; + } + return NativeAgent.EMPTY_RESPONSE_FALLBACK; + } }