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.
This commit is contained in:
parent
cb11020c65
commit
03dc897929
|
|
@ -841,21 +841,47 @@ export class OpenMultiAgent {
|
||||||
if (agentConfigs.length > 0 && isSimpleGoal(goal)) {
|
if (agentConfigs.length > 0 && isSimpleGoal(goal)) {
|
||||||
const bestAgent = selectBestAgent(goal, agentConfigs)
|
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?.({
|
this.config.onProgress?.({
|
||||||
type: 'agent_start',
|
type: 'agent_start',
|
||||||
agent: bestAgent.name,
|
agent: bestAgent.name,
|
||||||
data: { phase: 'short-circuit', goal },
|
data: { phase: 'short-circuit', goal },
|
||||||
})
|
})
|
||||||
|
|
||||||
// Forward the caller's abort signal so short-circuit honours the same
|
const traceFields = this.config.onTrace
|
||||||
// cancellation contract as the full coordinator path.
|
? { onTrace: this.config.onTrace, runId: generateRunId(), traceAgent: bestAgent.name }
|
||||||
const result = await this.runAgent(
|
: null
|
||||||
bestAgent,
|
const abortFields = options?.abortSignal ? { abortSignal: options.abortSignal } : null
|
||||||
goal,
|
const runOptions: Partial<RunOptions> | undefined =
|
||||||
options?.abortSignal ? { abortSignal: options.abortSignal } : undefined,
|
traceFields || abortFields
|
||||||
)
|
? { ...(traceFields ?? {}), ...(abortFields ?? {}) }
|
||||||
const agentResults = new Map<string, AgentRunResult>()
|
: undefined
|
||||||
agentResults.set(bestAgent.name, result)
|
|
||||||
|
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?.({
|
this.config.onProgress?.({
|
||||||
type: 'agent_complete',
|
type: 'agent_complete',
|
||||||
|
|
@ -863,6 +889,8 @@ export class OpenMultiAgent {
|
||||||
data: { phase: 'short-circuit', result },
|
data: { phase: 'short-circuit', result },
|
||||||
})
|
})
|
||||||
|
|
||||||
|
const agentResults = new Map<string, AgentRunResult>()
|
||||||
|
agentResults.set(bestAgent.name, result)
|
||||||
return this.buildTeamRunResult(agentResults)
|
return this.buildTeamRunResult(agentResults)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -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
|
// The short-circuit path must emit exactly one agent_start and one
|
||||||
// through to runAgent, otherwise simple-goal cancellations are silently
|
// agent_complete event. Before the fix, calling this.runAgent() added
|
||||||
// ignored — a regression vs the full coordinator path which already
|
// a second pair of events on top of the ones emitted by the short-circuit
|
||||||
// honours the signal via PR #69.
|
// 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']
|
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())
|
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')
|
await oma.runTeam(team, 'Say hello')
|
||||||
|
|
||||||
expect(runAgentSpy).toHaveBeenCalledTimes(1)
|
const starts = events.filter(e => e.type === 'agent_start')
|
||||||
const callArgs = runAgentSpy.mock.calls[0]!
|
const completes = events.filter(e => e.type === 'agent_complete')
|
||||||
// Third positional arg should be undefined when caller doesn't pass one
|
expect(starts).toHaveLength(1)
|
||||||
expect(callArgs[2]).toBeUndefined()
|
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 () => {
|
it('aborted signal causes the underlying agent loop to skip the LLM call', async () => {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue