refactor(cli): migrate non-interactive flow to event-driven scheduler (#17572)

This commit is contained in:
Abhi
2026-01-26 22:11:29 -05:00
committed by GitHub
parent a79051d9f8
commit 67b00252d3
6 changed files with 383 additions and 211 deletions
+25 -4
View File
@@ -35,7 +35,10 @@ vi.mock('../telemetry/types.js', () => ({
ToolCallEvent: vi.fn().mockImplementation((call) => ({ ...call })),
}));
import { SchedulerStateManager } from './state-manager.js';
import {
SchedulerStateManager,
type TerminalCallHandler,
} from './state-manager.js';
import { resolveConfirmation } from './confirmation.js';
import { checkPolicy, updatePolicy } from './policy.js';
import { ToolExecutor } from './tool-executor.js';
@@ -64,6 +67,7 @@ import type {
SuccessfulToolCall,
ErroredToolCall,
CancelledToolCall,
CompletedToolCall,
ToolCallResponseInfo,
} from './types.js';
import { ROOT_SCHEDULER_ID } from './types.js';
@@ -201,10 +205,27 @@ describe('Scheduler (Orchestrator)', () => {
applyInlineModify: vi.fn(),
} as unknown as Mocked<ToolModificationHandler>;
// Wire up class constructors to return our mock instances
vi.mocked(SchedulerStateManager).mockReturnValue(
mockStateManager as unknown as Mocked<SchedulerStateManager>,
let capturedTerminalHandler: TerminalCallHandler | undefined;
vi.mocked(SchedulerStateManager).mockImplementation(
(_messageBus, _schedulerId, onTerminalCall) => {
capturedTerminalHandler = onTerminalCall;
return mockStateManager as unknown as SchedulerStateManager;
},
);
mockStateManager.finalizeCall.mockImplementation((callId: string) => {
const call = mockStateManager.getToolCall(callId);
if (call) {
capturedTerminalHandler?.(call as CompletedToolCall);
}
});
mockStateManager.cancelAllQueued.mockImplementation((_reason: string) => {
// In tests, we usually mock the queue or completed batch.
// For the sake of telemetry tests, we manually trigger if needed,
// but most tests here check if finalizing is called.
});
vi.mocked(ToolExecutor).mockReturnValue(
mockExecutor as unknown as Mocked<ToolExecutor>,
);
+7 -11
View File
@@ -101,7 +101,11 @@ export class Scheduler {
this.getPreferredEditor = options.getPreferredEditor;
this.schedulerId = options.schedulerId;
this.parentCallId = options.parentCallId;
this.state = new SchedulerStateManager(this.messageBus, this.schedulerId);
this.state = new SchedulerStateManager(
this.messageBus,
this.schedulerId,
(call) => logToolCall(this.config, new ToolCallEvent(call)),
);
this.executor = new ToolExecutor(this.config);
this.modifier = new ToolModificationHandler();
@@ -388,16 +392,6 @@ export class Scheduler {
}
}
// Fetch the updated call from state before finalizing to capture the
// terminal status.
const terminalCall = this.state.getToolCall(active.request.callId);
if (terminalCall && this.isTerminal(terminalCall.status)) {
logToolCall(
this.config,
new ToolCallEvent(terminalCall as CompletedToolCall),
);
}
this.state.finalizeCall(active.request.callId);
}
@@ -422,6 +416,7 @@ export class Scheduler {
ToolErrorType.POLICY_VIOLATION,
),
);
this.state.finalizeCall(callId);
return;
}
@@ -453,6 +448,7 @@ export class Scheduler {
// Handle cancellation (cascades to entire batch)
if (outcome === ToolConfirmationOutcome.Cancel) {
this.state.updateStatus(callId, 'cancelled', 'User denied execution.');
this.state.finalizeCall(callId);
this.state.cancelAllQueued('User cancelled operation');
return; // Skip execution
}
@@ -23,6 +23,7 @@ import {
} from '../tools/tools.js';
import { MessageBusType } from '../confirmation-bus/types.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js';
import { ROOT_SCHEDULER_ID } from './types.js';
describe('SchedulerStateManager', () => {
const mockRequest: ToolCallRequestInfo = {
@@ -83,6 +84,60 @@ describe('SchedulerStateManager', () => {
stateManager = new SchedulerStateManager(mockMessageBus);
});
describe('Observer Callback', () => {
it('should trigger onTerminalCall when finalizing a call', () => {
const onTerminalCall = vi.fn();
const manager = new SchedulerStateManager(
mockMessageBus,
ROOT_SCHEDULER_ID,
onTerminalCall,
);
const call = createValidatingCall();
manager.enqueue([call]);
manager.dequeue();
manager.updateStatus(
call.request.callId,
'success',
createMockResponse(call.request.callId),
);
manager.finalizeCall(call.request.callId);
expect(onTerminalCall).toHaveBeenCalledTimes(1);
expect(onTerminalCall).toHaveBeenCalledWith(
expect.objectContaining({
status: 'success',
request: expect.objectContaining({ callId: call.request.callId }),
}),
);
});
it('should trigger onTerminalCall for every call in cancelAllQueued', () => {
const onTerminalCall = vi.fn();
const manager = new SchedulerStateManager(
mockMessageBus,
ROOT_SCHEDULER_ID,
onTerminalCall,
);
manager.enqueue([createValidatingCall('1'), createValidatingCall('2')]);
manager.cancelAllQueued('Test cancel');
expect(onTerminalCall).toHaveBeenCalledTimes(2);
expect(onTerminalCall).toHaveBeenCalledWith(
expect.objectContaining({
status: 'cancelled',
request: expect.objectContaining({ callId: '1' }),
}),
);
expect(onTerminalCall).toHaveBeenCalledWith(
expect.objectContaining({
status: 'cancelled',
request: expect.objectContaining({ callId: '2' }),
}),
);
});
});
describe('Initialization', () => {
it('should start with empty state', () => {
expect(stateManager.isActive).toBe(false);
+12 -1
View File
@@ -31,6 +31,11 @@ import {
type SerializableConfirmationDetails,
} from '../confirmation-bus/types.js';
/**
* Handler for terminal tool calls.
*/
export type TerminalCallHandler = (call: CompletedToolCall) => void;
/**
* Manages the state of tool calls.
* Publishes state changes to the MessageBus via TOOL_CALLS_UPDATE events.
@@ -43,6 +48,7 @@ export class SchedulerStateManager {
constructor(
private readonly messageBus: MessageBus,
private readonly schedulerId: string = ROOT_SCHEDULER_ID,
private readonly onTerminalCall?: TerminalCallHandler,
) {}
addToolCalls(calls: ToolCall[]): void {
@@ -134,6 +140,8 @@ export class SchedulerStateManager {
if (this.isTerminalCall(call)) {
this._completedBatch.push(call);
this.activeCalls.delete(callId);
this.onTerminalCall?.(call);
this.emitUpdate();
}
}
@@ -173,9 +181,12 @@ export class SchedulerStateManager {
const queuedCall = this.queue.shift()!;
if (queuedCall.status === 'error') {
this._completedBatch.push(queuedCall);
this.onTerminalCall?.(queuedCall);
continue;
}
this._completedBatch.push(this.toCancelled(queuedCall, reason));
const cancelledCall = this.toCancelled(queuedCall, reason);
this._completedBatch.push(cancelledCall);
this.onTerminalCall?.(cancelledCall);
}
this.emitUpdate();
}