Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 46 additions & 8 deletions packages/a2a-server/src/agent/task-event-driven.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ describe('Task Event-Driven Scheduler', () => {
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall],
schedulerId: 'task-id',
});

expect(mockEventBus.publish).toHaveBeenCalledWith(
Expand Down Expand Up @@ -106,6 +107,7 @@ describe('Task Event-Driven Scheduler', () => {
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall],
schedulerId: 'task-id',
});

// Simulate A2A client confirmation
Expand Down Expand Up @@ -148,7 +150,11 @@ describe('Task Event-Driven Scheduler', () => {
const handler = (messageBus.subscribe as Mock).mock.calls.find(
(call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE,
)?.[1];
handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] });
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall],
schedulerId: 'task-id',
});

// Simulate Rejection (Cancel)
const handled = await (
Expand All @@ -174,7 +180,11 @@ describe('Task Event-Driven Scheduler', () => {
correlationId: 'corr-2',
confirmationDetails: { type: 'info', title: 'test', prompt: 'test' },
};
handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall2] });
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall2],
schedulerId: 'task-id',
});

// Simulate ModifyWithEditor
const handled2 = await (
Expand Down Expand Up @@ -215,7 +225,11 @@ describe('Task Event-Driven Scheduler', () => {
const handler = (messageBus.subscribe as Mock).mock.calls.find(
(call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE,
)?.[1];
handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] });
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall],
schedulerId: 'task-id',
});

// Simulate ProceedOnce for MCP
const handled = await (
Expand Down Expand Up @@ -255,7 +269,11 @@ describe('Task Event-Driven Scheduler', () => {
const handler = (messageBus.subscribe as Mock).mock.calls.find(
(call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE,
)?.[1];
handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] });
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall],
schedulerId: 'task-id',
});

const handled = await (
task as unknown as {
Expand Down Expand Up @@ -294,7 +312,11 @@ describe('Task Event-Driven Scheduler', () => {
const handler = (messageBus.subscribe as Mock).mock.calls.find(
(call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE,
)?.[1];
handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] });
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall],
schedulerId: 'task-id',
});

const handled = await (
task as unknown as {
Expand Down Expand Up @@ -333,7 +355,11 @@ describe('Task Event-Driven Scheduler', () => {
const handler = (messageBus.subscribe as Mock).mock.calls.find(
(call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE,
)?.[1];
handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] });
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall],
schedulerId: 'task-id',
});

const handled = await (
task as unknown as {
Expand Down Expand Up @@ -376,7 +402,11 @@ describe('Task Event-Driven Scheduler', () => {
const handler = (yoloMessageBus.subscribe as Mock).mock.calls.find(
(call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE,
)?.[1];
handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] });
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall],
schedulerId: 'task-id',
});

// Should NOT auto-publish ProceedOnce anymore, because PolicyEngine handles it directly
expect(yoloMessageBus.publish).not.toHaveBeenCalledWith(
Expand Down Expand Up @@ -419,6 +449,7 @@ describe('Task Event-Driven Scheduler', () => {
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall],
schedulerId: 'task-id',
});

// Should publish artifact update for output
Expand Down Expand Up @@ -453,7 +484,11 @@ describe('Task Event-Driven Scheduler', () => {
const handler = (messageBus.subscribe as Mock).mock.calls.find(
(call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE,
)?.[1];
handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] });
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall],
schedulerId: 'task-id',
});

// The tool should be complete and registered appropriately, eventually
// triggering the toolCompletionPromise resolution when all clear.
Expand Down Expand Up @@ -533,6 +568,7 @@ describe('Task Event-Driven Scheduler', () => {
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall1, toolCall2],
schedulerId: 'task-id',
});

// Confirm first tool call
Expand Down Expand Up @@ -600,6 +636,7 @@ describe('Task Event-Driven Scheduler', () => {
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall1, toolCall2],
schedulerId: 'task-id',
});

// Should NOT transition to input-required yet
Expand All @@ -621,6 +658,7 @@ describe('Task Event-Driven Scheduler', () => {
handler({
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [toolCall1Complete, toolCall2],
schedulerId: 'task-id',
});

// Now it should transition
Expand Down
203 changes: 203 additions & 0 deletions packages/a2a-server/src/agent/task.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import {
type ToolCallRequestInfo,
type GitService,
type CompletedToolCall,
type ToolCall,
type ToolCallsUpdateMessage,
MessageBusType,
} from '@google/gemini-cli-core';
import { createMockConfig } from '../utils/testing_utils.js';
import type { ExecutionEventBus, RequestContext } from '@a2a-js/sdk/server';
Expand Down Expand Up @@ -460,4 +463,204 @@ describe('Task', () => {
expect(task.currentPromptId).toBe(expectedPromptId2);
});
});

describe('Race Condition Fix', () => {
const mockConfig = createMockConfig();
const mockEventBus: ExecutionEventBus = {
publish: vi.fn(),
on: vi.fn(),
off: vi.fn(),
once: vi.fn(),
removeAllListeners: vi.fn(),
finished: vi.fn(),
};

beforeEach(() => {
vi.clearAllMocks();
});

it('should NOT transition to input-required if a tool is still validating', async () => {
// @ts-expect-error - Calling private constructor
const task = new Task(
'task-id',
'context-id',
mockConfig as Config,
mockEventBus,
);

// Manually register two tool calls
task['_registerToolCall']('tool-1', 'awaiting_approval');
task['_registerToolCall']('tool-2', 'validating');

// Call checkInputRequiredState (private)
task['checkInputRequiredState']();

// Verify task state did NOT change to input-required
expect(task.taskState).not.toBe('input-required');
expect(mockEventBus.publish).not.toHaveBeenCalledWith(
expect.objectContaining({
status: expect.objectContaining({ state: 'input-required' }),
}),
);
});

it('should transition to input-required if all active tools are awaiting approval', async () => {
// @ts-expect-error - Calling private constructor
const task = new Task(
'task-id',
'context-id',
mockConfig as Config,
mockEventBus,
);

// Transition from submitted to working first to simulate normal flow
task.taskState = 'working';

// Manually register tool calls
task['_registerToolCall']('tool-1', 'awaiting_approval');

// Call checkInputRequiredState
task['checkInputRequiredState']();

// Verify task state changed to input-required
expect(task.taskState).toBe('input-required');
expect(mockEventBus.publish).toHaveBeenCalledWith(
expect.objectContaining({
status: expect.objectContaining({ state: 'input-required' }),
}),
);
});

it('handleEventDrivenToolCallsUpdate should ignore events for other schedulers', async () => {
// @ts-expect-error - Calling private constructor
const task = new Task(
'task-id',
'context-id',
mockConfig as Config,
mockEventBus,
);

const handleEventDrivenToolCallSpy = vi.spyOn(
task as unknown as {
handleEventDrivenToolCall: Task['handleEventDrivenToolCall'];
},
'handleEventDrivenToolCall',
);

const otherEvent: ToolCallsUpdateMessage = {
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [
{ request: { callId: '1' }, status: 'executing' } as ToolCall,
],
schedulerId: 'other-task-id',
};

task['handleEventDrivenToolCallsUpdate'](otherEvent);

expect(handleEventDrivenToolCallSpy).not.toHaveBeenCalled();

const ownEvent: ToolCallsUpdateMessage = {
type: MessageBusType.TOOL_CALLS_UPDATE,
toolCalls: [
{ request: { callId: '1' }, status: 'executing' } as ToolCall,
],
schedulerId: 'task-id',
};

task['handleEventDrivenToolCallsUpdate'](ownEvent);

expect(handleEventDrivenToolCallSpy).toHaveBeenCalled();
});
});

describe('Serialization and Mapping', () => {
it('should map internal "validating" status to "scheduled" for the client and include outcome', async () => {
const mockConfig = createMockConfig();
const mockEventBus: ExecutionEventBus = {
publish: vi.fn(),
on: vi.fn(),
off: vi.fn(),
once: vi.fn(),
removeAllListeners: vi.fn(),
finished: vi.fn(),
};

// @ts-expect-error - Calling private constructor
const task = new Task(
'task-id',
'context-id',
mockConfig as Config,
mockEventBus,
);

const mockToolCall = {
request: { callId: 'tool-1' },
status: 'validating',
outcome: 'accepted',
tool: { name: 'test-tool' },
};

const message = task['toolStatusMessage'](
mockToolCall as unknown as ToolCall,
'task-id',
'context-id',
);
const serialized = (
message.parts![0] as {
data: { status: string; outcome: string };
}
).data;

expect(serialized.status).toBe('scheduled');
expect(serialized.outcome).toBe('accepted');
});

it('should correctly detect changes when status or outcome changes', async () => {
const mockConfig = createMockConfig();
const mockEventBus: ExecutionEventBus = {
publish: vi.fn(),
on: vi.fn(),
off: vi.fn(),
once: vi.fn(),
removeAllListeners: vi.fn(),
finished: vi.fn(),
};

// @ts-expect-error - Calling private constructor
const task = new Task(
'task-id',
'context-id',
mockConfig as Config,
mockEventBus,
);

const toolCall1 = {
request: { callId: 'tool-1' },
status: 'awaiting_approval',
};

// First update - should trigger change
const changed1 = task['handleEventDrivenToolCall'](
toolCall1 as unknown as ToolCall,
);
expect(changed1).toBe(true);

// Second update with same status - should NOT trigger change
const changed2 = task['handleEventDrivenToolCall'](
toolCall1 as unknown as ToolCall,
);
expect(changed2).toBe(false);

// Update with new outcome - SHOULD trigger change
const toolCall2 = {
request: { callId: 'tool-1' },
status: 'awaiting_approval',
outcome: 'accepted',
};
const changed3 = task['handleEventDrivenToolCall'](
toolCall2 as unknown as ToolCall,
);
expect(changed3).toBe(true);
});
});
});
Loading
Loading