mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-18 15:52:53 -07:00
fix(core): resolve scheduler hang and verify with unit tests
This commit: 1. Rips out the redundant MessageBus listener in Scheduler.ts that caused race conditions. 2. Adds a unit test to verify the Scheduler no longer subscribes to TOOL_CONFIRMATION_REQUEST. 3. Adds emitFeedback to MessageBus.ts for policy rejections. 4. Adds a unit test to verify that user feedback is emitted on policy denial.
This commit is contained in:
@@ -8,6 +8,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest';
|
||||
import { MessageBus } from './message-bus.js';
|
||||
import { PolicyEngine } from '../policy/policy-engine.js';
|
||||
import { PolicyDecision } from '../policy/types.js';
|
||||
import { coreEvents } from '../utils/events.js';
|
||||
import {
|
||||
MessageBusType,
|
||||
type ToolConfirmationRequest,
|
||||
@@ -23,6 +24,7 @@ describe('MessageBus', () => {
|
||||
beforeEach(() => {
|
||||
policyEngine = new PolicyEngine();
|
||||
messageBus = new MessageBus(policyEngine);
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe('publish', () => {
|
||||
@@ -80,11 +82,14 @@ describe('MessageBus', () => {
|
||||
expect(responseHandler).toHaveBeenCalledWith(expectedResponse);
|
||||
});
|
||||
|
||||
it('should emit rejection and response when policy denies', async () => {
|
||||
it('should emit rejection, response, and user feedback when policy denies', async () => {
|
||||
vi.spyOn(policyEngine, 'check').mockResolvedValue({
|
||||
decision: PolicyDecision.DENY,
|
||||
});
|
||||
|
||||
const feedbackSpy = vi
|
||||
.spyOn(coreEvents, 'emitFeedback')
|
||||
.mockImplementation(() => {});
|
||||
const responseHandler = vi.fn();
|
||||
const rejectionHandler = vi.fn();
|
||||
messageBus.subscribe(
|
||||
@@ -104,6 +109,11 @@ describe('MessageBus', () => {
|
||||
|
||||
await messageBus.publish(request);
|
||||
|
||||
expect(feedbackSpy).toHaveBeenCalledWith(
|
||||
'error',
|
||||
expect.stringContaining('test-tool'),
|
||||
);
|
||||
|
||||
const expectedRejection: ToolPolicyRejection = {
|
||||
type: MessageBusType.TOOL_POLICY_REJECTION,
|
||||
toolCall: { name: 'test-tool', args: {} },
|
||||
|
||||
@@ -66,6 +66,7 @@ vi.mock('./tool-modifier.js');
|
||||
|
||||
import { Scheduler } from './scheduler.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
import { MessageBusType } from '../confirmation-bus/types.js';
|
||||
import type { MessageBus } from '../confirmation-bus/message-bus.js';
|
||||
import type { PolicyEngine } from '../policy/policy-engine.js';
|
||||
import type { ToolRegistry } from '../tools/tool-registry.js';
|
||||
@@ -327,6 +328,15 @@ describe('Scheduler (Orchestrator)', () => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe('Initialization', () => {
|
||||
it('should NOT subscribe to TOOL_CONFIRMATION_REQUEST on message bus', () => {
|
||||
expect(mockMessageBus.subscribe).not.toHaveBeenCalledWith(
|
||||
MessageBusType.TOOL_CONFIRMATION_REQUEST,
|
||||
expect.any(Function),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Phase 1: Ingestion & Resolution', () => {
|
||||
it('should create an ErroredToolCall if tool is not found', async () => {
|
||||
vi.mocked(mockToolRegistry.getTool).mockReturnValue(undefined);
|
||||
|
||||
Reference in New Issue
Block a user