fix(paths): Add cross-platform path normalization (#18939)

packages/core/src/scheduler/confirmation.test.ts

@@ -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,
@@ -31,7 +31,7 @@ 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
@@ -39,10 +39,19 @@ 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');
@@ -53,6 +62,7 @@ describe('confirmation.ts', () => {
   });
 
   afterEach(() => {
+    vi.unstubAllEnvs();
     vi.restoreAllMocks();
   });
 
@@ -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>;
@@ -286,8 +259,13 @@ describe('confirmation.ts', () => {
       };
       invocationMock.shouldConfirmExecute.mockResolvedValue(details);
 
-      // 1. User says Modify
+      // Set up modifier mock before starting the flow
-      // 2. User says Proceed
+      mockModifier.handleModifyWithEditor.mockResolvedValue({
+        updatedParams: { foo: 'bar' },
+      });
+      toolMock.build.mockReturnValue({} as unknown as AnyToolInvocation);
+
+      // Start the confirmation flow
       const listenerPromise1 = waitForListener(
         MessageBusType.TOOL_CONFIRMATION_RESPONSE,
       );
@@ -302,7 +280,12 @@ 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',
@@ -310,22 +293,12 @@ describe('confirmation.ts', () => {
         outcome: ToolConfirmationOutcome.ModifyWithEditor,
       });
 
-      // Mock the modifier action
+      // Wait for the loop to process the modification and re-subscribe
-      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,
-      );
       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',

packages/core/src/utils/paths.test.ts

@@ -13,6 +13,7 @@ import {
   unescapePath,
   isSubpath,
   shortenPath,
+  normalizePath,
   resolveToRealPath,
 } from './paths.js';
 
@@ -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');
+    });
+  });
+});

packages/core/src/utils/paths.ts

@@ -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;
 }
 
 /**