fix(agent): add model request timeouts and empty-response fallback
This commit is contained in:
@@ -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<ChatResponse>(() => {})),
|
||||
};
|
||||
|
||||
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<ChatResponse>(() => {})),
|
||||
};
|
||||
|
||||
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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 = `<thinking>\n${response.thinkingContent}\n</thinking>\n\n${response.content}`;
|
||||
finalContent = `<thinking>\n${response.thinkingContent}\n</thinking>\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 = `<thinking>\n${response.thinkingContent}\n</thinking>\n\n${finalContent}`;
|
||||
finalContent = `<thinking>\n${response.thinkingContent}\n</thinking>\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<ChatResponse> {
|
||||
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<never>((_, 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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user