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
81 changes: 27 additions & 54 deletions packages/core/src/scheduler/confirmation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
type Mock,
} from 'vitest';
import { EventEmitter } from 'node:events';
import { awaitConfirmation, resolveConfirmation } from './confirmation.js';
import { resolveConfirmation } from './confirmation.js';
import {
MessageBusType,
type ToolConfirmationResponse,
Expand All @@ -31,18 +31,27 @@ import type { ToolModificationHandler } from './tool-modifier.js';
import type { ValidatingToolCall, WaitingToolCall } from './types.js';
import { ROOT_SCHEDULER_ID } from './types.js';
import type { Config } from '../config/config.js';
import type { EditorType } from '../utils/editor.js';
import { type EditorType } from '../utils/editor.js';
import { randomUUID } from 'node:crypto';

// Mock Dependencies
vi.mock('node:crypto', () => ({
randomUUID: vi.fn(),
}));

vi.mock('../utils/editor.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('../utils/editor.js')>();
return {
...actual,
resolveEditorAsync: () => Promise.resolve('vim'),
};
});

describe('confirmation.ts', () => {
let mockMessageBus: MessageBus;

beforeEach(() => {
vi.stubEnv('SANDBOX', '');
mockMessageBus = new EventEmitter() as unknown as MessageBus;
mockMessageBus.publish = vi.fn().mockResolvedValue(undefined);
vi.spyOn(mockMessageBus, 'on');
Expand All @@ -53,6 +62,7 @@ describe('confirmation.ts', () => {
});

afterEach(() => {
vi.unstubAllEnvs();
vi.restoreAllMocks();
});

Expand All @@ -75,43 +85,6 @@ describe('confirmation.ts', () => {
mockMessageBus.on('newListener', handler);
});

describe('awaitConfirmation', () => {
it('should resolve when confirmed response matches correlationId', async () => {
const correlationId = 'test-correlation-id';
const abortController = new AbortController();

const promise = awaitConfirmation(
mockMessageBus,
correlationId,
abortController.signal,
);

emitResponse({
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
correlationId,
confirmed: true,
});

const result = await promise;
expect(result).toEqual({
outcome: ToolConfirmationOutcome.ProceedOnce,
payload: undefined,
});
});

it('should reject when abort signal is triggered', async () => {
const correlationId = 'abort-id';
const abortController = new AbortController();
const promise = awaitConfirmation(
mockMessageBus,
correlationId,
abortController.signal,
);
abortController.abort();
await expect(promise).rejects.toThrow('Operation cancelled');
});
});

describe('resolveConfirmation', () => {
let mockState: Mocked<SchedulerStateManager>;
let mockModifier: Mocked<ToolModificationHandler>;
Expand Down Expand Up @@ -286,8 +259,13 @@ describe('confirmation.ts', () => {
};
invocationMock.shouldConfirmExecute.mockResolvedValue(details);

// 1. User says Modify
// 2. User says Proceed
// Set up modifier mock before starting the flow
mockModifier.handleModifyWithEditor.mockResolvedValue({
updatedParams: { foo: 'bar' },
});
toolMock.build.mockReturnValue({} as unknown as AnyToolInvocation);

// Start the confirmation flow
const listenerPromise1 = waitForListener(
MessageBusType.TOOL_CONFIRMATION_RESPONSE,
);
Expand All @@ -302,30 +280,25 @@ describe('confirmation.ts', () => {

await listenerPromise1;

// First response: Modify
// Prepare to detect when the loop re-subscribes after modification
const listenerPromise2 = waitForListener(
MessageBusType.TOOL_CONFIRMATION_RESPONSE,
);

// First response: User chooses to modify with editor
emitResponse({
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
correlationId: '123e4567-e89b-12d3-a456-426614174000',
confirmed: true,
outcome: ToolConfirmationOutcome.ModifyWithEditor,
});

// Mock the modifier action
mockModifier.handleModifyWithEditor.mockResolvedValue({
updatedParams: { foo: 'bar' },
});
toolMock.build.mockReturnValue({} as unknown as AnyToolInvocation);

// Wait for loop to cycle and re-subscribe
const listenerPromise2 = waitForListener(
MessageBusType.TOOL_CONFIRMATION_RESPONSE,
);
// Wait for the loop to process the modification and re-subscribe
await listenerPromise2;

// Expect state update
expect(mockState.updateArgs).toHaveBeenCalled();

// Second response: Proceed
// Second response: User approves the modified params
emitResponse({
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
correlationId: '123e4567-e89b-12d3-a456-426614174000',
Expand Down
44 changes: 44 additions & 0 deletions packages/core/src/utils/paths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
unescapePath,
isSubpath,
shortenPath,
normalizePath,
resolveToRealPath,
} from './paths.js';

Expand Down Expand Up @@ -521,3 +522,46 @@ describe('resolveToRealPath', () => {
expect(resolveToRealPath(input)).toBe(expected);
});
});

describe('normalizePath', () => {
it('should resolve a relative path to an absolute path', () => {
const result = normalizePath('some/relative/path');
expect(result).toMatch(/^\/|^[a-z]:\//);
});

it('should convert all backslashes to forward slashes', () => {
const result = normalizePath(path.resolve('some', 'path'));
expect(result).not.toContain('\\');
});

describe.skipIf(process.platform !== 'win32')('on Windows', () => {
it('should lowercase the entire path', () => {
const result = normalizePath('C:\\Users\\TEST');
expect(result).toBe(result.toLowerCase());
});

it('should normalize drive letters to lowercase', () => {
const result = normalizePath('C:\\');
expect(result).toMatch(/^c:\//);
});

it('should handle mixed separators', () => {
const result = normalizePath('C:/Users\\Test/file.txt');
expect(result).not.toContain('\\');
expect(result).toMatch(/^c:\/users\/test\/file\.txt$/);
});
});

describe.skipIf(process.platform === 'win32')('on POSIX', () => {
it('should preserve case', () => {
const result = normalizePath('/usr/Local/Bin');
expect(result).toContain('Local');
expect(result).toContain('Bin');
});

it('should use forward slashes', () => {
const result = normalizePath('/usr/local/bin');
expect(result).toBe('/usr/local/bin');
});
});
});
6 changes: 4 additions & 2 deletions packages/core/src/utils/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,13 +319,15 @@ export function getProjectHash(projectRoot: string): string {
}

/**
* Normalizes a path for reliable comparison.
* Normalizes a path for reliable comparison across platforms.
* - Resolves to an absolute path.
* - Converts all path separators to forward slashes.
* - On Windows, converts to lowercase for case-insensitivity.
*/
export function normalizePath(p: string): string {
const resolved = path.resolve(p);
return process.platform === 'win32' ? resolved.toLowerCase() : resolved;
const normalized = resolved.replace(/\\/g, '/');
return process.platform === 'win32' ? normalized.toLowerCase() : normalized;
}

/**
Expand Down
Loading