From 91676dd968d4b8b1bc625d2b8ac2154d526a1625 Mon Sep 17 00:00:00 2001 From: DavertMik Date: Sat, 11 Jul 2026 14:00:28 +0300 Subject: [PATCH 1/3] fix: abort in-flight AI request when idle timeout fires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The provider races each LLM call against rejectAfterIdle(timeout). When the timer won it only rejected (and the retry condition retries on that message), but nothing aborted the original request — so a legitimately slow generateText/ generateObject kept running while a retry started, producing duplicate concurrent LLM calls (extra tokens/cost, spurious failures). Create a per-attempt AbortController inside the withRetry callback, combine it with the global executionController signal via AbortSignal.any, pass the combined signal to the SDK call (after the ...config spread so it wins), and call controller.abort() inside rejectAfterIdle before rejecting. Applied at both the generateWithTools and generateObject race sites. Deferred (see plan 005): making the 30s idle timeout configurable per agent. Co-Authored-By: Claude Opus 4.8 --- src/ai/provider.ts | 13 +++++++--- tests/unit/provider.test.ts | 51 +++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/ai/provider.ts b/src/ai/provider.ts index c626ceb..df1696f 100644 --- a/src/ai/provider.ts +++ b/src/ai/provider.ts @@ -31,7 +31,7 @@ function extractCachedTokens(usage: any): number { return typeof fromRaw === 'number' ? fromRaw : 0; } -function rejectAfterIdle(ms: number, signal: { cancelled: boolean }): Promise { +function rejectAfterIdle(ms: number, signal: { cancelled: boolean }, controller: AbortController): Promise { return new Promise((_, reject) => { const tick = () => { if (signal.cancelled) return; @@ -39,6 +39,7 @@ function rejectAfterIdle(ms: number, signal: { cancelled: boolean }): Promise { const timeout = config.timeout || 30000; const cancel = { cancelled: false }; + const controller = new AbortController(); + const combinedSignal = AbortSignal.any([controller.signal, executionController.getAbortSignal()].filter(Boolean) as AbortSignal[]); try { const result = (await Promise.race([ generateText({ messages, ...config, + abortSignal: combinedSignal, }), - rejectAfterIdle(timeout, cancel), + rejectAfterIdle(timeout, cancel, controller), ])) as any; const hasToolCall = (result.toolCalls?.length || 0) > 0; if (!result.text && !hasToolCall && result.finishReason === 'length') { @@ -455,13 +459,16 @@ export class Provider { const response = await withRetry(async () => { const timeout = config.timeout || 30000; const cancel = { cancelled: false }; + const controller = new AbortController(); + const combinedSignal = AbortSignal.any([controller.signal, executionController.getAbortSignal()].filter(Boolean) as AbortSignal[]); try { return (await Promise.race([ generateObject({ messages, ...config, + abortSignal: combinedSignal, }), - rejectAfterIdle(timeout, cancel), + rejectAfterIdle(timeout, cancel, controller), ])) as any; } finally { cancel.cancelled = true; diff --git a/tests/unit/provider.test.ts b/tests/unit/provider.test.ts index c3e7a34..0bb2a0e 100644 --- a/tests/unit/provider.test.ts +++ b/tests/unit/provider.test.ts @@ -1,10 +1,25 @@ import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; import type { ModelMessage } from 'ai'; +import { MockLanguageModelV3 } from 'ai/test'; import { AiError, Provider } from '../../src/ai/provider.js'; import { ConfigParser } from '../../src/config.js'; import type { AIConfig } from '../../src/config.js'; +import { executionController } from '../../src/execution-controller.js'; import { MockAIProvider } from '../mocks/ai-provider.mock.js'; +function buildHangingModel(capture: { signal?: AbortSignal }): MockLanguageModelV3 { + return new MockLanguageModelV3({ + provider: 'test', + modelId: 'hanging-model', + doGenerate: async (params: any) => { + capture.signal = params.abortSignal; + return await new Promise((_resolve, reject) => { + params.abortSignal?.addEventListener('abort', () => reject(new Error('aborted'))); + }); + }, + }); +} + function makeToolCallMessage(calls: Array<{ id: string; name: string; input: any }>): ModelMessage { return { role: 'assistant', @@ -352,4 +367,40 @@ describe('Provider', () => { expect(result).toBeNull(); }); }); + + describe('abort on idle timeout', () => { + it('aborts the in-flight request when the idle timeout fires', async () => { + const capture: { signal?: AbortSignal } = {}; + const model = buildHangingModel(capture); + const messages: ModelMessage[] = [{ role: 'user', content: 'hi' }]; + + const rejected = await provider + .generateWithTools(messages, model, {}, { timeout: 20, maxRetries: 1 }) + .then(() => false) + .catch(() => true); + + expect(rejected).toBe(true); + expect(capture.signal?.aborted).toBe(true); + }); + + it('propagates the global execution abort through the combined signal', async () => { + executionController.startExecution(); + const capture: { signal?: AbortSignal } = {}; + const model = buildHangingModel(capture); + const messages: ModelMessage[] = [{ role: 'user', content: 'hi' }]; + + const pending = provider.generateWithTools(messages, model, {}, { timeout: 60000, maxRetries: 1 }); + while (!capture.signal) { + await new Promise((resolve) => setTimeout(resolve, 5)); + } + executionController.interrupt(); + + const rejected = await pending.then(() => false).catch(() => true); + const aborted = capture.signal?.aborted === true; + executionController.reset(); + + expect(rejected).toBe(true); + expect(aborted).toBe(true); + }); + }); }); From 19a52838f00431b3aab78a7be919addba5158427 Mon Sep 17 00:00:00 2001 From: DavertMik Date: Sat, 11 Jul 2026 14:11:33 +0300 Subject: [PATCH 2/3] fix: unify tool-success mapping and honor telemetryFunctionId option Provider.invokeConversation paired tool calls with results by array position and defaulted a missing `success` field to failed; Conversation.getToolExecutions paired by toolCallId, unwrapped the {type:json,value} envelope, and defaulted to passed. Tester (former) and Pilot (latter) could disagree about the same call. Extract one shared toToolExecution mapper (owns unwrap + `success !== false` default) and route both sites through it; invokeConversation now pairs by toolCallId via a Map. Data-shaped tools with no `success` field now count as successful in both views, matching what Pilot already trusted. Honor the telemetryFunctionId provider option in getTelemetry (ten call sites passed it expecting a named Langfuse trace; only `telemetry.functionId` was read). Explicit telemetry.functionId still wins. Delete the dead Conversation.telemetryFunctionId field/param/clone-arg (never read by anything). Co-Authored-By: Claude Opus 4.8 --- src/ai/conversation.ts | 21 +++++++++------------ src/ai/provider.ts | 23 ++++++++++++----------- tests/unit/conversation.test.ts | 26 +++++++++++++++++++++++++- tests/unit/provider.test.ts | 14 ++++++++++++++ 4 files changed, 60 insertions(+), 24 deletions(-) diff --git a/src/ai/conversation.ts b/src/ai/conversation.ts index f66febf..c5b4997 100644 --- a/src/ai/conversation.ts +++ b/src/ai/conversation.ts @@ -7,6 +7,12 @@ export interface ToolExecution { wasSuccessful: boolean; } +export function toToolExecution(toolName: string, input: any, rawOutput: any): ToolExecution { + let output = rawOutput; + if (rawOutput?.type === 'json' && rawOutput?.value) output = rawOutput.value; + return { toolName, input, output, wasSuccessful: output?.success !== false }; +} + export function toolExecutionLabel(input: Record | undefined): string { return input?.explanation || input?.assertion || input?.reason || input?.request || ''; } @@ -18,15 +24,13 @@ export class Conversation { id: string; messages: ModelMessage[]; model: any; - telemetryFunctionId?: string; protectedPrefixCount = 0; private autoTrimRules: Map; - constructor(messages: ModelMessage[] = [], model?: any, telemetryFunctionId?: string) { + constructor(messages: ModelMessage[] = [], model?: any) { this.id = this.generateId(); this.messages = messages; this.model = model || ''; - this.telemetryFunctionId = telemetryFunctionId; this.autoTrimRules = new Map(); } @@ -83,7 +87,7 @@ export class Conversation { } clone(): Conversation { - return new Conversation([...this.messages], this.model, this.telemetryFunctionId); + return new Conversation([...this.messages], this.model); } cleanupTag(tagName: string, replacement: string, keepLast = 0): void { @@ -243,14 +247,7 @@ export class Conversation { if (!Array.isArray(message.content)) continue; for (const part of message.content) { if (part.type !== 'tool-result') continue; - const rawOutput = part.output as Record; - const output = rawOutput?.type === 'json' && rawOutput?.value ? rawOutput.value : rawOutput; - executions.push({ - toolName: part.toolName, - input: toolCalls.get(part.toolCallId) || {}, - output, - wasSuccessful: output?.success !== false, - }); + executions.push(toToolExecution(part.toolName, toolCalls.get(part.toolCallId) || {}, part.output)); } } diff --git a/src/ai/provider.ts b/src/ai/provider.ts index df1696f..420e898 100644 --- a/src/ai/provider.ts +++ b/src/ai/provider.ts @@ -11,7 +11,7 @@ import { Stats } from '../stats.ts'; import { createDebug, tag } from '../utils/logger.js'; import { type RetryOptions, withRetry } from '../utils/retry.js'; import { RulesLoader } from '../utils/rules-loader.ts'; -import { Conversation } from './conversation.js'; +import { Conversation, toToolExecution } from './conversation.js'; const debugLog = createDebug('explorbot:provider'); const promptLog = createDebug('explorbot:provider:out'); @@ -207,20 +207,25 @@ export class Provider { const runTelemetry = Observability.getTelemetry(); - if (!options.telemetry) { + let optionTelemetry = options.telemetry; + if (options.telemetryFunctionId && !optionTelemetry?.functionId) { + optionTelemetry = { ...optionTelemetry, functionId: options.telemetryFunctionId }; + } + + if (!optionTelemetry) { return runTelemetry; } if (!runTelemetry) { - return options.telemetry; + return optionTelemetry; } return { ...runTelemetry, - ...options.telemetry, + ...optionTelemetry, metadata: { ...runTelemetry.metadata, - ...options.telemetry.metadata, + ...optionTelemetry.metadata, }, }; } @@ -254,12 +259,8 @@ export class Provider { const toolCalls = response.toolCalls || []; const toolResults = response.toolResults || []; - const toolExecutions = toolCalls.map((call: any, index: number) => ({ - toolName: call.toolName || '', - input: call.input, - output: toolResults[index]?.output, - wasSuccessful: toolResults[index]?.output?.success || false, - })); + const resultsById = new Map(toolResults.map((r: any) => [r.toolCallId, r])); + const toolExecutions = toolCalls.map((call: any) => toToolExecution(call.toolName || '', call.input, resultsById.get(call.toolCallId)?.output)); return { conversation, response, toolExecutions }; } diff --git a/tests/unit/conversation.test.ts b/tests/unit/conversation.test.ts index d4e403b..321c016 100644 --- a/tests/unit/conversation.test.ts +++ b/tests/unit/conversation.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'bun:test'; -import { Conversation } from '../../src/ai/conversation'; +import { Conversation, toToolExecution } from '../../src/ai/conversation'; describe('Conversation', () => { describe('cleanupTag', () => { @@ -405,4 +405,28 @@ describe('Conversation', () => { expect(output.pageDiff.htmlParts).toBeDefined(); }); }); + + describe('toToolExecution', () => { + it('unwraps the {type:json, value} envelope', () => { + const exec = toToolExecution('click', { locator: 'Save' }, { type: 'json', value: { success: true, url: '/x' } }); + expect(exec.output).toEqual({ success: true, url: '/x' }); + expect(exec.wasSuccessful).toBe(true); + }); + + it('defaults missing success to true', () => { + const exec = toToolExecution('getVisitedStates', {}, { states: ['/a', '/b'] }); + expect(exec.wasSuccessful).toBe(true); + }); + + it('respects explicit success: false', () => { + const exec = toToolExecution('click', { locator: 'Save' }, { success: false, message: 'not found' }); + expect(exec.wasSuccessful).toBe(false); + }); + + it('handles a missing output', () => { + const exec = toToolExecution('click', { locator: 'Save' }, undefined); + expect(exec.wasSuccessful).toBe(true); + expect(exec.output).toBeUndefined(); + }); + }); }); diff --git a/tests/unit/provider.test.ts b/tests/unit/provider.test.ts index 0bb2a0e..fa090d4 100644 --- a/tests/unit/provider.test.ts +++ b/tests/unit/provider.test.ts @@ -368,6 +368,20 @@ describe('Provider', () => { }); }); + describe('getTelemetry', () => { + it('honors the telemetryFunctionId option shorthand', () => { + (provider as any).telemetryEnabled = true; + const telemetry = (provider as any).getTelemetry({ telemetryFunctionId: 'researcher.textContent' }); + expect(telemetry?.functionId).toBe('researcher.textContent'); + }); + + it('lets an explicit telemetry.functionId win over the shorthand', () => { + (provider as any).telemetryEnabled = true; + const telemetry = (provider as any).getTelemetry({ telemetry: { functionId: 'explicit' }, telemetryFunctionId: 'shorthand' }); + expect(telemetry?.functionId).toBe('explicit'); + }); + }); + describe('abort on idle timeout', () => { it('aborts the in-flight request when the idle timeout fires', async () => { const capture: { signal?: AbortSignal } = {}; From 63a9cc2ffd11faa28f6a64fe9fcaccfbdb42cde4 Mon Sep 17 00:00:00 2001 From: DavertMik Date: Sat, 11 Jul 2026 16:42:08 +0300 Subject: [PATCH 3/3] refactor: extract combinedAbortSignal, rename abortAfterIdle, drop dead abortSignal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per re-review of the idle-timeout abort fix: - Remove the now-dead `abortSignal: executionController.getAbortSignal()` from the generateWithTools/generateObject config builders — both are overridden by the combined signal at the race site, so they were misleading. - Collapse the duplicated `AbortSignal.any([...].filter(Boolean) as AbortSignal[])` at both race sites into a `combinedAbortSignal(controller)` helper (also drops the cast). - Rename `rejectAfterIdle` -> `abortAfterIdle` (it now aborts, not just rejects) and its sentinel param `signal` -> `cancel` (it is not an AbortSignal). Co-Authored-By: Claude Opus 4.8 --- src/ai/provider.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/ai/provider.ts b/src/ai/provider.ts index df1696f..f2ba9f9 100644 --- a/src/ai/provider.ts +++ b/src/ai/provider.ts @@ -31,10 +31,10 @@ function extractCachedTokens(usage: any): number { return typeof fromRaw === 'number' ? fromRaw : 0; } -function rejectAfterIdle(ms: number, signal: { cancelled: boolean }, controller: AbortController): Promise { +function abortAfterIdle(ms: number, cancel: { cancelled: boolean }, controller: AbortController): Promise { return new Promise((_, reject) => { const tick = () => { - if (signal.cancelled) return; + if (cancel.cancelled) return; if (executionController.isAwaitingInput()) { setTimeout(tick, ms); return; @@ -46,6 +46,12 @@ function rejectAfterIdle(ms: number, signal: { cancelled: boolean }, controller: }); } +function combinedAbortSignal(controller: AbortController): AbortSignal { + const global = executionController.getAbortSignal(); + if (!global) return controller.signal; + return AbortSignal.any([controller.signal, global]); +} + export class Provider { private config: AIConfig; private telemetryEnabled = false; @@ -359,7 +365,6 @@ export class Provider { ...optionsWithoutStop, stopWhen: stopConditions, model, - abortSignal: executionController.getAbortSignal(), }, options.agentName ); @@ -369,7 +374,7 @@ export class Provider { const timeout = config.timeout || 30000; const cancel = { cancelled: false }; const controller = new AbortController(); - const combinedSignal = AbortSignal.any([controller.signal, executionController.getAbortSignal()].filter(Boolean) as AbortSignal[]); + const combinedSignal = combinedAbortSignal(controller); try { const result = (await Promise.race([ generateText({ @@ -377,7 +382,7 @@ export class Provider { ...config, abortSignal: combinedSignal, }), - rejectAfterIdle(timeout, cancel, controller), + abortAfterIdle(timeout, cancel, controller), ])) as any; const hasToolCall = (result.toolCalls?.length || 0) > 0; if (!result.text && !hasToolCall && result.finishReason === 'length') { @@ -448,7 +453,6 @@ export class Provider { ...(this.config.config || {}), ...options, model: modelToUse, - abortSignal: executionController.getAbortSignal(), }, options.agentName ); @@ -460,7 +464,7 @@ export class Provider { const timeout = config.timeout || 30000; const cancel = { cancelled: false }; const controller = new AbortController(); - const combinedSignal = AbortSignal.any([controller.signal, executionController.getAbortSignal()].filter(Boolean) as AbortSignal[]); + const combinedSignal = combinedAbortSignal(controller); try { return (await Promise.race([ generateObject({ @@ -468,7 +472,7 @@ export class Provider { ...config, abortSignal: combinedSignal, }), - rejectAfterIdle(timeout, cancel, controller), + abortAfterIdle(timeout, cancel, controller), ])) as any; } finally { cancel.cancelled = true;