Skip to content

Commit 307883a

Browse files
committed
fix(server): yield Task snapshot on resubscribe when bus is inactive
1 parent 4e1d4f7 commit 307883a

3 files changed

Lines changed: 276 additions & 13 deletions

File tree

src/server/request_handler/default_request_handler.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,15 +1081,21 @@ export class DefaultRequestHandler implements A2ARequestHandler {
10811081
);
10821082
}
10831083

1084-
if (!eventQueue) {
1085-
throw new UnsupportedOperationError(`Resubscribe: No active event bus for task ${taskId}.`);
1086-
}
1087-
10881084
// Per spec 3.1.6: "The operation MUST return a Task object as the first event
10891085
// in the stream, representing the current state of the task at the time of
10901086
// subscription."
10911087
yield { payload: { $case: 'task', value: task } };
10921088

1089+
// No active event bus means there is no live executor to drain
1090+
// from — but per §3.1.6 the snapshot above is still a valid
1091+
// response. Closing the stream here (instead of throwing
1092+
// `UnsupportedOperationError`) lets clients reconnect to a
1093+
// long-running task after server restart, executor pause, or an
1094+
// INPUT_REQUIRED bus-sleep window.
1095+
if (!eventQueue) {
1096+
return;
1097+
}
1098+
10931099
// Stream live events, filtering by taskId.
10941100
// The ResultManager is already handled by the original execution flow;
10951101
// resubscribe only listens for new events.

test/server/default_request_handler.spec.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,7 +1474,7 @@ describe('DefaultRequestHandler as A2ARequestHandler', () => {
14741474
assert.deepEqual((results[0].payload as { $case: 'task'; value: Task }).value, fakeTask);
14751475
});
14761476

1477-
it('resubscribe: should throw UnsupportedOperationError when no active event bus exists', async () => {
1477+
it('resubscribe: should yield the Task snapshot and close when no active event bus exists', async () => {
14781478
const taskId = 'task-resub-no-bus';
14791479
const fakeTask: Task = {
14801480
id: taskId,
@@ -1487,15 +1487,14 @@ describe('DefaultRequestHandler as A2ARequestHandler', () => {
14871487
await mockTaskStore.save(fakeTask, serverCallContext);
14881488

14891489
const generator = handler.resubscribe({ id: taskId, tenant: '' }, serverCallContext);
1490-
try {
1491-
await generator.next();
1492-
assert.fail('Should have thrown UnsupportedOperationError');
1493-
} catch (error: unknown) {
1494-
expect(error).to.be.instanceOf(UnsupportedOperationError);
1495-
expect((error as Error).message).to.contain(
1496-
`Resubscribe: No active event bus for task ${taskId}`
1497-
);
1490+
const results: StreamResponse[] = [];
1491+
for await (const event of generator) {
1492+
results.push(event);
14981493
}
1494+
1495+
assert.lengthOf(results, 1, 'Should yield exactly one event (the Task snapshot)');
1496+
assert.equal(results[0].payload?.$case, 'task');
1497+
assert.deepEqual((results[0].payload as { $case: 'task'; value: Task }).value, fakeTask);
14991498
});
15001499

15011500
it('sendMessageStream: should close stream after a single message (§3.1.2 message-only pattern)', async () => {
Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest';
2+
3+
import {
4+
DefaultRequestHandler,
5+
ExecutionEventBus,
6+
InMemoryTaskStore,
7+
TaskStore,
8+
} from '../../../src/server/index.js';
9+
import {
10+
AgentCard,
11+
StreamResponse,
12+
Task,
13+
TaskState,
14+
TaskStatusUpdateEvent,
15+
} from '../../../src/types/pb/a2a.js';
16+
import { DefaultExecutionEventBusManager } from '../../../src/server/events/execution_event_bus_manager.js';
17+
import { AgentEvent } from '../../../src/server/events/execution_event_bus.js';
18+
import { ServerCallContext } from '../../../src/server/context.js';
19+
import { TaskNotFoundError, UnsupportedOperationError } from '../../../src/errors.js';
20+
import { TERMINAL_STATE_LIST } from '../../../src/server/utils.js';
21+
import { MockAgentExecutor } from '../mocks/agent-executor.mock.js';
22+
23+
/**
24+
* Focused coverage for {@link DefaultRequestHandler.resubscribe} per
25+
* spec §3.1.6.
26+
*
27+
* The contract verified here:
28+
*
29+
* 1. **Non-terminal task with NO active bus** — the handler MUST
30+
* yield the Task snapshot loaded from the store and close the
31+
* stream cleanly. This is the regression scenario for the
32+
* previously-thrown `UnsupportedOperationError('No active event
33+
* bus...')`, which broke reconnection after server restart,
34+
* executor pause, or an INPUT_REQUIRED bus-sleep window. The new
35+
* behaviour mirrors a2a-go's `distributedManager.Resubscribe`
36+
* (`internal/taskexec/distributed_manager.go:73-82`).
37+
*
38+
* 2. **Terminal task** — still throws `UnsupportedOperationError`
39+
* per the §3.1.6 errors list (no further events will be
40+
* delivered, so a snapshot is not a meaningful response).
41+
*
42+
* 3. **Unknown task** — still throws `TaskNotFoundError` per the
43+
* §3.1.6 errors list.
44+
*
45+
* 4. **Non-terminal task WITH active bus** — the snapshot is still
46+
* the first yielded event and live events from the bus are
47+
* forwarded afterwards. Pinned as a regression guard so the
48+
* no-bus early-return doesn't accidentally short-circuit the
49+
* live-bus path.
50+
*/
51+
describe('DefaultRequestHandler.resubscribe (§3.1.6)', () => {
52+
let handler: DefaultRequestHandler;
53+
let taskStore: TaskStore;
54+
let mockExecutor: MockAgentExecutor;
55+
let eventBusManager: DefaultExecutionEventBusManager;
56+
57+
const agentCard: AgentCard = {
58+
name: 'Resubscribe Agent',
59+
description: 'Test agent for §3.1.6 resubscribe contract',
60+
version: '1.0.0',
61+
provider: undefined,
62+
documentationUrl: '',
63+
supportedInterfaces: [
64+
{
65+
url: 'http://localhost/a2a',
66+
protocolBinding: 'HTTP+JSON',
67+
tenant: '',
68+
protocolVersion: '1.0',
69+
},
70+
],
71+
capabilities: {
72+
extensions: [],
73+
streaming: true,
74+
pushNotifications: false,
75+
},
76+
securitySchemes: {},
77+
securityRequirements: [],
78+
defaultInputModes: ['text/plain'],
79+
defaultOutputModes: ['text/plain'],
80+
skills: [],
81+
signatures: [],
82+
};
83+
84+
const serverContext = new ServerCallContext();
85+
86+
beforeEach(() => {
87+
taskStore = new InMemoryTaskStore();
88+
mockExecutor = new MockAgentExecutor();
89+
eventBusManager = new DefaultExecutionEventBusManager();
90+
handler = new DefaultRequestHandler(agentCard, taskStore, mockExecutor, eventBusManager);
91+
});
92+
93+
afterEach(() => {
94+
vi.restoreAllMocks();
95+
});
96+
97+
const makeTask = (
98+
id: string,
99+
state: TaskState = TaskState.TASK_STATE_WORKING,
100+
contextId = `ctx-${id}`
101+
): Task => ({
102+
id,
103+
contextId,
104+
status: { state, message: undefined, timestamp: undefined },
105+
artifacts: [],
106+
history: [],
107+
metadata: {},
108+
});
109+
110+
it('yields the Task snapshot and closes when no active event bus exists (server-restart scenario)', async () => {
111+
// Simulates the post-restart / executor-paused / INPUT_REQUIRED
112+
// bus-sleep scenarios: the task is persisted and non-terminal,
113+
// but the in-memory `eventBusManager` has nothing for this id.
114+
// Previously this raised `UnsupportedOperationError` and broke
115+
// reconnection; the contract is now to yield the snapshot and
116+
// close the stream cleanly.
117+
const taskId = 'task-restart';
118+
const persisted = makeTask(taskId, TaskState.TASK_STATE_WORKING);
119+
await taskStore.save(persisted, serverContext);
120+
121+
// Sanity: no bus is registered for this task before resubscribe.
122+
expect(eventBusManager.getByTaskId(taskId)).toBeUndefined();
123+
124+
const events: StreamResponse[] = [];
125+
for await (const event of handler.resubscribe({ id: taskId, tenant: '' }, serverContext)) {
126+
events.push(event);
127+
}
128+
129+
expect(events).toHaveLength(1);
130+
const payload = events[0].payload as { $case: 'task'; value: Task };
131+
expect(payload.$case).toBe('task');
132+
expect(payload.value).toEqual(persisted);
133+
});
134+
135+
it('yields the snapshot when bus is inactive even at INPUT_REQUIRED (post-bus-sleep reconnection)', async () => {
136+
// INPUT_REQUIRED is a non-terminal state that keeps the bus alive
137+
// across the original `_settleBus` call, but a long enough idle
138+
// window (or a different server instance) may have torn it down.
139+
// The snapshot path must work for INPUT_REQUIRED too — it's one
140+
// of the primary motivating scenarios for this fix.
141+
const taskId = 'task-input-required';
142+
const persisted = makeTask(taskId, TaskState.TASK_STATE_INPUT_REQUIRED);
143+
await taskStore.save(persisted, serverContext);
144+
145+
const events: StreamResponse[] = [];
146+
for await (const event of handler.resubscribe({ id: taskId, tenant: '' }, serverContext)) {
147+
events.push(event);
148+
}
149+
150+
expect(events).toHaveLength(1);
151+
const payload = events[0].payload as { $case: 'task'; value: Task };
152+
expect(payload.value.status?.state).toBe(TaskState.TASK_STATE_INPUT_REQUIRED);
153+
});
154+
155+
it('still throws UnsupportedOperationError for terminal tasks', async () => {
156+
// Per §3.1.6's errors list, terminal tasks are explicitly
157+
// unsubscribable — there will be no further events to deliver,
158+
// so a snapshot would mislead the caller into waiting for a
159+
// stream that will never produce more events.
160+
for (const state of TERMINAL_STATE_LIST) {
161+
const taskId = `task-terminal-${state}`;
162+
await taskStore.save(makeTask(taskId, state as TaskState), serverContext);
163+
164+
const generator = handler.resubscribe({ id: taskId, tenant: '' }, serverContext);
165+
await expect(generator.next()).rejects.toThrow(UnsupportedOperationError);
166+
}
167+
});
168+
169+
it('still throws TaskNotFoundError when the task id is unknown', async () => {
170+
// Per §3.1.6's errors list. The new snapshot path is gated on a
171+
// successful `taskStore.load` — there is no snapshot to yield
172+
// when the id is unknown, so the error contract is preserved.
173+
const generator = handler.resubscribe({ id: 'does-not-exist', tenant: '' }, serverContext);
174+
await expect(generator.next()).rejects.toThrow(TaskNotFoundError);
175+
});
176+
177+
it('throws UnsupportedOperationError if the agent does not advertise streaming', async () => {
178+
// The capability gate at the top of `resubscribe` must still
179+
// short-circuit before we even look at the store — re-checked
180+
// here so the new snapshot path doesn't accidentally bypass it.
181+
const nonStreamingCard: AgentCard = {
182+
...agentCard,
183+
capabilities: { ...agentCard.capabilities!, streaming: false },
184+
};
185+
const nonStreamingHandler = new DefaultRequestHandler(
186+
nonStreamingCard,
187+
taskStore,
188+
mockExecutor,
189+
eventBusManager
190+
);
191+
await taskStore.save(makeTask('any-task'), serverContext);
192+
193+
const generator = nonStreamingHandler.resubscribe(
194+
{ id: 'any-task', tenant: '' },
195+
serverContext
196+
);
197+
await expect(generator.next()).rejects.toThrow(UnsupportedOperationError);
198+
});
199+
200+
it('yields the snapshot first and then forwards live bus events when a bus IS active', async () => {
201+
// Regression guard: the no-bus early-return must not short-circuit
202+
// the live-bus path. With an active bus, the snapshot is still
203+
// the first event and subsequent status updates flow through.
204+
const taskId = 'task-live-bus';
205+
const contextId = `ctx-${taskId}`;
206+
const persisted = makeTask(taskId, TaskState.TASK_STATE_WORKING, contextId);
207+
await taskStore.save(persisted, serverContext);
208+
209+
const bus: ExecutionEventBus = eventBusManager.createOrGetByTaskId(taskId);
210+
211+
const generator = handler.resubscribe({ id: taskId, tenant: '' }, serverContext);
212+
const iterator = generator[Symbol.asyncIterator]();
213+
214+
// Pull the snapshot first — must arrive before any live event is
215+
// published so the consumer always observes the §3.1.6
216+
// "Task-snapshot as first event" guarantee.
217+
const first = await iterator.next();
218+
expect(first.done).toBe(false);
219+
// The generator's declared return type is `void`, so the
220+
// `IteratorResult.value` union widens to `StreamResponse | void`
221+
// even after a `done: false` runtime check — narrow with an
222+
// explicit cast, matching the convention used elsewhere in this
223+
// suite (see `default_request_handler.spec.ts:1332`).
224+
const firstEvent = first.value as StreamResponse;
225+
const firstPayload = firstEvent.payload as { $case: 'task'; value: Task };
226+
expect(firstPayload.$case).toBe('task');
227+
expect(firstPayload.value.id).toBe(taskId);
228+
229+
// Now publish a live status update and let the executor close
230+
// the bus so the generator can settle.
231+
bus.publish(
232+
AgentEvent.statusUpdate({
233+
taskId,
234+
contextId,
235+
status: {
236+
state: TaskState.TASK_STATE_COMPLETED,
237+
message: undefined,
238+
timestamp: undefined,
239+
},
240+
metadata: {},
241+
})
242+
);
243+
bus.finished();
244+
245+
const remaining: StreamResponse[] = [];
246+
for await (const event of { [Symbol.asyncIterator]: () => iterator }) {
247+
remaining.push(event);
248+
}
249+
250+
expect(remaining).toHaveLength(1);
251+
const livePayload = remaining[0].payload as {
252+
$case: 'statusUpdate';
253+
value: TaskStatusUpdateEvent;
254+
};
255+
expect(livePayload.$case).toBe('statusUpdate');
256+
expect(livePayload.value.status?.state).toBe(TaskState.TASK_STATE_COMPLETED);
257+
});
258+
});

0 commit comments

Comments
 (0)