From 03dc8979295929328d5b8b55e12d693b0591c93f Mon Sep 17 00:00:00 2001 From: JackChen Date: Wed, 8 Apr 2026 12:49:13 +0800 Subject: [PATCH] fix: eliminate duplicate progress events and double completedTaskCount in short-circuit path (#82) The short-circuit block in runTeam() called this.runAgent(), which emits its own agent_start/agent_complete events and increments completedTaskCount. The short-circuit block then emitted the same events again, and buildTeamRunResult() incremented the count a second time. Fix: call buildAgent() + agent.run() directly, bypassing runAgent(). Events and counting are handled once by the short-circuit block and buildTeamRunResult() respectively. --- src/orchestrator/orchestrator.ts | 46 +++++++++++++++++++++----- tests/short-circuit.test.ts | 57 ++++++++++++++------------------ 2 files changed, 62 insertions(+), 41 deletions(-) diff --git a/src/orchestrator/orchestrator.ts b/src/orchestrator/orchestrator.ts index 169741d..5fd6060 100644 --- a/src/orchestrator/orchestrator.ts +++ b/src/orchestrator/orchestrator.ts @@ -841,21 +841,47 @@ export class OpenMultiAgent { if (agentConfigs.length > 0 && isSimpleGoal(goal)) { const bestAgent = selectBestAgent(goal, agentConfigs) + // Use buildAgent() + agent.run() directly instead of this.runAgent() + // to avoid duplicate progress events and double completedTaskCount. + // Events are emitted here; counting is handled by buildTeamRunResult(). + const effectiveBudget = resolveTokenBudget(bestAgent.maxTokenBudget, this.config.maxTokenBudget) + const effective: AgentConfig = { + ...bestAgent, + provider: bestAgent.provider ?? this.config.defaultProvider, + baseURL: bestAgent.baseURL ?? this.config.defaultBaseURL, + apiKey: bestAgent.apiKey ?? this.config.defaultApiKey, + maxTokenBudget: effectiveBudget, + } + const agent = buildAgent(effective) + this.config.onProgress?.({ type: 'agent_start', agent: bestAgent.name, data: { phase: 'short-circuit', goal }, }) - // Forward the caller's abort signal so short-circuit honours the same - // cancellation contract as the full coordinator path. - const result = await this.runAgent( - bestAgent, - goal, - options?.abortSignal ? { abortSignal: options.abortSignal } : undefined, - ) - const agentResults = new Map() - agentResults.set(bestAgent.name, result) + const traceFields = this.config.onTrace + ? { onTrace: this.config.onTrace, runId: generateRunId(), traceAgent: bestAgent.name } + : null + const abortFields = options?.abortSignal ? { abortSignal: options.abortSignal } : null + const runOptions: Partial | undefined = + traceFields || abortFields + ? { ...(traceFields ?? {}), ...(abortFields ?? {}) } + : undefined + + const result = await agent.run(goal, runOptions) + + if (result.budgetExceeded) { + this.config.onProgress?.({ + type: 'budget_exceeded', + agent: bestAgent.name, + data: new TokenBudgetExceededError( + bestAgent.name, + result.tokenUsage.input_tokens + result.tokenUsage.output_tokens, + effectiveBudget ?? 0, + ), + }) + } this.config.onProgress?.({ type: 'agent_complete', @@ -863,6 +889,8 @@ export class OpenMultiAgent { data: { phase: 'short-circuit', result }, }) + const agentResults = new Map() + agentResults.set(bestAgent.name, result) return this.buildTeamRunResult(agentResults) } diff --git a/tests/short-circuit.test.ts b/tests/short-circuit.test.ts index b1f13e2..583a7e6 100644 --- a/tests/short-circuit.test.ts +++ b/tests/short-circuit.test.ts @@ -348,46 +348,39 @@ describe('runTeam short-circuit', () => { }) // ------------------------------------------------------------------------- - // Regression: abortSignal forwarding (PR #70 review point 4) + // Regression: no duplicate progress events (#82) // - // The short-circuit path must forward `options.abortSignal` from runTeam - // through to runAgent, otherwise simple-goal cancellations are silently - // ignored — a regression vs the full coordinator path which already - // honours the signal via PR #69. + // The short-circuit path must emit exactly one agent_start and one + // agent_complete event. Before the fix, calling this.runAgent() added + // a second pair of events on top of the ones emitted by the short-circuit + // block itself, and buildTeamRunResult() double-counted completedTasks. // ------------------------------------------------------------------------- - it('forwards abortSignal from runTeam to runAgent in short-circuit path', async () => { + it('emits exactly one agent_start and one agent_complete (no duplicates)', async () => { mockAdapterResponses = ['done'] - const oma = new OpenMultiAgent({ defaultModel: 'mock-model' }) + const events: OrchestratorEvent[] = [] + const oma = new OpenMultiAgent({ + defaultModel: 'mock-model', + onProgress: (e) => events.push(e), + }) const team = oma.createTeam('t', teamCfg()) - // Spy on runAgent to capture the options argument - const runAgentSpy = vi.spyOn(oma, 'runAgent') - - const controller = new AbortController() - await oma.runTeam(team, 'Say hello', { abortSignal: controller.signal }) - - expect(runAgentSpy).toHaveBeenCalledTimes(1) - const callArgs = runAgentSpy.mock.calls[0]! - // Third positional arg must contain the same signal we passed in - expect(callArgs[2]).toBeDefined() - expect(callArgs[2]?.abortSignal).toBe(controller.signal) - }) - - it('runAgent invoked without abortSignal when caller omits it', async () => { - mockAdapterResponses = ['done'] - - const oma = new OpenMultiAgent({ defaultModel: 'mock-model' }) - const team = oma.createTeam('t', teamCfg()) - - const runAgentSpy = vi.spyOn(oma, 'runAgent') - await oma.runTeam(team, 'Say hello') - expect(runAgentSpy).toHaveBeenCalledTimes(1) - const callArgs = runAgentSpy.mock.calls[0]! - // Third positional arg should be undefined when caller doesn't pass one - expect(callArgs[2]).toBeUndefined() + const starts = events.filter(e => e.type === 'agent_start') + const completes = events.filter(e => e.type === 'agent_complete') + expect(starts).toHaveLength(1) + expect(completes).toHaveLength(1) + }) + + it('completedTaskCount is exactly 1 after a successful short-circuit run', async () => { + mockAdapterResponses = ['done'] + const oma = new OpenMultiAgent({ defaultModel: 'mock-model' }) + const team = oma.createTeam('t', teamCfg()) + + await oma.runTeam(team, 'Say hello') + + expect(oma.getStatus().completedTasks).toBe(1) }) it('aborted signal causes the underlying agent loop to skip the LLM call', async () => {