From df04b8d2fb03389cd4303f27e8e342ee321d0a13 Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Fri, 29 May 2026 23:24:26 +0800 Subject: [PATCH] fix(swarm): resolve reassign orphan row, enrich stall context, decision-aware recovery UI --- .../messages/swarm-dashboard-model.ts | 22 ++- .../src/tui/components/messages/tool-call.ts | 6 +- .../tui/controllers/session-event-handler.ts | 16 +- .../messages/swarm-dashboard-model.test.ts | 73 ++++++++ .../components/messages/swarm-routing.test.ts | 157 ++++++++++++++++++ .../messages/tool-call-swarm.test.ts | 2 +- packages/agent-core/src/agent/index.ts | 8 +- .../agent-core/src/agent/swarm/coordinator.ts | 3 + .../agent-core/src/agent/swarm/stall-hook.ts | 30 +++- packages/agent-core/src/agent/swarm/types.ts | 15 +- .../agent-core/src/session/subagent-host.ts | 5 +- .../src/tools/builtin/collaboration/swarm.ts | 1 - .../agent/swarm/stall-hook-turn.e2e.test.ts | 2 + .../test/agent/swarm/stall-hook.test.ts | 21 +++ .../swarm/subagent-hooks-unaffected.test.ts | 96 +++++++++++ .../agent-core/test/swarm/coordinator.test.ts | 44 ++++- 16 files changed, 476 insertions(+), 25 deletions(-) create mode 100644 packages/agent-core/test/agent/swarm/subagent-hooks-unaffected.test.ts diff --git a/apps/kimi-code/src/tui/components/messages/swarm-dashboard-model.ts b/apps/kimi-code/src/tui/components/messages/swarm-dashboard-model.ts index 79d6d98b1..a1128544f 100644 --- a/apps/kimi-code/src/tui/components/messages/swarm-dashboard-model.ts +++ b/apps/kimi-code/src/tui/components/messages/swarm-dashboard-model.ts @@ -24,7 +24,7 @@ export interface SwarmModel { export type SwarmEvent = | { t: 'planned'; total: number } | { t: 'synthesizing' } - | { t: 'done'; succeeded: number; failed: number; dropped?: number } + | { t: 'done'; succeeded: number; failed: number } | { t: 'cancelled' } | { t: 'worker.spawned'; id: string; role: string } | { t: 'worker.toolcall'; id: string; activity: string } @@ -32,6 +32,7 @@ export type SwarmEvent = | { t: 'worker.done'; id: string; tokens?: number } | { t: 'worker.failed'; id: string; error: string } | { t: 'worker.retrying'; role: string } + | { t: 'worker.reassigned'; fromRole: string; toRole: string } | { t: 'worker.dropped'; role: string; reason: string }; export function initialSwarmModel(task: string): SwarmModel { @@ -169,6 +170,25 @@ export function applySwarmEvent(model: SwarmModel, event: SwarmEvent): SwarmMode workers.set(prior.id, { ...prior, status: 'retrying', latestActivity: undefined }); return { ...model, workers, ...withCounts(model, adj) }; } + case 'worker.reassigned': { + // The reviser moved this subtask to a new role. Re-key the SAME row from + // the old role to the new one and mark it retrying so the subsequent + // worker.spawned for the new role reuses THIS row (one row per subtask) + // instead of stranding the old-role row in 'retrying' forever. If no + // old-role row exists, no-op — there is nothing to correlate. + const prior = findReusableRoleRow(model.workers, event.fromRole); + if (prior === undefined) return model; + const workers = new Map(model.workers); + const adj = countAdjustments(prior.status, 'retrying'); + workers.set(prior.id, { + ...prior, + role: event.toRole, + status: 'retrying', + latestActivity: undefined, + error: undefined, + }); + return { ...model, workers, ...withCounts(model, adj) }; + } case 'worker.dropped': { // The coordinator gave up on this role's subtask. Mark its row dropped // (or create a dropped row if the subtask never spawned a worker) and diff --git a/apps/kimi-code/src/tui/components/messages/tool-call.ts b/apps/kimi-code/src/tui/components/messages/tool-call.ts index 7d540b5e9..d62a98d63 100644 --- a/apps/kimi-code/src/tui/components/messages/tool-call.ts +++ b/apps/kimi-code/src/tui/components/messages/tool-call.ts @@ -1379,10 +1379,12 @@ export class ToolCallComponent extends Container { if (w.status === 'done') { return [line1]; } - // Retrying is a transient in-flight state shown as a single dim line so the + // Retrying is a transient in-flight state shown as a single line so the // role's row stays visible (and stable) while the coordinator re-runs it. + // Dim the role label to match the 'dropped' convention: non-running rows + // (retrying, dropped) use a dimmed label, running/done/failed keep primary. if (w.status === 'retrying') { - return [line1]; + return [` ${branch1} ${chalk.dim(w.role)}${statsPart}`]; } if (w.status === 'failed') { const errLine = chalk.hex(c.error)(`failed: ${w.error ?? 'error'}`); diff --git a/apps/kimi-code/src/tui/controllers/session-event-handler.ts b/apps/kimi-code/src/tui/controllers/session-event-handler.ts index c04ec3ca8..7de54c6ab 100644 --- a/apps/kimi-code/src/tui/controllers/session-event-handler.ts +++ b/apps/kimi-code/src/tui/controllers/session-event-handler.ts @@ -538,6 +538,8 @@ export class SessionEventHandler { phase?: string; total?: number; role?: string; + newRole?: string; + decision?: string; reason?: string; }; if (p.phase === 'planned' && typeof p.total === 'number') { @@ -547,8 +549,18 @@ export class SessionEventHandler { } else if (p.phase === 'done') { tc.applySwarm({ t: 'done', succeeded: 0, failed: 0 }); } else if (p.phase === 'revising' && typeof p.role === 'string') { - // The reviser decided to re-run this role's subtask — show it retrying. - tc.applySwarm({ t: 'worker.retrying', role: p.role }); + // Route by the reviser's decision so each recovery path shows the right + // transient state: + // - retry/regenerate re-run the same role → mark it retrying. + // - reassign moves the subtask to a new role → re-key the existing + // row so the subtask keeps ONE row (no orphan left in retrying). + // - drop emits nothing here; the subsequent 'dropped' event fully + // describes it (and skipping this avoids a drop→retrying flash). + if (p.decision === 'reassign' && typeof p.newRole === 'string') { + tc.applySwarm({ t: 'worker.reassigned', fromRole: p.role, toRole: p.newRole }); + } else if (p.decision === 'retry' || p.decision === 'regenerate') { + tc.applySwarm({ t: 'worker.retrying', role: p.role }); + } } else if (p.phase === 'dropped' && typeof p.role === 'string') { // The subtask was given up on — show it as a dropped gap with the reason. tc.applySwarm({ t: 'worker.dropped', role: p.role, reason: p.reason ?? '' }); diff --git a/apps/kimi-code/test/tui/components/messages/swarm-dashboard-model.test.ts b/apps/kimi-code/test/tui/components/messages/swarm-dashboard-model.test.ts index 45448b117..0860a474f 100644 --- a/apps/kimi-code/test/tui/components/messages/swarm-dashboard-model.test.ts +++ b/apps/kimi-code/test/tui/components/messages/swarm-dashboard-model.test.ts @@ -186,6 +186,79 @@ describe('applySwarmEvent', () => { expect(m.workers.get('a2')?.role).toBe('R2'); }); + it('reassign collapses to ONE row: failed(OLD) -> reassigned(OLD->NEW) -> spawned(NEW) -> done', () => { + // The reassign-orphan regression: before the fix, a reassign marked the OLD + // role row retrying then the re-spawn created a NEW role row, stranding the + // old one in 'retrying' forever. The reassigned event re-keys the SAME row. + const m = reduce([ + { t: 'planned', total: 1 }, + { t: 'worker.spawned', id: 'a1', role: 'OldRole' }, + { t: 'worker.failed', id: 'a1', error: 'boom' }, + { t: 'worker.reassigned', fromRole: 'OldRole', toRole: 'NewRole' }, + { t: 'worker.spawned', id: 'a2', role: 'NewRole' }, + { t: 'worker.done', id: 'a2', tokens: 1500 }, + ]); + // Exactly one row, final role NewRole, status done. + expect(m.workers.size).toBe(1); + const w = [...m.workers.values()][0]; + expect(w?.role).toBe('NewRole'); + expect(w?.status).toBe('done'); + expect(w?.tokens).toBe(1500); + // No row left dangling in 'retrying', and no stray OldRole row. + expect([...m.workers.values()].some((r) => r.status === 'retrying')).toBe(false); + expect([...m.workers.values()].some((r) => r.role === 'OldRole')).toBe(false); + expect(m.doneCount).toBe(1); + expect(m.failedCount).toBe(0); + }); + + it('worker.reassigned re-keys the failed row to the new role and marks it retrying', () => { + const m = reduce([ + { t: 'planned', total: 1 }, + { t: 'worker.spawned', id: 'a1', role: 'OldRole' }, + { t: 'worker.failed', id: 'a1', error: 'boom' }, + { t: 'worker.reassigned', fromRole: 'OldRole', toRole: 'NewRole' }, + ]); + expect(m.workers.size).toBe(1); + const w = m.workers.get('a1'); + expect(w?.role).toBe('NewRole'); + expect(w?.status).toBe('retrying'); + expect(w?.error).toBeUndefined(); + // The transient failed count is reversed when the row leaves the failed state. + expect(m.failedCount).toBe(0); + }); + + it('worker.reassigned is a no-op when no fromRole row exists', () => { + const before = reduce([ + { t: 'planned', total: 1 }, + { t: 'worker.spawned', id: 'a1', role: 'Other' }, + ]); + const after = applySwarmEvent(before, { + t: 'worker.reassigned', + fromRole: 'Missing', + toRole: 'NewRole', + }); + expect(after).toBe(before); + }); + + it('full failed->retrying->respawn(running)->done on ONE role keeps counts consistent', () => { + // Locks count bookkeeping: the transient failed must be reversed, so the + // surviving row is done and the failed/dropped counts return to zero. + const m = reduce([ + { t: 'planned', total: 1 }, + { t: 'worker.spawned', id: 'a1', role: 'Worker' }, + { t: 'worker.failed', id: 'a1', error: 'boom' }, + { t: 'worker.retrying', role: 'Worker' }, + { t: 'worker.spawned', id: 'a2', role: 'Worker' }, + { t: 'worker.done', id: 'a2', tokens: 900 }, + ]); + expect(m.workers.size).toBe(1); + const w = [...m.workers.values()][0]; + expect(w?.status).toBe('done'); + expect(m.doneCount).toBe(1); + expect(m.failedCount).toBe(0); + expect(m.droppedCount).toBe(0); + }); + it('single-run (no retry) leaves running rows untouched by reuse logic', () => { const m = reduce([ { t: 'planned', total: 2 }, diff --git a/apps/kimi-code/test/tui/components/messages/swarm-routing.test.ts b/apps/kimi-code/test/tui/components/messages/swarm-routing.test.ts index 34b7db833..d9c9b3439 100644 --- a/apps/kimi-code/test/tui/components/messages/swarm-routing.test.ts +++ b/apps/kimi-code/test/tui/components/messages/swarm-routing.test.ts @@ -171,6 +171,163 @@ describe('swarm dashboard wiring (translation)', () => { expect(out).toContain('dropped: impossible'); }); + it('routes a reassign decision so the subtask keeps ONE row (no orphan)', () => { + const parentToolCallId = 'tc-swarm'; + const dash = makeSwarm(); + const mockHost = { + streamingUI: { + setTurnId: (): void => {}, + getToolComponent: (id: string): ToolCallComponent | undefined => + id === parentToolCallId ? dash : undefined, + }, + } as unknown as SessionEventHost; + const handler = new SessionEventHandler(mockHost); + const noop = (): void => {}; + + const progress = (customData: Record): void => { + handler.handleEvent( + { + type: 'tool.progress', + agentId: 'main', + sessionId: 's', + turnId: 1, + toolCallId: parentToolCallId, + update: { kind: 'custom', customKind: 'swarm', customData }, + } as unknown as Event, + noop, + ); + }; + const spawn = (subagentId: string, role: string): void => { + handler.handleEvent( + { + type: 'subagent.spawned', + agentId: 'main', + sessionId: 's', + subagentId, + subagentName: `swarm:${role}`, + parentToolCallId, + description: role, + runInBackground: false, + } as unknown as Event, + noop, + ); + }; + const fail = (subagentId: string): void => { + handler.handleEvent( + { + type: 'subagent.failed', + agentId: 'main', + sessionId: 's', + subagentId, + parentToolCallId, + error: 'boom', + } as unknown as Event, + noop, + ); + }; + const complete = (subagentId: string): void => { + handler.handleEvent( + { + type: 'subagent.completed', + agentId: 'main', + sessionId: 's', + subagentId, + parentToolCallId, + resultSummary: 'ok', + } as unknown as Event, + noop, + ); + }; + + progress({ phase: 'planned', total: 1 }); + spawn('w1', 'OldRole'); + fail('w1'); + // Reviser reassigns OldRole -> NewRole; the re-spawn uses the NEW role. + progress({ + phase: 'revising', + subtaskId: 'task-1', + role: 'OldRole', + newRole: 'NewRole', + decision: 'reassign', + attempt: 1, + }); + spawn('w2', 'NewRole'); + complete('w2'); + + const out = strip(dash.render(80).join('\n')); + // Exactly one row, now labeled with the new role; the old role is gone. + expect(out).toContain('NewRole'); + expect(out).not.toContain('OldRole'); + // No stray retrying row left behind. + expect(out).not.toContain('retrying'); + }); + + it('a drop decision then dropped produces a single dropped row with no transient retrying', () => { + const parentToolCallId = 'tc-swarm'; + const dash = makeSwarm(); + const mockHost = { + streamingUI: { + setTurnId: (): void => {}, + getToolComponent: (id: string): ToolCallComponent | undefined => + id === parentToolCallId ? dash : undefined, + }, + } as unknown as SessionEventHost; + const handler = new SessionEventHandler(mockHost); + const noop = (): void => {}; + + const progress = (customData: Record): void => { + handler.handleEvent( + { + type: 'tool.progress', + agentId: 'main', + sessionId: 's', + turnId: 1, + toolCallId: parentToolCallId, + update: { kind: 'custom', customKind: 'swarm', customData }, + } as unknown as Event, + noop, + ); + }; + + progress({ phase: 'planned', total: 1 }); + handler.handleEvent( + { + type: 'subagent.spawned', + agentId: 'main', + sessionId: 's', + subagentId: 'w1', + subagentName: 'swarm:Worker', + parentToolCallId, + description: 'Worker', + runInBackground: false, + } as unknown as Event, + noop, + ); + handler.handleEvent( + { + type: 'subagent.failed', + agentId: 'main', + sessionId: 's', + subagentId: 'w1', + parentToolCallId, + error: 'boom', + } as unknown as Event, + noop, + ); + // The reviser decides to DROP. The 'revising' event with decision 'drop' + // must emit NOTHING (no transient retrying flash); the subsequent 'dropped' + // event fully describes the gap. + progress({ phase: 'revising', subtaskId: 'task-1', role: 'Worker', decision: 'drop', attempt: 1 }); + const afterRevise = strip(dash.render(80).join('\n')); + expect(afterRevise).not.toContain('retrying'); + + progress({ phase: 'dropped', subtaskId: 'task-1', role: 'Worker', reason: 'impossible' }); + const out = strip(dash.render(80).join('\n')); + expect(out.match(/Worker/g)?.length).toBe(1); + expect(out).toContain('dropped: impossible'); + expect(out).not.toContain('retrying'); + }); + it('counts only real workers — planner/synthesizer/retry never become rows', () => { const parentToolCallId = 'tc-swarm'; const dash = makeSwarm(); diff --git a/apps/kimi-code/test/tui/components/messages/tool-call-swarm.test.ts b/apps/kimi-code/test/tui/components/messages/tool-call-swarm.test.ts index 756e7a6b2..a83208d25 100644 --- a/apps/kimi-code/test/tui/components/messages/tool-call-swarm.test.ts +++ b/apps/kimi-code/test/tui/components/messages/tool-call-swarm.test.ts @@ -148,7 +148,7 @@ describe('ToolCallComponent swarm mode', () => { c.applySwarm({ t: 'worker.spawned', id: 'a2', role: 'A' }); c.applySwarm({ t: 'worker.failed', id: 'a2', error: 'x' }); c.applySwarm({ t: 'worker.dropped', role: 'A', reason: 'gave up' }); - c.applySwarm({ t: 'done', succeeded: 1, failed: 0, dropped: 1 }); + c.applySwarm({ t: 'done', succeeded: 1, failed: 0 }); c.setResult({ tool_call_id: 'tc-swarm', output: 'final report', is_error: false }); const out = strip(c.render(80).join('\n')); expect(out).toContain('1✓'); diff --git a/packages/agent-core/src/agent/index.ts b/packages/agent-core/src/agent/index.ts index a44ccad0e..281f458cb 100644 --- a/packages/agent-core/src/agent/index.ts +++ b/packages/agent-core/src/agent/index.ts @@ -14,7 +14,7 @@ import { import type { EnabledPluginSessionStart } from '#/plugin'; -import type { LoopHooks } from '../loop'; +import type { SubagentLoopHooks } from './swarm/stall-hook'; import type { McpConnectionManager } from '../mcp'; import type { PreparedSystemPromptContext, ResolvedAgentProfile } from '../profile'; import type { ModelProvider } from '../session/provider-manager'; @@ -119,9 +119,11 @@ export class Agent { * Loop hooks scoped to this agent when it runs as a subagent (e.g. swarm * worker stall detection). Set by {@link SessionSubagentHost} when spawning; * `undefined` for the main agent and regular subagents, so they run with - * identical (default) turn hooks. + * identical (default) turn hooks. Narrowed to the only phase `TurnFlow` + * consumes (`prepareToolExecution`) so the unaffected-paths invariant is + * enforced by the type. */ - subagentLoopHooks?: Partial | undefined; + subagentLoopHooks?: SubagentLoopHooks | undefined; private lastLlmConfigLogSignature?: string; diff --git a/packages/agent-core/src/agent/swarm/coordinator.ts b/packages/agent-core/src/agent/swarm/coordinator.ts index 81d1f41c3..d5a87c009 100644 --- a/packages/agent-core/src/agent/swarm/coordinator.ts +++ b/packages/agent-core/src/agent/swarm/coordinator.ts @@ -115,6 +115,9 @@ export class SwarmCoordinator { // correlates to the existing dashboard row keyed by the old role. role: st.role, decision: decision.kind, + // For a reassign, carry the NEW role too so the dashboard can re-key + // the existing old-role row instead of stranding it in `retrying`. + ...(decision.kind === 'reassign' ? { newRole: decision.role } : {}), attempt: st.attempts, }); this.applyDecision(st, decision); diff --git a/packages/agent-core/src/agent/swarm/stall-hook.ts b/packages/agent-core/src/agent/swarm/stall-hook.ts index eafe52132..046f90f71 100644 --- a/packages/agent-core/src/agent/swarm/stall-hook.ts +++ b/packages/agent-core/src/agent/swarm/stall-hook.ts @@ -15,13 +15,25 @@ import { canonicalTelemetryArgs } from '../turn/canonical-args'; import type { LoopHooks, PrepareToolExecutionResult } from '../../loop/types'; +/** + * The only loop-hook phase a subagent (swarm worker) overrides. `TurnFlow` + * composes just `prepareToolExecution` ahead of its built-in dedup, so a + * purpose-named subset keeps the surface honest and the main agent / regular + * subagent paths provably unaffected. + */ +export type SubagentLoopHooks = Pick; + +/** Max length of the repeated-call args snippet embedded in a stall reason. */ +const STALL_ARGS_PREVIEW_MAX_CHARS = 120; + export interface StallDetectionHookOptions { /** Repeat count (inclusive) at which a call is treated as a stall. */ readonly repeatThreshold: number; /** * Invoked exactly once, the first time the threshold is reached. Receives a - * distinguishable reason string (e.g. `stalled: repeated x`) so a - * caller can abort a per-worker controller with it. + * distinguishable reason string (e.g. + * `stalled: repeated () x`) so a caller can abort a per-worker + * controller with it. */ readonly onStall: (reason: string) => void; } @@ -37,20 +49,28 @@ export interface StallDetectionHookOptions { */ export function createStallDetectionHook( options: StallDetectionHookOptions, -): Partial { +): SubagentLoopHooks { const { repeatThreshold, onStall } = options; const counts = new Map(); let stalled = false; return { prepareToolExecution: async (ctx): Promise => { - const key = `${ctx.toolCall.name} ${canonicalTelemetryArgs(ctx.args)}`; + const canonicalArgs = canonicalTelemetryArgs(ctx.args); + const key = `${ctx.toolCall.name} ${canonicalArgs}`; const next = (counts.get(key) ?? 0) + 1; counts.set(key, next); if (next < repeatThreshold) return undefined; - const reason = `stalled: repeated ${ctx.toolCall.name} x${String(next)}`; + // Include the repeated call's canonical args (truncated) so the reviser + // can see WHAT was repeated, not just which tool — e.g. + // `stalled: repeated Read({"path":"/a"}) x10`. + const argsPreview = + canonicalArgs.length > STALL_ARGS_PREVIEW_MAX_CHARS + ? `${canonicalArgs.slice(0, STALL_ARGS_PREVIEW_MAX_CHARS)}…` + : canonicalArgs; + const reason = `stalled: repeated ${ctx.toolCall.name}(${argsPreview}) x${String(next)}`; if (!stalled) { stalled = true; onStall(reason); diff --git a/packages/agent-core/src/agent/swarm/types.ts b/packages/agent-core/src/agent/swarm/types.ts index 3d9664f9e..f907374d9 100644 --- a/packages/agent-core/src/agent/swarm/types.ts +++ b/packages/agent-core/src/agent/swarm/types.ts @@ -48,6 +48,14 @@ export type SwarmProgress = */ role: string; decision: 'retry' | 'regenerate' | 'reassign' | 'drop'; + /** + * For a `reassign`, the NEW role the subtask is being moved to (the + * decision's role). Lets the dashboard re-key the existing OLD-role row to + * the new role so the subtask keeps a single row across the reassign, + * rather than stranding the old row in `retrying`. Absent for other + * decisions. + */ + newRole?: string; attempt: number; } | { phase: 'dropped'; subtaskId: string; role: string; reason: string } @@ -60,13 +68,6 @@ export interface SwarmCoordinatorDeps { onProgress?: ((text: string) => void) | undefined; onProgressCustom?: ((progress: SwarmProgress) => void) | undefined; maxConcurrency?: number | undefined; - /** - * Repeat count at which a worker that keeps issuing the SAME tool call is - * treated as stalled and hard-stopped (its turn fails with a distinguishable - * reason so this wave records it as a failed subtask). Defaults to - * {@link DEFAULT_STALL_REPEAT_THRESHOLD}. - */ - stallRepeatThreshold?: number | undefined; /** * Maximum number of times a single subtask is executed before it is * force-dropped (counting the original run). Defaults to diff --git a/packages/agent-core/src/session/subagent-host.ts b/packages/agent-core/src/session/subagent-host.ts index 2882fa2fc..58ccc5c1e 100644 --- a/packages/agent-core/src/session/subagent-host.ts +++ b/packages/agent-core/src/session/subagent-host.ts @@ -2,7 +2,8 @@ import type { TokenUsage } from '@moonshot-ai/kosong'; import type { Agent } from '../agent'; import type { PromptOrigin } from '../agent/context'; -import type { LoopHooks, LoopTurnStopReason } from '../loop'; +import type { LoopTurnStopReason } from '../loop'; +import type { SubagentLoopHooks } from '../agent/swarm/stall-hook'; import { DEFAULT_AGENT_PROFILES, prepareSystemPromptContext, @@ -50,7 +51,7 @@ type RunSubagentOptions = { * built-in ones. Absent for the main agent and regular subagents, so their * behavior is unchanged. */ - readonly loopHooks?: Partial | undefined; + readonly loopHooks?: SubagentLoopHooks | undefined; }; type SubagentCompletion = { diff --git a/packages/agent-core/src/tools/builtin/collaboration/swarm.ts b/packages/agent-core/src/tools/builtin/collaboration/swarm.ts index bc4e3369b..1924a145d 100644 --- a/packages/agent-core/src/tools/builtin/collaboration/swarm.ts +++ b/packages/agent-core/src/tools/builtin/collaboration/swarm.ts @@ -68,7 +68,6 @@ export class SwarmTool implements BuiltinTool { const coordinator = new SwarmCoordinator({ signal: ctx.signal, maxConcurrency: DEFAULT_MAX_CONCURRENCY, - stallRepeatThreshold, onProgress: (text) => ctx.onUpdate?.({ kind: 'status', text }), onProgressCustom: (progress) => ctx.onUpdate?.({ kind: 'custom', customKind: 'swarm', customData: progress }), diff --git a/packages/agent-core/test/agent/swarm/stall-hook-turn.e2e.test.ts b/packages/agent-core/test/agent/swarm/stall-hook-turn.e2e.test.ts index dd978fb9c..c59c4d767 100644 --- a/packages/agent-core/test/agent/swarm/stall-hook-turn.e2e.test.ts +++ b/packages/agent-core/test/agent/swarm/stall-hook-turn.e2e.test.ts @@ -53,6 +53,8 @@ describe('swarm stall hook — turn level', () => { expect(worker.signal.aborted).toBe(true); expect(stallReason).toMatch(/stalled/i); expect(stallReason).toContain('echo'); + // The reason carries the repeated call's args so a reviser sees WHAT spun. + expect(stallReason).toContain('spin'); // Crucially the coordinator's signal is NOT aborted — a single worker // failure, not a whole-swarm cancel. expect(parent.signal.aborted).toBe(false); diff --git a/packages/agent-core/test/agent/swarm/stall-hook.test.ts b/packages/agent-core/test/agent/swarm/stall-hook.test.ts index 52da01654..6f4ee643e 100644 --- a/packages/agent-core/test/agent/swarm/stall-hook.test.ts +++ b/packages/agent-core/test/agent/swarm/stall-hook.test.ts @@ -33,7 +33,12 @@ describe('createStallDetectionHook', () => { expect(r3?.block).toBe(true); expect(r3?.reason).toMatch(/stalled/i); expect(r3?.reason).toContain('Read'); + // The reason includes the repeated call's canonical args so the reviser + // can tell WHAT was repeated, not just which tool. + expect(r3?.reason).toContain('/a'); + expect(r3?.reason).toContain('"path"'); expect(onStall).toHaveBeenCalledTimes(1); + expect(onStall).toHaveBeenLastCalledWith(r3?.reason); // Further repeats keep blocking but never re-fire onStall. const r4 = await prepare!(ctx); @@ -41,6 +46,22 @@ describe('createStallDetectionHook', () => { expect(onStall).toHaveBeenCalledTimes(1); }); + it('truncates very long repeated args in the stall reason', async () => { + const onStall = vi.fn(); + const hook = createStallDetectionHook({ repeatThreshold: 2, onStall }); + const prepare = hook.prepareToolExecution!; + + const longPattern = 'x'.repeat(500); + const ctx = makeCtx('Grep', { pattern: longPattern }); + await prepare(ctx); + const r = await prepare(ctx); + expect(r?.block).toBe(true); + // Truncated with an ellipsis — the full 500-char pattern is not embedded. + expect(r?.reason).toContain('…'); + expect(r?.reason?.length).toBeLessThan(longPattern.length); + expect(r?.reason).toContain('Grep'); + }); + it('never triggers on distinct progressing calls', async () => { const onStall = vi.fn(); const hook = createStallDetectionHook({ repeatThreshold: 3, onStall }); diff --git a/packages/agent-core/test/agent/swarm/subagent-hooks-unaffected.test.ts b/packages/agent-core/test/agent/swarm/subagent-hooks-unaffected.test.ts new file mode 100644 index 000000000..8af36edf7 --- /dev/null +++ b/packages/agent-core/test/agent/swarm/subagent-hooks-unaffected.test.ts @@ -0,0 +1,96 @@ +/** + * Locks the "main agent / regular subagent unaffected" invariant: when + * `agent.subagentLoopHooks` is UNDEFINED (every path except a swarm worker), + * `TurnFlow.runTurn` composes ONLY the built-in `prepareToolExecution` (the + * tool-call deduplicator). This test replicates that composition exactly and + * proves the built-in same-step dedup still short-circuits identical calls, + * so non-swarm turns run with unchanged behavior. + */ + +import { describe, expect, it } from 'vitest'; + +import { ToolCallDeduplicator } from '../../../src/agent/turn/tool-dedup'; +import type { + LoopHooks, + PrepareToolExecutionHook, +} from '../../../src/loop/index'; +import { makeToolCall, makeToolUseResponse, makeEndTurnResponse } from '../../loop/fixtures/fake-llm'; +import { runTurn } from '../../loop/fixtures/helpers'; +import { EchoTool } from '../../loop/fixtures/tools'; + +/** + * Build the same `prepareToolExecution` hook `TurnFlow.runTurn` builds: run the + * subagent-scoped hook first (when present), then fall through to the built-in + * dedup. With `subagentPrepareToolExecution` undefined this is exactly the + * non-swarm composition. + */ +function buildHooks( + deduper: ToolCallDeduplicator, + subagentPrepareToolExecution: PrepareToolExecutionHook | undefined, +): LoopHooks { + return { + beforeStep: async () => { + deduper.beginStep(); + return; + }, + afterStep: async () => { + deduper.endStep(); + }, + prepareToolExecution: async (ctx) => { + if (subagentPrepareToolExecution !== undefined) { + const subagentResult = await subagentPrepareToolExecution(ctx); + if (subagentResult !== undefined) return subagentResult; + } + const cached = deduper.checkSameStep(ctx.toolCall.id, ctx.toolCall.name, ctx.args); + if (cached !== null) return { syntheticResult: cached }; + return undefined; + }, + finalizeToolResult: async (ctx) => { + return deduper.finalizeResult(ctx.toolCall.id, ctx.toolCall.name, ctx.args, ctx.result); + }, + }; +} + +describe('subagentLoopHooks undefined — non-swarm turn unaffected', () => { + it('built-in same-step dedup still short-circuits identical calls (no subagent hook)', async () => { + const deduper = new ToolCallDeduplicator(); + // subagentLoopHooks UNDEFINED — the non-swarm / regular-subagent case. + const hooks = buildHooks(deduper, undefined); + + const echo = new EchoTool(); + // One step emits the identical tool call twice; the built-in dedup must + // execute the tool only once and serve the second from the placeholder. + const responses = [ + makeToolUseResponse([ + makeToolCall('echo', { text: 'same' }, 'c1'), + makeToolCall('echo', { text: 'same' }, 'c2'), + ]), + makeEndTurnResponse('done'), + ]; + + const { result } = await runTurn({ hooks, tools: [echo], responses }); + + expect(result.stopReason).toBe('end_turn'); + // Dedup short-circuits the duplicate: the tool ran exactly once. + expect(echo.calls.length).toBe(1); + }); + + it('distinct same-step calls all execute (dedup does not over-collapse)', async () => { + const deduper = new ToolCallDeduplicator(); + const hooks = buildHooks(deduper, undefined); + + const echo = new EchoTool(); + const responses = [ + makeToolUseResponse([ + makeToolCall('echo', { text: 'a' }, 'c1'), + makeToolCall('echo', { text: 'b' }, 'c2'), + ]), + makeEndTurnResponse('done'), + ]; + + const { result } = await runTurn({ hooks, tools: [echo], responses }); + + expect(result.stopReason).toBe('end_turn'); + expect(echo.calls.length).toBe(2); + }); +}); diff --git a/packages/agent-core/test/swarm/coordinator.test.ts b/packages/agent-core/test/swarm/coordinator.test.ts index d7a582c25..b9a4fbd64 100644 --- a/packages/agent-core/test/swarm/coordinator.test.ts +++ b/packages/agent-core/test/swarm/coordinator.test.ts @@ -230,13 +230,15 @@ describe('SwarmCoordinator failure recovery', () => { expect(seen[1]?.systemPrompt).toBe('SP2'); expect(seen[1]?.tools).toEqual(['Read']); // The 'revising' event carries the role as it was BEFORE the reassign so - // the dashboard can correlate it to the existing worker row. + // the dashboard can correlate it to the existing worker row, plus the NEW + // role so the dashboard can re-key that row instead of stranding it. const payloads = (onProgressCustom as ReturnType).mock.calls.map((c) => c[0]); expect(payloads).toContainEqual({ phase: 'revising', subtaskId: 'task-1', role: 'Worker', decision: 'reassign', + newRole: 'R2', attempt: 1, }); }); @@ -367,6 +369,46 @@ describe('SwarmCoordinator failure recovery', () => { expect(calls['swarm:B']).toBe(2); }); + it('all subtasks dropped: still synthesizes with a gap-only prompt (no crash)', async () => { + const TWO_PLAN = JSON.stringify({ + subtasks: [ + { id: 'task-1', role: 'A', systemPrompt: 'spa', prompt: 'pa' }, + { id: 'task-2', role: 'B', systemPrompt: 'spb', prompt: 'pb' }, + ], + }); + let synthesizerPrompt: string | undefined; + const spawn = vi.fn(async (args) => { + if (args.profileName === 'swarm-planner') return { result: TWO_PLAN }; + if (args.profileName === 'swarm-synthesizer') { + synthesizerPrompt = args.prompt; + return { result: 'SYNTH' }; + } + if (args.profileName === 'swarm-reviser') + return { result: '{"kind":"drop","reason":"impossible"}' }; + // Every worker fails on its first (only) run, then is dropped. + throw new Error('boom'); + }); + const onProgressCustom = vi.fn(); + const coordinator = new SwarmCoordinator({ + spawnSubagent: spawn, + signal: new AbortController().signal, + onProgressCustom, + }); + const result = await coordinator.run('x'); + expect(result).toBe('SYNTH'); + // Synthesizer was consulted and its prompt surfaces both subtasks as gaps, + // never inventing a success. + expect(synthesizerPrompt).toBeDefined(); + expect(synthesizerPrompt).toMatch(/DROPPED/); + expect(synthesizerPrompt).not.toMatch(/done\)/); + const payloads = (onProgressCustom as ReturnType).mock.calls.map((c) => c[0]); + expect( + payloads.some( + (p) => p.phase === 'done' && p.succeeded === 0 && p.dropped === 2 && p.failed === 0, + ), + ).toBe(true); + }); + it('does not revise on a genuine swarm-wide cancel (re-throws the abort)', async () => { const controller = new AbortController(); const spawn = vi.fn(async (args) => {