diff --git a/apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts b/apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts index ea4cec57d3..e67a978778 100644 --- a/apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts +++ b/apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts @@ -6,24 +6,28 @@ * Integration tests for terminal copy/paste functionality * Tests xterm.js selection API integration with clipboard operations */ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { render, act } from '@testing-library/react'; -import React from 'react'; -import type { Mock } from 'vitest'; -import { Terminal as XTerm } from '@xterm/xterm'; -import { FitAddon } from '@xterm/addon-fit'; -import { WebLinksAddon } from '@xterm/addon-web-links'; -import { SerializeAddon } from '@xterm/addon-serialize'; +import { describe, it, expect, vi, beforeEach, afterEach, beforeAll, afterAll } from "vitest"; +import { render, act } from "@testing-library/react"; +import React from "react"; +import type { Mock } from "vitest"; +import { Terminal as XTerm } from "@xterm/xterm"; +import { FitAddon } from "@xterm/addon-fit"; +import { WebLinksAddon } from "@xterm/addon-web-links"; +import { SerializeAddon } from "@xterm/addon-serialize"; // Mock xterm.js and its addons -vi.mock('@xterm/xterm', () => ({ - Terminal: vi.fn().mockImplementation(function() { +vi.mock("@xterm/xterm", () => ({ + Terminal: vi.fn().mockImplementation(function () { return { open: vi.fn(), loadAddon: vi.fn(), attachCustomKeyEventHandler: vi.fn(), - hasSelection: vi.fn(function() { return false; }), - getSelection: vi.fn(function() { return ''; }), + hasSelection: vi.fn(function () { + return false; + }), + getSelection: vi.fn(function () { + return ""; + }), paste: vi.fn(), input: vi.fn(), onData: vi.fn(), @@ -31,137 +35,249 @@ vi.mock('@xterm/xterm', () => ({ dispose: vi.fn(), write: vi.fn(), cols: 80, - rows: 24 + rows: 24, }; - }) + }), })); -vi.mock('@xterm/addon-fit', () => ({ - FitAddon: vi.fn().mockImplementation(function() { +vi.mock("@xterm/addon-fit", () => ({ + FitAddon: vi.fn().mockImplementation(function () { return { - fit: vi.fn() + fit: vi.fn(), }; - }) + }), })); -vi.mock('@xterm/addon-web-links', () => ({ - WebLinksAddon: vi.fn().mockImplementation(function() { +vi.mock("@xterm/addon-web-links", () => ({ + WebLinksAddon: vi.fn().mockImplementation(function () { return {}; - }) + }), })); -vi.mock('@xterm/addon-serialize', () => ({ - SerializeAddon: vi.fn().mockImplementation(function() { +vi.mock("@xterm/addon-serialize", () => ({ + SerializeAddon: vi.fn().mockImplementation(function () { return { - serialize: vi.fn(function() { return ''; }), - dispose: vi.fn() + serialize: vi.fn(function () { + return ""; + }), + dispose: vi.fn(), }; - }) + }), })); -describe('Terminal copy/paste integration', () => { +// Mock requestAnimationFrame for jsdom environment (not provided by default) +// Save originals to restore in afterAll hook +const origRequestAnimationFrame = global.requestAnimationFrame; +const origCancelAnimationFrame = global.cancelAnimationFrame; +const origResizeObserver = global.ResizeObserver; + +/** + * Helper function to set up XTerm and addon mocks with a key event handler capture. + * Returns a mutable wrapper containing the keyEventHandler (set when component renders). + * + * @param mockHasSelection - Optional mock for xterm.hasSelection() + * @param mockGetSelection - Optional mock for xterm.getSelection() + * @param mockPaste - Optional mock for xterm.paste() + * @param mockInput - Optional mock for xterm.input() + * @param mockOnDataCallback - Optional callback to capture the onData callback + * @param mockWrite - Optional mock for xterm.write() + * @returns A wrapper object with a `handler` property that will be set when the component attaches the key event handler + */ +function setupMockXTermWithHandler({ + mockHasSelection, + mockGetSelection, + mockPaste, + mockInput, + mockOnDataCallback, + mockWrite, +}: { + mockHasSelection?: Mock; + mockGetSelection?: Mock; + mockPaste?: Mock; + mockInput?: Mock; + mockOnDataCallback?: ((callback: (data: string) => void) => void) | null; + mockWrite?: Mock; +} = {}): { handler: ((event: KeyboardEvent) => boolean) | null } { + const result = { handler: null as ((event: KeyboardEvent) => boolean) | null }; + + // Override XTerm mock to be constructable with custom keyEventHandler capture + (XTerm as unknown as Mock).mockImplementation(function () { + const mockInstance: Record = { + open: vi.fn(), + loadAddon: vi.fn(), + attachCustomKeyEventHandler: vi.fn(function (handler: (event: KeyboardEvent) => boolean) { + result.handler = handler; + }), + hasSelection: + mockHasSelection || + vi.fn(function () { + return false; + }), + getSelection: + mockGetSelection || + vi.fn(function () { + return ""; + }), + paste: mockPaste || vi.fn(), + input: mockInput || vi.fn(), + onData: vi.fn(), + onResize: vi.fn(), + dispose: vi.fn(), + write: mockWrite || vi.fn(), + cols: 80, + rows: 24, + }; + + // Handle onData callback capture if provided + if (mockOnDataCallback) { + mockInstance.onData = mockOnDataCallback; + } + + return mockInstance; + }); + + // Override addon mocks to be constructable + (FitAddon as unknown as Mock).mockImplementation(function () { + return { fit: vi.fn() }; + }); + + (WebLinksAddon as unknown as Mock).mockImplementation(function () { + return {}; + }); + + (SerializeAddon as unknown as Mock).mockImplementation(function () { + return { + serialize: vi.fn(function () { + return ""; + }), + dispose: vi.fn(), + }; + }); + + return result; +} + +describe("Terminal copy/paste integration", () => { let mockClipboard: { writeText: Mock; readText: Mock; }; + beforeAll(() => { + // Set up global mocks once for all tests + // These persist across tests but are not affected by vi.clearAllMocks() + global.requestAnimationFrame = vi.fn( + (cb: FrameRequestCallback) => setTimeout(cb, 0) as unknown as number + ); + global.cancelAnimationFrame = vi.fn((id: unknown) => clearTimeout(id as number)); + + // Mock ResizeObserver + global.ResizeObserver = vi.fn().mockImplementation(function () { + return { + observe: vi.fn(), + unobserve: vi.fn(), + disconnect: vi.fn(), + }; + }); + }); + beforeEach(() => { vi.clearAllMocks(); + // Mock requestAnimationFrame + global.requestAnimationFrame = vi.fn( + (cb: FrameRequestCallback) => setTimeout(cb, 0) as unknown as number + ); + global.cancelAnimationFrame = vi.fn((id: unknown) => clearTimeout(id as number)); + // Mock ResizeObserver - global.ResizeObserver = vi.fn().mockImplementation(function() { + global.ResizeObserver = vi.fn().mockImplementation(function () { return { observe: vi.fn(), unobserve: vi.fn(), - disconnect: vi.fn() + disconnect: vi.fn(), }; }); // Mock navigator.clipboard mockClipboard = { writeText: vi.fn().mockResolvedValue(undefined), - readText: vi.fn().mockResolvedValue('clipboard content') + readText: vi.fn().mockResolvedValue("clipboard content"), }; - Object.defineProperty(global.navigator, 'clipboard', { + Object.defineProperty(global.navigator, "clipboard", { value: mockClipboard, - writable: true + writable: true, }); // Mock window.electronAPI (window as unknown as { electronAPI: unknown }).electronAPI = { - sendTerminalInput: vi.fn() + sendTerminalInput: vi.fn(), }; }); afterEach(() => { vi.restoreAllMocks(); + + // Re-apply global mocks for pending async callbacks (setTimeout, etc.) + // vi.restoreAllMocks() restores the original undefined values, but pending + // callbacks from the component (e.g., performInitialFit via setTimeout) may + // still need these mocks after the test completes. + global.requestAnimationFrame = vi.fn( + (cb: FrameRequestCallback) => setTimeout(cb, 0) as unknown as number + ); + global.cancelAnimationFrame = vi.fn((id: unknown) => clearTimeout(id as number)); + global.ResizeObserver = vi.fn().mockImplementation(function () { + return { + observe: vi.fn(), + unobserve: vi.fn(), + disconnect: vi.fn(), + }; + }); }); - describe('xterm.js selection API integration with clipboard write', () => { - it('should integrate xterm.hasSelection() with clipboard write', async () => { - const { useXterm } = await import('../../renderer/components/terminal/useXterm'); - - let keyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; - const mockHasSelection = vi.fn(function() { return true; }); - const mockGetSelection = vi.fn(function() { return 'selected terminal text'; }); - - // Override XTerm mock to be constructable - (XTerm as unknown as Mock).mockImplementation(function() { - return { - open: vi.fn(), - loadAddon: vi.fn(), - attachCustomKeyEventHandler: vi.fn(function(handler: (event: KeyboardEvent) => boolean) { - keyEventHandler = handler; - }), - hasSelection: mockHasSelection, - getSelection: mockGetSelection, - paste: vi.fn(), - input: vi.fn(), - onData: vi.fn(), - onResize: vi.fn(), - dispose: vi.fn(), - write: vi.fn(), - cols: 80, - rows: 24 - }; - }); + afterAll(() => { + // Note: We intentionally do NOT restore the original global shims (origRequestAnimationFrame, etc.) + // because jsdom doesn't provide these APIs anyway, and pending setTimeout callbacks from + // the component (e.g., performInitialFit) may still fire after all tests complete. + // The mocks are harmless and will be cleaned up when the test process exits. + }); - // Need to also override the addon mocks to be constructable - (FitAddon as unknown as Mock).mockImplementation(function() { - return { fit: vi.fn() }; - }); + describe("xterm.js selection API integration with clipboard write", () => { + it("should integrate xterm.hasSelection() with clipboard write", async () => { + const { useXterm } = await import("../../renderer/components/terminal/useXterm"); - (WebLinksAddon as unknown as Mock).mockImplementation(function() { - return {}; + const mockHasSelection = vi.fn(function () { + return true; + }); + const mockGetSelection = vi.fn(function () { + return "selected terminal text"; }); - (SerializeAddon as unknown as Mock).mockImplementation(function() { - return { - serialize: vi.fn(function() { return ''; }), - dispose: vi.fn() - }; + const keyEventHandlerWrapper = setupMockXTermWithHandler({ + mockHasSelection, + mockGetSelection, }); // Create a test wrapper component that provides the DOM element const TestWrapper = () => { - const { terminalRef } = useXterm({ terminalId: 'test-terminal' }); - return React.createElement('div', { ref: terminalRef }); + const { terminalRef } = useXterm({ terminalId: "test-terminal" }); + return React.createElement("div", { ref: terminalRef }); }; render(React.createElement(TestWrapper)); await act(async () => { // Simulate copy operation - const event = new KeyboardEvent('keydown', { - key: 'c', - ctrlKey: true + const event = new KeyboardEvent("keydown", { + key: "c", + ctrlKey: true, }); - if (keyEventHandler) { - keyEventHandler(event); + if (keyEventHandlerWrapper.handler) { + keyEventHandlerWrapper.handler(event); // Wait for clipboard write - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); } }); @@ -172,69 +288,40 @@ describe('Terminal copy/paste integration', () => { expect(mockGetSelection).toHaveBeenCalled(); // Verify integration: clipboard.writeText() called with selection - expect(mockClipboard.writeText).toHaveBeenCalledWith('selected terminal text'); + expect(mockClipboard.writeText).toHaveBeenCalledWith("selected terminal text"); }); - it('should not call getSelection when hasSelection returns false', async () => { - const { useXterm } = await import('../../renderer/components/terminal/useXterm'); - - let keyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; - const mockHasSelection = vi.fn(function() { return false; }); - const mockGetSelection = vi.fn(function() { return ''; }); - - // Override XTerm mock to be constructable - (XTerm as unknown as Mock).mockImplementation(function() { - return { - open: vi.fn(), - loadAddon: vi.fn(), - attachCustomKeyEventHandler: vi.fn(function(handler: (event: KeyboardEvent) => boolean) { - keyEventHandler = handler; - }), - hasSelection: mockHasSelection, - getSelection: mockGetSelection, - paste: vi.fn(), - input: vi.fn(), - onData: vi.fn(), - onResize: vi.fn(), - dispose: vi.fn(), - write: vi.fn(), - cols: 80, - rows: 24 - }; - }); + it("should not call getSelection when hasSelection returns false", async () => { + const { useXterm } = await import("../../renderer/components/terminal/useXterm"); - // Need to also override the addon mocks to be constructable - (FitAddon as unknown as Mock).mockImplementation(function() { - return { fit: vi.fn() }; + const mockHasSelection = vi.fn(function () { + return false; }); - - (WebLinksAddon as unknown as Mock).mockImplementation(function() { - return {}; + const mockGetSelection = vi.fn(function () { + return ""; }); - (SerializeAddon as unknown as Mock).mockImplementation(function() { - return { - serialize: vi.fn(function() { return ''; }), - dispose: vi.fn() - }; + const keyEventHandlerWrapper = setupMockXTermWithHandler({ + mockHasSelection, + mockGetSelection, }); // Create a test wrapper component that provides the DOM element const TestWrapper = () => { - const { terminalRef } = useXterm({ terminalId: 'test-terminal' }); - return React.createElement('div', { ref: terminalRef }); + const { terminalRef } = useXterm({ terminalId: "test-terminal" }); + return React.createElement("div", { ref: terminalRef }); }; render(React.createElement(TestWrapper)); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'c', - ctrlKey: true + const event = new KeyboardEvent("keydown", { + key: "c", + ctrlKey: true, }); - if (keyEventHandler) { - keyEventHandler(event); + if (keyEventHandlerWrapper.handler) { + keyEventHandlerWrapper.handler(event); } }); @@ -249,7 +336,7 @@ describe('Terminal copy/paste integration', () => { }); }); - describe('clipboard read with xterm paste integration', () => { + describe("clipboard read with xterm paste integration", () => { let originalNavigatorPlatform: string; beforeEach(() => { @@ -259,81 +346,47 @@ describe('Terminal copy/paste integration', () => { afterEach(() => { // Restore navigator.platform - Object.defineProperty(navigator, 'platform', { + Object.defineProperty(navigator, "platform", { value: originalNavigatorPlatform, - writable: true + writable: true, }); }); - it('should integrate clipboard.readText() with xterm.paste()', async () => { - const { useXterm } = await import('../../renderer/components/terminal/useXterm'); + it("should integrate clipboard.readText() with xterm.paste()", async () => { + const { useXterm } = await import("../../renderer/components/terminal/useXterm"); // Mock Windows platform - Object.defineProperty(navigator, 'platform', { - value: 'Win32', - writable: true + Object.defineProperty(navigator, "platform", { + value: "Win32", + writable: true, }); - let keyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; const mockPaste = vi.fn(); - // Override XTerm mock to be constructable - (XTerm as unknown as Mock).mockImplementation(function() { - return { - open: vi.fn(), - loadAddon: vi.fn(), - attachCustomKeyEventHandler: vi.fn(function(handler: (event: KeyboardEvent) => boolean) { - keyEventHandler = handler; - }), - hasSelection: vi.fn(), - getSelection: vi.fn(), - paste: mockPaste, - input: vi.fn(), - onData: vi.fn(), - onResize: vi.fn(), - dispose: vi.fn(), - write: vi.fn(), - cols: 80, - rows: 24 - }; + const keyEventHandlerWrapper = setupMockXTermWithHandler({ + mockPaste, }); - // Need to also override the addon mocks to be constructable - (FitAddon as unknown as Mock).mockImplementation(function() { - return { fit: vi.fn() }; - }); - - (WebLinksAddon as unknown as Mock).mockImplementation(function() { - return {}; - }); - - (SerializeAddon as unknown as Mock).mockImplementation(function() { - return { - serialize: vi.fn(function() { return ''; }), - dispose: vi.fn() - }; - }); - - mockClipboard.readText.mockResolvedValue('pasted text'); + mockClipboard.readText.mockResolvedValue("pasted text"); // Create a test wrapper component that provides the DOM element const TestWrapper = () => { - const { terminalRef } = useXterm({ terminalId: 'test-terminal' }); - return React.createElement('div', { ref: terminalRef }); + const { terminalRef } = useXterm({ terminalId: "test-terminal" }); + return React.createElement("div", { ref: terminalRef }); }; render(React.createElement(TestWrapper)); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'v', - ctrlKey: true + const event = new KeyboardEvent("keydown", { + key: "v", + ctrlKey: true, }); - if (keyEventHandler) { - keyEventHandler(event); + if (keyEventHandlerWrapper.handler) { + keyEventHandlerWrapper.handler(event); // Wait for clipboard read and paste - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); } }); @@ -341,79 +394,45 @@ describe('Terminal copy/paste integration', () => { expect(mockClipboard.readText).toHaveBeenCalled(); // Verify integration: xterm.paste() called with clipboard content - expect(mockPaste).toHaveBeenCalledWith('pasted text'); + expect(mockPaste).toHaveBeenCalledWith("pasted text"); }); - it('should not paste when clipboard is empty', async () => { - const { useXterm } = await import('../../renderer/components/terminal/useXterm'); + it("should not paste when clipboard is empty", async () => { + const { useXterm } = await import("../../renderer/components/terminal/useXterm"); // Mock Linux platform - Object.defineProperty(navigator, 'platform', { - value: 'Linux', - writable: true + Object.defineProperty(navigator, "platform", { + value: "Linux", + writable: true, }); - let keyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; const mockPaste = vi.fn(); - // Override XTerm mock to be constructable - (XTerm as unknown as Mock).mockImplementation(function() { - return { - open: vi.fn(), - loadAddon: vi.fn(), - attachCustomKeyEventHandler: vi.fn(function(handler: (event: KeyboardEvent) => boolean) { - keyEventHandler = handler; - }), - hasSelection: vi.fn(), - getSelection: vi.fn(), - paste: mockPaste, - input: vi.fn(), - onData: vi.fn(), - onResize: vi.fn(), - dispose: vi.fn(), - write: vi.fn(), - cols: 80, - rows: 24 - }; - }); - - // Need to also override the addon mocks to be constructable - (FitAddon as unknown as Mock).mockImplementation(function() { - return { fit: vi.fn() }; - }); - - (WebLinksAddon as unknown as Mock).mockImplementation(function() { - return {}; - }); - - (SerializeAddon as unknown as Mock).mockImplementation(function() { - return { - serialize: vi.fn(function() { return ''; }), - dispose: vi.fn() - }; + const keyEventHandlerWrapper = setupMockXTermWithHandler({ + mockPaste, }); // Mock empty clipboard - mockClipboard.readText.mockResolvedValue(''); + mockClipboard.readText.mockResolvedValue(""); // Create a test wrapper component that provides the DOM element const TestWrapper = () => { - const { terminalRef } = useXterm({ terminalId: 'test-terminal' }); - return React.createElement('div', { ref: terminalRef }); + const { terminalRef } = useXterm({ terminalId: "test-terminal" }); + return React.createElement("div", { ref: terminalRef }); }; render(React.createElement(TestWrapper)); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'v', - ctrlKey: true + const event = new KeyboardEvent("keydown", { + key: "v", + ctrlKey: true, }); - if (keyEventHandler) { - keyEventHandler(event); + if (keyEventHandlerWrapper.handler) { + keyEventHandlerWrapper.handler(event); // Wait for clipboard read - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); } }); @@ -425,102 +444,75 @@ describe('Terminal copy/paste integration', () => { }); }); - describe('keyboard event propagation', () => { - it('should prevent copy/paste events from interfering with other shortcuts', async () => { - const { useXterm } = await import('../../renderer/components/terminal/useXterm'); + describe("keyboard event propagation", () => { + it("should prevent copy/paste events from interfering with other shortcuts", async () => { + const { useXterm } = await import("../../renderer/components/terminal/useXterm"); - let keyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; let eventCallOrder: string[] = []; - - // Override XTerm mock to be constructable - (XTerm as unknown as Mock).mockImplementation(function() { - return { - open: vi.fn(), - loadAddon: vi.fn(), - attachCustomKeyEventHandler: vi.fn(function(handler: (event: KeyboardEvent) => boolean) { - keyEventHandler = handler; - }), - hasSelection: vi.fn(function() { return true; }), - getSelection: vi.fn(function() { return 'selection'; }), - paste: vi.fn(), - input: vi.fn(function(data: string) { - eventCallOrder.push(`input:${data}`); - }), - onData: vi.fn(), - onResize: vi.fn(), - dispose: vi.fn(), - write: vi.fn(), - cols: 80, - rows: 24 - }; + const mockInput = vi.fn(function (data: string) { + eventCallOrder.push(`input:${data}`); }); - // Need to also override the addon mocks to be constructable - (FitAddon as unknown as Mock).mockImplementation(function() { - return { fit: vi.fn() }; - }); - - (WebLinksAddon as unknown as Mock).mockImplementation(function() { - return {}; - }); - - (SerializeAddon as unknown as Mock).mockImplementation(function() { - return { - serialize: vi.fn(function() { return ''; }), - dispose: vi.fn() - }; + const keyEventHandlerWrapper = setupMockXTermWithHandler({ + mockHasSelection: vi.fn(function () { + return true; + }), + mockGetSelection: vi.fn(function () { + return "selection"; + }), + mockInput, }); // Create a test wrapper component that provides the DOM element const TestWrapper = () => { - const { terminalRef } = useXterm({ terminalId: 'test-terminal' }); - return React.createElement('div', { ref: terminalRef }); + const { terminalRef } = useXterm({ terminalId: "test-terminal" }); + return React.createElement("div", { ref: terminalRef }); }; render(React.createElement(TestWrapper)); await act(async () => { // Test SHIFT+Enter (should work independently of copy/paste) - const shiftEnterEvent = new KeyboardEvent('keydown', { - key: 'Enter', + const shiftEnterEvent = new KeyboardEvent("keydown", { + key: "Enter", shiftKey: true, ctrlKey: false, - metaKey: false + metaKey: false, }); - if (keyEventHandler) { - keyEventHandler(shiftEnterEvent); + if (keyEventHandlerWrapper.handler) { + keyEventHandlerWrapper.handler(shiftEnterEvent); } // Verify SHIFT+Enter still works (sends newline) - expect(eventCallOrder.some(s => s.includes('\x1b\n'))).toBe(true); + expect(eventCallOrder.some((s) => s.includes("\x1b\n"))).toBe(true); // Test CTRL+C with selection (should not interfere) eventCallOrder = []; - const copyEvent = new KeyboardEvent('keydown', { - key: 'c', - ctrlKey: true + const copyEvent = new KeyboardEvent("keydown", { + key: "c", + ctrlKey: true, }); - if (keyEventHandler) { - keyEventHandler(copyEvent); + if (keyEventHandlerWrapper.handler) { + keyEventHandlerWrapper.handler(copyEvent); // Wait for clipboard write - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); } // Copy should not send input to terminal expect(eventCallOrder).toHaveLength(0); // Test CTRL+V (should not interfere) - const pasteEvent = new KeyboardEvent('keydown', { - key: 'v', - ctrlKey: true + const pasteEvent = new KeyboardEvent("keydown", { + key: "v", + ctrlKey: true, }); - if (keyEventHandler) { - keyEventHandler(pasteEvent); + if (keyEventHandlerWrapper.handler) { + keyEventHandlerWrapper.handler(pasteEvent); // Wait for clipboard read - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); } // Paste should use xterm.paste(), not xterm.input() @@ -529,87 +521,55 @@ describe('Terminal copy/paste integration', () => { }); }); - it('should maintain correct handler ordering for existing shortcuts', async () => { - const { useXterm } = await import('../../renderer/components/terminal/useXterm'); + it("should maintain correct handler ordering for existing shortcuts", async () => { + const { useXterm } = await import("../../renderer/components/terminal/useXterm"); - let keyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; let handlerResults: { key: string; handled: boolean }[] = []; - const mockHasSelection = vi.fn(function() { return false; }); - - // Override XTerm mock to be constructable - (XTerm as unknown as Mock).mockImplementation(function() { - return { - open: vi.fn(), - loadAddon: vi.fn(), - attachCustomKeyEventHandler: vi.fn(function(handler: (event: KeyboardEvent) => boolean) { - keyEventHandler = handler; - }), - hasSelection: mockHasSelection, - getSelection: vi.fn(), - paste: vi.fn(), - input: vi.fn(), - onData: vi.fn(), - onResize: vi.fn(), - dispose: vi.fn(), - write: vi.fn(), - cols: 80, - rows: 24 - }; - }); - - // Need to also override the addon mocks to be constructable - (FitAddon as unknown as Mock).mockImplementation(function() { - return { fit: vi.fn() }; - }); - - (WebLinksAddon as unknown as Mock).mockImplementation(function() { - return {}; + const mockHasSelection = vi.fn(function () { + return false; }); - (SerializeAddon as unknown as Mock).mockImplementation(function() { - return { - serialize: vi.fn(function() { return ''; }), - dispose: vi.fn() - }; + const keyEventHandlerWrapper = setupMockXTermWithHandler({ + mockHasSelection, }); // Create a test wrapper component that provides the DOM element const TestWrapper = () => { - const { terminalRef } = useXterm({ terminalId: 'test-terminal' }); - return React.createElement('div', { ref: terminalRef }); + const { terminalRef } = useXterm({ terminalId: "test-terminal" }); + return React.createElement("div", { ref: terminalRef }); }; render(React.createElement(TestWrapper)); // Helper to test key handling const testKey = (key: string, ctrl: boolean, meta: boolean, shift: boolean) => { - const event = new KeyboardEvent('keydown', { + const event = new KeyboardEvent("keydown", { key, ctrlKey: ctrl, metaKey: meta, - shiftKey: shift + shiftKey: shift, }); - if (keyEventHandler) { - const handled = keyEventHandler(event); + if (keyEventHandlerWrapper.handler) { + const handled = keyEventHandlerWrapper.handler(event); handlerResults.push({ key, handled }); } }; await act(async () => { // Test existing shortcuts (should return false to bubble up) - testKey('1', true, false, false); // Ctrl+1 - testKey('Tab', true, false, false); // Ctrl+Tab - testKey('t', true, false, false); // Ctrl+T - testKey('w', true, false, false); // Ctrl+W + testKey("1", true, false, false); // Ctrl+1 + testKey("Tab", true, false, false); // Ctrl+Tab + testKey("t", true, false, false); // Ctrl+T + testKey("w", true, false, false); // Ctrl+W // Verify these return false (bubble to window handler) - expect(handlerResults.filter(r => !r.handled)).toHaveLength(4); + expect(handlerResults.filter((r) => !r.handled)).toHaveLength(4); // Test copy/paste WITHOUT selection (should pass through to send ^C) handlerResults = []; mockHasSelection.mockReturnValue(false); - testKey('c', true, false, false); // Ctrl+C without selection + testKey("c", true, false, false); // Ctrl+C without selection // Should return true (let ^C pass through to terminal for interrupt signal) expect(handlerResults[0].handled).toBe(true); @@ -617,95 +577,65 @@ describe('Terminal copy/paste integration', () => { }); }); - describe('clipboard error handling without breaking terminal', () => { - it('should continue terminal operation after clipboard error', async () => { - const { useXterm } = await import('../../renderer/components/terminal/useXterm'); + describe("clipboard error handling without breaking terminal", () => { + it("should continue terminal operation after clipboard error", async () => { + const { useXterm } = await import("../../renderer/components/terminal/useXterm"); // Mock Windows platform to enable custom paste handler - Object.defineProperty(navigator, 'platform', { - value: 'Win32', - writable: true + Object.defineProperty(navigator, "platform", { + value: "Win32", + writable: true, }); - let keyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; const mockPaste = vi.fn(); const mockInput = vi.fn(); const mockSendTerminalInput = vi.fn(); let onDataCallback: ((data: string) => void) | undefined; let errorLogged = false; - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(function(...args: unknown[]) { - if (String(args[0]).includes('[useXterm]')) { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(function ( + ...args: unknown[] + ) { + if (String(args[0]).includes("[useXterm]")) { errorLogged = true; } }); // Mock clipboard error - mockClipboard.readText = vi.fn().mockRejectedValue(new Error('Clipboard denied')); + mockClipboard.readText = vi.fn().mockRejectedValue(new Error("Clipboard denied")); // Mock window.electronAPI with sendTerminalInput (window as unknown as { electronAPI: { sendTerminalInput: Mock } }).electronAPI = { - sendTerminalInput: mockSendTerminalInput + sendTerminalInput: mockSendTerminalInput, }; - // Override XTerm mock to be constructable - (XTerm as unknown as Mock).mockImplementation(function() { - return { - open: vi.fn(), - loadAddon: vi.fn(), - attachCustomKeyEventHandler: vi.fn(function(handler: (event: KeyboardEvent) => boolean) { - keyEventHandler = handler; - }), - hasSelection: vi.fn(), - getSelection: vi.fn(), - paste: mockPaste, - input: mockInput, - onData: vi.fn(function(callback: (data: string) => void) { - onDataCallback = callback; - }), - onResize: vi.fn(), - dispose: vi.fn(), - write: vi.fn(), - cols: 80, - rows: 24 - }; - }); - - // Need to also override the addon mocks to be constructable - (FitAddon as unknown as Mock).mockImplementation(function() { - return { fit: vi.fn() }; - }); - - (WebLinksAddon as unknown as Mock).mockImplementation(function() { - return {}; - }); - - (SerializeAddon as unknown as Mock).mockImplementation(function() { - return { - serialize: vi.fn(function() { return ''; }), - dispose: vi.fn() - }; + const keyEventHandlerWrapper = setupMockXTermWithHandler({ + mockPaste, + mockInput, + mockOnDataCallback: vi.fn(function (callback: (data: string) => void) { + onDataCallback = callback; + }), }); // Create a test wrapper component that provides the DOM element const TestWrapper = () => { - const { terminalRef } = useXterm({ terminalId: 'test-terminal' }); - return React.createElement('div', { ref: terminalRef }); + const { terminalRef } = useXterm({ terminalId: "test-terminal" }); + return React.createElement("div", { ref: terminalRef }); }; render(React.createElement(TestWrapper)); await act(async () => { // Try to paste (will fail) - const pasteEvent = new KeyboardEvent('keydown', { - key: 'v', - ctrlKey: true + const pasteEvent = new KeyboardEvent("keydown", { + key: "v", + ctrlKey: true, }); - if (keyEventHandler) { - keyEventHandler(pasteEvent); + if (keyEventHandlerWrapper.handler) { + keyEventHandlerWrapper.handler(pasteEvent); // Wait for clipboard error - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); } }); @@ -713,14 +643,14 @@ describe('Terminal copy/paste integration', () => { expect(errorLogged).toBe(true); // Verify terminal still works (can accept input through onData callback) - const inputData = 'test command'; + const inputData = "test command"; if (onDataCallback) { onDataCallback(inputData); } // Verify input was sent to electronAPI (terminal still functional) - expect(mockSendTerminalInput).toHaveBeenCalledWith('test-terminal', 'test command'); + expect(mockSendTerminalInput).toHaveBeenCalledWith("test-terminal", "test command"); consoleErrorSpy.mockRestore(); }); diff --git a/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts b/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts index 5222690779..0167a0c9f5 100644 --- a/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts +++ b/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts @@ -3,51 +3,51 @@ * Tests CLI tool detection with focus on NVM path detection */ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { existsSync, readdirSync } from 'fs'; -import os from 'os'; -import { execFileSync } from 'child_process'; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { existsSync, readdirSync } from "fs"; +import os from "os"; +import { execFileSync } from "child_process"; import { getToolInfo, getToolPathAsync, clearToolCache, getClaudeDetectionPaths, sortNvmVersionDirs, - buildClaudeDetectionResult -} from '../cli-tool-manager'; + buildClaudeDetectionResult, +} from "../cli-tool-manager"; import { findWindowsExecutableViaWhere, - findWindowsExecutableViaWhereAsync -} from '../utils/windows-paths'; -import { findExecutable, findExecutableAsync } from '../env-utils'; + findWindowsExecutableViaWhereAsync, +} from "../utils/windows-paths"; +import { findExecutable, findExecutableAsync } from "../env-utils"; // Mock Electron app -vi.mock('electron', () => ({ +vi.mock("electron", () => ({ app: { isPackaged: false, - getPath: vi.fn() - } + getPath: vi.fn(), + }, })); // Mock os module -vi.mock('os', () => ({ +vi.mock("os", () => ({ default: { - homedir: vi.fn(() => '/mock/home') - } + homedir: vi.fn(() => "/mock/home"), + }, })); // Mock fs module - need to mock both sync and promises -vi.mock('fs', () => ({ +vi.mock("fs", () => ({ existsSync: vi.fn(), readdirSync: vi.fn(), - promises: {} + promises: {}, })); // Mock child_process for execFileSync, execFile, execSync, and exec (used in validation) // execFile and exec need to be promisify-compatible // IMPORTANT: execSync and execFileSync share the same mock so tests that set one will affect both // This is because validateClaude() uses execSync for .cmd files and execFileSync for others -vi.mock('child_process', () => { +vi.mock("child_process", () => { // Shared mock for sync execution - both execFileSync and execSync use this // so when tests call vi.mocked(execFileSync).mockReturnValue(), it affects execSync too const sharedSyncMock = vi.fn(); @@ -57,12 +57,12 @@ vi.mock('child_process', () => { const childProcess = { stdout: { on: vi.fn() }, stderr: { on: vi.fn() }, - on: vi.fn() + on: vi.fn(), }; // If callback is provided, call it asynchronously - if (typeof callback === 'function') { - setImmediate(() => callback(null, 'claude-code version 1.0.0\n', '')); + if (typeof callback === "function") { + setImmediate(() => callback(null, "claude-code version 1.0.0\n", "")); } return childProcess as any; @@ -73,12 +73,12 @@ vi.mock('child_process', () => { const childProcess = { stdout: { on: vi.fn() }, stderr: { on: vi.fn() }, - on: vi.fn() + on: vi.fn(), }; // If callback is provided, call it asynchronously - if (typeof callback === 'function') { - setImmediate(() => callback(null, 'claude-code version 1.0.0\n', '')); + if (typeof callback === "function") { + setImmediate(() => callback(null, "claude-code version 1.0.0\n", "")); } return childProcess as any; @@ -87,49 +87,59 @@ vi.mock('child_process', () => { return { execFileSync: sharedSyncMock, execFile: mockExecFile, - execSync: sharedSyncMock, // Share with execFileSync so tests work for both - exec: mockExec + execSync: sharedSyncMock, // Share with execFileSync so tests work for both + exec: mockExec, }; }); // Mock env-utils to avoid PATH augmentation complexity -vi.mock('../env-utils', () => ({ +vi.mock("../env-utils", () => ({ findExecutable: vi.fn(() => null), // Return null to force platform-specific path checking findExecutableAsync: vi.fn(() => Promise.resolve(null)), - getAugmentedEnv: vi.fn(() => ({ PATH: '' })), - getAugmentedEnvAsync: vi.fn(() => Promise.resolve({ PATH: '' })), + getAugmentedEnv: vi.fn(() => ({ PATH: "" })), + getAugmentedEnvAsync: vi.fn(() => Promise.resolve({ PATH: "" })), + prepareShellCommand: vi.fn((command: string) => { + // Mock prepareShellCommand to match actual behavior + const needsShell = process.platform === "win32" && /\.(cmd|bat)$/i.test(command); + return { command, needsShell }; + }), + preparePythonSubprocessCommand: vi.fn((command: string) => { + // Mock preparePythonSubprocessCommand to match actual behavior + const needsShell = process.platform === "win32" && /\.(cmd|bat)$/i.test(command); + return { command, needsShell }; + }), shouldUseShell: vi.fn((command: string) => { // Mock shouldUseShell to match actual behavior - if (process.platform !== 'win32') { + if (process.platform !== "win32") { return false; } return /\.(cmd|bat)$/i.test(command); }), getSpawnOptions: vi.fn((command: string, baseOptions?: any) => ({ ...baseOptions, - shell: /\.(cmd|bat)$/i.test(command) && process.platform === 'win32' + shell: /\.(cmd|bat)$/i.test(command) && process.platform === "win32", })), - existsAsync: vi.fn(() => Promise.resolve(false)) + existsAsync: vi.fn(() => Promise.resolve(false)), })); // Mock homebrew-python utility -vi.mock('../utils/homebrew-python', () => ({ - findHomebrewPython: vi.fn(() => null) +vi.mock("../utils/homebrew-python", () => ({ + findHomebrewPython: vi.fn(() => null), })); // Mock windows-paths utility -vi.mock('../utils/windows-paths', () => ({ +vi.mock("../utils/windows-paths", () => ({ findWindowsExecutableViaWhere: vi.fn(() => null), - findWindowsExecutableViaWhereAsync: vi.fn(() => Promise.resolve(null)) + findWindowsExecutableViaWhereAsync: vi.fn(() => Promise.resolve(null)), })); -describe('cli-tool-manager - Claude CLI NVM detection', () => { +describe("cli-tool-manager - Claude CLI NVM detection", () => { beforeEach(() => { vi.clearAllMocks(); // Set default platform to Linux - Object.defineProperty(process, 'platform', { - value: 'linux', - writable: true + Object.defineProperty(process, "platform", { + value: "linux", + writable: true, }); }); @@ -137,10 +147,10 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { clearToolCache(); }); - const mockHomeDir = '/mock/home'; + const mockHomeDir = "/mock/home"; - describe('NVM path detection on Unix/Linux/macOS', () => { - it('should detect Claude CLI in NVM directory when multiple Node versions exist', () => { + describe("NVM path detection on Unix/Linux/macOS", () => { + it("should detect Claude CLI in NVM directory when multiple Node versions exist", () => { // Mock home directory vi.mocked(os.homedir).mockReturnValue(mockHomeDir); @@ -148,11 +158,11 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); // NVM versions directory exists - if (pathStr.includes('.nvm/versions/node') || pathStr.includes('.nvm\\versions\\node')) { + if (pathStr.includes(".nvm/versions/node") || pathStr.includes(".nvm\\versions\\node")) { return true; } // Claude CLI exists in v22.17.0 - if (pathStr.includes('v22.17.0/bin/claude') || pathStr.includes('v22.17.0\\bin\\claude')) { + if (pathStr.includes("v22.17.0/bin/claude") || pathStr.includes("v22.17.0\\bin\\claude")) { return true; } return false; @@ -160,356 +170,360 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { // Mock Node.js version directories (three versions) const mockDirents = [ - { name: 'v20.0.0', isDirectory: () => true }, - { name: 'v22.17.0', isDirectory: () => true }, - { name: 'v18.20.0', isDirectory: () => true }, + { name: "v20.0.0", isDirectory: () => true }, + { name: "v22.17.0", isDirectory: () => true }, + { name: "v18.20.0", isDirectory: () => true }, ]; vi.mocked(readdirSync).mockReturnValue(mockDirents as any); // Mock execFileSync to simulate successful version check - vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); + vi.mocked(execFileSync).mockReturnValue("claude-code version 1.0.0\n"); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); expect(result.found).toBe(true); // Path should contain version and claude (works with both / and \ separators) expect(result.path).toMatch(/v22\.17\.0[/\\]bin[/\\]claude/); - expect(result.version).toBe('1.0.0'); - expect(result.source).toBe('nvm'); - expect(result.message).toContain('Using NVM Claude CLI'); + expect(result.version).toBe("1.0.0"); + expect(result.source).toBe("nvm"); + expect(result.message).toContain("Using NVM Claude CLI"); }); - it('should skip NVM path detection on Windows', () => { + it("should skip NVM path detection on Windows", () => { // Set platform to Windows - Object.defineProperty(process, 'platform', { - value: 'win32', - writable: true + Object.defineProperty(process, "platform", { + value: "win32", + writable: true, }); - vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + vi.mocked(os.homedir).mockReturnValue("C:\\Users\\test"); vi.mocked(existsSync).mockReturnValue(false); vi.mocked(readdirSync).mockReturnValue([]); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); // readdirSync should not be called for NVM on Windows expect(readdirSync).not.toHaveBeenCalled(); - expect(result.source).toBe('fallback'); // Should fallback since no other paths exist + expect(result.source).toBe("fallback"); // Should fallback since no other paths exist }); - it('should handle missing NVM directory gracefully', () => { + it("should handle missing NVM directory gracefully", () => { vi.mocked(os.homedir).mockReturnValue(mockHomeDir); // NVM directory doesn't exist vi.mocked(existsSync).mockReturnValue(false); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); // Should not crash, should continue to platform paths expect(result).toBeDefined(); expect(result.found).toBe(false); }); - it('should try next version if Claude not found in newest Node version', () => { + it("should try next version if Claude not found in newest Node version", () => { vi.mocked(os.homedir).mockReturnValue(mockHomeDir); // NVM directory exists, but Claude only exists in v20.0.0 vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); // Check for claude binary paths first (more specific) - if (pathStr.includes('claude')) { + if (pathStr.includes("claude")) { // Claude only exists in v20.0.0, not in v22.17.0 - return pathStr.includes('v20.0.0'); + return pathStr.includes("v20.0.0"); } // NVM versions directory exists - if (pathStr.includes('.nvm')) { + if (pathStr.includes(".nvm")) { return true; } return false; }); const mockDirents = [ - { name: 'v22.17.0', isDirectory: () => true }, - { name: 'v20.0.0', isDirectory: () => true }, + { name: "v22.17.0", isDirectory: () => true }, + { name: "v20.0.0", isDirectory: () => true }, ]; vi.mocked(readdirSync).mockReturnValue(mockDirents as any); - vi.mocked(execFileSync).mockReturnValue('claude-code version 1.5.0\n'); + vi.mocked(execFileSync).mockReturnValue("claude-code version 1.5.0\n"); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); expect(result.found).toBe(true); expect(result.path).toMatch(/v20\.0\.0[/\\]bin[/\\]claude/); }); - it('should validate Claude CLI before returning NVM path', () => { + it("should validate Claude CLI before returning NVM path", () => { vi.mocked(os.homedir).mockReturnValue(mockHomeDir); vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); // Check for claude binary paths first - if (pathStr.includes('claude')) { - return pathStr.includes('v22.17.0'); + if (pathStr.includes("claude")) { + return pathStr.includes("v22.17.0"); } // NVM directory exists - if (pathStr.includes('.nvm')) return true; + if (pathStr.includes(".nvm")) return true; return false; }); - const mockDirents = [ - { name: 'v22.17.0', isDirectory: () => true }, - ]; + const mockDirents = [{ name: "v22.17.0", isDirectory: () => true }]; vi.mocked(readdirSync).mockReturnValue(mockDirents as any); // Mock validation failure vi.mocked(execFileSync).mockImplementation(() => { - throw new Error('Command not found or invalid'); + throw new Error("Command not found or invalid"); }); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); // Should not return invalid Claude path, should continue to platform paths expect(result.found).toBe(false); - expect(result.source).toBe('fallback'); + expect(result.source).toBe("fallback"); }); - it('should use version sorting to prioritize newest Node version', () => { + it("should use version sorting to prioritize newest Node version", () => { vi.mocked(os.homedir).mockReturnValue(mockHomeDir); vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); - if (pathStr.includes('.nvm/versions/node') || pathStr.includes('.nvm\\versions\\node')) return true; + if (pathStr.includes(".nvm/versions/node") || pathStr.includes(".nvm\\versions\\node")) + return true; // Claude exists in all versions - if (pathStr.includes('/bin/claude') || pathStr.includes('\\bin\\claude')) return true; + if (pathStr.includes("/bin/claude") || pathStr.includes("\\bin\\claude")) return true; return false; }); // Versions in random order const mockDirents = [ - { name: 'v18.20.0', isDirectory: () => true }, - { name: 'v22.17.0', isDirectory: () => true }, - { name: 'v20.5.0', isDirectory: () => true }, + { name: "v18.20.0", isDirectory: () => true }, + { name: "v22.17.0", isDirectory: () => true }, + { name: "v20.5.0", isDirectory: () => true }, ]; vi.mocked(readdirSync).mockReturnValue(mockDirents as any); - vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); + vi.mocked(execFileSync).mockReturnValue("claude-code version 1.0.0\n"); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); expect(result.found).toBe(true); - expect(result.path).toContain('v22.17.0'); // Highest version + expect(result.path).toContain("v22.17.0"); // Highest version }); }); - describe('Platform-specific path detection', () => { - it('should detect Claude CLI in Windows AppData npm global path', () => { - Object.defineProperty(process, 'platform', { - value: 'win32', - writable: true + describe("Platform-specific path detection", () => { + it("should detect Claude CLI in Windows AppData npm global path", () => { + Object.defineProperty(process, "platform", { + value: "win32", + writable: true, }); - vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + vi.mocked(os.homedir).mockReturnValue("C:\\Users\\test"); vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); // Check path components (path.join uses host OS separator) - if (pathStr.includes('AppData') && - pathStr.includes('npm') && - pathStr.includes('claude.cmd')) { + if ( + pathStr.includes("AppData") && + pathStr.includes("npm") && + pathStr.includes("claude.cmd") + ) { return true; } return false; }); - vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); + vi.mocked(execFileSync).mockReturnValue("claude-code version 1.0.0\n"); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); expect(result.found).toBe(true); expect(result.path).toMatch(/AppData[/\\]Roaming[/\\]npm[/\\]claude\.cmd/); - expect(result.source).toBe('system-path'); + expect(result.source).toBe("system-path"); }); - it('should detect Claude CLI in Unix .local/bin path', () => { - vi.mocked(os.homedir).mockReturnValue('/home/user'); + it("should detect Claude CLI in Unix .local/bin path", () => { + vi.mocked(os.homedir).mockReturnValue("/home/user"); vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); - if (pathStr.includes('.local/bin/claude') || pathStr.includes('.local\\bin\\claude')) { + if (pathStr.includes(".local/bin/claude") || pathStr.includes(".local\\bin\\claude")) { return true; } return false; }); - vi.mocked(execFileSync).mockReturnValue('claude-code version 2.0.0\n'); + vi.mocked(execFileSync).mockReturnValue("claude-code version 2.0.0\n"); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); expect(result.found).toBe(true); expect(result.path).toMatch(/\.local[/\\]bin[/\\]claude/); - expect(result.version).toBe('2.0.0'); + expect(result.version).toBe("2.0.0"); }); - it('should return fallback when Claude CLI not found anywhere', () => { - vi.mocked(os.homedir).mockReturnValue('/home/user'); + it("should return fallback when Claude CLI not found anywhere", () => { + vi.mocked(os.homedir).mockReturnValue("/home/user"); vi.mocked(existsSync).mockReturnValue(false); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); expect(result.found).toBe(false); - expect(result.source).toBe('fallback'); - expect(result.message).toContain('Claude CLI not found'); + expect(result.source).toBe("fallback"); + expect(result.message).toContain("Claude CLI not found"); }); }); - }); /** * Unit tests for helper functions */ -describe('cli-tool-manager - Helper Functions', () => { - describe('getClaudeDetectionPaths', () => { - it('should return homebrew paths on macOS', () => { - Object.defineProperty(process, 'platform', { - value: 'darwin', - writable: true +describe("cli-tool-manager - Helper Functions", () => { + describe("getClaudeDetectionPaths", () => { + it("should return homebrew paths on macOS", () => { + Object.defineProperty(process, "platform", { + value: "darwin", + writable: true, }); - const paths = getClaudeDetectionPaths('/Users/test'); + const paths = getClaudeDetectionPaths("/Users/test"); - expect(paths.homebrewPaths).toContain('/opt/homebrew/bin/claude'); - expect(paths.homebrewPaths).toContain('/usr/local/bin/claude'); + expect(paths.homebrewPaths).toContain("/opt/homebrew/bin/claude"); + expect(paths.homebrewPaths).toContain("/usr/local/bin/claude"); }); - it('should return Windows paths on win32', () => { - Object.defineProperty(process, 'platform', { - value: 'win32', - writable: true + it("should return Windows paths on win32", () => { + Object.defineProperty(process, "platform", { + value: "win32", + writable: true, }); - const paths = getClaudeDetectionPaths('C:\\Users\\test'); + const paths = getClaudeDetectionPaths("C:\\Users\\test"); // Windows paths should include AppData and Program Files - expect(paths.platformPaths.some(p => p.includes('AppData'))).toBe(true); - expect(paths.platformPaths.some(p => p.includes('Program Files'))).toBe(true); + expect(paths.platformPaths.some((p) => p.includes("AppData"))).toBe(true); + expect(paths.platformPaths.some((p) => p.includes("Program Files"))).toBe(true); }); - it('should return Unix paths on Linux', () => { - Object.defineProperty(process, 'platform', { - value: 'linux', - writable: true + it("should return Unix paths on Linux", () => { + Object.defineProperty(process, "platform", { + value: "linux", + writable: true, }); - const paths = getClaudeDetectionPaths('/home/test'); + const paths = getClaudeDetectionPaths("/home/test"); // Check for paths containing the expected components (works with both / and \ separators) - expect(paths.platformPaths.some(p => p.includes('.local') && p.includes('bin') && p.includes('claude'))).toBe(true); - expect(paths.platformPaths.some(p => p.includes('bin') && p.includes('claude'))).toBe(true); + expect( + paths.platformPaths.some( + (p) => p.includes(".local") && p.includes("bin") && p.includes("claude") + ) + ).toBe(true); + expect(paths.platformPaths.some((p) => p.includes("bin") && p.includes("claude"))).toBe(true); }); - it('should return correct NVM versions directory', () => { - const paths = getClaudeDetectionPaths('/home/test'); + it("should return correct NVM versions directory", () => { + const paths = getClaudeDetectionPaths("/home/test"); // Check path components exist (works with both / and \ separators) - expect(paths.nvmVersionsDir).toContain('.nvm'); - expect(paths.nvmVersionsDir).toContain('versions'); - expect(paths.nvmVersionsDir).toContain('node'); + expect(paths.nvmVersionsDir).toContain(".nvm"); + expect(paths.nvmVersionsDir).toContain("versions"); + expect(paths.nvmVersionsDir).toContain("node"); }); }); - describe('sortNvmVersionDirs', () => { - it('should sort versions in descending order (newest first)', () => { + describe("sortNvmVersionDirs", () => { + it("should sort versions in descending order (newest first)", () => { const entries = [ - { name: 'v18.20.0', isDirectory: () => true }, - { name: 'v22.17.0', isDirectory: () => true }, - { name: 'v20.5.0', isDirectory: () => true }, + { name: "v18.20.0", isDirectory: () => true }, + { name: "v22.17.0", isDirectory: () => true }, + { name: "v20.5.0", isDirectory: () => true }, ]; const sorted = sortNvmVersionDirs(entries); - expect(sorted).toEqual(['v22.17.0', 'v20.5.0', 'v18.20.0']); + expect(sorted).toEqual(["v22.17.0", "v20.5.0", "v18.20.0"]); }); - it('should filter out non-version directories', () => { + it("should filter out non-version directories", () => { const entries = [ - { name: 'v20.0.0', isDirectory: () => true }, - { name: 'current', isDirectory: () => true }, - { name: '.DS_Store', isDirectory: () => false }, - { name: 'system', isDirectory: () => true }, + { name: "v20.0.0", isDirectory: () => true }, + { name: "current", isDirectory: () => true }, + { name: ".DS_Store", isDirectory: () => false }, + { name: "system", isDirectory: () => true }, ]; const sorted = sortNvmVersionDirs(entries); - expect(sorted).toEqual(['v20.0.0']); - expect(sorted).not.toContain('current'); - expect(sorted).not.toContain('system'); + expect(sorted).toEqual(["v20.0.0"]); + expect(sorted).not.toContain("current"); + expect(sorted).not.toContain("system"); }); - it('should handle malformed version strings', () => { + it("should handle malformed version strings", () => { const entries = [ - { name: 'v22.17.0', isDirectory: () => true }, - { name: 'v20.abc.1', isDirectory: () => true }, // Invalid version - { name: 'v18.20.0', isDirectory: () => true }, + { name: "v22.17.0", isDirectory: () => true }, + { name: "v20.abc.1", isDirectory: () => true }, // Invalid version + { name: "v18.20.0", isDirectory: () => true }, ]; const sorted = sortNvmVersionDirs(entries); // Should filter out malformed versions - expect(sorted).toContain('v22.17.0'); - expect(sorted).toContain('v18.20.0'); - expect(sorted).not.toContain('v20.abc.1'); + expect(sorted).toContain("v22.17.0"); + expect(sorted).toContain("v18.20.0"); + expect(sorted).not.toContain("v20.abc.1"); }); - it('should handle patch version comparison correctly', () => { + it("should handle patch version comparison correctly", () => { const entries = [ - { name: 'v20.0.1', isDirectory: () => true }, - { name: 'v20.0.10', isDirectory: () => true }, - { name: 'v20.0.2', isDirectory: () => true }, + { name: "v20.0.1", isDirectory: () => true }, + { name: "v20.0.10", isDirectory: () => true }, + { name: "v20.0.2", isDirectory: () => true }, ]; const sorted = sortNvmVersionDirs(entries); - expect(sorted).toEqual(['v20.0.10', 'v20.0.2', 'v20.0.1']); + expect(sorted).toEqual(["v20.0.10", "v20.0.2", "v20.0.1"]); }); }); - describe('buildClaudeDetectionResult', () => { - it('should return null when validation fails', () => { + describe("buildClaudeDetectionResult", () => { + it("should return null when validation fails", () => { const result = buildClaudeDetectionResult( - '/path/to/claude', - { valid: false, message: 'Not valid' }, - 'nvm', - 'Found via NVM' + "/path/to/claude", + { valid: false, message: "Not valid" }, + "nvm", + "Found via NVM" ); expect(result).toBeNull(); }); - it('should return proper result when validation succeeds', () => { + it("should return proper result when validation succeeds", () => { const result = buildClaudeDetectionResult( - '/path/to/claude', - { valid: true, version: '1.0.0', message: 'Valid' }, - 'nvm', - 'Found via NVM' + "/path/to/claude", + { valid: true, version: "1.0.0", message: "Valid" }, + "nvm", + "Found via NVM" ); expect(result).not.toBeNull(); expect(result?.found).toBe(true); - expect(result?.path).toBe('/path/to/claude'); - expect(result?.version).toBe('1.0.0'); - expect(result?.source).toBe('nvm'); - expect(result?.message).toContain('Found via NVM'); - expect(result?.message).toContain('/path/to/claude'); + expect(result?.path).toBe("/path/to/claude"); + expect(result?.version).toBe("1.0.0"); + expect(result?.source).toBe("nvm"); + expect(result?.message).toContain("Found via NVM"); + expect(result?.message).toContain("/path/to/claude"); }); - it('should include path in message', () => { + it("should include path in message", () => { const result = buildClaudeDetectionResult( - '/home/user/.nvm/versions/node/v22.17.0/bin/claude', - { valid: true, version: '2.0.0', message: 'OK' }, - 'nvm', - 'Detected Claude CLI' + "/home/user/.nvm/versions/node/v22.17.0/bin/claude", + { valid: true, version: "2.0.0", message: "OK" }, + "nvm", + "Detected Claude CLI" ); - expect(result?.message).toContain('Detected Claude CLI'); - expect(result?.message).toContain('/home/user/.nvm/versions/node/v22.17.0/bin/claude'); + expect(result?.message).toContain("Detected Claude CLI"); + expect(result?.message).toContain("/home/user/.nvm/versions/node/v22.17.0/bin/claude"); }); }); }); @@ -517,12 +531,12 @@ describe('cli-tool-manager - Helper Functions', () => { /** * Unit tests for Claude CLI Windows where.exe detection */ -describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { +describe("cli-tool-manager - Claude CLI Windows where.exe detection", () => { beforeEach(() => { vi.clearAllMocks(); - Object.defineProperty(process, 'platform', { - value: 'win32', - writable: true + Object.defineProperty(process, "platform", { + value: "win32", + writable: true, }); }); @@ -530,42 +544,42 @@ describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { clearToolCache(); }); - it('should detect Claude CLI via where.exe when not in PATH', () => { - vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + it("should detect Claude CLI via where.exe when not in PATH", () => { + vi.mocked(os.homedir).mockReturnValue("C:\\Users\\test"); // Mock findExecutable returns null (not in PATH) vi.mocked(findExecutable).mockReturnValue(null); // Mock where.exe finds it in nvm-windows location vi.mocked(findWindowsExecutableViaWhere).mockReturnValue( - 'D:\\Program Files\\nvm4w\\nodejs\\claude.cmd' + "D:\\Program Files\\nvm4w\\nodejs\\claude.cmd" ); // Mock file system checks vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); - if (pathStr.includes('nvm4w') && pathStr.includes('claude.cmd')) { + if (pathStr.includes("nvm4w") && pathStr.includes("claude.cmd")) { return true; } return false; }); // Mock validation success - vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); + vi.mocked(execFileSync).mockReturnValue("claude-code version 1.0.0\n"); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); expect(result.found).toBe(true); - expect(result.path).toContain('nvm4w'); - expect(result.path).toContain('claude.cmd'); - expect(result.source).toBe('system-path'); - expect(result.message).toContain('Using Windows Claude CLI'); + expect(result.path).toContain("nvm4w"); + expect(result.path).toContain("claude.cmd"); + expect(result.source).toBe("system-path"); + expect(result.message).toContain("Using Windows Claude CLI"); }); - it('should skip where.exe on non-Windows platforms', () => { - Object.defineProperty(process, 'platform', { - value: 'darwin', - writable: true + it("should skip where.exe on non-Windows platforms", () => { + Object.defineProperty(process, "platform", { + value: "darwin", + writable: true, }); vi.mocked(findWindowsExecutableViaWhere).mockReturnValue(null); @@ -573,25 +587,23 @@ describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { // Mock other detection methods to fail vi.mocked(existsSync).mockReturnValue(false); - getToolInfo('claude'); + getToolInfo("claude"); // where.exe should not be called on macOS expect(findWindowsExecutableViaWhere).not.toHaveBeenCalled(); }); - it('should validate Claude CLI before returning where.exe path', () => { - vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + it("should validate Claude CLI before returning where.exe path", () => { + vi.mocked(os.homedir).mockReturnValue("C:\\Users\\test"); vi.mocked(findExecutable).mockReturnValue(null); - vi.mocked(findWindowsExecutableViaWhere).mockReturnValue( - 'D:\\Tools\\claude.cmd' - ); + vi.mocked(findWindowsExecutableViaWhere).mockReturnValue("D:\\Tools\\claude.cmd"); // Mock file system to return false for all paths except where.exe result vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); - if (pathStr.includes('Tools') && pathStr.includes('claude.cmd')) { + if (pathStr.includes("Tools") && pathStr.includes("claude.cmd")) { return true; } return false; @@ -599,18 +611,18 @@ describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { // Mock validation failure (executable doesn't respond correctly) vi.mocked(execFileSync).mockImplementation(() => { - throw new Error('Command failed'); + throw new Error("Command failed"); }); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); // Should not return the unvalidated path, fallback to not found expect(result.found).toBe(false); - expect(result.source).toBe('fallback'); + expect(result.source).toBe("fallback"); }); - it('should fallback to platform paths if where.exe fails', () => { - vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + it("should fallback to platform paths if where.exe fails", () => { + vi.mocked(os.homedir).mockReturnValue("C:\\Users\\test"); vi.mocked(findExecutable).mockReturnValue(null); @@ -619,44 +631,48 @@ describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { // Mock platform path exists (AppData npm global) vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); - if (pathStr.includes('AppData') && pathStr.includes('npm') && pathStr.includes('claude.cmd')) { + if ( + pathStr.includes("AppData") && + pathStr.includes("npm") && + pathStr.includes("claude.cmd") + ) { return true; } return false; }); - vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); + vi.mocked(execFileSync).mockReturnValue("claude-code version 1.0.0\n"); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); expect(result.found).toBe(true); - expect(result.path).toContain('AppData'); - expect(result.path).toContain('npm'); - expect(result.path).toContain('claude.cmd'); + expect(result.path).toContain("AppData"); + expect(result.path).toContain("npm"); + expect(result.path).toContain("claude.cmd"); }); - it('should prefer .cmd/.exe paths when where.exe returns multiple results', () => { - vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + it("should prefer .cmd/.exe paths when where.exe returns multiple results", () => { + vi.mocked(os.homedir).mockReturnValue("C:\\Users\\test"); vi.mocked(findExecutable).mockReturnValue(null); // Simulate where.exe returning path with .cmd extension (preferred over no extension) vi.mocked(findWindowsExecutableViaWhere).mockReturnValue( - 'D:\\Program Files\\nvm4w\\nodejs\\claude.cmd' + "D:\\Program Files\\nvm4w\\nodejs\\claude.cmd" ); vi.mocked(existsSync).mockReturnValue(true); - vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); + vi.mocked(execFileSync).mockReturnValue("claude-code version 1.0.0\n"); - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); expect(result.found).toBe(true); - expect(result.path).toBe('D:\\Program Files\\nvm4w\\nodejs\\claude.cmd'); + expect(result.path).toBe("D:\\Program Files\\nvm4w\\nodejs\\claude.cmd"); expect(result.path).toMatch(/\.(cmd|exe)$/i); }); - it('should handle where.exe execution errors gracefully', () => { - vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + it("should handle where.exe execution errors gracefully", () => { + vi.mocked(os.homedir).mockReturnValue("C:\\Users\\test"); vi.mocked(findExecutable).mockReturnValue(null); @@ -666,23 +682,23 @@ describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { vi.mocked(existsSync).mockReturnValue(false); // Should not crash, should continue to next detection method - const result = getToolInfo('claude'); + const result = getToolInfo("claude"); expect(result).toBeDefined(); expect(result.found).toBe(false); - expect(result.source).toBe('fallback'); + expect(result.source).toBe("fallback"); }); }); /** * Unit tests for async Claude CLI Windows where.exe detection */ -describe('cli-tool-manager - Claude CLI async Windows where.exe detection', () => { +describe("cli-tool-manager - Claude CLI async Windows where.exe detection", () => { beforeEach(() => { vi.clearAllMocks(); - Object.defineProperty(process, 'platform', { - value: 'win32', - writable: true + Object.defineProperty(process, "platform", { + value: "win32", + writable: true, }); }); @@ -690,8 +706,8 @@ describe('cli-tool-manager - Claude CLI async Windows where.exe detection', () = clearToolCache(); }); - it('should detect Claude CLI via where.exe asynchronously', async () => { - vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + it("should detect Claude CLI via where.exe asynchronously", async () => { + vi.mocked(os.homedir).mockReturnValue("C:\\Users\\test"); vi.mocked(findExecutableAsync).mockResolvedValue(null); @@ -700,14 +716,14 @@ describe('cli-tool-manager - Claude CLI async Windows where.exe detection', () = // Mock file system - no platform paths exist vi.mocked(existsSync).mockReturnValue(false); - await getToolPathAsync('claude'); + await getToolPathAsync("claude"); // Verify where.exe was called on Windows - expect(findWindowsExecutableViaWhereAsync).toHaveBeenCalledWith('claude', '[Claude CLI]'); + expect(findWindowsExecutableViaWhereAsync).toHaveBeenCalledWith("claude", "[Claude CLI]"); }); - it('should handle async where.exe errors gracefully', async () => { - vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + it("should handle async where.exe errors gracefully", async () => { + vi.mocked(os.homedir).mockReturnValue("C:\\Users\\test"); vi.mocked(findExecutableAsync).mockResolvedValue(null); @@ -716,8 +732,8 @@ describe('cli-tool-manager - Claude CLI async Windows where.exe detection', () = vi.mocked(existsSync).mockReturnValue(false); // Should not crash - const result = await getToolPathAsync('claude'); + const result = await getToolPathAsync("claude"); - expect(result).toBe('claude'); // Fallback + expect(result).toBe("claude"); // Fallback }); }); diff --git a/apps/frontend/src/main/changelog/formatter.ts b/apps/frontend/src/main/changelog/formatter.ts index a43be3affa..b84b205561 100644 --- a/apps/frontend/src/main/changelog/formatter.ts +++ b/apps/frontend/src/main/changelog/formatter.ts @@ -1,15 +1,13 @@ -import type { - ChangelogGenerationRequest, - TaskSpecContent, - GitCommit -} from '../../shared/types'; -import { extractSpecOverview } from './parser'; +import type { ChangelogGenerationRequest, TaskSpecContent, GitCommit } from "../../shared/types"; +import { extractSpecOverview } from "./parser"; +import { preparePythonSubprocessCommand } from "../env-utils"; +import { isSecurePath } from "../utils/windows-paths"; /** * Format instructions for different changelog styles */ const FORMAT_TEMPLATES = { - 'keep-a-changelog': (version: string, date: string) => `## [${version}] - ${date} + "keep-a-changelog": (version: string, date: string) => `## [${version}] - ${date} ### Added - [New features] @@ -20,7 +18,7 @@ const FORMAT_TEMPLATES = { ### Fixed - [Bug fixes]`, - 'simple-list': (version: string, date: string) => `# Release v${version} (${date}) + "simple-list": (version: string, date: string) => `# Release v${version} (${date}) **New Features:** - [List features] @@ -31,7 +29,7 @@ const FORMAT_TEMPLATES = { **Bug Fixes:** - [List fixes]`, - 'github-release': (version: string, date: string) => `## ${version} - ${date} + "github-release": (version: string, date: string) => `## ${version} - ${date} ### New Features @@ -53,37 +51,40 @@ const FORMAT_TEMPLATES = { ## Thanks to all contributors -@contributor1, @contributor2` +@contributor1, @contributor2`, }; /** * Audience-specific writing instructions */ const AUDIENCE_INSTRUCTIONS = { - 'technical': 'You are a technical documentation specialist creating a changelog for developers. Use precise technical language.', - 'user-facing': 'You are a product manager writing release notes for end users. Use clear, non-technical language focusing on user benefits.', - 'marketing': 'You are a marketing specialist writing release notes. Focus on outcomes and user impact with compelling language.' + technical: + "You are a technical documentation specialist creating a changelog for developers. Use precise technical language.", + "user-facing": + "You are a product manager writing release notes for end users. Use clear, non-technical language focusing on user benefits.", + marketing: + "You are a marketing specialist writing release notes. Focus on outcomes and user impact with compelling language.", }; /** * Get emoji usage instructions based on level and format */ function getEmojiInstructions(emojiLevel?: string, format?: string): string { - if (!emojiLevel || emojiLevel === 'none') { - return ''; + if (!emojiLevel || emojiLevel === "none") { + return ""; } // GitHub Release format uses specific emoji style matching Gemini CLI pattern - if (format === 'github-release') { + if (format === "github-release") { const githubInstructions: Record = { - 'little': `Add emojis ONLY to section headings. Use these specific emoji-heading pairs: + little: `Add emojis ONLY to section headings. Use these specific emoji-heading pairs: - "### ✨ New Features" - "### 🛠️ Improvements" - "### 🐛 Bug Fixes" - "### 📚 Documentation" - "### 🔧 Other Changes" Do NOT add emojis to individual line items.`, - 'medium': `Add emojis to section headings AND to notable/important items only. + medium: `Add emojis to section headings AND to notable/important items only. Section headings MUST use these specific emoji-heading pairs: - "### ✨ New Features" - "### 🛠️ Improvements" @@ -91,35 +92,35 @@ Section headings MUST use these specific emoji-heading pairs: - "### 📚 Documentation" - "### 🔧 Other Changes" Add emojis to 2-3 highlighted items per section that are particularly significant.`, - 'high': `Add emojis to section headings AND every line item. + high: `Add emojis to section headings AND every line item. Section headings MUST use these specific emoji-heading pairs: - "### ✨ New Features" - "### 🛠️ Improvements" - "### 🐛 Bug Fixes" - "### 📚 Documentation" - "### 🔧 Other Changes" -Every line item should start with a contextual emoji.` +Every line item should start with a contextual emoji.`, }; - return githubInstructions[emojiLevel] || ''; + return githubInstructions[emojiLevel] || ""; } // Default instructions for other formats const instructions: Record = { - 'little': `Add emojis ONLY to section headings. Each heading should have one contextual emoji at the start. + little: `Add emojis ONLY to section headings. Each heading should have one contextual emoji at the start. Examples: - "### ✨ New Features" or "### 🚀 New Features" - "### 🐛 Bug Fixes" - "### 🔧 Improvements" or "### ⚡ Improvements" - "### 📚 Documentation" Do NOT add emojis to individual line items.`, - 'medium': `Add emojis to section headings AND to notable/important items only. + medium: `Add emojis to section headings AND to notable/important items only. Section headings should have one emoji (e.g., "### ✨ New Features", "### 🐛 Bug Fixes"). Add emojis to 2-3 highlighted items per section that are particularly significant. Examples of highlighted items: - "- 🎉 **Major Feature**: Description" - "- 🔒 **Security Fix**: Description" Most regular line items should NOT have emojis.`, - 'high': `Add emojis to section headings AND every line item for maximum visual appeal. + high: `Add emojis to section headings AND every line item for maximum visual appeal. Section headings: "### ✨ New Features", "### 🐛 Bug Fixes", "### ⚡ Improvements" Every line item should start with a contextual emoji: - "- ✨ Added new feature..." @@ -127,10 +128,10 @@ Every line item should start with a contextual emoji: - "- 🔧 Improved performance of..." - "- 📝 Updated documentation for..." - "- 🎨 Refined UI styling..." -Use diverse, contextually appropriate emojis for each item.` +Use diverse, contextually appropriate emojis for each item.`, }; - return instructions[emojiLevel] || ''; + return instructions[emojiLevel] || ""; } /** @@ -145,28 +146,30 @@ export function buildChangelogPrompt( const emojiInstruction = getEmojiInstructions(request.emojiLevel, request.format); // Build CONCISE task summaries (key to avoiding timeout) - const taskSummaries = specs.map(spec => { - const parts: string[] = [`- **${spec.specId}**`]; + const taskSummaries = specs + .map((spec) => { + const parts: string[] = [`- **${spec.specId}**`]; - // Get workflow type if available - if (spec.implementationPlan?.workflow_type) { - parts.push(`(${spec.implementationPlan.workflow_type})`); - } + // Get workflow type if available + if (spec.implementationPlan?.workflow_type) { + parts.push(`(${spec.implementationPlan.workflow_type})`); + } - // Extract just the overview/purpose - if (spec.spec) { - const overview = extractSpecOverview(spec.spec); - if (overview) { - parts.push(`: ${overview}`); + // Extract just the overview/purpose + if (spec.spec) { + const overview = extractSpecOverview(spec.spec); + if (overview) { + parts.push(`: ${overview}`); + } } - } - return parts.join(''); - }).join('\n'); + return parts.join(""); + }) + .join("\n"); // Format-specific instructions for tasks mode - let formatSpecificInstructions = ''; - if (request.format === 'github-release') { + let formatSpecificInstructions = ""; + if (request.format === "github-release") { formatSpecificInstructions = ` For GitHub Release format: @@ -188,13 +191,13 @@ RELEASE TITLE (CRITICAL): Format: ${formatInstruction} -${emojiInstruction ? `\nEmoji Usage:\n${emojiInstruction}` : ''} +${emojiInstruction ? `\nEmoji Usage:\n${emojiInstruction}` : ""} ${formatSpecificInstructions} Completed tasks: ${taskSummaries} -${request.customInstructions ? `Note: ${request.customInstructions}` : ''} +${request.customInstructions ? `Note: ${request.customInstructions}` : ""} CRITICAL: Output ONLY the raw changelog content. Do NOT include ANY introductory text, analysis, or explanation. Start directly with the changelog heading (## or #). No "Here's the changelog" or similar phrases.`; } @@ -202,51 +205,50 @@ CRITICAL: Output ONLY the raw changelog content. Do NOT include ANY introductory /** * Build changelog prompt from git commits */ -export function buildGitPrompt( - request: ChangelogGenerationRequest, - commits: GitCommit[] -): string { +export function buildGitPrompt(request: ChangelogGenerationRequest, commits: GitCommit[]): string { const audienceInstruction = AUDIENCE_INSTRUCTIONS[request.audience]; const formatInstruction = FORMAT_TEMPLATES[request.format](request.version, request.date); const emojiInstruction = getEmojiInstructions(request.emojiLevel, request.format); // Format commits for the prompt // Include author info for github-release format - const commitLines = commits.map(commit => { - const hash = commit.hash; - const subject = commit.subject; - const author = commit.author; - - // Detect conventional commit format: type(scope): message - const conventionalMatch = subject.match(/^(\w+)(?:\(([^)]+)\))?:\s*(.+)$/); - if (conventionalMatch) { - const [, type, scope, message] = conventionalMatch; - return `- ${hash} | ${type}${scope ? `(${scope})` : ''}: ${message} | by ${author}`; - } - return `- ${hash} | ${subject} | by ${author}`; - }).join('\n'); + const commitLines = commits + .map((commit) => { + const hash = commit.hash; + const subject = commit.subject; + const author = commit.author; + + // Detect conventional commit format: type(scope): message + const conventionalMatch = subject.match(/^(\w+)(?:\(([^)]+)\))?:\s*(.+)$/); + if (conventionalMatch) { + const [, type, scope, message] = conventionalMatch; + return `- ${hash} | ${type}${scope ? `(${scope})` : ""}: ${message} | by ${author}`; + } + return `- ${hash} | ${subject} | by ${author}`; + }) + .join("\n"); // Add context about branch/range if available - let sourceContext = ''; + let sourceContext = ""; if (request.branchDiff) { sourceContext = `These commits are from branch "${request.branchDiff.compareBranch}" that are not in "${request.branchDiff.baseBranch}".`; } else if (request.gitHistory) { switch (request.gitHistory.type) { - case 'recent': + case "recent": sourceContext = `These are the ${commits.length} most recent commits.`; break; - case 'since-date': + case "since-date": sourceContext = `These are commits since ${request.gitHistory.sinceDate}.`; break; - case 'tag-range': - sourceContext = `These are commits between tag "${request.gitHistory.fromTag}" and "${request.gitHistory.toTag || 'HEAD'}".`; + case "tag-range": + sourceContext = `These are commits between tag "${request.gitHistory.fromTag}" and "${request.gitHistory.toTag || "HEAD"}".`; break; } } // Format-specific instructions - let formatSpecificInstructions = ''; - if (request.format === 'github-release') { + let formatSpecificInstructions = ""; + if (request.format === "github-release") { formatSpecificInstructions = ` For GitHub Release format, you MUST follow this structure: @@ -306,12 +308,12 @@ ${formatSpecificInstructions} Format: ${formatInstruction} -${emojiInstruction ? `\nEmoji Usage:\n${emojiInstruction}` : ''} +${emojiInstruction ? `\nEmoji Usage:\n${emojiInstruction}` : ""} Git commits (${commits.length} total): ${commitLines} -${request.customInstructions ? `Note: ${request.customInstructions}` : ''} +${request.customInstructions ? `Note: ${request.customInstructions}` : ""} CRITICAL: Output ONLY the raw changelog content. Do NOT include ANY introductory text, analysis, or explanation. Start directly with the changelog heading (## or #). No "Here's the changelog" or similar phrases. Intelligently group and summarize related commits - don't just list each commit individually. Only include sections that have actual changes.`; } @@ -320,16 +322,24 @@ CRITICAL: Output ONLY the raw changelog content. Do NOT include ANY introductory * Create Python script for Claude generation */ export function createGenerationScript(prompt: string, claudePath: string): string { + // Validate claudePath is secure before using it in shell commands + if (!isSecurePath(claudePath)) { + throw new Error( + `Invalid Claude CLI path: path contains potentially dangerous characters. Path: ${claudePath}` + ); + } + // Convert prompt to base64 to avoid any string escaping issues in Python - const base64Prompt = Buffer.from(prompt, 'utf-8').toString('base64'); + const base64Prompt = Buffer.from(prompt, "utf-8").toString("base64"); - // Escape the claude path for Python string - const escapedClaudePath = claudePath.replace(/\\/g, '\\\\').replace(/'/g, "\\'"); + // Prepare the path for Python subprocess execution (handles Windows quoting rules) + const { commandPath, needsShell } = preparePythonSubprocessCommand(claudePath); return ` import subprocess import sys import base64 +import shlex try: # Decode the base64 prompt to avoid string escaping issues @@ -337,13 +347,28 @@ try: # Use Claude Code CLI to generate # stdin=DEVNULL prevents hanging when claude checks for interactive input + ${ + needsShell + ? `# On Windows with .cmd/.bat files, use shell=True with properly escaped prompt + # Use shlex.quote to safely escape the prompt for shell + escaped_prompt = shlex.quote(prompt) + command = ${commandPath} + ' -p ' + escaped_prompt + ' --output-format text --model haiku' result = subprocess.run( - ['${escapedClaudePath}', '-p', prompt, '--output-format', 'text', '--model', 'haiku'], + command, + capture_output=True, + text=True, + stdin=subprocess.DEVNULL, + timeout=300, + shell=True + )` + : `result = subprocess.run( + ['${commandPath}', '-p', prompt, '--output-format', 'text', '--model', 'haiku'], capture_output=True, text=True, stdin=subprocess.DEVNULL, timeout=300 - ) + )` + } if result.returncode == 0: print(result.stdout) diff --git a/apps/frontend/src/main/changelog/version-suggester.ts b/apps/frontend/src/main/changelog/version-suggester.ts index 6d4a9b9126..c7d72424a8 100644 --- a/apps/frontend/src/main/changelog/version-suggester.ts +++ b/apps/frontend/src/main/changelog/version-suggester.ts @@ -1,14 +1,15 @@ -import { spawn } from 'child_process'; -import * as os from 'os'; -import type { GitCommit } from '../../shared/types'; -import { getProfileEnv } from '../rate-limit-detector'; -import { parsePythonCommand } from '../python-detector'; -import { getAugmentedEnv } from '../env-utils'; +import { spawn } from "child_process"; +import * as os from "os"; +import type { GitCommit } from "../../shared/types"; +import { getProfileEnv } from "../rate-limit-detector"; +import { parsePythonCommand } from "../python-detector"; +import { getAugmentedEnv, preparePythonSubprocessCommand } from "../env-utils"; +import { isSecurePath } from "../utils/windows-paths"; interface VersionSuggestion { version: string; reason: string; - bumpType: 'major' | 'minor' | 'patch'; + bumpType: "major" | "minor" | "patch"; } /** @@ -24,12 +25,18 @@ export class VersionSuggester { private autoBuildSourcePath: string, debugEnabled: boolean ) { + // Validate claudePath is secure before using it in shell commands + if (!isSecurePath(claudePath)) { + throw new Error( + `Invalid Claude CLI path: path contains potentially dangerous characters. Path: ${claudePath}` + ); + } this.debugEnabled = debugEnabled; } private debug(...args: unknown[]): void { if (this.debugEnabled) { - console.warn('[VersionSuggester]', ...args); + console.warn("[VersionSuggester]", ...args); } } @@ -40,9 +47,9 @@ export class VersionSuggester { commits: GitCommit[], currentVersion: string ): Promise { - this.debug('suggestVersionBump called', { + this.debug("suggestVersionBump called", { commitCount: commits.length, - currentVersion + currentVersion, }); // Build prompt for Claude to analyze commits @@ -55,42 +62,42 @@ export class VersionSuggester { return new Promise((resolve, _reject) => { // Parse Python command to handle space-separated commands like "py -3" const [pythonCommand, pythonBaseArgs] = parsePythonCommand(this.pythonPath); - const childProcess = spawn(pythonCommand, [...pythonBaseArgs, '-c', script], { + const childProcess = spawn(pythonCommand, [...pythonBaseArgs, "-c", script], { cwd: this.autoBuildSourcePath, - env: spawnEnv + env: spawnEnv, }); - let output = ''; - let errorOutput = ''; + let output = ""; + let errorOutput = ""; - childProcess.stdout?.on('data', (data: Buffer) => { + childProcess.stdout?.on("data", (data: Buffer) => { output += data.toString(); }); - childProcess.stderr?.on('data', (data: Buffer) => { + childProcess.stderr?.on("data", (data: Buffer) => { errorOutput += data.toString(); }); - childProcess.on('exit', (code: number | null) => { + childProcess.on("exit", (code: number | null) => { if (code === 0 && output.trim()) { try { const result = this.parseAIResponse(output.trim(), currentVersion); - this.debug('AI suggestion parsed', result); + this.debug("AI suggestion parsed", result); resolve(result); } catch (error) { - this.debug('Failed to parse AI response', error); + this.debug("Failed to parse AI response", error); // Fallback to simple bump resolve(this.fallbackSuggestion(currentVersion)); } } else { - this.debug('AI analysis failed', { code, error: errorOutput }); + this.debug("AI analysis failed", { code, error: errorOutput }); // Fallback to simple bump resolve(this.fallbackSuggestion(currentVersion)); } }); - childProcess.on('error', (err: Error) => { - this.debug('Process error', err); + childProcess.on("error", (err: Error) => { + this.debug("Process error", err); resolve(this.fallbackSuggestion(currentVersion)); }); }); @@ -100,9 +107,7 @@ export class VersionSuggester { * Build prompt for Claude to analyze commits and suggest version bump */ private buildPrompt(commits: GitCommit[], currentVersion: string): string { - const commitSummary = commits - .map((c, i) => `${i + 1}. ${c.hash} - ${c.subject}`) - .join('\n'); + const commitSummary = commits.map((c, i) => `${i + 1}. ${c.hash} - ${c.subject}`).join("\n"); return `You are a semantic versioning expert analyzing git commits to suggest the appropriate version bump. @@ -128,26 +133,42 @@ Respond with ONLY a JSON object in this exact format (no markdown, no extra text * Create Python script to run Claude analysis */ private createAnalysisScript(prompt: string): string { - // Escape the prompt for Python string literal - const escapedPrompt = prompt - .replace(/\\/g, '\\\\') - .replace(/"/g, '\\"') - .replace(/\n/g, '\\n'); + // Convert prompt to base64 to avoid any string escaping issues and shell injection + const base64Prompt = Buffer.from(prompt, "utf-8").toString("base64"); + + // Prepare the path for Python subprocess execution (handles Windows quoting rules) + const { commandPath, needsShell } = preparePythonSubprocessCommand(this.claudePath); return ` import subprocess import sys +import base64 # Use haiku model for fast, cost-effective analysis -prompt = "${escapedPrompt}" +prompt = base64.b64decode('${base64Prompt}').decode('utf-8') try: + ${ + needsShell + ? `# On Windows with .cmd/.bat files, use shell=True with properly escaped prompt + # Use shlex.quote to properly escape the prompt for shell + import shlex + escaped_prompt = shlex.quote(prompt) + command = ${commandPath} + ' chat --model haiku --prompt ' + escaped_prompt result = subprocess.run( - ["${this.claudePath}", "chat", "--model", "haiku", "--prompt", prompt], + command, + capture_output=True, + text=True, + check=True, + shell=True + )` + : `result = subprocess.run( + ['${commandPath}', 'chat', '--model', 'haiku', '--prompt', prompt], capture_output=True, text=True, check=True - ) + )` + } print(result.stdout) except subprocess.CalledProcessError as e: print(f"Error: {e.stderr}", file=sys.stderr) @@ -165,25 +186,25 @@ except Exception as e: // Extract JSON from output (Claude might wrap it in markdown or other text) const jsonMatch = output.match(/\{[\s\S]*"bumpType"[\s\S]*"reason"[\s\S]*\}/); if (!jsonMatch) { - throw new Error('No JSON found in AI response'); + throw new Error("No JSON found in AI response"); } const parsed = JSON.parse(jsonMatch[0]); - const bumpType = parsed.bumpType as 'major' | 'minor' | 'patch'; - const reason = parsed.reason || 'AI analysis of commits'; + const bumpType = parsed.bumpType as "major" | "minor" | "patch"; + const reason = parsed.reason || "AI analysis of commits"; // Calculate new version - const [major, minor, patch] = currentVersion.split('.').map(Number); + const [major, minor, patch] = currentVersion.split(".").map(Number); let newVersion: string; switch (bumpType) { - case 'major': + case "major": newVersion = `${major + 1}.0.0`; break; - case 'minor': + case "minor": newVersion = `${major}.${minor + 1}.0`; break; - case 'patch': + case "patch": default: newVersion = `${major}.${minor}.${patch + 1}`; break; @@ -192,7 +213,7 @@ except Exception as e: return { version: newVersion, reason, - bumpType + bumpType, }; } @@ -200,11 +221,11 @@ except Exception as e: * Fallback suggestion if AI analysis fails */ private fallbackSuggestion(currentVersion: string): VersionSuggestion { - const [major, minor, patch] = currentVersion.split('.').map(Number); + const [major, minor, patch] = currentVersion.split(".").map(Number); return { version: `${major}.${minor}.${patch + 1}`, - reason: 'Patch version bump (default)', - bumpType: 'patch' + reason: "Patch version bump (default)", + bumpType: "patch", }; } @@ -213,7 +234,7 @@ except Exception as e: */ private buildSpawnEnvironment(): Record { const homeDir = os.homedir(); - const isWindows = process.platform === 'win32'; + const isWindows = process.platform === "win32"; // Use getAugmentedEnv() to ensure common tool paths are available // even when app is launched from Finder/Dock @@ -227,10 +248,10 @@ except Exception as e: ...profileEnv, // Ensure critical env vars are set for claude CLI ...(isWindows ? { USERPROFILE: homeDir } : { HOME: homeDir }), - USER: process.env.USER || process.env.USERNAME || 'user', - PYTHONUNBUFFERED: '1', - PYTHONIOENCODING: 'utf-8', - PYTHONUTF8: '1' + USER: process.env.USER || process.env.USERNAME || "user", + PYTHONUNBUFFERED: "1", + PYTHONIOENCODING: "utf-8", + PYTHONUTF8: "1", }; return spawnEnv; diff --git a/apps/frontend/src/main/cli-tool-manager.ts b/apps/frontend/src/main/cli-tool-manager.ts index 3aa514d1e6..f3b8ff1529 100644 --- a/apps/frontend/src/main/cli-tool-manager.ts +++ b/apps/frontend/src/main/cli-tool-manager.ts @@ -26,7 +26,7 @@ import path from 'path'; import os from 'os'; import { promisify } from 'util'; import { app } from 'electron'; -import { findExecutable, findExecutableAsync, getAugmentedEnv, getAugmentedEnvAsync, shouldUseShell, existsAsync } from './env-utils'; +import { findExecutable, findExecutableAsync, getAugmentedEnv, getAugmentedEnvAsync, prepareShellCommand, existsAsync } from './env-utils'; import type { ToolDetectionResult } from '../shared/types'; const execFileAsync = promisify(execFile); @@ -44,7 +44,7 @@ import { /** * Supported CLI tools managed by this system */ -export type CLITool = 'python' | 'git' | 'gh' | 'claude'; +export type CLITool = "python" | "git" | "gh" | "claude"; /** * User configuration for CLI tool paths @@ -86,12 +86,12 @@ interface CacheEntry { function isWrongPlatformPath(pathStr: string | undefined): boolean { if (!pathStr) return false; - const isWindows = process.platform === 'win32'; + const isWindows = process.platform === "win32"; if (isWindows) { // On Windows, reject Unix-style absolute paths (starting with /) // but allow relative paths and Windows paths - if (pathStr.startsWith('/') && !pathStr.startsWith('//')) { + if (pathStr.startsWith("/") && !pathStr.startsWith("//")) { // Unix absolute path on Windows return true; } @@ -102,11 +102,11 @@ function isWrongPlatformPath(pathStr: string | undefined): boolean { // Drive letter path (C:\, D:/, etc.) return true; } - if (pathStr.includes('\\')) { + if (pathStr.includes("\\")) { // Contains backslashes (Windows path separators) return true; } - if (pathStr.includes('AppData') || pathStr.includes('Program Files')) { + if (pathStr.includes("AppData") || pathStr.includes("Program Files")) { // Contains Windows-specific directory names return true; } @@ -147,24 +147,22 @@ interface ClaudeDetectionPaths { */ export function getClaudeDetectionPaths(homeDir: string): ClaudeDetectionPaths { const homebrewPaths = [ - '/opt/homebrew/bin/claude', // Apple Silicon - '/usr/local/bin/claude', // Intel Mac + "/opt/homebrew/bin/claude", // Apple Silicon + "/usr/local/bin/claude", // Intel Mac ]; - const platformPaths = process.platform === 'win32' - ? [ - path.join(homeDir, 'AppData', 'Local', 'Programs', 'claude', 'claude.exe'), - path.join(homeDir, 'AppData', 'Roaming', 'npm', 'claude.cmd'), - path.join(homeDir, '.local', 'bin', 'claude.exe'), - 'C:\\Program Files\\Claude\\claude.exe', - 'C:\\Program Files (x86)\\Claude\\claude.exe', - ] - : [ - path.join(homeDir, '.local', 'bin', 'claude'), - path.join(homeDir, 'bin', 'claude'), - ]; + const platformPaths = + process.platform === "win32" + ? [ + path.join(homeDir, "AppData", "Local", "Programs", "claude", "claude.exe"), + path.join(homeDir, "AppData", "Roaming", "npm", "claude.cmd"), + path.join(homeDir, ".local", "bin", "claude.exe"), + "C:\\Program Files\\Claude\\claude.exe", + "C:\\Program Files (x86)\\Claude\\claude.exe", + ] + : [path.join(homeDir, ".local", "bin", "claude"), path.join(homeDir, "bin", "claude")]; - const nvmVersionsDir = path.join(homeDir, '.nvm', 'versions', 'node'); + const nvmVersionsDir = path.join(homeDir, ".nvm", "versions", "node"); return { homebrewPaths, platformPaths, nvmVersionsDir }; } @@ -197,8 +195,8 @@ export function sortNvmVersionDirs( .filter((entry) => entry.isDirectory() && semverRegex.test(entry.name)) .sort((a, b) => { // Parse version numbers: v20.0.0 -> [20, 0, 0] - const vA = a.name.slice(1).split('.').map(Number); - const vB = b.name.slice(1).split('.').map(Number); + const vA = a.name.slice(1).split(".").map(Number); + const vB = b.name.slice(1).split(".").map(Number); // Compare major, minor, patch in order (descending) for (let i = 0; i < 3; i++) { const diff = (vB[i] ?? 0) - (vA[i] ?? 0); @@ -233,7 +231,7 @@ export function sortNvmVersionDirs( export function buildClaudeDetectionResult( claudePath: string, validation: ToolValidation, - source: ToolDetectionResult['source'], + source: ToolDetectionResult["source"], messagePrefix: string ): ToolDetectionResult | null { if (!validation.valid) { @@ -279,7 +277,7 @@ class CLIToolManager { configure(config: ToolConfig): void { this.userConfig = config; this.cache.clear(); - console.warn('[CLI Tools] Configuration updated, cache cleared'); + console.warn("[CLI Tools] Configuration updated, cache cleared"); } /** @@ -295,9 +293,7 @@ class CLIToolManager { // Check cache first const cached = this.cache.get(tool); if (cached) { - console.warn( - `[CLI Tools] Using cached ${tool}: ${cached.path} (${cached.source})` - ); + console.warn(`[CLI Tools] Using cached ${tool}: ${cached.path} (${cached.source})`); return cached.path; } @@ -328,18 +324,18 @@ class CLIToolManager { */ private detectToolPath(tool: CLITool): ToolDetectionResult { switch (tool) { - case 'python': + case "python": return this.detectPython(); - case 'git': + case "git": return this.detectGit(); - case 'gh': + case "gh": return this.detectGitHubCLI(); - case 'claude': + case "claude": return this.detectClaude(); default: return { found: false, - source: 'fallback', + source: "fallback", message: `Unknown tool: ${tool}`, }; } @@ -359,7 +355,7 @@ class CLIToolManager { * @returns Detection result for Python */ private detectPython(): ToolDetectionResult { - const MINIMUM_VERSION = '3.10.0'; + const MINIMUM_VERSION = "3.10.0"; // 1. User configuration if (this.userConfig.pythonPath) { @@ -375,13 +371,11 @@ class CLIToolManager { found: true, path: this.userConfig.pythonPath, version: validation.version, - source: 'user-config', + source: "user-config", message: `Using user-configured Python: ${this.userConfig.pythonPath}`, }; } - console.warn( - `[Python] User-configured path invalid: ${validation.message}` - ); + console.warn(`[Python] User-configured path invalid: ${validation.message}`); } } @@ -395,7 +389,7 @@ class CLIToolManager { found: true, path: bundledPath, version: validation.version, - source: 'bundled', + source: "bundled", message: `Using bundled Python: ${bundledPath}`, }; } @@ -403,7 +397,7 @@ class CLIToolManager { } // 3. Homebrew Python (macOS) - if (process.platform === 'darwin') { + if (process.platform === "darwin") { const homebrewPath = this.findHomebrewPython(); if (homebrewPath) { const validation = this.validatePython(homebrewPath); @@ -412,7 +406,7 @@ class CLIToolManager { found: true, path: homebrewPath, version: validation.version, - source: 'homebrew', + source: "homebrew", message: `Using Homebrew Python: ${homebrewPath}`, }; } @@ -421,20 +415,18 @@ class CLIToolManager { // 4. System PATH (augmented) const candidates = - process.platform === 'win32' - ? ['py -3', 'python', 'python3', 'py'] - : ['python3', 'python']; + process.platform === "win32" ? ["py -3", "python", "python3", "py"] : ["python3", "python"]; for (const cmd of candidates) { // Special handling for Windows 'py -3' launcher - if (cmd.startsWith('py ')) { + if (cmd.startsWith("py ")) { const validation = this.validatePython(cmd); if (validation.valid) { return { found: true, path: cmd, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using system Python: ${cmd}`, }; } @@ -448,7 +440,7 @@ class CLIToolManager { found: true, path: pythonPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using system Python: ${pythonPath}`, }; } @@ -459,10 +451,10 @@ class CLIToolManager { // 5. Not found return { found: false, - source: 'fallback', + source: "fallback", message: `Python ${MINIMUM_VERSION}+ not found. ` + - 'Please install Python or configure in Settings.', + "Please install Python or configure in Settings.", }; } @@ -491,7 +483,7 @@ class CLIToolManager { found: true, path: this.userConfig.gitPath, version: validation.version, - source: 'user-config', + source: "user-config", message: `Using user-configured Git: ${this.userConfig.gitPath}`, }; } @@ -500,10 +492,10 @@ class CLIToolManager { } // 2. Homebrew (macOS) - if (process.platform === 'darwin') { + if (process.platform === "darwin") { const homebrewPaths = [ - '/opt/homebrew/bin/git', // Apple Silicon - '/usr/local/bin/git', // Intel Mac + "/opt/homebrew/bin/git", // Apple Silicon + "/usr/local/bin/git", // Intel Mac ]; for (const gitPath of homebrewPaths) { @@ -514,7 +506,7 @@ class CLIToolManager { found: true, path: gitPath, version: validation.version, - source: 'homebrew', + source: "homebrew", message: `Using Homebrew Git: ${gitPath}`, }; } @@ -523,7 +515,7 @@ class CLIToolManager { } // 3. System PATH (augmented) - const gitPath = findExecutable('git'); + const gitPath = findExecutable("git"); if (gitPath) { const validation = this.validateGit(gitPath); if (validation.valid) { @@ -531,16 +523,16 @@ class CLIToolManager { found: true, path: gitPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using system Git: ${gitPath}`, }; } } // 4. Windows-specific detection using 'where' command (most reliable for custom installs) - if (process.platform === 'win32') { + if (process.platform === "win32") { // First try 'where' command - finds git regardless of installation location - const whereGitPath = findWindowsExecutableViaWhere('git', '[Git]'); + const whereGitPath = findWindowsExecutableViaWhere("git", "[Git]"); if (whereGitPath) { const validation = this.validateGit(whereGitPath); if (validation.valid) { @@ -548,14 +540,14 @@ class CLIToolManager { found: true, path: whereGitPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using Windows Git: ${whereGitPath}`, }; } } // Fallback to checking common installation paths - const windowsPaths = getWindowsExecutablePaths(WINDOWS_GIT_PATHS, '[Git]'); + const windowsPaths = getWindowsExecutablePaths(WINDOWS_GIT_PATHS, "[Git]"); for (const winGitPath of windowsPaths) { const validation = this.validateGit(winGitPath); if (validation.valid) { @@ -563,7 +555,7 @@ class CLIToolManager { found: true, path: winGitPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using Windows Git: ${winGitPath}`, }; } @@ -573,7 +565,7 @@ class CLIToolManager { // 5. Not found - fallback to 'git' return { found: false, - source: 'fallback', + source: "fallback", message: 'Git not found in standard locations. Using fallback "git".', }; } @@ -604,21 +596,19 @@ class CLIToolManager { found: true, path: this.userConfig.githubCLIPath, version: validation.version, - source: 'user-config', + source: "user-config", message: `Using user-configured GitHub CLI: ${this.userConfig.githubCLIPath}`, }; } - console.warn( - `[GitHub CLI] User-configured path invalid: ${validation.message}` - ); + console.warn(`[GitHub CLI] User-configured path invalid: ${validation.message}`); } } // 2. Homebrew (macOS) - if (process.platform === 'darwin') { + if (process.platform === "darwin") { const homebrewPaths = [ - '/opt/homebrew/bin/gh', // Apple Silicon - '/usr/local/bin/gh', // Intel Mac + "/opt/homebrew/bin/gh", // Apple Silicon + "/usr/local/bin/gh", // Intel Mac ]; for (const ghPath of homebrewPaths) { @@ -629,7 +619,7 @@ class CLIToolManager { found: true, path: ghPath, version: validation.version, - source: 'homebrew', + source: "homebrew", message: `Using Homebrew GitHub CLI: ${ghPath}`, }; } @@ -638,7 +628,7 @@ class CLIToolManager { } // 3. System PATH (augmented) - const ghPath = findExecutable('gh'); + const ghPath = findExecutable("gh"); if (ghPath) { const validation = this.validateGitHubCLI(ghPath); if (validation.valid) { @@ -646,17 +636,17 @@ class CLIToolManager { found: true, path: ghPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using system GitHub CLI: ${ghPath}`, }; } } // 4. Windows Program Files - if (process.platform === 'win32') { + if (process.platform === "win32") { const windowsPaths = [ - 'C:\\Program Files\\GitHub CLI\\gh.exe', - 'C:\\Program Files (x86)\\GitHub CLI\\gh.exe', + "C:\\Program Files\\GitHub CLI\\gh.exe", + "C:\\Program Files (x86)\\GitHub CLI\\gh.exe", ]; for (const ghPath of windowsPaths) { @@ -667,7 +657,7 @@ class CLIToolManager { found: true, path: ghPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using Windows GitHub CLI: ${ghPath}`, }; } @@ -678,8 +668,8 @@ class CLIToolManager { // 5. Not found return { found: false, - source: 'fallback', - message: 'GitHub CLI (gh) not found. Install from https://cli.github.com', + source: "fallback", + message: "GitHub CLI (gh) not found. Install from https://cli.github.com", }; } @@ -713,7 +703,10 @@ class CLIToolManager { } else { const validation = this.validateClaude(this.userConfig.claudePath); const result = buildClaudeDetectionResult( - this.userConfig.claudePath, validation, 'user-config', 'Using user-configured Claude CLI' + this.userConfig.claudePath, + validation, + "user-config", + "Using user-configured Claude CLI" ); if (result) return result; console.warn(`[Claude CLI] User-configured path invalid: ${validation.message}`); @@ -721,21 +714,31 @@ class CLIToolManager { } // 2. Homebrew (macOS) - if (process.platform === 'darwin') { + if (process.platform === "darwin") { for (const claudePath of paths.homebrewPaths) { if (existsSync(claudePath)) { const validation = this.validateClaude(claudePath); - const result = buildClaudeDetectionResult(claudePath, validation, 'homebrew', 'Using Homebrew Claude CLI'); + const result = buildClaudeDetectionResult( + claudePath, + validation, + "homebrew", + "Using Homebrew Claude CLI" + ); if (result) return result; } } } // 3. System PATH (augmented) - const systemClaudePath = findExecutable('claude'); + const systemClaudePath = findExecutable("claude"); if (systemClaudePath) { const validation = this.validateClaude(systemClaudePath); - const result = buildClaudeDetectionResult(systemClaudePath, validation, 'system-path', 'Using system Claude CLI'); + const result = buildClaudeDetectionResult( + systemClaudePath, + validation, + "system-path", + "Using system Claude CLI" + ); if (result) return result; } @@ -757,10 +760,15 @@ class CLIToolManager { const versionNames = sortNvmVersionDirs(nodeVersions); for (const versionName of versionNames) { - const nvmClaudePath = path.join(paths.nvmVersionsDir, versionName, 'bin', 'claude'); + const nvmClaudePath = path.join(paths.nvmVersionsDir, versionName, "bin", "claude"); if (existsSync(nvmClaudePath)) { const validation = this.validateClaude(nvmClaudePath); - const result = buildClaudeDetectionResult(nvmClaudePath, validation, 'nvm', 'Using NVM Claude CLI'); + const result = buildClaudeDetectionResult( + nvmClaudePath, + validation, + "nvm", + "Using NVM Claude CLI" + ); if (result) return result; } } @@ -774,7 +782,12 @@ class CLIToolManager { for (const claudePath of paths.platformPaths) { if (existsSync(claudePath)) { const validation = this.validateClaude(claudePath); - const result = buildClaudeDetectionResult(claudePath, validation, 'system-path', 'Using Claude CLI'); + const result = buildClaudeDetectionResult( + claudePath, + validation, + "system-path", + "Using Claude CLI" + ); if (result) return result; } } @@ -782,8 +795,8 @@ class CLIToolManager { // 7. Not found return { found: false, - source: 'fallback', - message: 'Claude CLI not found. Install from https://claude.ai/download', + source: "fallback", + message: "Claude CLI not found. Install from https://claude.ai/download", }; } @@ -797,17 +810,17 @@ class CLIToolManager { * @returns Validation result with version information */ private validatePython(pythonCmd: string): ToolValidation { - const MINIMUM_VERSION = '3.10.0'; + const MINIMUM_VERSION = "3.10.0"; try { // Parse command to handle cases like 'py -3' on Windows // This avoids command injection by using execFileSync instead of execSync - const parts = pythonCmd.split(' '); + const parts = pythonCmd.split(" "); const cmd = parts[0]; - const args = [...parts.slice(1), '--version']; + const args = [...parts.slice(1), "--version"]; const version = execFileSync(cmd, args, { - encoding: 'utf-8', + encoding: "utf-8", timeout: 5000, windowsHide: true, }).trim(); @@ -816,16 +829,15 @@ class CLIToolManager { if (!match) { return { valid: false, - message: 'Unable to detect Python version', + message: "Unable to detect Python version", }; } const versionStr = match[1]; - const [major, minor] = versionStr.split('.').map(Number); - const [reqMajor, reqMinor] = MINIMUM_VERSION.split('.').map(Number); + const [major, minor] = versionStr.split(".").map(Number); + const [reqMajor, reqMinor] = MINIMUM_VERSION.split(".").map(Number); - const meetsRequirement = - major > reqMajor || (major === reqMajor && minor >= reqMinor); + const meetsRequirement = major > reqMajor || (major === reqMajor && minor >= reqMinor); if (!meetsRequirement) { return { @@ -856,8 +868,8 @@ class CLIToolManager { */ private validateGit(gitCmd: string): ToolValidation { try { - const version = execFileSync(gitCmd, ['--version'], { - encoding: 'utf-8', + const version = execFileSync(gitCmd, ["--version"], { + encoding: "utf-8", timeout: 5000, windowsHide: true, }).trim(); @@ -886,14 +898,14 @@ class CLIToolManager { */ private validateGitHubCLI(ghCmd: string): ToolValidation { try { - const version = execFileSync(ghCmd, ['--version'], { - encoding: 'utf-8', + const version = execFileSync(ghCmd, ["--version"], { + encoding: "utf-8", timeout: 5000, windowsHide: true, }).trim(); const match = version.match(/gh version (\d+\.\d+\.\d+)/); - const versionStr = match ? match[1] : version.split('\n')[0]; + const versionStr = match ? match[1] : version.split("\n")[0]; return { valid: true, @@ -916,15 +928,19 @@ class CLIToolManager { */ private validateClaude(claudeCmd: string): ToolValidation { try { - const needsShell = shouldUseShell(claudeCmd); + // On Windows, .cmd files need shell: true to execute properly. + // SECURITY NOTE: shell: true is safe here because: + // 1. claudeCmd comes from internal path detection (user config or known system paths) + // 2. Only '--version' is passed as an argument (no user input) + // If claudeCmd origin ever changes to accept user input, use escapeShellArgWindows. + const { command, needsShell } = prepareShellCommand(claudeCmd); let version: string; if (needsShell) { // For .cmd/.bat files on Windows, use execSync with quoted path // execFileSync doesn't handle spaces in .cmd paths correctly even with shell:true - const quotedCmd = `"${claudeCmd}"`; - version = execSync(`${quotedCmd} --version`, { + version = execSync(`${command} --version`, { encoding: 'utf-8', timeout: 5000, windowsHide: true, @@ -943,7 +959,7 @@ class CLIToolManager { // Claude CLI version output format: "claude-code version X.Y.Z" or similar const match = version.match(/(\d+\.\d+\.\d+)/); - const versionStr = match ? match[1] : version.split('\n')[0]; + const versionStr = match ? match[1] : version.split("\n")[0]; return { valid: true, @@ -975,9 +991,7 @@ class CLIToolManager { // Check cache first (instant return if cached) const cached = this.cache.get(tool); if (cached) { - console.warn( - `[CLI Tools] Using cached ${tool}: ${cached.path} (${cached.source})` - ); + console.warn(`[CLI Tools] Using cached ${tool}: ${cached.path} (${cached.source})`); return cached.path; } @@ -1008,18 +1022,18 @@ class CLIToolManager { */ private async detectToolPathAsync(tool: CLITool): Promise { switch (tool) { - case 'claude': + case "claude": return this.detectClaudeAsync(); - case 'python': + case "python": return this.detectPythonAsync(); - case 'git': + case "git": return this.detectGitAsync(); - case 'gh': + case "gh": return this.detectGitHubCLIAsync(); default: return { found: false, - source: 'fallback', + source: "fallback", message: `Unknown tool: ${tool}`, }; } @@ -1033,14 +1047,13 @@ class CLIToolManager { */ private async validateClaudeAsync(claudeCmd: string): Promise { try { - const needsShell = shouldUseShell(claudeCmd); + const { command, needsShell } = prepareShellCommand(claudeCmd); let stdout: string; if (needsShell) { // For .cmd/.bat files on Windows, use exec with quoted path - const quotedCmd = `"${claudeCmd}"`; - const result = await execAsync(`${quotedCmd} --version`, { + const result = await execAsync(`${command} --version`, { encoding: 'utf-8', timeout: 5000, windowsHide: true, @@ -1061,7 +1074,7 @@ class CLIToolManager { const version = stdout.trim(); const match = version.match(/(\d+\.\d+\.\d+)/); - const versionStr = match ? match[1] : version.split('\n')[0]; + const versionStr = match ? match[1] : version.split("\n")[0]; return { valid: true, @@ -1083,15 +1096,15 @@ class CLIToolManager { * @returns Promise resolving to validation result */ private async validatePythonAsync(pythonCmd: string): Promise { - const MINIMUM_VERSION = '3.10.0'; + const MINIMUM_VERSION = "3.10.0"; try { - const parts = pythonCmd.split(' '); + const parts = pythonCmd.split(" "); const cmd = parts[0]; - const args = [...parts.slice(1), '--version']; + const args = [...parts.slice(1), "--version"]; const { stdout } = await execFileAsync(cmd, args, { - encoding: 'utf-8', + encoding: "utf-8", timeout: 5000, windowsHide: true, env: await getAugmentedEnvAsync(), @@ -1102,16 +1115,15 @@ class CLIToolManager { if (!match) { return { valid: false, - message: 'Unable to detect Python version', + message: "Unable to detect Python version", }; } const versionStr = match[1]; - const [major, minor] = versionStr.split('.').map(Number); - const [reqMajor, reqMinor] = MINIMUM_VERSION.split('.').map(Number); + const [major, minor] = versionStr.split(".").map(Number); + const [reqMajor, reqMinor] = MINIMUM_VERSION.split(".").map(Number); - const meetsRequirement = - major > reqMajor || (major === reqMajor && minor >= reqMinor); + const meetsRequirement = major > reqMajor || (major === reqMajor && minor >= reqMinor); if (!meetsRequirement) { return { @@ -1142,8 +1154,8 @@ class CLIToolManager { */ private async validateGitAsync(gitCmd: string): Promise { try { - const { stdout } = await execFileAsync(gitCmd, ['--version'], { - encoding: 'utf-8', + const { stdout } = await execFileAsync(gitCmd, ["--version"], { + encoding: "utf-8", timeout: 5000, windowsHide: true, env: await getAugmentedEnvAsync(), @@ -1174,8 +1186,8 @@ class CLIToolManager { */ private async validateGitHubCLIAsync(ghCmd: string): Promise { try { - const { stdout } = await execFileAsync(ghCmd, ['--version'], { - encoding: 'utf-8', + const { stdout } = await execFileAsync(ghCmd, ["--version"], { + encoding: "utf-8", timeout: 5000, windowsHide: true, env: await getAugmentedEnvAsync(), @@ -1183,7 +1195,7 @@ class CLIToolManager { const version = stdout.trim(); const match = version.match(/gh version (\d+\.\d+\.\d+)/); - const versionStr = match ? match[1] : version.split('\n')[0]; + const versionStr = match ? match[1] : version.split("\n")[0]; return { valid: true, @@ -1228,7 +1240,10 @@ class CLIToolManager { } else { const validation = await this.validateClaudeAsync(this.userConfig.claudePath); const result = buildClaudeDetectionResult( - this.userConfig.claudePath, validation, 'user-config', 'Using user-configured Claude CLI' + this.userConfig.claudePath, + validation, + "user-config", + "Using user-configured Claude CLI" ); if (result) return result; console.warn(`[Claude CLI] User-configured path invalid: ${validation.message}`); @@ -1236,21 +1251,31 @@ class CLIToolManager { } // 2. Homebrew (macOS) - if (process.platform === 'darwin') { + if (process.platform === "darwin") { for (const claudePath of paths.homebrewPaths) { if (await existsAsync(claudePath)) { const validation = await this.validateClaudeAsync(claudePath); - const result = buildClaudeDetectionResult(claudePath, validation, 'homebrew', 'Using Homebrew Claude CLI'); + const result = buildClaudeDetectionResult( + claudePath, + validation, + "homebrew", + "Using Homebrew Claude CLI" + ); if (result) return result; } } } // 3. System PATH (augmented) - using async findExecutable - const systemClaudePath = await findExecutableAsync('claude'); + const systemClaudePath = await findExecutableAsync("claude"); if (systemClaudePath) { const validation = await this.validateClaudeAsync(systemClaudePath); - const result = buildClaudeDetectionResult(systemClaudePath, validation, 'system-path', 'Using system Claude CLI'); + const result = buildClaudeDetectionResult( + systemClaudePath, + validation, + "system-path", + "Using system Claude CLI" + ); if (result) return result; } @@ -1268,14 +1293,21 @@ class CLIToolManager { if (process.platform !== 'win32') { try { if (await existsAsync(paths.nvmVersionsDir)) { - const nodeVersions = await fsPromises.readdir(paths.nvmVersionsDir, { withFileTypes: true }); + const nodeVersions = await fsPromises.readdir(paths.nvmVersionsDir, { + withFileTypes: true, + }); const versionNames = sortNvmVersionDirs(nodeVersions); for (const versionName of versionNames) { - const nvmClaudePath = path.join(paths.nvmVersionsDir, versionName, 'bin', 'claude'); + const nvmClaudePath = path.join(paths.nvmVersionsDir, versionName, "bin", "claude"); if (await existsAsync(nvmClaudePath)) { const validation = await this.validateClaudeAsync(nvmClaudePath); - const result = buildClaudeDetectionResult(nvmClaudePath, validation, 'nvm', 'Using NVM Claude CLI'); + const result = buildClaudeDetectionResult( + nvmClaudePath, + validation, + "nvm", + "Using NVM Claude CLI" + ); if (result) return result; } } @@ -1289,7 +1321,12 @@ class CLIToolManager { for (const claudePath of paths.platformPaths) { if (await existsAsync(claudePath)) { const validation = await this.validateClaudeAsync(claudePath); - const result = buildClaudeDetectionResult(claudePath, validation, 'system-path', 'Using Claude CLI'); + const result = buildClaudeDetectionResult( + claudePath, + validation, + "system-path", + "Using Claude CLI" + ); if (result) return result; } } @@ -1297,8 +1334,8 @@ class CLIToolManager { // 7. Not found return { found: false, - source: 'fallback', - message: 'Claude CLI not found. Install from https://claude.ai/download', + source: "fallback", + message: "Claude CLI not found. Install from https://claude.ai/download", }; } @@ -1310,7 +1347,7 @@ class CLIToolManager { * @returns Promise resolving to detection result */ private async detectPythonAsync(): Promise { - const MINIMUM_VERSION = '3.10.0'; + const MINIMUM_VERSION = "3.10.0"; // 1. User configuration if (this.userConfig.pythonPath) { @@ -1325,7 +1362,7 @@ class CLIToolManager { found: true, path: this.userConfig.pythonPath, version: validation.version, - source: 'user-config', + source: "user-config", message: `Using user-configured Python: ${this.userConfig.pythonPath}`, }; } @@ -1343,7 +1380,7 @@ class CLIToolManager { found: true, path: bundledPath, version: validation.version, - source: 'bundled', + source: "bundled", message: `Using bundled Python: ${bundledPath}`, }; } @@ -1351,13 +1388,13 @@ class CLIToolManager { } // 3. Homebrew Python (macOS) - simplified async version - if (process.platform === 'darwin') { + if (process.platform === "darwin") { const homebrewPaths = [ - '/opt/homebrew/bin/python3', - '/opt/homebrew/bin/python3.12', - '/opt/homebrew/bin/python3.11', - '/opt/homebrew/bin/python3.10', - '/usr/local/bin/python3', + "/opt/homebrew/bin/python3", + "/opt/homebrew/bin/python3.12", + "/opt/homebrew/bin/python3.11", + "/opt/homebrew/bin/python3.10", + "/usr/local/bin/python3", ]; for (const pythonPath of homebrewPaths) { if (await existsAsync(pythonPath)) { @@ -1367,7 +1404,7 @@ class CLIToolManager { found: true, path: pythonPath, version: validation.version, - source: 'homebrew', + source: "homebrew", message: `Using Homebrew Python: ${pythonPath}`, }; } @@ -1377,19 +1414,17 @@ class CLIToolManager { // 4. System PATH (augmented) const candidates = - process.platform === 'win32' - ? ['py -3', 'python', 'python3', 'py'] - : ['python3', 'python']; + process.platform === "win32" ? ["py -3", "python", "python3", "py"] : ["python3", "python"]; for (const cmd of candidates) { - if (cmd.startsWith('py ')) { + if (cmd.startsWith("py ")) { const validation = await this.validatePythonAsync(cmd); if (validation.valid) { return { found: true, path: cmd, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using system Python: ${cmd}`, }; } @@ -1402,7 +1437,7 @@ class CLIToolManager { found: true, path: pythonPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using system Python: ${pythonPath}`, }; } @@ -1413,10 +1448,10 @@ class CLIToolManager { // 5. Not found return { found: false, - source: 'fallback', + source: "fallback", message: `Python ${MINIMUM_VERSION}+ not found. ` + - 'Please install Python or configure in Settings.', + "Please install Python or configure in Settings.", }; } @@ -1441,7 +1476,7 @@ class CLIToolManager { found: true, path: this.userConfig.gitPath, version: validation.version, - source: 'user-config', + source: "user-config", message: `Using user-configured Git: ${this.userConfig.gitPath}`, }; } @@ -1450,11 +1485,8 @@ class CLIToolManager { } // 2. Homebrew (macOS) - if (process.platform === 'darwin') { - const homebrewPaths = [ - '/opt/homebrew/bin/git', - '/usr/local/bin/git', - ]; + if (process.platform === "darwin") { + const homebrewPaths = ["/opt/homebrew/bin/git", "/usr/local/bin/git"]; for (const gitPath of homebrewPaths) { if (await existsAsync(gitPath)) { @@ -1464,7 +1496,7 @@ class CLIToolManager { found: true, path: gitPath, version: validation.version, - source: 'homebrew', + source: "homebrew", message: `Using Homebrew Git: ${gitPath}`, }; } @@ -1473,7 +1505,7 @@ class CLIToolManager { } // 3. System PATH (augmented) - const gitPath = await findExecutableAsync('git'); + const gitPath = await findExecutableAsync("git"); if (gitPath) { const validation = await this.validateGitAsync(gitPath); if (validation.valid) { @@ -1481,15 +1513,15 @@ class CLIToolManager { found: true, path: gitPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using system Git: ${gitPath}`, }; } } // 4. Windows-specific detection (async to avoid blocking main process) - if (process.platform === 'win32') { - const whereGitPath = await findWindowsExecutableViaWhereAsync('git', '[Git]'); + if (process.platform === "win32") { + const whereGitPath = await findWindowsExecutableViaWhereAsync("git", "[Git]"); if (whereGitPath) { const validation = await this.validateGitAsync(whereGitPath); if (validation.valid) { @@ -1497,13 +1529,13 @@ class CLIToolManager { found: true, path: whereGitPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using Windows Git: ${whereGitPath}`, }; } } - const windowsPaths = await getWindowsExecutablePathsAsync(WINDOWS_GIT_PATHS, '[Git]'); + const windowsPaths = await getWindowsExecutablePathsAsync(WINDOWS_GIT_PATHS, "[Git]"); for (const winGitPath of windowsPaths) { const validation = await this.validateGitAsync(winGitPath); if (validation.valid) { @@ -1511,7 +1543,7 @@ class CLIToolManager { found: true, path: winGitPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using Windows Git: ${winGitPath}`, }; } @@ -1521,7 +1553,7 @@ class CLIToolManager { // 5. Not found return { found: false, - source: 'fallback', + source: "fallback", message: 'Git not found in standard locations. Using fallback "git".', }; } @@ -1547,7 +1579,7 @@ class CLIToolManager { found: true, path: this.userConfig.githubCLIPath, version: validation.version, - source: 'user-config', + source: "user-config", message: `Using user-configured GitHub CLI: ${this.userConfig.githubCLIPath}`, }; } @@ -1556,11 +1588,8 @@ class CLIToolManager { } // 2. Homebrew (macOS) - if (process.platform === 'darwin') { - const homebrewPaths = [ - '/opt/homebrew/bin/gh', - '/usr/local/bin/gh', - ]; + if (process.platform === "darwin") { + const homebrewPaths = ["/opt/homebrew/bin/gh", "/usr/local/bin/gh"]; for (const ghPath of homebrewPaths) { if (await existsAsync(ghPath)) { @@ -1570,7 +1599,7 @@ class CLIToolManager { found: true, path: ghPath, version: validation.version, - source: 'homebrew', + source: "homebrew", message: `Using Homebrew GitHub CLI: ${ghPath}`, }; } @@ -1579,7 +1608,7 @@ class CLIToolManager { } // 3. System PATH (augmented) - const ghPath = await findExecutableAsync('gh'); + const ghPath = await findExecutableAsync("gh"); if (ghPath) { const validation = await this.validateGitHubCLIAsync(ghPath); if (validation.valid) { @@ -1587,17 +1616,17 @@ class CLIToolManager { found: true, path: ghPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using system GitHub CLI: ${ghPath}`, }; } } // 4. Windows Program Files - if (process.platform === 'win32') { + if (process.platform === "win32") { const windowsPaths = [ - 'C:\\Program Files\\GitHub CLI\\gh.exe', - 'C:\\Program Files (x86)\\GitHub CLI\\gh.exe', + "C:\\Program Files\\GitHub CLI\\gh.exe", + "C:\\Program Files (x86)\\GitHub CLI\\gh.exe", ]; for (const winGhPath of windowsPaths) { @@ -1608,7 +1637,7 @@ class CLIToolManager { found: true, path: winGhPath, version: validation.version, - source: 'system-path', + source: "system-path", message: `Using Windows GitHub CLI: ${winGhPath}`, }; } @@ -1619,8 +1648,8 @@ class CLIToolManager { // 5. Not found return { found: false, - source: 'fallback', - message: 'GitHub CLI (gh) not found. Install from https://cli.github.com', + source: "fallback", + message: "GitHub CLI (gh) not found. Install from https://cli.github.com", }; } @@ -1638,11 +1667,11 @@ class CLIToolManager { } const resourcesPath = process.resourcesPath; - const isWindows = process.platform === 'win32'; + const isWindows = process.platform === "win32"; const pythonPath = isWindows - ? path.join(resourcesPath, 'python', 'python.exe') - : path.join(resourcesPath, 'python', 'bin', 'python3'); + ? path.join(resourcesPath, "python", "python.exe") + : path.join(resourcesPath, "python", "bin", "python3"); return existsSync(pythonPath) ? pythonPath : null; } @@ -1654,10 +1683,7 @@ class CLIToolManager { * @returns Path to Homebrew Python or null if not found */ private findHomebrewPython(): string | null { - return findHomebrewPythonUtil( - (pythonPath) => this.validatePython(pythonPath), - '[CLI Tools]' - ); + return findHomebrewPythonUtil((pythonPath) => this.validatePython(pythonPath), "[CLI Tools]"); } /** @@ -1668,7 +1694,7 @@ class CLIToolManager { */ clearCache(): void { this.cache.clear(); - console.warn('[CLI Tools] Cache cleared'); + console.warn("[CLI Tools] Cache cleared"); } /** @@ -1841,8 +1867,8 @@ export async function getToolPathAsync(tool: CLITool): Promise { * }); * ``` */ -export async function preWarmToolCache(tools: CLITool[] = ['claude']): Promise { - console.warn('[CLI Tools] Pre-warming cache for:', tools.join(', ')); - await Promise.all(tools.map(tool => cliToolManager.getToolPathAsync(tool))); - console.warn('[CLI Tools] Cache pre-warming complete'); +export async function preWarmToolCache(tools: CLITool[] = ["claude"]): Promise { + console.warn("[CLI Tools] Pre-warming cache for:", tools.join(", ")); + await Promise.all(tools.map((tool) => cliToolManager.getToolPathAsync(tool))); + console.warn("[CLI Tools] Cache pre-warming complete"); } diff --git a/apps/frontend/src/main/env-utils.ts b/apps/frontend/src/main/env-utils.ts index 8463a2ace4..1499e5517e 100644 --- a/apps/frontend/src/main/env-utils.ts +++ b/apps/frontend/src/main/env-utils.ts @@ -9,12 +9,13 @@ * which isn't in PATH when the Electron app launches from Finder/Dock. */ -import * as os from 'os'; -import * as path from 'path'; -import * as fs from 'fs'; -import { promises as fsPromises } from 'fs'; -import { execFileSync, execFile } from 'child_process'; -import { promisify } from 'util'; +import * as os from "os"; +import * as path from "path"; +import * as fs from "fs"; +import { promises as fsPromises } from "fs"; +import { execFileSync, execFile } from "child_process"; +import { promisify } from "util"; +import { isSecurePath } from "./utils/windows-paths"; const execFileAsync = promisify(execFile); @@ -53,15 +54,15 @@ let npmGlobalPrefixCachePromise: Promise | null = null; function getNpmGlobalPrefix(): string | null { try { // On Windows, use npm.cmd for proper command resolution - const npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm'; + const npmCommand = process.platform === "win32" ? "npm.cmd" : "npm"; // Use --location=global to bypass workspace context and avoid ENOWORKSPACES error - const rawPrefix = execFileSync(npmCommand, ['config', 'get', 'prefix', '--location=global'], { - encoding: 'utf-8', + const rawPrefix = execFileSync(npmCommand, ["config", "get", "prefix", "--location=global"], { + encoding: "utf-8", timeout: 3000, windowsHide: true, cwd: os.homedir(), // Run from home dir to avoid ENOWORKSPACES error in monorepos - shell: process.platform === 'win32', // Enable shell on Windows for .cmd resolution + shell: process.platform === "win32", // Enable shell on Windows for .cmd resolution }).trim(); if (!rawPrefix) { @@ -70,9 +71,7 @@ function getNpmGlobalPrefix(): string | null { // On non-Windows platforms, npm globals are installed in prefix/bin // On Windows, they're installed directly in the prefix directory - const binPath = process.platform === 'win32' - ? rawPrefix - : path.join(rawPrefix, 'bin'); + const binPath = process.platform === "win32" ? rawPrefix : path.join(rawPrefix, "bin"); // Normalize and verify the path exists const normalizedPath = path.normalize(binPath); @@ -89,26 +88,26 @@ function getNpmGlobalPrefix(): string | null { */ export const COMMON_BIN_PATHS: Record = { darwin: [ - '/opt/homebrew/bin', // Apple Silicon Homebrew - '/usr/local/bin', // Intel Homebrew / system - '/usr/local/share/dotnet', // .NET SDK - '/opt/homebrew/sbin', // Apple Silicon Homebrew sbin - '/usr/local/sbin', // Intel Homebrew sbin - '~/.local/bin', // User-local binaries (Claude CLI) - '~/.dotnet/tools', // .NET global tools + "/opt/homebrew/bin", // Apple Silicon Homebrew + "/usr/local/bin", // Intel Homebrew / system + "/usr/local/share/dotnet", // .NET SDK + "/opt/homebrew/sbin", // Apple Silicon Homebrew sbin + "/usr/local/sbin", // Intel Homebrew sbin + "~/.local/bin", // User-local binaries (Claude CLI) + "~/.dotnet/tools", // .NET global tools ], linux: [ - '/usr/local/bin', - '/usr/bin', // System binaries (Python, etc.) - '/snap/bin', // Snap packages - '~/.local/bin', // User-local binaries - '~/.dotnet/tools', // .NET global tools - '/usr/sbin', // System admin binaries + "/usr/local/bin", + "/usr/bin", // System binaries (Python, etc.) + "/snap/bin", // Snap packages + "~/.local/bin", // User-local binaries + "~/.dotnet/tools", // .NET global tools + "/usr/sbin", // System admin binaries ], win32: [ // Windows usually handles PATH better, but we can add common locations - 'C:\\Program Files\\Git\\cmd', - 'C:\\Program Files\\GitHub CLI', + "C:\\Program Files\\Git\\cmd", + "C:\\Program Files\\GitHub CLI", ], }; @@ -116,7 +115,7 @@ export const COMMON_BIN_PATHS: Record = { * Essential system directories that must always be in PATH * Required for core system functionality (e.g., /usr/bin/security for Keychain access) */ -const ESSENTIAL_SYSTEM_PATHS: string[] = ['/usr/bin', '/bin', '/usr/sbin', '/sbin']; +const ESSENTIAL_SYSTEM_PATHS: string[] = ["/usr/bin", "/bin", "/usr/sbin", "/sbin"]; /** * Get expanded platform paths for PATH augmentation @@ -128,19 +127,17 @@ const ESSENTIAL_SYSTEM_PATHS: string[] = ['/usr/bin', '/bin', '/usr/sbin', '/sbi * @returns Array of expanded paths (without existence checking) */ function getExpandedPlatformPaths(additionalPaths?: string[]): string[] { - const platform = process.platform as 'darwin' | 'linux' | 'win32'; + const platform = process.platform as "darwin" | "linux" | "win32"; const homeDir = os.homedir(); // Get platform-specific paths and expand home directory const platformPaths = COMMON_BIN_PATHS[platform] || []; - const expandedPaths = platformPaths.map(p => - p.startsWith('~') ? p.replace('~', homeDir) : p - ); + const expandedPaths = platformPaths.map((p) => (p.startsWith("~") ? p.replace("~", homeDir) : p)); // Add user-requested additional paths (expanded) if (additionalPaths) { for (const p of additionalPaths) { - const expanded = p.startsWith('~') ? p.replace('~', homeDir) : p; + const expanded = p.startsWith("~") ? p.replace("~", homeDir) : p; expandedPaths.push(expanded); } } @@ -195,8 +192,8 @@ function buildPathsToAdd( */ export function getAugmentedEnv(additionalPaths?: string[]): Record { const env = { ...process.env } as Record; - const platform = process.platform as 'darwin' | 'linux' | 'win32'; - const pathSeparator = platform === 'win32' ? ';' : ':'; + const platform = process.platform as "darwin" | "linux" | "win32"; + const pathSeparator = platform === "win32" ? ";" : ":"; // Get all candidate paths (platform + additional) const candidatePaths = getExpandedPlatformPaths(additionalPaths); @@ -204,12 +201,12 @@ export function getAugmentedEnv(additionalPaths?: string[]): Record !pathSetForEssentials.has(p)); + const missingEssentials = ESSENTIAL_SYSTEM_PATHS.filter((p) => !pathSetForEssentials.has(p)); if (missingEssentials.length > 0) { // Append essential paths if missing (append, not prepend, to respect user's PATH) @@ -223,7 +220,7 @@ export function getAugmentedEnv(additionalPaths?: string[]): Record fs.existsSync(p))); + const existingPaths = new Set(candidatePaths.filter((p) => fs.existsSync(p))); // Get npm global prefix dynamically const npmPrefix = getNpmGlobalPrefix(); @@ -251,14 +248,12 @@ export function getAugmentedEnv(additionalPaths?: string[]): Record { // Start the async fetch npmGlobalPrefixCachePromise = (async () => { try { - const npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm'; - - const { stdout } = await execFileAsync(npmCommand, ['config', 'get', 'prefix', '--location=global'], { - encoding: 'utf-8', - timeout: 3000, - windowsHide: true, - cwd: os.homedir(), // Run from home dir to avoid ENOWORKSPACES error in monorepos - shell: process.platform === 'win32', - }); + const npmCommand = process.platform === "win32" ? "npm.cmd" : "npm"; + + const { stdout } = await execFileAsync( + npmCommand, + ["config", "get", "prefix", "--location=global"], + { + encoding: "utf-8", + timeout: 3000, + windowsHide: true, + cwd: os.homedir(), // Run from home dir to avoid ENOWORKSPACES error in monorepos + shell: process.platform === "win32", + } + ); const rawPrefix = stdout.trim(); if (!rawPrefix) { @@ -324,12 +323,10 @@ async function getNpmGlobalPrefixAsync(): Promise { return null; } - const binPath = process.platform === 'win32' - ? rawPrefix - : path.join(rawPrefix, 'bin'); + const binPath = process.platform === "win32" ? rawPrefix : path.join(rawPrefix, "bin"); const normalizedPath = path.normalize(binPath); - npmGlobalPrefixCache = await existsAsync(normalizedPath) ? normalizedPath : null; + npmGlobalPrefixCache = (await existsAsync(normalizedPath)) ? normalizedPath : null; return npmGlobalPrefixCache; } catch (error) { console.warn(`[env-utils] Failed to get npm global prefix: ${error}`); @@ -352,20 +349,22 @@ async function getNpmGlobalPrefixAsync(): Promise { * @param additionalPaths - Optional array of additional paths to include * @returns Promise resolving to environment object with augmented PATH */ -export async function getAugmentedEnvAsync(additionalPaths?: string[]): Promise> { +export async function getAugmentedEnvAsync( + additionalPaths?: string[] +): Promise> { const env = { ...process.env } as Record; - const platform = process.platform as 'darwin' | 'linux' | 'win32'; - const pathSeparator = platform === 'win32' ? ';' : ':'; + const platform = process.platform as "darwin" | "linux" | "win32"; + const pathSeparator = platform === "win32" ? ";" : ":"; // Get all candidate paths (platform + additional) const candidatePaths = getExpandedPlatformPaths(additionalPaths); // Ensure essential system paths are present (for macOS Keychain access) - let currentPath = env.PATH || ''; + let currentPath = env.PATH || ""; - if (platform !== 'win32') { + if (platform !== "win32") { const pathSetForEssentials = new Set(currentPath.split(pathSeparator).filter(Boolean)); - const missingEssentials = ESSENTIAL_SYSTEM_PATHS.filter(p => !pathSetForEssentials.has(p)); + const missingEssentials = ESSENTIAL_SYSTEM_PATHS.filter((p) => !pathSetForEssentials.has(p)); if (missingEssentials.length > 0) { currentPath = currentPath @@ -381,13 +380,11 @@ export async function getAugmentedEnvAsync(additionalPaths?: string[]): Promise< const pathChecks = await Promise.all( candidatePaths.map(async (p) => ({ path: p, exists: await existsAsync(p) })) ); - const existingPaths = new Set( - pathChecks.filter(({ exists }) => exists).map(({ path: p }) => p) - ); + const existingPaths = new Set(pathChecks.filter(({ exists }) => exists).map(({ path: p }) => p)); // Get npm global prefix dynamically (async - non-blocking) const npmPrefix = await getNpmGlobalPrefixAsync(); - if (npmPrefix && await existsAsync(npmPrefix)) { + if (npmPrefix && (await existsAsync(npmPrefix))) { existingPaths.add(npmPrefix); } @@ -410,12 +407,10 @@ export async function getAugmentedEnvAsync(additionalPaths?: string[]): Promise< */ export async function findExecutableAsync(command: string): Promise { const env = await getAugmentedEnvAsync(); - const pathSeparator = process.platform === 'win32' ? ';' : ':'; - const pathDirs = (env.PATH || '').split(pathSeparator); + const pathSeparator = process.platform === "win32" ? ";" : ":"; + const pathDirs = (env.PATH || "").split(pathSeparator); - const extensions = process.platform === 'win32' - ? ['.exe', '.cmd', '.bat', '.ps1', ''] - : ['']; + const extensions = process.platform === "win32" ? [".exe", ".cmd", ".bat", ".ps1", ""] : [""]; for (const dir of pathDirs) { for (const ext of extensions) { @@ -439,6 +434,219 @@ export function clearNpmPrefixCache(): void { npmGlobalPrefixCachePromise = null; } +// ============================================================================ +// PYTHON SUBPROCESS UTILITIES +// ============================================================================ + +/** + * Result of analyzing path quoting requirements + * + * @internal Shared helper for preparePythonSubprocessCommand and prepareShellCommand + */ +interface QuotingDecision { + /** Whether shell mode is required (only true for .cmd/.bat files on Windows) */ + needsShell: boolean; + /** Whether the path needs to be quoted (only when needsShell=true and path has spaces/metacharacters) */ + needsQuoting: boolean; + /** Whether the path is already wrapped in quotes */ + isAlreadyQuoted: boolean; +} + +/** + * Analyze a path to determine quoting requirements for shell execution + * + * This is a shared helper that detects: + * - Windows batch files (.cmd/.bat) that require shell mode + * - Paths already wrapped in quotes + * - Shell metacharacters (& | < > ^ % ()) and spaces that require quoting + * + * @param executablePath - The path to analyze + * @returns Quoting decision object + * @internal + */ +function analyzePathQuoting(executablePath: string): QuotingDecision { + // On Windows, .cmd and .bat files need shell mode for proper execution + // Use case-insensitive check since Windows file extensions are case-insensitive + const lowerPath = executablePath.toLowerCase(); + const isWindowsBatchFile = + process.platform === "win32" && (lowerPath.endsWith(".cmd") || lowerPath.endsWith(".bat")); + const needsShell = isWindowsBatchFile; + + // Check if path is already wrapped in quotes + const isAlreadyQuoted = executablePath.startsWith('"') && executablePath.endsWith('"'); + + // Detect shell metacharacters that require quoting in shell mode + // Windows cmd.exe interprets: & | < > ^ % ( ) as special characters + const hasShellMetacharacters = /[&|<>^%()]/.test(executablePath); + const hasSpaces = executablePath.includes(" "); + + // Add quotes if: shell mode AND (has spaces OR shell metacharacters) AND not already quoted + const needsQuoting = needsShell && (hasSpaces || hasShellMetacharacters) && !isAlreadyQuoted; + + return { needsShell, needsQuoting, isAlreadyQuoted }; +} + +/** + * Result of preparing a path for Python subprocess execution + */ +export interface PythonSubprocessCommand { + /** The escaped path to use in the subprocess command (with proper quotes if needed) */ + commandPath: string; + /** Whether shell=True should be used for subprocess.run() */ + needsShell: boolean; +} + +/** + * Prepare a Windows executable path for Python subprocess execution + * + * Handles the complex quoting rules for Windows paths: + * - .cmd and .bat files require shell=True + * - Paths with spaces need quotes, but only if not already quoted + * - Backslashes and quotes need escaping for Python string representation + * + * This is used when generating Python scripts that will execute Windows commands + * via subprocess.run() with shell=True. + * + * @param executablePath - The path to the executable (e.g., "C:\\Users\\Jane Smith\\...\\claude.cmd") + * @returns Object containing commandPath (escaped for Python) and needsShell flag + * + * @example + * ```typescript + * const { commandPath, needsShell } = preparePythonSubprocessCommand("C:\\Users\\Jane Smith\\AppData\\Roaming\\npm\\claude.cmd"); + * // commandPath = "\\"C:\\\\Users\\\\Jane\\\\Smith\\\\AppData\\\\Roaming\\\\npm\\\\claude.cmd\\"" + * // needsShell = true + * + * // In generated Python script: + * // command = ${commandPath} + ' chat --model haiku --prompt "' + prompt + '"' + * // result = subprocess.run(command, shell=True) + * ``` + */ +export function preparePythonSubprocessCommand(executablePath: string): PythonSubprocessCommand { + // Validate input + if (!executablePath || typeof executablePath !== "string") { + throw new Error( + "preparePythonSubprocessCommand: executablePath is required and must be a string" + ); + } + + // Security validation: ensure path doesn't contain dangerous characters + if (!isSecurePath(executablePath)) { + throw new Error( + `preparePythonSubprocessCommand: executablePath contains potentially dangerous characters. Path: ${executablePath}` + ); + } + + // Analyze path to determine quoting requirements + const { needsShell, needsQuoting, isAlreadyQuoted } = analyzePathQuoting(executablePath); + + // For shell mode, build the properly quoted and escaped path for Python string representation. + // For list mode (no shell), just escape backslashes and quotes. + let commandPath: string; + if (needsShell) { + // If path is already quoted, just escape it for Python without adding more quotes + // Otherwise, quote it if needed and then escape + const pathToEscape = isAlreadyQuoted + ? executablePath + : needsQuoting + ? `"${executablePath}"` + : executablePath; + commandPath = `"${pathToEscape.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; + } else { + // List mode: escape backslashes and single quotes + commandPath = executablePath.replace(/\\/g, "\\\\").replace(/'/g, "\\'"); + } + + return { commandPath, needsShell }; +} + +/** + * Result of preparing a path for Node.js shell execution + */ +export interface ShellCommandResult { + /** The command to use (with quotes if needed) */ + command: string; + /** + * Whether shell mode is required for Windows batch files (.cmd/.bat) + * + * This is true ONLY when the path ends with .cmd/.bat on Windows. + * The returned {command} will be quoted ONLY when needsShell is true and the + * path contains spaces or shell metacharacters. + * + * IMPORTANT: If you set shell: true for other reasons (e.g., custom shell + * scripts, environment variable expansion), you MUST perform your own quoting + * and escaping of executablePath before calling this function. + */ + needsShell: boolean; +} + +/** + * Prepare a Windows executable path for Node.js shell execution + * + * This function prepares paths for use with Node.js child_process functions + * (execFileSync, execFile) when shell: true is required. + * + * Behavior: + * - Sets needsShell=true ONLY for Windows .cmd/.bat files + * - Quotes the path ONLY when needsShell=true AND path contains spaces or + * shell metacharacters (& | < > ^ % ()) + * - Does NOT quote .exe or other files even if they contain spaces + * + * @param executablePath - The path to the executable + * @returns {ShellCommandResult} Object containing: + * - command: The path to use (quoted if needsShell=true and quoting is needed) + * - needsShell: true ONLY for .cmd/.bat files on Windows + * + * @example + * ```typescript + * const { command, needsShell } = prepareShellCommand('C:\\npm\\claude.cmd'); + * // command: 'C:\\npm\\claude.cmd' (no spaces, not quoted) + * // needsShell: true + * + * execFileSync(command, ['--version'], { shell: needsShell }); + * ``` + * + * @example + * ```typescript + * const { command, needsShell } = prepareShellCommand('C:\\Jane Smith\\npm\\claude.cmd'); + * // command: '"C:\\Jane Smith\\npm\\claude.cmd"' (quoted due to spaces) + * // needsShell: true + * + * execFileSync(command, ['--version'], { shell: needsShell }); + * ``` + * + * @example + * ```typescript + * const { command, needsShell } = prepareShellCommand('C:\\Program Files\\app.exe'); + * // command: 'C:\\Program Files\\app.exe' (NOT quoted - needsShell is false for .exe) + * // needsShell: false + * + * // Caller must quote if using shell: true for other reasons: + * const quotedCmd = command.includes(' ') ? `"${command}"` : command; + * execFileSync(quotedCmd, ['--version'], { shell: true }); + * ``` + */ +export function prepareShellCommand(executablePath: string): ShellCommandResult { + // Validate input + if (!executablePath || typeof executablePath !== "string") { + throw new Error("prepareShellCommand: executablePath is required and must be a string"); + } + + // Security validation: ensure path doesn't contain dangerous characters + if (!isSecurePath(executablePath)) { + throw new Error( + `prepareShellCommand: executablePath contains potentially dangerous characters. Path: ${executablePath}` + ); + } + + // Analyze path to determine quoting requirements + const { needsShell, needsQuoting } = analyzePathQuoting(executablePath); + + // Add quotes if: shell mode AND quoting is needed + const command = needsQuoting ? `"${executablePath}"` : executablePath; + + return { command, needsShell }; +} + /** * Determine if a command requires shell execution on Windows * @@ -458,23 +666,18 @@ export function clearNpmPrefixCache(): void { * ``` */ export function shouldUseShell(command: string): boolean { - // Only Windows needs special handling for .cmd/.bat files - if (process.platform !== 'win32') { - return false; - } - - // Check if command ends with .cmd or .bat (case-insensitive) - return /\.(cmd|bat)$/i.test(command); + const { needsShell } = prepareShellCommand(command); + return needsShell; } /** - * Get spawn options with correct shell setting for Windows compatibility + * Get spawn options for executing a command * - * Provides a consistent way to create spawn options that work across platforms. - * Handles the shell requirement for Windows .cmd/.bat files automatically. + * Returns appropriate spawn options for Node.js child_process functions, + * including the correct shell setting for Windows batch files. * - * @param command - The command path to execute - * @param baseOptions - Base spawn options to merge with (optional) + * @param command - The command path being executed + * @param options - Base spawn options to merge with (optional) * @returns Spawn options with correct shell setting * * @example @@ -485,23 +688,24 @@ export function shouldUseShell(command: string): boolean { */ export function getSpawnOptions( command: string, - baseOptions?: { + options?: { cwd?: string; env?: Record; timeout?: number; windowsHide?: boolean; - stdio?: 'inherit' | 'pipe' | Array<'inherit' | 'pipe'>; + stdio?: "inherit" | "pipe" | Array<"inherit" | "pipe">; } ): { cwd?: string; env?: Record; - shell: boolean; + shell: boolean | string; timeout?: number; windowsHide?: boolean; - stdio?: 'inherit' | 'pipe' | Array<'inherit' | 'pipe'>; + stdio?: "inherit" | "pipe" | Array<"inherit" | "pipe">; } { + const { needsShell } = prepareShellCommand(command); return { - ...baseOptions, - shell: shouldUseShell(command), + ...options, + shell: needsShell, }; } diff --git a/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts b/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts index 96edd3c437..aaf3a476bc 100644 --- a/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts @@ -1,16 +1,27 @@ -import { ipcMain } from 'electron'; -import type { BrowserWindow } from 'electron'; -import { IPC_CHANNELS } from '../../shared/constants'; -import type { IPCResult, TerminalCreateOptions, ClaudeProfile, ClaudeProfileSettings, ClaudeUsageSnapshot } from '../../shared/types'; -import { getClaudeProfileManager } from '../claude-profile-manager'; -import { getUsageMonitor } from '../claude-profile/usage-monitor'; -import { TerminalManager } from '../terminal-manager'; -import { projectStore } from '../project-store'; -import { terminalNameGenerator } from '../terminal-name-generator'; -import { debugLog, debugError } from '../../shared/utils/debug-logger'; -import { escapeShellArg, escapeShellArgWindows } from '../../shared/utils/shell-escape'; -import { getClaudeCliInvocationAsync } from '../claude-cli-utils'; - +import { ipcMain } from "electron"; +import type { BrowserWindow } from "electron"; +import { IPC_CHANNELS } from "../../shared/constants"; +import type { + IPCResult, + TerminalCreateOptions, + ClaudeProfile, + ClaudeProfileSettings, + ClaudeUsageSnapshot, +} from "../../shared/types"; +import { getClaudeProfileManager } from "../claude-profile-manager"; +import { getUsageMonitor } from "../claude-profile/usage-monitor"; +import { TerminalManager } from "../terminal-manager"; +import { projectStore } from "../project-store"; +import { terminalNameGenerator } from "../terminal-name-generator"; +import { debugLog, debugError } from "../../shared/utils/debug-logger"; +import { + buildPathAssignment, + buildEnvVarAssignment, + buildEnvVarReference, + buildCommandInvocation, + type ShellType, +} from "../../shared/utils/shell-escape"; +import { getClaudeCliInvocationAsync } from "../claude-cli-utils"; /** * Register all terminal-related IPC handlers @@ -30,36 +41,24 @@ export function registerTerminalHandlers( } ); - ipcMain.handle( - IPC_CHANNELS.TERMINAL_DESTROY, - async (_, id: string): Promise => { - return terminalManager.destroy(id); - } - ); + ipcMain.handle(IPC_CHANNELS.TERMINAL_DESTROY, async (_, id: string): Promise => { + return terminalManager.destroy(id); + }); - ipcMain.on( - IPC_CHANNELS.TERMINAL_INPUT, - (_, id: string, data: string) => { - terminalManager.write(id, data); - } - ); + ipcMain.on(IPC_CHANNELS.TERMINAL_INPUT, (_, id: string, data: string) => { + terminalManager.write(id, data); + }); - ipcMain.on( - IPC_CHANNELS.TERMINAL_RESIZE, - (_, id: string, cols: number, rows: number) => { - terminalManager.resize(id, cols, rows); - } - ); + ipcMain.on(IPC_CHANNELS.TERMINAL_RESIZE, (_, id: string, cols: number, rows: number) => { + terminalManager.resize(id, cols, rows); + }); - ipcMain.on( - IPC_CHANNELS.TERMINAL_INVOKE_CLAUDE, - (_, id: string, cwd?: string) => { - // Use async version to avoid blocking main process during CLI detection - terminalManager.invokeClaudeAsync(id, cwd).catch((error) => { - console.error('[terminal-handlers] Failed to invoke Claude:', error); - }); - } - ); + ipcMain.on(IPC_CHANNELS.TERMINAL_INVOKE_CLAUDE, (_, id: string, cwd?: string) => { + // Use async version to avoid blocking main process during CLI detection + terminalManager.invokeClaudeAsync(id, cwd).catch((error) => { + console.error("[terminal-handlers] Failed to invoke Claude:", error); + }); + }); ipcMain.handle( IPC_CHANNELS.TERMINAL_GENERATE_NAME, @@ -69,29 +68,26 @@ export function registerTerminalHandlers( if (name) { return { success: true, data: name }; } else { - return { success: false, error: 'Failed to generate terminal name' }; + return { success: false, error: "Failed to generate terminal name" }; } } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to generate terminal name' + error: error instanceof Error ? error.message : "Failed to generate terminal name", }; } } ); // Set terminal title (user renamed terminal in renderer) - ipcMain.on( - IPC_CHANNELS.TERMINAL_SET_TITLE, - (_, id: string, title: string) => { - terminalManager.setTitle(id, title); - } - ); + ipcMain.on(IPC_CHANNELS.TERMINAL_SET_TITLE, (_, id: string, title: string) => { + terminalManager.setTitle(id, title); + }); // Set terminal worktree config (user changed worktree association in renderer) ipcMain.on( IPC_CHANNELS.TERMINAL_SET_WORKTREE_CONFIG, - (_, id: string, config: import('../../shared/types').TerminalWorktreeConfig | undefined) => { + (_, id: string, config: import("../../shared/types").TerminalWorktreeConfig | undefined) => { terminalManager.setWorktreeConfig(id, config); } ); @@ -107,7 +103,7 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to get Claude profiles' + error: error instanceof Error ? error.message : "Failed to get Claude profiles", }; } } @@ -126,7 +122,7 @@ export function registerTerminalHandlers( // Ensure config directory exists for non-default profiles if (!profile.isDefault && profile.configDir) { - const { mkdirSync, existsSync } = await import('fs'); + const { mkdirSync, existsSync } = await import("fs"); if (!existsSync(profile.configDir)) { mkdirSync(profile.configDir, { recursive: true }); } @@ -137,7 +133,7 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to save Claude profile' + error: error instanceof Error ? error.message : "Failed to save Claude profile", }; } } @@ -150,13 +146,13 @@ export function registerTerminalHandlers( const profileManager = getClaudeProfileManager(); const success = profileManager.deleteProfile(profileId); if (!success) { - return { success: false, error: 'Cannot delete default or last profile' }; + return { success: false, error: "Cannot delete default or last profile" }; } return { success: true }; } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to delete Claude profile' + error: error instanceof Error ? error.message : "Failed to delete Claude profile", }; } } @@ -169,13 +165,13 @@ export function registerTerminalHandlers( const profileManager = getClaudeProfileManager(); const success = profileManager.renameProfile(profileId, newName); if (!success) { - return { success: false, error: 'Profile not found or invalid name' }; + return { success: false, error: "Profile not found or invalid name" }; } return { success: true }; } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to rename Claude profile' + error: error instanceof Error ? error.message : "Failed to rename Claude profile", }; } } @@ -184,8 +180,10 @@ export function registerTerminalHandlers( ipcMain.handle( IPC_CHANNELS.CLAUDE_PROFILE_SET_ACTIVE, async (_, profileId: string): Promise => { - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] ========== PROFILE SWITCH START =========='); - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Requested profile ID:', profileId); + debugLog( + "[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] ========== PROFILE SWITCH START ==========" + ); + debugLog("[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Requested profile ID:", profileId); try { const profileManager = getClaudeProfileManager(); @@ -193,39 +191,47 @@ export function registerTerminalHandlers( const previousProfileId = previousProfile.id; const newProfile = profileManager.getProfile(profileId); - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Previous profile:', { + debugLog("[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Previous profile:", { id: previousProfile.id, name: previousProfile.name, hasOAuthToken: !!previousProfile.oauthToken, - isDefault: previousProfile.isDefault + isDefault: previousProfile.isDefault, }); - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] New profile:', newProfile ? { - id: newProfile.id, - name: newProfile.name, - hasOAuthToken: !!newProfile.oauthToken, - isDefault: newProfile.isDefault - } : 'NOT FOUND'); + debugLog( + "[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] New profile:", + newProfile + ? { + id: newProfile.id, + name: newProfile.name, + hasOAuthToken: !!newProfile.oauthToken, + isDefault: newProfile.isDefault, + } + : "NOT FOUND" + ); const success = profileManager.setActiveProfile(profileId); - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] setActiveProfile result:', success); + debugLog("[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] setActiveProfile result:", success); if (!success) { - debugError('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Profile not found, aborting'); - return { success: false, error: 'Profile not found' }; + debugError("[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Profile not found, aborting"); + return { success: false, error: "Profile not found" }; } // If the profile actually changed, restart Claude in active terminals // This ensures existing Claude sessions use the new profile's OAuth token const profileChanged = previousProfileId !== profileId; - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Profile changed:', profileChanged, { + debugLog("[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Profile changed:", profileChanged, { previousProfileId, - newProfileId: profileId + newProfileId: profileId, }); if (profileChanged) { const activeTerminalIds = terminalManager.getActiveTerminalIds(); - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Active terminal IDs:', activeTerminalIds); + debugLog( + "[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Active terminal IDs:", + activeTerminalIds + ); const switchPromises: Promise[] = []; const terminalsInClaudeMode: string[] = []; @@ -233,21 +239,32 @@ export function registerTerminalHandlers( for (const terminalId of activeTerminalIds) { const isClaudeMode = terminalManager.isClaudeMode(terminalId); - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal check:', { + debugLog("[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal check:", { terminalId, - isClaudeMode + isClaudeMode, }); if (isClaudeMode) { terminalsInClaudeMode.push(terminalId); - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Queuing terminal for profile switch:', terminalId); + debugLog( + "[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Queuing terminal for profile switch:", + terminalId + ); switchPromises.push( - terminalManager.switchClaudeProfile(terminalId, profileId) + terminalManager + .switchClaudeProfile(terminalId, profileId) .then(() => { - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal profile switch SUCCESS:', terminalId); + debugLog( + "[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal profile switch SUCCESS:", + terminalId + ); }) .catch((err) => { - debugError('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal profile switch FAILED:', terminalId, err); + debugError( + "[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal profile switch FAILED:", + terminalId, + err + ); throw err; // Re-throw so Promise.allSettled correctly reports rejections }) ); @@ -256,39 +273,49 @@ export function registerTerminalHandlers( } } - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal summary:', { + debugLog("[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Terminal summary:", { total: activeTerminalIds.length, inClaudeMode: terminalsInClaudeMode.length, notInClaudeMode: terminalsNotInClaudeMode.length, terminalsToSwitch: terminalsInClaudeMode, - terminalsSkipped: terminalsNotInClaudeMode + terminalsSkipped: terminalsNotInClaudeMode, }); // Wait for all switches to complete (but don't fail the main operation if some fail) if (switchPromises.length > 0) { - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Waiting for', switchPromises.length, 'terminal switches...'); + debugLog( + "[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Waiting for", + switchPromises.length, + "terminal switches..." + ); const results = await Promise.allSettled(switchPromises); - const fulfilled = results.filter(r => r.status === 'fulfilled').length; - const rejected = results.filter(r => r.status === 'rejected').length; - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Switch results:', { + const fulfilled = results.filter((r) => r.status === "fulfilled").length; + const rejected = results.filter((r) => r.status === "rejected").length; + debugLog("[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Switch results:", { total: results.length, fulfilled, - rejected + rejected, }); } else { - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] No terminals in Claude mode to switch'); + debugLog( + "[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] No terminals in Claude mode to switch" + ); } } else { - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Same profile selected, no terminal switches needed'); + debugLog( + "[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] Same profile selected, no terminal switches needed" + ); } - debugLog('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] ========== PROFILE SWITCH COMPLETE =========='); + debugLog( + "[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] ========== PROFILE SWITCH COMPLETE ==========" + ); return { success: true }; } catch (error) { - debugError('[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] EXCEPTION:', error); + debugError("[terminal-handlers:CLAUDE_PROFILE_SET_ACTIVE] EXCEPTION:", error); return { success: false, - error: error instanceof Error ? error.message : 'Failed to set active Claude profile' + error: error instanceof Error ? error.message : "Failed to set active Claude profile", }; } } @@ -303,7 +330,7 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to switch Claude profile' + error: error instanceof Error ? error.message : "Failed to switch Claude profile", }; } } @@ -316,28 +343,28 @@ export function registerTerminalHandlers( const profileManager = getClaudeProfileManager(); const profile = profileManager.getProfile(profileId); if (!profile) { - return { success: false, error: 'Profile not found' }; + return { success: false, error: "Profile not found" }; } // Ensure the config directory exists for non-default profiles if (!profile.isDefault && profile.configDir) { - const { mkdirSync, existsSync } = await import('fs'); + const { mkdirSync, existsSync } = await import("fs"); if (!existsSync(profile.configDir)) { mkdirSync(profile.configDir, { recursive: true }); - debugLog('[IPC] Created config directory:', profile.configDir); + debugLog("[IPC] Created config directory:", profile.configDir); } } // Create a terminal and run claude setup-token there // This is needed because claude setup-token requires TTY/raw mode const terminalId = `claude-login-${profileId}-${Date.now()}`; - const homeDir = process.env.HOME || process.env.USERPROFILE || '/tmp'; + const homeDir = process.env.HOME || process.env.USERPROFILE || "/tmp"; - debugLog('[IPC] Initializing Claude profile:', { + debugLog("[IPC] Initializing Claude profile:", { profileId, profileName: profile.name, configDir: profile.configDir, - isDefault: profile.isDefault + isDefault: profile.isDefault, }); // Create a new terminal for the login process @@ -347,43 +374,56 @@ export function registerTerminalHandlers( if (!createResult.success) { return { success: false, - error: createResult.error || 'Failed to create terminal for authentication' + error: createResult.error || "Failed to create terminal for authentication", }; } // Wait a moment for the terminal to initialize - await new Promise(resolve => setTimeout(resolve, 500)); + await new Promise((resolve) => setTimeout(resolve, 500)); + + // Get the shell type from the terminal for appropriate command syntax + const terminal = terminalManager.getTerminal(terminalId); + const shellType = terminal?.shellType || "unknown"; + debugLog("[IPC] Terminal shell type:", shellType); // Build the login command with the profile's config dir - // Use platform-specific syntax and escaping for environment variables + // Use shell-type-specific syntax and escaping for environment variables let loginCommand: string; const { command: claudeCmd, env: claudeEnv } = await getClaudeCliInvocationAsync(); + + // Build PATH prefix with shell-specific syntax const pathPrefix = claudeEnv.PATH - ? (process.platform === 'win32' - ? `set "PATH=${escapeShellArgWindows(claudeEnv.PATH)}" && ` - : `export PATH=${escapeShellArg(claudeEnv.PATH)} && `) - : ''; - const shellClaudeCmd = process.platform === 'win32' - ? `"${escapeShellArgWindows(claudeCmd)}"` - : escapeShellArg(claudeCmd); + ? buildPathAssignment(claudeEnv.PATH, shellType) + " && " + : ""; if (!profile.isDefault && profile.configDir) { - if (process.platform === 'win32') { - // SECURITY: Use Windows-specific escaping for cmd.exe - const escapedConfigDir = escapeShellArgWindows(profile.configDir); - // Windows cmd.exe syntax: set "VAR=value" with %VAR% for expansion - loginCommand = `${pathPrefix}set "CLAUDE_CONFIG_DIR=${escapedConfigDir}" && echo Config dir: %CLAUDE_CONFIG_DIR% && ${shellClaudeCmd} setup-token`; + // Build environment variable assignment and echo command for the shell type + const envVarAssignment = buildEnvVarAssignment( + "CLAUDE_CONFIG_DIR", + profile.configDir, + shellType + ); + const envVarReference = buildEnvVarReference("CLAUDE_CONFIG_DIR", shellType); + const claudeInvocation = buildCommandInvocation(claudeCmd, ["setup-token"], shellType); + + // Build echo command for verification + let echoCommand: string; + if (shellType === "powershell") { + echoCommand = `Write-Host "Config dir: ${envVarReference}"`; + } else if (shellType === "cmd") { + echoCommand = `echo Config dir: ${envVarReference}`; } else { - // SECURITY: Use POSIX escaping for bash/zsh - const escapedConfigDir = escapeShellArg(profile.configDir); - // Unix/Mac bash/zsh syntax: export VAR=value with $VAR for expansion - loginCommand = `${pathPrefix}export CLAUDE_CONFIG_DIR=${escapedConfigDir} && echo "Config dir: $CLAUDE_CONFIG_DIR" && ${shellClaudeCmd} setup-token`; + echoCommand = `echo Config dir: ${envVarReference}`; } + + loginCommand = `${pathPrefix}${envVarAssignment} && ${echoCommand} && ${claudeInvocation}`; } else { - loginCommand = `${pathPrefix}${shellClaudeCmd} setup-token`; + // Default profile - no config dir needed + const claudeInvocation = buildCommandInvocation(claudeCmd, ["setup-token"], shellType); + loginCommand = `${pathPrefix}${claudeInvocation}`; } - debugLog('[IPC] Sending login command to terminal:', loginCommand); + debugLog("[IPC] Sending login command to terminal:", loginCommand); // Write the login command to the terminal terminalManager.write(terminalId, `${loginCommand}\r`); @@ -395,7 +435,7 @@ export function registerTerminalHandlers( mainWindow.webContents.send(IPC_CHANNELS.TERMINAL_AUTH_CREATED, { terminalId, profileId, - profileName: profile.name + profileName: profile.name, }); } @@ -403,14 +443,14 @@ export function registerTerminalHandlers( success: true, data: { terminalId, - message: `A terminal has been opened to authenticate "${profile.name}". Complete the OAuth flow in your browser, then copy the token shown in the terminal.` - } + message: `A terminal has been opened to authenticate "${profile.name}". Complete the OAuth flow in your browser, then copy the token shown in the terminal.`, + }, }; } catch (error) { - debugError('[IPC] Failed to initialize Claude profile:', error); + debugError("[IPC] Failed to initialize Claude profile:", error); return { success: false, - error: error instanceof Error ? error.message : 'Failed to initialize Claude profile' + error: error instanceof Error ? error.message : "Failed to initialize Claude profile", }; } } @@ -424,14 +464,14 @@ export function registerTerminalHandlers( const profileManager = getClaudeProfileManager(); const success = profileManager.setProfileToken(profileId, token, email); if (!success) { - return { success: false, error: 'Profile not found' }; + return { success: false, error: "Profile not found" }; } return { success: true }; } catch (error) { - debugError('[IPC] Failed to set OAuth token:', error); + debugError("[IPC] Failed to set OAuth token:", error); return { success: false, - error: error instanceof Error ? error.message : 'Failed to set OAuth token' + error: error instanceof Error ? error.message : "Failed to set OAuth token", }; } } @@ -440,7 +480,7 @@ export function registerTerminalHandlers( // Get auto-switch settings ipcMain.handle( IPC_CHANNELS.CLAUDE_PROFILE_AUTO_SWITCH_SETTINGS, - async (): Promise> => { + async (): Promise> => { try { const profileManager = getClaudeProfileManager(); const settings = profileManager.getAutoSwitchSettings(); @@ -448,7 +488,7 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to get auto-switch settings' + error: error instanceof Error ? error.message : "Failed to get auto-switch settings", }; } } @@ -457,7 +497,10 @@ export function registerTerminalHandlers( // Update auto-switch settings ipcMain.handle( IPC_CHANNELS.CLAUDE_PROFILE_UPDATE_AUTO_SWITCH, - async (_, settings: Partial): Promise => { + async ( + _, + settings: Partial + ): Promise => { try { const profileManager = getClaudeProfileManager(); profileManager.updateAutoSwitchSettings(settings); @@ -471,7 +514,7 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to update auto-switch settings' + error: error instanceof Error ? error.message : "Failed to update auto-switch settings", }; } } @@ -483,12 +526,12 @@ export function registerTerminalHandlers( async (_, terminalId: string): Promise => { try { // Send /usage command to the terminal - terminalManager.write(terminalId, '/usage\r'); + terminalManager.write(terminalId, "/usage\r"); return { success: true }; } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to fetch usage' + error: error instanceof Error ? error.message : "Failed to fetch usage", }; } } @@ -505,7 +548,7 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to get best profile' + error: error instanceof Error ? error.message : "Failed to get best profile", }; } } @@ -514,7 +557,10 @@ export function registerTerminalHandlers( // Retry rate-limited operation with a different profile ipcMain.handle( IPC_CHANNELS.CLAUDE_RETRY_WITH_PROFILE, - async (_, request: import('../../shared/types').RetryWithProfileRequest): Promise => { + async ( + _, + request: import("../../shared/types").RetryWithProfileRequest + ): Promise => { try { const profileManager = getClaudeProfileManager(); @@ -524,26 +570,26 @@ export function registerTerminalHandlers( // Get the project const project = projectStore.getProject(request.projectId); if (!project) { - return { success: false, error: 'Project not found' }; + return { success: false, error: "Project not found" }; } // Retry based on the source switch (request.source) { - case 'changelog': + case "changelog": // The changelog UI will handle retrying by re-submitting the form // We just need to confirm the profile switch was successful return { success: true }; - case 'task': + case "task": // For tasks, we would need to restart the task // This is complex and would need task state restoration - return { success: true, data: { message: 'Please restart the task manually' } }; + return { success: true, data: { message: "Please restart the task manually" } }; - case 'roadmap': + case "roadmap": // For roadmap, the UI can trigger a refresh return { success: true }; - case 'ideation': + case "ideation": // For ideation, the UI can trigger a refresh return { success: true }; @@ -553,7 +599,7 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to retry with profile' + error: error instanceof Error ? error.message : "Failed to retry with profile", }; } } @@ -566,7 +612,7 @@ export function registerTerminalHandlers( // Request current usage snapshot ipcMain.handle( IPC_CHANNELS.USAGE_REQUEST, - async (): Promise> => { + async (): Promise> => { try { const monitor = getUsageMonitor(); const usage = monitor.getCurrentUsage(); @@ -574,24 +620,26 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to get current usage' + error: error instanceof Error ? error.message : "Failed to get current usage", }; } } ); - // Terminal session management (persistence/restore) ipcMain.handle( IPC_CHANNELS.TERMINAL_GET_SESSIONS, - async (_, projectPath: string): Promise> => { + async ( + _, + projectPath: string + ): Promise> => { try { const sessions = terminalManager.getSavedSessions(projectPath); return { success: true, data: sessions }; } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to get terminal sessions' + error: error instanceof Error ? error.message : "Failed to get terminal sessions", }; } } @@ -599,7 +647,12 @@ export function registerTerminalHandlers( ipcMain.handle( IPC_CHANNELS.TERMINAL_RESTORE_SESSION, - async (_, session: import('../../shared/types').TerminalSession, cols?: number, rows?: number): Promise> => { + async ( + _, + session: import("../../shared/types").TerminalSession, + cols?: number, + rows?: number + ): Promise> => { try { const result = await terminalManager.restore(session, cols, rows); return { @@ -608,13 +661,13 @@ export function registerTerminalHandlers( success: result.success, terminalId: session.id, outputBuffer: result.outputBuffer, - error: result.error - } + error: result.error, + }, }; } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to restore terminal session' + error: error instanceof Error ? error.message : "Failed to restore terminal session", }; } } @@ -629,48 +682,39 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to clear terminal sessions' + error: error instanceof Error ? error.message : "Failed to clear terminal sessions", }; } } ); - ipcMain.on( - IPC_CHANNELS.TERMINAL_RESUME_CLAUDE, - (_, id: string, sessionId?: string) => { - // Use async version to avoid blocking main process during CLI detection - terminalManager.resumeClaudeAsync(id, sessionId).catch((error) => { - console.error('[terminal-handlers] Failed to resume Claude:', error); - }); - } - ); + ipcMain.on(IPC_CHANNELS.TERMINAL_RESUME_CLAUDE, (_, id: string, sessionId?: string) => { + // Use async version to avoid blocking main process during CLI detection + terminalManager.resumeClaudeAsync(id, sessionId).catch((error) => { + console.error("[terminal-handlers] Failed to resume Claude:", error); + }); + }); // Activate deferred Claude resume when terminal becomes active // This is triggered by the renderer when a terminal with pendingClaudeResume becomes the active tab - ipcMain.on( - IPC_CHANNELS.TERMINAL_ACTIVATE_DEFERRED_RESUME, - (_, id: string) => { - terminalManager.activateDeferredResume(id).catch((error) => { - console.error('[terminal-handlers] Failed to activate deferred Claude resume:', error); - }); - } - ); + ipcMain.on(IPC_CHANNELS.TERMINAL_ACTIVATE_DEFERRED_RESUME, (_, id: string) => { + terminalManager.activateDeferredResume(id).catch((error) => { + console.error("[terminal-handlers] Failed to activate deferred Claude resume:", error); + }); + }); // Get available session dates for a project - ipcMain.handle( - IPC_CHANNELS.TERMINAL_GET_SESSION_DATES, - async (_, projectPath?: string) => { - try { - const dates = terminalManager.getAvailableSessionDates(projectPath); - return { success: true, data: dates }; - } catch (error) { - return { - success: false, - error: error instanceof Error ? error.message : 'Failed to get session dates' - }; - } + ipcMain.handle(IPC_CHANNELS.TERMINAL_GET_SESSION_DATES, async (_, projectPath?: string) => { + try { + const dates = terminalManager.getAvailableSessionDates(projectPath); + return { success: true, data: dates }; + } catch (error) { + return { + success: false, + error: error instanceof Error ? error.message : "Failed to get session dates", + }; } - ); + }); // Get sessions for a specific date and project ipcMain.handle( @@ -682,7 +726,7 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to get sessions for date' + error: error instanceof Error ? error.message : "Failed to get sessions for date", }; } } @@ -703,7 +747,7 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to restore sessions from date' + error: error instanceof Error ? error.message : "Failed to restore sessions from date", }; } } @@ -719,7 +763,7 @@ export function registerTerminalHandlers( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to check terminal status' + error: error instanceof Error ? error.message : "Failed to check terminal status", }; } } @@ -734,14 +778,14 @@ export function initializeUsageMonitorForwarding(mainWindow: BrowserWindow): voi const monitor = getUsageMonitor(); // Forward usage updates to renderer - monitor.on('usage-updated', (usage: ClaudeUsageSnapshot) => { + monitor.on("usage-updated", (usage: ClaudeUsageSnapshot) => { mainWindow.webContents.send(IPC_CHANNELS.USAGE_UPDATED, usage); }); // Forward proactive swap notifications to renderer - monitor.on('show-swap-notification', (notification: unknown) => { + monitor.on("show-swap-notification", (notification: unknown) => { mainWindow.webContents.send(IPC_CHANNELS.PROACTIVE_SWAP_NOTIFICATION, notification); }); - debugLog('[terminal-handlers] Usage monitor event forwarding initialized'); + debugLog("[terminal-handlers] Usage monitor event forwarding initialized"); } diff --git a/apps/frontend/src/main/terminal/claude-integration-handler.ts b/apps/frontend/src/main/terminal/claude-integration-handler.ts index ae420b2d97..b552c2dd66 100644 --- a/apps/frontend/src/main/terminal/claude-integration-handler.ts +++ b/apps/frontend/src/main/terminal/claude-integration-handler.ts @@ -3,41 +3,223 @@ * Manages Claude-specific operations including profile switching, rate limiting, and OAuth token detection */ -import * as os from 'os'; -import * as fs from 'fs'; -import { promises as fsPromises } from 'fs'; -import * as path from 'path'; -import * as crypto from 'crypto'; -import { IPC_CHANNELS } from '../../shared/constants'; -import { getClaudeProfileManager, initializeClaudeProfileManager } from '../claude-profile-manager'; -import * as OutputParser from './output-parser'; -import * as SessionHandler from './session-handler'; -import { debugLog, debugError } from '../../shared/utils/debug-logger'; -import { escapeShellArg, buildCdCommand } from '../../shared/utils/shell-escape'; -import { getClaudeCliInvocation, getClaudeCliInvocationAsync } from '../claude-cli-utils'; -import type { - TerminalProcess, - WindowGetter, - RateLimitEvent, - OAuthTokenEvent -} from './types'; - -function normalizePathForBash(envPath: string): string { - return process.platform === 'win32' ? envPath.replace(/;/g, ':') : envPath; -} +import * as os from "os"; +import * as fs from "fs"; +import { promises as fsPromises } from "fs"; +import * as path from "path"; +import * as crypto from "crypto"; +import { execSync } from "child_process"; +import { IPC_CHANNELS } from "../../shared/constants"; +import { getClaudeProfileManager, initializeClaudeProfileManager } from "../claude-profile-manager"; +import * as OutputParser from "./output-parser"; +import * as SessionHandler from "./session-handler"; +import { debugLog, debugError } from "../../shared/utils/debug-logger"; +import { + escapeShellArg, + escapePowerShellArg, + escapeShellArgWindows, + buildPathAssignment, + buildCommandInvocation, + buildCdCommandForShell, + type ShellType, +} from "../../shared/utils/shell-escape"; +import { getClaudeCliInvocation, getClaudeCliInvocationAsync } from "../claude-cli-utils"; +import type { TerminalProcess, WindowGetter, RateLimitEvent, OAuthTokenEvent } from "./types"; // ============================================================================ // SHARED HELPERS - Used by both sync and async invokeClaude // ============================================================================ +/** + * Check if bash is available on Windows. + * On Unix platforms, always returns true (bash is assumed to be available). + * On Windows, checks if bash command can be found. + * + * Result is cached to avoid redundant execSync calls. + * + * @returns true if bash is available, false otherwise + */ +// Cache result at module level - result doesn't change during app lifetime +// Caches the absolute path to bash executable, or null if not found +let bashPathCache: string | null | undefined; + +/** + * Get the absolute path to bash executable. + * On Unix systems, returns "bash" (assumed to be in PATH). + * On Windows, searches for bash in PATH first, then common installation locations. + * Returns the absolute path if found, null otherwise. + */ +function getBashPath(): string | null { + // Return cached result if available + if (bashPathCache !== undefined) { + return bashPathCache; + } + + if (process.platform !== "win32") { + bashPathCache = "bash"; // Unix systems always have bash in PATH + return "bash"; + } + + // On Windows, check if bash is in PATH by trying common locations + const commonBashPaths = [ + "C:\\Program Files\\Git\\bin\\bash.exe", + "C:\\Program Files\\Git\\usr\\bin\\bash.exe", + "C:\\msys64\\usr\\bin\\bash.exe", + "C:\\cygwin64\\bin\\bash.exe", + ]; + + // Check if bash is in PATH (returns "bash" if found via PATH) + try { + execSync("bash --version", { + stdio: "ignore", + timeout: 1000, // Add timeout to prevent blocking + }); + bashPathCache = "bash"; + return "bash"; + } catch { + // bash not found in PATH, check common installation locations + for (const bashPath of commonBashPaths) { + if (fs.existsSync(bashPath)) { + bashPathCache = bashPath; + return bashPath; + } + } + // bash not found anywhere + bashPathCache = null; + return null; + } +} + +/** + * Parameters for building profile-specific Claude commands + */ +interface BuildProfileCommandParams { + /** Shell type (PowerShell, cmd, bash, zsh, fish, or unknown) */ + shellType: ShellType; + /** Command to change directory (empty string if no change needed) */ + cwdCommand: string; + /** PATH prefix for Claude CLI (empty string if not needed) */ + pathPrefix: string; + /** Claude CLI command (not escaped) */ + claudeCmd: string; + /** OAuth token (for token-based authentication) */ + token?: string; + /** Custom config directory (for configDir-based profiles) */ + configDir?: string; + /** Temp file path (required for token-based method) */ + tempFile?: string; +} + +/** + * Build the command string for profile-specific Claude invocation. + * + * This helper function reduces code duplication between sync and async versions + * by centralizing the shell-specific command building logic. + * + * Handles two methods: + * - 'token': Uses temp file with OAuth token, supports bash fallback and native Windows shells + * - 'configDir': Sets CLAUDE_CONFIG_DIR for custom profile location + * + * @param params - Command building parameters + * @returns The command string, or null if default method should be used + * + * @example + * // Token-based method with bash available + * buildProfileCommandString({ + * shellType: 'powershell', + * cwdCommand: 'cd C:\\path && ', + * pathPrefix: '', + * claudeCmd: 'claude', + * token: 'my-token', + * tempFile: 'C:\\Users\\...\\token-file' + * }); + */ +function buildProfileCommandString(params: BuildProfileCommandParams): string | null { + const { shellType, cwdCommand, pathPrefix, claudeCmd, token, configDir, tempFile } = params; + + // Token-based method (OAuth) + if (token && tempFile) { + if (shellType === "powershell" || shellType === "cmd") { + // On Windows, check if bash is available + const bashPath = getBashPath(); + if (bashPath) { + // Use bash-style temp file with bash -c wrapper + const tempFileBash = tempFile.replace(/\\/g, "/"); + const bashClaudeCmd = claudeCmd.replace(/\\/g, "/"); + const escapedClaudeBash = escapeShellArg(bashClaudeCmd); + const escapedTempFileBash = escapeShellArg(tempFileBash); + // Use the discovered bash path (may be absolute path on Windows) + const bashExe = bashPath === "bash" ? "bash" : bashPath.replace(/\\/g, "/"); + + // Both PowerShell and cmd.exe use the same bash command + return `${cwdCommand}${pathPrefix}${bashExe} -c "source ${escapedTempFileBash} && rm -f ${escapedTempFileBash} && exec ${escapedClaudeBash}"\r`; + } else { + // Bash not available - use native PowerShell/cmd syntax + // + // SECURITY TRADE-OFF: When bash is unavailable, the OAuth token is embedded + // directly in the shell command string (e.g., `$env:CLAUDE_CODE_OAUTH_TOKEN=...` + // or `set "CLAUDE_CODE_OAUTH_TOKEN=..."`). This means the token may be visible + // in process listings (e.g., Task Manager, Process Explorer, ps.exe) during + // command execution. The token is NOT written to shell history files. + // + // This is an acceptable fallback for Windows systems without bash because: + // 1. Process listings are transient (cleared on process exit) + // 2. The exposure window is brief (command executes immediately) + // 3. Alternative approaches (e.g., temp script files) have their own trade-offs + // + // For better security, users should install Git for Windows or WSL to provide bash. + if (shellType === "powershell") { + // PowerShell: set environment variable, invoke claude, then remove temp file + const escapedToken = escapePowerShellArg(token); + const escapedClaude = escapePowerShellArg(claudeCmd); + const escapedTempFile = escapePowerShellArg(tempFile); + return `Clear-Host; ${cwdCommand}$env:CLAUDE_CODE_OAUTH_TOKEN=${escapedToken}; ${pathPrefix}${escapedClaude}; Remove-Item -Force ${escapedTempFile}\r`; + } else { + // cmd.exe: set environment variable, invoke claude, then remove temp file + const escapedToken = escapeShellArgWindows(token); + const escapedClaudeCmdWin = escapeShellArgWindows(claudeCmd); + const escapedTempFileWin = escapeShellArgWindows(tempFile); + return `cls && ${cwdCommand}set "CLAUDE_CODE_OAUTH_TOKEN=${escapedToken}" && ${pathPrefix}"${escapedClaudeCmdWin}" && del /f "${escapedTempFileWin}"\r`; + } + } + } else { + // Unix shells - use existing temp file method + const escapedTempFile = escapeShellArg(tempFile); + const escapedClaudeCmd = escapeShellArg(claudeCmd); + return buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { + method: "temp-file", + escapedTempFile, + }); + } + } + + // ConfigDir-based method + if (configDir) { + if (shellType === "powershell") { + return `${cwdCommand}${pathPrefix}$env:CLAUDE_CONFIG_DIR=${escapePowerShellArg(configDir)}; ${buildCommandInvocation(claudeCmd, [], shellType)}\r`; + } else if (shellType === "cmd") { + const escapedConfigDir = escapeShellArgWindows(configDir); + return `${cwdCommand}set "CLAUDE_CONFIG_DIR=${escapedConfigDir}" && ${pathPrefix}${buildCommandInvocation(claudeCmd, [], shellType)}\r`; + } else { + // Bash/zsh/fish - use history-safe prefix with bash -c "exec ..." to replace shell + const escapedConfigDir = escapeShellArg(configDir); + const escapedClaudeCmd = escapeShellArg(claudeCmd); + return `clear && ${cwdCommand}HISTFILE= HISTCONTROL=ignorespace CLAUDE_CONFIG_DIR=${escapedConfigDir} ${pathPrefix}bash -c "exec ${escapedClaudeCmd}"\r`; + } + } + + // No profile-specific method applicable + return null; +} + /** * Configuration for building Claude shell commands using discriminated union. * This provides type safety by ensuring the correct options are provided for each method. */ type ClaudeCommandConfig = - | { method: 'default' } - | { method: 'temp-file'; escapedTempFile: string } - | { method: 'config-dir'; escapedConfigDir: string }; + | { method: "default" } + | { method: "temp-file"; escapedTempFile: string } + | { method: "config-dir"; escapedConfigDir: string }; /** * Build the shell command for invoking Claude CLI. @@ -72,10 +254,10 @@ export function buildClaudeShellCommand( config: ClaudeCommandConfig ): string { switch (config.method) { - case 'temp-file': + case "temp-file": return `clear && ${cwdCommand}HISTFILE= HISTCONTROL=ignorespace ${pathPrefix}bash -c "source ${config.escapedTempFile} && rm -f ${config.escapedTempFile} && exec ${escapedClaudeCmd}"\r`; - case 'config-dir': + case "config-dir": return `clear && ${cwdCommand}HISTFILE= HISTCONTROL=ignorespace CLAUDE_CONFIG_DIR=${config.escapedConfigDir} ${pathPrefix}bash -c "exec ${escapedClaudeCmd}"\r`; default: @@ -131,9 +313,8 @@ export function finalizeClaudeInvoke( onSessionCapture: SessionCaptureCallback ): void { // Set terminal title based on profile - const title = activeProfile && !activeProfile.isDefault - ? `Claude (${activeProfile.name})` - : 'Claude'; + const title = + activeProfile && !activeProfile.isDefault ? `Claude (${activeProfile.name})` : "Claude"; terminal.title = title; // Notify renderer of title change @@ -174,16 +355,16 @@ export function handleRateLimit( } lastNotifiedRateLimitReset.set(terminal.id, resetTime); - console.warn('[ClaudeIntegration] Rate limit detected, reset:', resetTime); + console.warn("[ClaudeIntegration] Rate limit detected, reset:", resetTime); const profileManager = getClaudeProfileManager(); - const currentProfileId = terminal.claudeProfileId || 'default'; + const currentProfileId = terminal.claudeProfileId || "default"; try { const rateLimitEvent = profileManager.recordRateLimitEvent(currentProfileId, resetTime); - console.warn('[ClaudeIntegration] Recorded rate limit event:', rateLimitEvent.type); + console.warn("[ClaudeIntegration] Recorded rate limit event:", rateLimitEvent.type); } catch (err) { - console.error('[ClaudeIntegration] Failed to record rate limit event:', err); + console.error("[ClaudeIntegration] Failed to record rate limit event:", err); } const autoSwitchSettings = profileManager.getAutoSwitchSettings(); @@ -198,17 +379,19 @@ export function handleRateLimit( profileId: currentProfileId, suggestedProfileId: bestProfile?.id, suggestedProfileName: bestProfile?.name, - autoSwitchEnabled: autoSwitchSettings.autoSwitchOnRateLimit + autoSwitchEnabled: autoSwitchSettings.autoSwitchOnRateLimit, } as RateLimitEvent); } if (autoSwitchSettings.enabled && autoSwitchSettings.autoSwitchOnRateLimit && bestProfile) { - console.warn('[ClaudeIntegration] Auto-switching to profile:', bestProfile.name); - switchProfileCallback(terminal.id, bestProfile.id).then(_result => { - console.warn('[ClaudeIntegration] Auto-switch completed'); - }).catch(err => { - console.error('[ClaudeIntegration] Auto-switch failed:', err); - }); + console.warn("[ClaudeIntegration] Auto-switching to profile:", bestProfile.name); + switchProfileCallback(terminal.id, bestProfile.id) + .then((_result) => { + console.warn("[ClaudeIntegration] Auto-switch completed"); + }) + .catch((err) => { + console.error("[ClaudeIntegration] Auto-switch failed:", err); + }); } } @@ -225,7 +408,7 @@ export function handleOAuthToken( return; } - console.warn('[ClaudeIntegration] OAuth token detected, length:', token.length); + console.warn("[ClaudeIntegration] OAuth token detected, length:", token.length); const email = OutputParser.extractEmail(terminal.outputBuffer); // Match both custom profiles (profile-123456) and the default profile @@ -238,7 +421,7 @@ export function handleOAuthToken( const success = profileManager.setProfileToken(profileId, token, email || undefined); if (success) { - console.warn('[ClaudeIntegration] OAuth token auto-saved to profile:', profileId); + console.warn("[ClaudeIntegration] OAuth token auto-saved to profile:", profileId); const win = getWindow(); if (win) { @@ -247,21 +430,23 @@ export function handleOAuthToken( profileId, email, success: true, - detectedAt: new Date().toISOString() + detectedAt: new Date().toISOString(), } as OAuthTokenEvent); } } else { - console.error('[ClaudeIntegration] Failed to save OAuth token to profile:', profileId); + console.error("[ClaudeIntegration] Failed to save OAuth token to profile:", profileId); } } else { // No profile-specific terminal, save to active profile (GitHub OAuth flow, etc.) - console.warn('[ClaudeIntegration] OAuth token detected in non-profile terminal, saving to active profile'); + console.warn( + "[ClaudeIntegration] OAuth token detected in non-profile terminal, saving to active profile" + ); const profileManager = getClaudeProfileManager(); const activeProfile = profileManager.getActiveProfile(); // Defensive null check for active profile if (!activeProfile) { - console.error('[ClaudeIntegration] Failed to save OAuth token: no active profile found'); + console.error("[ClaudeIntegration] Failed to save OAuth token: no active profile found"); const win = getWindow(); if (win) { win.webContents.send(IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { @@ -269,8 +454,8 @@ export function handleOAuthToken( profileId: undefined, email, success: false, - message: 'No active profile found', - detectedAt: new Date().toISOString() + message: "No active profile found", + detectedAt: new Date().toISOString(), } as OAuthTokenEvent); } return; @@ -279,7 +464,10 @@ export function handleOAuthToken( const success = profileManager.setProfileToken(activeProfile.id, token, email || undefined); if (success) { - console.warn('[ClaudeIntegration] OAuth token auto-saved to active profile:', activeProfile.name); + console.warn( + "[ClaudeIntegration] OAuth token auto-saved to active profile:", + activeProfile.name + ); const win = getWindow(); if (win) { @@ -288,11 +476,14 @@ export function handleOAuthToken( profileId: activeProfile.id, email, success: true, - detectedAt: new Date().toISOString() + detectedAt: new Date().toISOString(), } as OAuthTokenEvent); } } else { - console.error('[ClaudeIntegration] Failed to save OAuth token to active profile:', activeProfile.name); + console.error( + "[ClaudeIntegration] Failed to save OAuth token to active profile:", + activeProfile.name + ); const win = getWindow(); if (win) { win.webContents.send(IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { @@ -300,8 +491,8 @@ export function handleOAuthToken( profileId: activeProfile?.id, email, success: false, - message: 'Failed to save token to active profile', - detectedAt: new Date().toISOString() + message: "Failed to save token to active profile", + detectedAt: new Date().toISOString(), } as OAuthTokenEvent); } } @@ -317,7 +508,7 @@ export function handleClaudeSessionId( getWindow: WindowGetter ): void { terminal.claudeSessionId = sessionId; - console.warn('[ClaudeIntegration] Captured Claude session ID:', sessionId); + console.warn("[ClaudeIntegration] Captured Claude session ID:", sessionId); if (terminal.projectPath) { SessionHandler.updateClaudeSessionId(terminal.projectPath, terminal.id, sessionId); @@ -339,10 +530,10 @@ export function invokeClaude( getWindow: WindowGetter, onSessionCapture: (terminalId: string, projectPath: string, startTime: number) => void ): void { - debugLog('[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE START =========='); - debugLog('[ClaudeIntegration:invokeClaude] Terminal ID:', terminal.id); - debugLog('[ClaudeIntegration:invokeClaude] Requested profile ID:', profileId); - debugLog('[ClaudeIntegration:invokeClaude] CWD:', cwd); + debugLog("[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE START =========="); + debugLog("[ClaudeIntegration:invokeClaude] Terminal ID:", terminal.id); + debugLog("[ClaudeIntegration:invokeClaude] Requested profile ID:", profileId); + debugLog("[ClaudeIntegration:invokeClaude] CWD:", cwd); terminal.isClaudeMode = true; SessionHandler.releaseSessionId(terminal.id); @@ -359,81 +550,152 @@ export function invokeClaude( const previousProfileId = terminal.claudeProfileId; terminal.claudeProfileId = activeProfile?.id; - debugLog('[ClaudeIntegration:invokeClaude] Profile resolution:', { + // Get shell type for command syntax generation (fallback to 'unknown' if not set) + const shellType = terminal.shellType || "unknown"; + debugLog("[ClaudeIntegration:invokeClaude] Shell type:", shellType); + + debugLog("[ClaudeIntegration:invokeClaude] Profile resolution:", { previousProfileId, newProfileId: activeProfile?.id, profileName: activeProfile?.name, hasOAuthToken: !!activeProfile?.oauthToken, - isDefault: activeProfile?.isDefault + isDefault: activeProfile?.isDefault, }); - const cwdCommand = buildCdCommand(cwd); + // Use shell-type-aware cd command + const cwdCommand = buildCdCommandForShell(cwd, shellType); const { command: claudeCmd, env: claudeEnv } = getClaudeCliInvocation(); - const escapedClaudeCmd = escapeShellArg(claudeCmd); - const pathPrefix = claudeEnv.PATH - ? `PATH=${escapeShellArg(normalizePathForBash(claudeEnv.PATH))} ` - : ''; + + // Build path prefix with shell-specific syntax + const pathPrefix = claudeEnv.PATH ? buildPathAssignment(claudeEnv.PATH, shellType) + " " : ""; + const needsEnvOverride = profileId && profileId !== previousProfileId; - debugLog('[ClaudeIntegration:invokeClaude] Environment override check:', { + debugLog("[ClaudeIntegration:invokeClaude] Environment override check:", { profileIdProvided: !!profileId, previousProfileId, - needsEnvOverride + needsEnvOverride, }); if (needsEnvOverride && activeProfile && !activeProfile.isDefault) { const token = profileManager.getProfileToken(activeProfile.id); - debugLog('[ClaudeIntegration:invokeClaude] Token retrieval:', { + debugLog("[ClaudeIntegration:invokeClaude] Token retrieval:", { hasToken: !!token, - tokenLength: token?.length + tokenLength: token?.length, }); if (token) { - const nonce = crypto.randomBytes(8).toString('hex'); + // For non-default profiles with OAuth token, use shell-specific temp file method + const nonce = crypto.randomBytes(8).toString("hex"); const tempFile = path.join(os.tmpdir(), `.claude-token-${Date.now()}-${nonce}`); - const escapedTempFile = escapeShellArg(tempFile); - debugLog('[ClaudeIntegration:invokeClaude] Writing token to temp file:', tempFile); + + debugLog("[ClaudeIntegration:invokeClaude] Writing token to temp file:", tempFile); + // Write token file with Unix-only POSIX file permissions - mode 0o600 (owner read/write only) + // is a POSIX-specific permission bit that is not applicable on Windows; Node.js on Windows + // ignores POSIX mode flags, so we only set writeOptions.mode when process.platform !== "win32" + const writeOptions: fs.WriteFileOptions = {}; + if (process.platform !== "win32") { + writeOptions.mode = 0o600; + } fs.writeFileSync( tempFile, `export CLAUDE_CODE_OAUTH_TOKEN=${escapeShellArg(token)}\n`, - { mode: 0o600 } + writeOptions ); - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'temp-file', escapedTempFile }); - debugLog('[ClaudeIntegration:invokeClaude] Executing command (temp file method, history-safe)'); - terminal.pty.write(command); - profileManager.markProfileUsed(activeProfile.id); - finalizeClaudeInvoke(terminal, activeProfile, projectPath, startTime, getWindow, onSessionCapture); - debugLog('[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE COMPLETE (temp file) =========='); - return; - } else if (activeProfile.configDir) { - const escapedConfigDir = escapeShellArg(activeProfile.configDir); - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'config-dir', escapedConfigDir }); - debugLog('[ClaudeIntegration:invokeClaude] Executing command (configDir method, history-safe)'); - terminal.pty.write(command); - profileManager.markProfileUsed(activeProfile.id); - finalizeClaudeInvoke(terminal, activeProfile, projectPath, startTime, getWindow, onSessionCapture); - debugLog('[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE COMPLETE (configDir) =========='); - return; + // Build shell-specific command for temp file method using shared helper + const command = buildProfileCommandString({ + shellType, + cwdCommand, + pathPrefix, + claudeCmd, + token, + tempFile, + }); + + if (command) { + debugLog( + "[ClaudeIntegration:invokeClaude] Executing command (temp file method, history-safe)" + ); + terminal.pty.write(command); + profileManager.markProfileUsed(activeProfile.id); + finalizeClaudeInvoke( + terminal, + activeProfile, + projectPath, + startTime, + getWindow, + onSessionCapture + ); + debugLog( + "[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE COMPLETE (temp file) ==========" + ); + return; + } + // If command is null, fall through to default method + } + // If no token, check for configDir + else if (activeProfile.configDir) { + // For profiles with custom config dir + const command = buildProfileCommandString({ + shellType, + cwdCommand, + pathPrefix, + claudeCmd, + configDir: activeProfile.configDir, + }); + + if (command) { + debugLog("[ClaudeIntegration:invokeClaude] Executing command (configDir method)"); + terminal.pty.write(command); + profileManager.markProfileUsed(activeProfile.id); + finalizeClaudeInvoke( + terminal, + activeProfile, + projectPath, + startTime, + getWindow, + onSessionCapture + ); + debugLog( + "[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE COMPLETE (configDir) ==========" + ); + return; + } } else { - debugLog('[ClaudeIntegration:invokeClaude] WARNING: No token or configDir available for non-default profile'); + debugLog( + "[ClaudeIntegration:invokeClaude] WARNING: No token or configDir available for non-default profile" + ); } } if (activeProfile && !activeProfile.isDefault) { - debugLog('[ClaudeIntegration:invokeClaude] Using terminal environment for non-default profile:', activeProfile.name); + debugLog( + "[ClaudeIntegration:invokeClaude] Using terminal environment for non-default profile:", + activeProfile.name + ); } - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'default' }); - debugLog('[ClaudeIntegration:invokeClaude] Executing command (default method):', command); + // Default method: use shell-type-aware command invocation + const command = `${cwdCommand}${pathPrefix}${buildCommandInvocation(claudeCmd, [], shellType)}\r`; + debugLog("[ClaudeIntegration:invokeClaude] Executing command (default method):", command); terminal.pty.write(command); if (activeProfile) { profileManager.markProfileUsed(activeProfile.id); } - finalizeClaudeInvoke(terminal, activeProfile, projectPath, startTime, getWindow, onSessionCapture); - debugLog('[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE COMPLETE (default) =========='); + finalizeClaudeInvoke( + terminal, + activeProfile, + projectPath, + startTime, + getWindow, + onSessionCapture + ); + debugLog( + "[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE COMPLETE (default) ==========" + ); } /** @@ -455,11 +717,11 @@ export function resumeClaude( terminal.isClaudeMode = true; SessionHandler.releaseSessionId(terminal.id); + const shellType = terminal.shellType || "unknown"; const { command: claudeCmd, env: claudeEnv } = getClaudeCliInvocation(); - const escapedClaudeCmd = escapeShellArg(claudeCmd); - const pathPrefix = claudeEnv.PATH - ? `PATH=${escapeShellArg(normalizePathForBash(claudeEnv.PATH))} ` - : ''; + + // Build path prefix with shell-specific syntax + const pathPrefix = claudeEnv.PATH ? buildPathAssignment(claudeEnv.PATH, shellType) + " " : ""; // Always use --continue which resumes the most recent session in the current directory. // This is more reliable than --resume with session IDs since Auto Claude already restores @@ -471,18 +733,20 @@ export function resumeClaude( // Deprecation warning for callers still passing sessionId if (_sessionId) { - console.warn('[ClaudeIntegration:resumeClaude] sessionId parameter is deprecated and ignored; using claude --continue instead'); + console.warn( + "[ClaudeIntegration:resumeClaude] sessionId parameter is deprecated and ignored; using claude --continue instead" + ); } - const command = `${pathPrefix}${escapedClaudeCmd} --continue`; + const command = `${pathPrefix}${buildCommandInvocation(claudeCmd, ["--continue"], shellType)}`; terminal.pty.write(`${command}\r`); // Update terminal title in main process and notify renderer - terminal.title = 'Claude'; + terminal.title = "Claude"; const win = getWindow(); if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_TITLE_CHANGE, terminal.id, 'Claude'); + win.webContents.send(IPC_CHANNELS.TERMINAL_TITLE_CHANGE, terminal.id, "Claude"); } // Persist session with updated title @@ -508,10 +772,12 @@ export async function invokeClaudeAsync( getWindow: WindowGetter, onSessionCapture: (terminalId: string, projectPath: string, startTime: number) => void ): Promise { - debugLog('[ClaudeIntegration:invokeClaudeAsync] ========== INVOKE CLAUDE START (async) =========='); - debugLog('[ClaudeIntegration:invokeClaudeAsync] Terminal ID:', terminal.id); - debugLog('[ClaudeIntegration:invokeClaudeAsync] Requested profile ID:', profileId); - debugLog('[ClaudeIntegration:invokeClaudeAsync] CWD:', cwd); + debugLog( + "[ClaudeIntegration:invokeClaudeAsync] ========== INVOKE CLAUDE START (async) ==========" + ); + debugLog("[ClaudeIntegration:invokeClaudeAsync] Terminal ID:", terminal.id); + debugLog("[ClaudeIntegration:invokeClaudeAsync] Requested profile ID:", profileId); + debugLog("[ClaudeIntegration:invokeClaudeAsync] CWD:", cwd); terminal.isClaudeMode = true; SessionHandler.releaseSessionId(terminal.id); @@ -529,82 +795,152 @@ export async function invokeClaudeAsync( const previousProfileId = terminal.claudeProfileId; terminal.claudeProfileId = activeProfile?.id; - debugLog('[ClaudeIntegration:invokeClaudeAsync] Profile resolution:', { + // Get shell type for command syntax generation (fallback to 'unknown' if not set) + const shellType = terminal.shellType || "unknown"; + debugLog("[ClaudeIntegration:invokeClaudeAsync] Shell type:", shellType); + + debugLog("[ClaudeIntegration:invokeClaudeAsync] Profile resolution:", { previousProfileId, newProfileId: activeProfile?.id, profileName: activeProfile?.name, hasOAuthToken: !!activeProfile?.oauthToken, - isDefault: activeProfile?.isDefault + isDefault: activeProfile?.isDefault, }); - // Async CLI invocation - non-blocking - const cwdCommand = buildCdCommand(cwd); + // Use shell-type-aware cd command + const cwdCommand = buildCdCommandForShell(cwd, shellType); const { command: claudeCmd, env: claudeEnv } = await getClaudeCliInvocationAsync(); - const escapedClaudeCmd = escapeShellArg(claudeCmd); - const pathPrefix = claudeEnv.PATH - ? `PATH=${escapeShellArg(normalizePathForBash(claudeEnv.PATH))} ` - : ''; + + // Build path prefix with shell-specific syntax + const pathPrefix = claudeEnv.PATH ? buildPathAssignment(claudeEnv.PATH, shellType) + " " : ""; + const needsEnvOverride = profileId && profileId !== previousProfileId; - debugLog('[ClaudeIntegration:invokeClaudeAsync] Environment override check:', { + debugLog("[ClaudeIntegration:invokeClaudeAsync] Environment override check:", { profileIdProvided: !!profileId, previousProfileId, - needsEnvOverride + needsEnvOverride, }); if (needsEnvOverride && activeProfile && !activeProfile.isDefault) { const token = profileManager.getProfileToken(activeProfile.id); - debugLog('[ClaudeIntegration:invokeClaudeAsync] Token retrieval:', { + debugLog("[ClaudeIntegration:invokeClaudeAsync] Token retrieval:", { hasToken: !!token, - tokenLength: token?.length + tokenLength: token?.length, }); if (token) { - const nonce = crypto.randomBytes(8).toString('hex'); + // For non-default profiles with OAuth token, use shell-specific temp file method + const nonce = crypto.randomBytes(8).toString("hex"); const tempFile = path.join(os.tmpdir(), `.claude-token-${Date.now()}-${nonce}`); - const escapedTempFile = escapeShellArg(tempFile); - debugLog('[ClaudeIntegration:invokeClaudeAsync] Writing token to temp file:', tempFile); + + debugLog("[ClaudeIntegration:invokeClaudeAsync] Writing token to temp file:", tempFile); + // Build options object - mode 0o600 is a Unix-only POSIX file permission flag + // that is ignored/unsupported on Windows; Node.js on Windows ignores POSIX mode flags, + // so we only set writeOptions.mode when process.platform !== "win32" to avoid errors + const writeOptions: fs.WriteFileOptions = {}; + if (process.platform !== "win32") { + writeOptions.mode = 0o600; + } await fsPromises.writeFile( tempFile, `export CLAUDE_CODE_OAUTH_TOKEN=${escapeShellArg(token)}\n`, - { mode: 0o600 } + writeOptions ); - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'temp-file', escapedTempFile }); - debugLog('[ClaudeIntegration:invokeClaudeAsync] Executing command (temp file method, history-safe)'); - terminal.pty.write(command); - profileManager.markProfileUsed(activeProfile.id); - finalizeClaudeInvoke(terminal, activeProfile, projectPath, startTime, getWindow, onSessionCapture); - debugLog('[ClaudeIntegration:invokeClaudeAsync] ========== INVOKE CLAUDE COMPLETE (temp file) =========='); - return; - } else if (activeProfile.configDir) { - const escapedConfigDir = escapeShellArg(activeProfile.configDir); - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'config-dir', escapedConfigDir }); - debugLog('[ClaudeIntegration:invokeClaudeAsync] Executing command (configDir method, history-safe)'); - terminal.pty.write(command); - profileManager.markProfileUsed(activeProfile.id); - finalizeClaudeInvoke(terminal, activeProfile, projectPath, startTime, getWindow, onSessionCapture); - debugLog('[ClaudeIntegration:invokeClaudeAsync] ========== INVOKE CLAUDE COMPLETE (configDir) =========='); - return; + // Build shell-specific command for temp file method using shared helper + const command = buildProfileCommandString({ + shellType, + cwdCommand, + pathPrefix, + claudeCmd, + token, + tempFile, + }); + + if (command) { + debugLog( + "[ClaudeIntegration:invokeClaudeAsync] Executing command (temp file method, history-safe)" + ); + terminal.pty.write(command); + profileManager.markProfileUsed(activeProfile.id); + finalizeClaudeInvoke( + terminal, + activeProfile, + projectPath, + startTime, + getWindow, + onSessionCapture + ); + debugLog( + "[ClaudeIntegration:invokeClaudeAsync] ========== INVOKE CLAUDE COMPLETE (temp file) ==========" + ); + return; + } + // If command is null, fall through to default method + } + // If no token, check for configDir + else if (activeProfile.configDir) { + // For profiles with custom config dir + const command = buildProfileCommandString({ + shellType, + cwdCommand, + pathPrefix, + claudeCmd, + configDir: activeProfile.configDir, + }); + + if (command) { + debugLog("[ClaudeIntegration:invokeClaudeAsync] Executing command (configDir method)"); + terminal.pty.write(command); + profileManager.markProfileUsed(activeProfile.id); + finalizeClaudeInvoke( + terminal, + activeProfile, + projectPath, + startTime, + getWindow, + onSessionCapture + ); + debugLog( + "[ClaudeIntegration:invokeClaudeAsync] ========== INVOKE CLAUDE COMPLETE (configDir) ==========" + ); + return; + } } else { - debugLog('[ClaudeIntegration:invokeClaudeAsync] WARNING: No token or configDir available for non-default profile'); + debugLog( + "[ClaudeIntegration:invokeClaudeAsync] WARNING: No token or configDir available for non-default profile" + ); } } if (activeProfile && !activeProfile.isDefault) { - debugLog('[ClaudeIntegration:invokeClaudeAsync] Using terminal environment for non-default profile:', activeProfile.name); + debugLog( + "[ClaudeIntegration:invokeClaudeAsync] Using terminal environment for non-default profile:", + activeProfile.name + ); } - const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'default' }); - debugLog('[ClaudeIntegration:invokeClaudeAsync] Executing command (default method):', command); + // Default method: use shell-type-aware command invocation + const command = `${cwdCommand}${pathPrefix}${buildCommandInvocation(claudeCmd, [], shellType)}\r`; + debugLog("[ClaudeIntegration:invokeClaudeAsync] Executing command (default method):", command); terminal.pty.write(command); if (activeProfile) { profileManager.markProfileUsed(activeProfile.id); } - finalizeClaudeInvoke(terminal, activeProfile, projectPath, startTime, getWindow, onSessionCapture); - debugLog('[ClaudeIntegration:invokeClaudeAsync] ========== INVOKE CLAUDE COMPLETE (default) =========='); + finalizeClaudeInvoke( + terminal, + activeProfile, + projectPath, + startTime, + getWindow, + onSessionCapture + ); + debugLog( + "[ClaudeIntegration:invokeClaudeAsync] ========== INVOKE CLAUDE COMPLETE (default) ==========" + ); } /** @@ -621,12 +957,11 @@ export async function resumeClaudeAsync( terminal.isClaudeMode = true; SessionHandler.releaseSessionId(terminal.id); - // Async CLI invocation - non-blocking + const shellType = terminal.shellType || "unknown"; const { command: claudeCmd, env: claudeEnv } = await getClaudeCliInvocationAsync(); - const escapedClaudeCmd = escapeShellArg(claudeCmd); - const pathPrefix = claudeEnv.PATH - ? `PATH=${escapeShellArg(normalizePathForBash(claudeEnv.PATH))} ` - : ''; + + // Build path prefix with shell-specific syntax + const pathPrefix = claudeEnv.PATH ? buildPathAssignment(claudeEnv.PATH, shellType) + " " : ""; // Always use --continue which resumes the most recent session in the current directory. // This is more reliable than --resume with session IDs since Auto Claude already restores @@ -638,17 +973,19 @@ export async function resumeClaudeAsync( // Deprecation warning for callers still passing sessionId if (sessionId) { - console.warn('[ClaudeIntegration:resumeClaudeAsync] sessionId parameter is deprecated and ignored; using claude --continue instead'); + console.warn( + "[ClaudeIntegration:resumeClaudeAsync] sessionId parameter is deprecated and ignored; using claude --continue instead" + ); } - const command = `${pathPrefix}${escapedClaudeCmd} --continue`; + const command = `${pathPrefix}${buildCommandInvocation(claudeCmd, ["--continue"], shellType)}`; terminal.pty.write(`${command}\r`); - terminal.title = 'Claude'; + terminal.title = "Claude"; const win = getWindow(); if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_TITLE_CHANGE, terminal.id, 'Claude'); + win.webContents.send(IPC_CHANNELS.TERMINAL_TITLE_CHANGE, terminal.id, "Claude"); } if (terminal.projectPath) { @@ -680,13 +1017,14 @@ interface WaitForExitResult { /** * Shell prompt patterns that indicate Claude has exited and shell is ready - * These patterns match common shell prompts across bash, zsh, fish, etc. + * These patterns match common shell prompts across bash, zsh, fish, PowerShell, etc. */ const SHELL_PROMPT_PATTERNS = [ - /[$%#>❯]\s*$/m, // Common prompt endings: $, %, #, >, ❯ - /\w+@[\w.-]+[:\s]/, // user@hostname: format - /^\s*\S+\s*[$%#>❯]\s*$/m, // hostname/path followed by prompt char - /\(.*\)\s*[$%#>❯]\s*$/m, // (venv) or (branch) followed by prompt + /[$%#>❯]\s*$/m, // Common prompt endings: $, %, #, >, ❯ + /\w+@[\w.-]+[:\s]/, // user@hostname: format + /^\s*\S+\s*[$%#>❯]\s*$/m, // hostname/path followed by prompt char + /\(.*\)\s*[$%#>❯]\s*$/m, // (venv) or (branch) followed by prompt + /^PS\s+[^\r\n>]+>\s*$/m, // PowerShell prompt: PS C:\path> or PS> ]; /** @@ -701,8 +1039,8 @@ async function waitForClaudeExit( ): Promise { const { timeout = 5000, pollInterval = 100 } = config; - debugLog('[ClaudeIntegration:waitForClaudeExit] Waiting for Claude to exit...'); - debugLog('[ClaudeIntegration:waitForClaudeExit] Config:', { timeout, pollInterval }); + debugLog("[ClaudeIntegration:waitForClaudeExit] Waiting for Claude to exit..."); + debugLog("[ClaudeIntegration:waitForClaudeExit] Config:", { timeout, pollInterval }); // Capture current buffer length to detect new output const initialBufferLength = terminal.outputBuffer.length; @@ -714,12 +1052,18 @@ async function waitForClaudeExit( // Check for timeout if (elapsed >= timeout) { - console.warn('[ClaudeIntegration:waitForClaudeExit] Timeout waiting for Claude to exit after', timeout, 'ms'); - debugLog('[ClaudeIntegration:waitForClaudeExit] Timeout reached, Claude may not have exited cleanly'); + console.warn( + "[ClaudeIntegration:waitForClaudeExit] Timeout waiting for Claude to exit after", + timeout, + "ms" + ); + debugLog( + "[ClaudeIntegration:waitForClaudeExit] Timeout reached, Claude may not have exited cleanly" + ); resolve({ success: false, error: `Timeout waiting for Claude to exit after ${timeout}ms`, - timedOut: true + timedOut: true, }); return; } @@ -730,8 +1074,12 @@ async function waitForClaudeExit( // Check if we can see a shell prompt in the new output for (const pattern of SHELL_PROMPT_PATTERNS) { if (pattern.test(newOutput)) { - debugLog('[ClaudeIntegration:waitForClaudeExit] Shell prompt detected after', elapsed, 'ms'); - debugLog('[ClaudeIntegration:waitForClaudeExit] Matched pattern:', pattern.toString()); + debugLog( + "[ClaudeIntegration:waitForClaudeExit] Shell prompt detected after", + elapsed, + "ms" + ); + debugLog("[ClaudeIntegration:waitForClaudeExit] Matched pattern:", pattern.toString()); resolve({ success: true }); return; } @@ -739,7 +1087,11 @@ async function waitForClaudeExit( // Also check if isClaudeMode was cleared (set by other handlers) if (!terminal.isClaudeMode) { - debugLog('[ClaudeIntegration:waitForClaudeExit] isClaudeMode flag cleared after', elapsed, 'ms'); + debugLog( + "[ClaudeIntegration:waitForClaudeExit] isClaudeMode flag cleared after", + elapsed, + "ms" + ); resolve({ success: true }); return; } @@ -760,99 +1112,160 @@ export async function switchClaudeProfile( terminal: TerminalProcess, profileId: string, getWindow: WindowGetter, - invokeClaudeCallback: (terminalId: string, cwd: string | undefined, profileId: string) => Promise, + invokeClaudeCallback: ( + terminalId: string, + cwd: string | undefined, + profileId: string + ) => Promise, clearRateLimitCallback: (terminalId: string) => void ): Promise<{ success: boolean; error?: string }> { // Always-on tracing - console.warn('[ClaudeIntegration:switchClaudeProfile] Called for terminal:', terminal.id, '| profileId:', profileId); - console.warn('[ClaudeIntegration:switchClaudeProfile] Terminal state: isClaudeMode=', terminal.isClaudeMode); - - debugLog('[ClaudeIntegration:switchClaudeProfile] ========== SWITCH PROFILE START =========='); - debugLog('[ClaudeIntegration:switchClaudeProfile] Terminal ID:', terminal.id); - debugLog('[ClaudeIntegration:switchClaudeProfile] Target profile ID:', profileId); - debugLog('[ClaudeIntegration:switchClaudeProfile] Terminal state:', { + console.warn( + "[ClaudeIntegration:switchClaudeProfile] Called for terminal:", + terminal.id, + "| profileId:", + profileId + ); + console.warn( + "[ClaudeIntegration:switchClaudeProfile] Terminal state: isClaudeMode=", + terminal.isClaudeMode + ); + + debugLog("[ClaudeIntegration:switchClaudeProfile] ========== SWITCH PROFILE START =========="); + debugLog("[ClaudeIntegration:switchClaudeProfile] Terminal ID:", terminal.id); + debugLog("[ClaudeIntegration:switchClaudeProfile] Target profile ID:", profileId); + debugLog("[ClaudeIntegration:switchClaudeProfile] Terminal state:", { isClaudeMode: terminal.isClaudeMode, currentProfileId: terminal.claudeProfileId, claudeSessionId: terminal.claudeSessionId, projectPath: terminal.projectPath, - cwd: terminal.cwd + cwd: terminal.cwd, }); // Ensure profile manager is initialized (async, yields to event loop) const profileManager = await initializeClaudeProfileManager(); const profile = profileManager.getProfile(profileId); - console.warn('[ClaudeIntegration:switchClaudeProfile] Profile found:', profile?.name || 'NOT FOUND'); - debugLog('[ClaudeIntegration:switchClaudeProfile] Target profile:', profile ? { - id: profile.id, - name: profile.name, - hasOAuthToken: !!profile.oauthToken, - isDefault: profile.isDefault - } : 'NOT FOUND'); + console.warn( + "[ClaudeIntegration:switchClaudeProfile] Profile found:", + profile?.name || "NOT FOUND" + ); + debugLog( + "[ClaudeIntegration:switchClaudeProfile] Target profile:", + profile + ? { + id: profile.id, + name: profile.name, + hasOAuthToken: !!profile.oauthToken, + isDefault: profile.isDefault, + } + : "NOT FOUND" + ); if (!profile) { - console.error('[ClaudeIntegration:switchClaudeProfile] Profile not found, aborting'); - debugError('[ClaudeIntegration:switchClaudeProfile] Profile not found, aborting'); - return { success: false, error: 'Profile not found' }; + console.error("[ClaudeIntegration:switchClaudeProfile] Profile not found, aborting"); + debugError("[ClaudeIntegration:switchClaudeProfile] Profile not found, aborting"); + return { success: false, error: "Profile not found" }; } - console.warn('[ClaudeIntegration:switchClaudeProfile] Switching to profile:', profile.name); - debugLog('[ClaudeIntegration:switchClaudeProfile] Switching to Claude profile:', profile.name); + console.warn("[ClaudeIntegration:switchClaudeProfile] Switching to profile:", profile.name); + debugLog("[ClaudeIntegration:switchClaudeProfile] Switching to Claude profile:", profile.name); if (terminal.isClaudeMode) { - console.warn('[ClaudeIntegration:switchClaudeProfile] Sending exit commands (Ctrl+C, /exit)'); - debugLog('[ClaudeIntegration:switchClaudeProfile] Terminal is in Claude mode, sending exit commands'); + console.warn("[ClaudeIntegration:switchClaudeProfile] Sending exit commands (Ctrl+C, /exit)"); + debugLog( + "[ClaudeIntegration:switchClaudeProfile] Terminal is in Claude mode, sending exit commands" + ); // Send Ctrl+C to interrupt any ongoing operation - debugLog('[ClaudeIntegration:switchClaudeProfile] Sending Ctrl+C (\\x03)'); - terminal.pty.write('\x03'); + debugLog("[ClaudeIntegration:switchClaudeProfile] Sending Ctrl+C (\\x03)"); + terminal.pty.write("\x03"); // Wait briefly for Ctrl+C to take effect before sending /exit - await new Promise(resolve => setTimeout(resolve, 100)); + await new Promise((resolve) => setTimeout(resolve, 100)); // Send /exit command - debugLog('[ClaudeIntegration:switchClaudeProfile] Sending /exit command'); - terminal.pty.write('/exit\r'); + debugLog("[ClaudeIntegration:switchClaudeProfile] Sending /exit command"); + terminal.pty.write("/exit\r"); // Wait for Claude to actually exit by monitoring for shell prompt - const exitResult = await waitForClaudeExit(terminal, { timeout: 5000, pollInterval: 100 }); + // Use retry logic with fallback for when Claude doesn't exit cleanly + const maxRetries = 2; + let exitResult = await waitForClaudeExit(terminal, { timeout: 5000, pollInterval: 100 }); + let retryCount = 0; + + while (exitResult.timedOut && retryCount < maxRetries) { + retryCount++; + console.warn( + "[ClaudeIntegration:switchClaudeProfile] Timeout waiting for Claude to exit, retry", + retryCount, + "of", + maxRetries + ); + debugLog( + "[ClaudeIntegration:switchClaudeProfile] Retry", + retryCount, + "- sending another Ctrl+C and /exit" + ); - if (exitResult.timedOut) { - console.warn('[ClaudeIntegration:switchClaudeProfile] Timed out waiting for Claude to exit, proceeding with caution'); - debugLog('[ClaudeIntegration:switchClaudeProfile] Exit timeout - terminal may be in inconsistent state'); + // Send another Ctrl+C and /exit for retry attempt + terminal.pty.write("\x03"); + await new Promise((resolve) => setTimeout(resolve, 100)); + terminal.pty.write("/exit\r"); + + exitResult = await waitForClaudeExit(terminal, { timeout: 5000, pollInterval: 100 }); + } - // Even on timeout, we'll try to proceed but log the warning - // The alternative would be to abort, but that could leave users stuck - // If this becomes a problem, we could add retry logic or abort option + if (exitResult.timedOut) { + console.error( + "[ClaudeIntegration:switchClaudeProfile] Failed to exit Claude after", + maxRetries + 1, + "attempts - terminal may be in inconsistent state" + ); + debugError( + "[ClaudeIntegration:switchClaudeProfile] All exit attempts timed out - proceeding with profile switch but terminal state is uncertain" + ); } else if (!exitResult.success) { - console.error('[ClaudeIntegration:switchClaudeProfile] Failed to exit Claude:', exitResult.error); - debugError('[ClaudeIntegration:switchClaudeProfile] Exit failed:', exitResult.error); + console.error( + "[ClaudeIntegration:switchClaudeProfile] Failed to exit Claude:", + exitResult.error + ); + debugError("[ClaudeIntegration:switchClaudeProfile] Exit failed:", exitResult.error); // Continue anyway - the /exit command was sent } else { - console.warn('[ClaudeIntegration:switchClaudeProfile] Claude exited successfully'); - debugLog('[ClaudeIntegration:switchClaudeProfile] Claude exited, ready to switch profile'); + console.warn("[ClaudeIntegration:switchClaudeProfile] Claude exited successfully"); + debugLog("[ClaudeIntegration:switchClaudeProfile] Claude exited, ready to switch profile"); } } else { - console.warn('[ClaudeIntegration:switchClaudeProfile] NOT in Claude mode, skipping exit commands'); - debugLog('[ClaudeIntegration:switchClaudeProfile] Terminal NOT in Claude mode, skipping exit commands'); + console.warn( + "[ClaudeIntegration:switchClaudeProfile] NOT in Claude mode, skipping exit commands" + ); + debugLog( + "[ClaudeIntegration:switchClaudeProfile] Terminal NOT in Claude mode, skipping exit commands" + ); } - debugLog('[ClaudeIntegration:switchClaudeProfile] Clearing rate limit state for terminal'); + debugLog("[ClaudeIntegration:switchClaudeProfile] Clearing rate limit state for terminal"); clearRateLimitCallback(terminal.id); const projectPath = terminal.projectPath || terminal.cwd; - console.warn('[ClaudeIntegration:switchClaudeProfile] Invoking Claude with profile:', profileId, '| cwd:', projectPath); - debugLog('[ClaudeIntegration:switchClaudeProfile] Invoking Claude with new profile:', { + console.warn( + "[ClaudeIntegration:switchClaudeProfile] Invoking Claude with profile:", + profileId, + "| cwd:", + projectPath + ); + debugLog("[ClaudeIntegration:switchClaudeProfile] Invoking Claude with new profile:", { terminalId: terminal.id, projectPath, - profileId + profileId, }); await invokeClaudeCallback(terminal.id, projectPath, profileId); - debugLog('[ClaudeIntegration:switchClaudeProfile] Setting active profile in profile manager'); + debugLog("[ClaudeIntegration:switchClaudeProfile] Setting active profile in profile manager"); profileManager.setActiveProfile(profileId); - console.warn('[ClaudeIntegration:switchClaudeProfile] COMPLETE'); - debugLog('[ClaudeIntegration:switchClaudeProfile] ========== SWITCH PROFILE COMPLETE =========='); + console.warn("[ClaudeIntegration:switchClaudeProfile] COMPLETE"); + debugLog("[ClaudeIntegration:switchClaudeProfile] ========== SWITCH PROFILE COMPLETE =========="); return { success: true }; } diff --git a/apps/frontend/src/main/terminal/pty-manager.ts b/apps/frontend/src/main/terminal/pty-manager.ts index bd38c07a5c..7208d6da82 100644 --- a/apps/frontend/src/main/terminal/pty-manager.ts +++ b/apps/frontend/src/main/terminal/pty-manager.ts @@ -3,42 +3,31 @@ * Handles low-level PTY process creation and lifecycle */ -import * as pty from '@lydell/node-pty'; -import * as os from 'os'; -import { existsSync } from 'fs'; -import type { TerminalProcess, WindowGetter } from './types'; -import { IPC_CHANNELS } from '../../shared/constants'; -import { getClaudeProfileManager } from '../claude-profile-manager'; -import { readSettingsFile } from '../settings-utils'; -import type { SupportedTerminal } from '../../shared/types/settings'; +import * as pty from "@lydell/node-pty"; +import * as os from "os"; +import { existsSync } from "fs"; +import type { TerminalProcess, WindowGetter, ShellType } from "./types"; +import { IPC_CHANNELS } from "../../shared/constants"; +import { getClaudeProfileManager } from "../claude-profile-manager"; +import { readSettingsFile } from "../settings-utils"; +import type { SupportedTerminal } from "../../shared/types/settings"; /** * Windows shell paths for different terminal preferences */ const WINDOWS_SHELL_PATHS: Record = { powershell: [ - 'C:\\Program Files\\PowerShell\\7\\pwsh.exe', // PowerShell 7 (Core) - 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe', // Windows PowerShell 5.1 + "C:\\Program Files\\PowerShell\\7\\pwsh.exe", // PowerShell 7 (Core) + "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", // Windows PowerShell 5.1 ], windowsterminal: [ - 'C:\\Program Files\\PowerShell\\7\\pwsh.exe', // Prefer PowerShell Core in Windows Terminal - 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe', - ], - cmd: [ - 'C:\\Windows\\System32\\cmd.exe', - ], - gitbash: [ - 'C:\\Program Files\\Git\\bin\\bash.exe', - 'C:\\Program Files (x86)\\Git\\bin\\bash.exe', - ], - cygwin: [ - 'C:\\cygwin64\\bin\\bash.exe', - 'C:\\cygwin\\bin\\bash.exe', - ], - msys2: [ - 'C:\\msys64\\usr\\bin\\bash.exe', - 'C:\\msys32\\usr\\bin\\bash.exe', + "C:\\Program Files\\PowerShell\\7\\pwsh.exe", // Prefer PowerShell Core in Windows Terminal + "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", ], + cmd: ["C:\\Windows\\System32\\cmd.exe"], + gitbash: ["C:\\Program Files\\Git\\bin\\bash.exe", "C:\\Program Files (x86)\\Git\\bin\\bash.exe"], + cygwin: ["C:\\cygwin64\\bin\\bash.exe", "C:\\cygwin\\bin\\bash.exe"], + msys2: ["C:\\msys64\\usr\\bin\\bash.exe", "C:\\msys32\\usr\\bin\\bash.exe"], }; /** @@ -46,8 +35,8 @@ const WINDOWS_SHELL_PATHS: Record = { */ function getWindowsShell(preferredTerminal: SupportedTerminal | undefined): string { // If no preference or 'system', use COMSPEC (usually cmd.exe) - if (!preferredTerminal || preferredTerminal === 'system') { - return process.env.COMSPEC || 'cmd.exe'; + if (!preferredTerminal || preferredTerminal === "system") { + return process.env.COMSPEC || "cmd.exe"; } // Check if we have paths defined for this terminal type @@ -62,7 +51,62 @@ function getWindowsShell(preferredTerminal: SupportedTerminal | undefined): stri } // Fallback to COMSPEC for unrecognized terminals - return process.env.COMSPEC || 'cmd.exe'; + return process.env.COMSPEC || "cmd.exe"; +} + +/** + * Detect shell type from shell path + * + * Determines whether the shell is PowerShell, cmd.exe, bash, zsh, or fish + * based on the executable path or name. + * + * @param shellPath - Full path to shell executable + * @returns Shell type for command syntax generation + */ +export function detectShellType(shellPath: string): ShellType { + const lowerPath = shellPath.toLowerCase(); + + // PowerShell detection (both PowerShell 5.1 and 7/Core) + if (lowerPath.includes("pwsh.exe") || lowerPath.includes("powershell.exe")) { + return "powershell"; + } + + // cmd.exe detection + if (lowerPath.includes("cmd.exe")) { + return "cmd"; + } + + // bash detection + if (lowerPath.includes("bash.exe") || lowerPath.includes("/bin/bash")) { + return "bash"; + } + + // zsh detection + if (lowerPath.includes("/zsh") || lowerPath.includes("zsh.exe")) { + return "zsh"; + } + + // fish detection + if (lowerPath.includes("/fish") || lowerPath.includes("fish.exe")) { + return "fish"; + } + + return "unknown"; +} + +/** + * Get the shell path for spawning a new terminal + * + * Returns the appropriate shell executable based on platform and user preference. + * + * @param preferredTerminal - User's preferred terminal type (optional) + * @returns Full path to shell executable + */ +export function getShellPath(preferredTerminal?: SupportedTerminal | undefined): string { + if (process.platform === "win32") { + return getWindowsShell(preferredTerminal); + } + return process.env.SHELL || "/bin/zsh"; } /** @@ -78,13 +122,21 @@ export function spawnPtyProcess( const settings = readSettingsFile(); const preferredTerminal = settings?.preferredTerminal as SupportedTerminal | undefined; - const shell = process.platform === 'win32' - ? getWindowsShell(preferredTerminal) - : process.env.SHELL || '/bin/zsh'; - - const shellArgs = process.platform === 'win32' ? [] : ['-l']; - - console.warn('[PtyManager] Spawning shell:', shell, shellArgs, '(preferred:', preferredTerminal || 'system', ')'); + const shell = getShellPath(preferredTerminal); + const shellType = detectShellType(shell); + const shellArgs = process.platform === "win32" ? [] : ["-l"]; + + console.warn( + "[PtyManager] Spawning shell:", + shell, + "type:", + shellType, + "args:", + shellArgs, + "(preferred:", + preferredTerminal || "system", + ")" + ); // Create a clean environment without DEBUG to prevent Claude Code from // enabling debug mode when the Electron app is run in development mode. @@ -95,15 +147,15 @@ export function spawnPtyProcess( const { DEBUG: _DEBUG, ANTHROPIC_API_KEY: _ANTHROPIC_API_KEY, ...cleanEnv } = process.env; return pty.spawn(shell, shellArgs, { - name: 'xterm-256color', + name: "xterm-256color", cols, rows, cwd: cwd || os.homedir(), env: { ...cleanEnv, ...profileEnv, - TERM: 'xterm-256color', - COLORTERM: 'truecolor', + TERM: "xterm-256color", + COLORTERM: "truecolor", }, }); } @@ -137,7 +189,7 @@ export function setupPtyHandlers( // Handle terminal exit ptyProcess.onExit(({ exitCode }) => { - console.warn('[PtyManager] Terminal exited:', id, 'code:', exitCode); + console.warn("[PtyManager] Terminal exited:", id, "code:", exitCode); const win = getWindow(); if (win) { diff --git a/apps/frontend/src/main/terminal/terminal-lifecycle.ts b/apps/frontend/src/main/terminal/terminal-lifecycle.ts index 22d7eaecee..d4b40998a5 100644 --- a/apps/frontend/src/main/terminal/terminal-lifecycle.ts +++ b/apps/frontend/src/main/terminal/terminal-lifecycle.ts @@ -3,19 +3,16 @@ * Handles terminal creation, restoration, and destruction operations */ -import * as os from 'os'; -import { existsSync } from 'fs'; -import type { TerminalCreateOptions } from '../../shared/types'; -import { IPC_CHANNELS } from '../../shared/constants'; -import type { TerminalSession } from '../terminal-session-store'; -import * as PtyManager from './pty-manager'; -import * as SessionHandler from './session-handler'; -import type { - TerminalProcess, - WindowGetter, - TerminalOperationResult -} from './types'; -import { debugLog, debugError } from '../../shared/utils/debug-logger'; +import * as os from "os"; +import { existsSync } from "fs"; +import type { TerminalCreateOptions } from "../../shared/types"; +import { IPC_CHANNELS } from "../../shared/constants"; +import type { TerminalSession } from "../terminal-session-store"; +import * as PtyManager from "./pty-manager"; +import * as SessionHandler from "./session-handler"; +import type { TerminalProcess, WindowGetter, TerminalOperationResult } from "./types"; +import { debugLog, debugError } from "../../shared/utils/debug-logger"; +import type { SupportedTerminal } from "../../shared/types/settings"; /** * Options for terminal restoration @@ -44,10 +41,10 @@ export async function createTerminal( ): Promise { const { id, cwd, cols = 80, rows = 24, projectPath } = options; - debugLog('[TerminalLifecycle] Creating terminal:', { id, cwd, cols, rows, projectPath }); + debugLog("[TerminalLifecycle] Creating terminal:", { id, cwd, cols, rows, projectPath }); if (terminals.has(id)) { - debugLog('[TerminalLifecycle] Terminal already exists, returning success:', id); + debugLog("[TerminalLifecycle] Terminal already exists, returning success:", id); return { success: true }; } @@ -55,14 +52,19 @@ export async function createTerminal( const profileEnv = PtyManager.getActiveProfileEnv(); if (profileEnv.CLAUDE_CODE_OAUTH_TOKEN) { - debugLog('[TerminalLifecycle] Injecting OAuth token from active profile'); + debugLog("[TerminalLifecycle] Injecting OAuth token from active profile"); } // Validate cwd exists - if the directory doesn't exist (e.g., worktree removed), // fall back to project path to prevent shell exit with code 1 let effectiveCwd = cwd; if (cwd && !existsSync(cwd)) { - debugLog('[TerminalLifecycle] Terminal cwd does not exist, falling back:', cwd, '->', projectPath || os.homedir()); + debugLog( + "[TerminalLifecycle] Terminal cwd does not exist, falling back:", + cwd, + "->", + projectPath || os.homedir() + ); effectiveCwd = projectPath || os.homedir(); } @@ -73,7 +75,12 @@ export async function createTerminal( profileEnv ); - debugLog('[TerminalLifecycle] PTY process spawned, pid:', ptyProcess.pid); + debugLog("[TerminalLifecycle] PTY process spawned, pid:", ptyProcess.pid); + + // Detect shell type for command syntax generation + const shellPath = PtyManager.getShellPath(); + const shellType = PtyManager.detectShellType(shellPath); + debugLog("[TerminalLifecycle] Detected shell type:", shellType, "for shell:", shellPath); const terminalCwd = effectiveCwd || os.homedir(); const terminal: TerminalProcess = { @@ -82,8 +89,9 @@ export async function createTerminal( isClaudeMode: false, projectPath, cwd: terminalCwd, - outputBuffer: '', - title: `Terminal ${terminals.size + 1}` + outputBuffer: "", + title: `Terminal ${terminals.size + 1}`, + shellType, }; terminals.set(id, terminal); @@ -100,13 +108,13 @@ export async function createTerminal( SessionHandler.persistSession(terminal); } - debugLog('[TerminalLifecycle] Terminal created successfully:', id); + debugLog("[TerminalLifecycle] Terminal created successfully:", id); return { success: true }; } catch (error) { - debugError('[TerminalLifecycle] Error creating terminal:', error); + debugError("[TerminalLifecycle] Error creating terminal:", error); return { success: false, - error: error instanceof Error ? error.message : 'Failed to create terminal', + error: error instanceof Error ? error.message : "Failed to create terminal", }; } } @@ -127,20 +135,31 @@ export async function restoreTerminal( // The renderer may pass isClaudeMode: false (by design), but we need the stored value // to determine whether to auto-resume Claude const storedSessions = SessionHandler.getSavedSessions(session.projectPath); - const storedSession = storedSessions.find(s => s.id === session.id); + const storedSession = storedSessions.find((s) => s.id === session.id); const storedIsClaudeMode = storedSession?.isClaudeMode ?? session.isClaudeMode; const storedClaudeSessionId = storedSession?.claudeSessionId ?? session.claudeSessionId; - debugLog('[TerminalLifecycle] Restoring terminal session:', session.id, - 'Passed Claude mode:', session.isClaudeMode, - 'Stored Claude mode:', storedIsClaudeMode, - 'Stored session ID:', storedClaudeSessionId); + debugLog( + "[TerminalLifecycle] Restoring terminal session:", + session.id, + "Passed Claude mode:", + session.isClaudeMode, + "Stored Claude mode:", + storedIsClaudeMode, + "Stored session ID:", + storedClaudeSessionId + ); // Validate cwd exists - if the directory was deleted (e.g., worktree removed), // fall back to project path to prevent shell exit with code 1 let effectiveCwd = session.cwd; if (!existsSync(session.cwd)) { - debugLog('[TerminalLifecycle] Session cwd does not exist, falling back to project path:', session.cwd, '->', session.projectPath); + debugLog( + "[TerminalLifecycle] Session cwd does not exist, falling back to project path:", + session.cwd, + "->", + session.projectPath + ); effectiveCwd = session.projectPath || os.homedir(); } @@ -150,7 +169,7 @@ export async function restoreTerminal( cwd: effectiveCwd, cols, rows, - projectPath: session.projectPath + projectPath: session.projectPath, }, terminals, getWindow, @@ -163,7 +182,7 @@ export async function restoreTerminal( const terminal = terminals.get(session.id); if (!terminal) { - return { success: false, error: 'Terminal not found after creation' }; + return { success: false, error: "Terminal not found after creation" }; } // Restore title and worktree config from session @@ -176,7 +195,10 @@ export async function restoreTerminal( // Worktree was deleted, clear the config and update terminal's cwd terminal.worktreeConfig = undefined; terminal.cwd = effectiveCwd; - debugLog('[TerminalLifecycle] Cleared worktree config for terminal with deleted worktree:', session.id); + debugLog( + "[TerminalLifecycle] Cleared worktree config for terminal with deleted worktree:", + session.id + ); } // Re-persist after restoring title and worktreeConfig @@ -204,12 +226,16 @@ export async function restoreTerminal( // Mark terminal as having a pending Claude resume // The actual resume will be triggered when the terminal becomes active terminal.pendingClaudeResume = true; - debugLog('[TerminalLifecycle] Marking terminal for deferred Claude resume:', terminal.id); + debugLog("[TerminalLifecycle] Marking terminal for deferred Claude resume:", terminal.id); // Notify renderer that this terminal has a pending Claude resume // The renderer will trigger the resume when the terminal tab becomes active if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_PENDING_RESUME, terminal.id, storedClaudeSessionId); + win.webContents.send( + IPC_CHANNELS.TERMINAL_PENDING_RESUME, + terminal.id, + storedClaudeSessionId + ); } // Persist the Claude mode and pending resume state @@ -220,7 +246,7 @@ export async function restoreTerminal( return { success: true, - outputBuffer: session.outputBuffer + outputBuffer: session.outputBuffer, }; } @@ -234,7 +260,7 @@ export async function destroyTerminal( ): Promise { const terminal = terminals.get(id); if (!terminal) { - return { success: false, error: 'Terminal not found' }; + return { success: false, error: "Terminal not found" }; } try { @@ -248,7 +274,7 @@ export async function destroyTerminal( } catch (error) { return { success: false, - error: error instanceof Error ? error.message : 'Failed to destroy terminal', + error: error instanceof Error ? error.message : "Failed to destroy terminal", }; } } @@ -312,7 +338,11 @@ export async function restoreSessionsFromDate( options: RestoreOptions, cols = 80, rows = 24 -): Promise<{ restored: number; failed: number; sessions: Array<{ id: string; success: boolean; error?: string }> }> { +): Promise<{ + restored: number; + failed: number; + sessions: Array<{ id: string; success: boolean; error?: string }>; +}> { const sessions = SessionHandler.getSessionsForDate(date, projectPath); const results: Array<{ id: string; success: boolean; error?: string }> = []; @@ -329,13 +359,13 @@ export async function restoreSessionsFromDate( results.push({ id: session.id, success: result.success, - error: result.error + error: result.error, }); } return { - restored: results.filter(r => r.success).length, - failed: results.filter(r => !r.success).length, - sessions: results + restored: results.filter((r) => r.success).length, + failed: results.filter((r) => !r.success).length, + sessions: results, }; } diff --git a/apps/frontend/src/main/terminal/terminal-manager.ts b/apps/frontend/src/main/terminal/terminal-manager.ts index 52b83a01f0..18aa54e2f0 100644 --- a/apps/frontend/src/main/terminal/terminal-manager.ts +++ b/apps/frontend/src/main/terminal/terminal-manager.ts @@ -3,20 +3,16 @@ * Main orchestrator for terminal lifecycle, Claude integration, and profile management */ -import type { TerminalCreateOptions } from '../../shared/types'; -import type { TerminalSession } from '../terminal-session-store'; +import type { TerminalCreateOptions } from "../../shared/types"; +import type { TerminalSession } from "../terminal-session-store"; // Internal modules -import type { - TerminalProcess, - WindowGetter, - TerminalOperationResult -} from './types'; -import * as PtyManager from './pty-manager'; -import * as SessionHandler from './session-handler'; -import * as TerminalLifecycle from './terminal-lifecycle'; -import * as TerminalEventHandler from './terminal-event-handler'; -import * as ClaudeIntegration from './claude-integration-handler'; +import type { TerminalProcess, WindowGetter, TerminalOperationResult } from "./types"; +import * as PtyManager from "./pty-manager"; +import * as SessionHandler from "./session-handler"; +import * as TerminalLifecycle from "./terminal-lifecycle"; +import * as TerminalEventHandler from "./terminal-event-handler"; +import * as ClaudeIntegration from "./claude-integration-handler"; export class TerminalManager { private terminals: Map = new Map(); @@ -60,11 +56,7 @@ export class TerminalManager { /** * Restore a terminal session */ - async restore( - session: TerminalSession, - cols = 80, - rows = 24 - ): Promise { + async restore(session: TerminalSession, cols = 80, rows = 24): Promise { return TerminalLifecycle.restoreTerminal( session, this.terminals, @@ -84,9 +76,9 @@ export class TerminalManager { onResumeNeeded: (terminalId, sessionId) => { // Use async version to avoid blocking main process this.resumeClaudeAsync(terminalId, sessionId).catch((error) => { - console.error('[terminal-manager] Failed to resume Claude session:', error); + console.error("[terminal-manager] Failed to resume Claude session:", error); }); - } + }, }, cols, rows @@ -97,23 +89,23 @@ export class TerminalManager { * Destroy a terminal process */ async destroy(id: string): Promise { - return TerminalLifecycle.destroyTerminal( - id, - this.terminals, - (terminalId) => { - this.lastNotifiedRateLimitReset.delete(terminalId); - } - ); + return TerminalLifecycle.destroyTerminal(id, this.terminals, (terminalId) => { + this.lastNotifiedRateLimitReset.delete(terminalId); + }); + } + + /** + * Get a terminal by ID + */ + getTerminal(id: string): TerminalProcess | undefined { + return this.terminals.get(id); } /** * Kill all terminal processes */ async killAll(): Promise { - this.saveTimer = await TerminalLifecycle.destroyAllTerminals( - this.terminals, - this.saveTimer - ); + this.saveTimer = await TerminalLifecycle.destroyAllTerminals(this.terminals, this.saveTimer); } /** @@ -195,7 +187,7 @@ export class TerminalManager { async switchClaudeProfile(id: string, profileId: string): Promise { const terminal = this.terminals.get(id); if (!terminal) { - return { success: false, error: 'Terminal not found' }; + return { success: false, error: "Terminal not found" }; } return ClaudeIntegration.switchClaudeProfile( @@ -271,7 +263,9 @@ export class TerminalManager { /** * Get available session dates */ - getAvailableSessionDates(projectPath?: string): import('../terminal-session-store').SessionDateInfo[] { + getAvailableSessionDates( + projectPath?: string + ): import("../terminal-session-store").SessionDateInfo[] { return SessionHandler.getAvailableSessionDates(projectPath); } @@ -290,7 +284,11 @@ export class TerminalManager { projectPath: string, cols = 80, rows = 24 - ): Promise<{ restored: number; failed: number; sessions: Array<{ id: string; success: boolean; error?: string }> }> { + ): Promise<{ + restored: number; + failed: number; + sessions: Array<{ id: string; success: boolean; error?: string }>; + }> { return TerminalLifecycle.restoreSessionsFromDate( date, projectPath, @@ -311,9 +309,9 @@ export class TerminalManager { onResumeNeeded: (terminalId, sessionId) => { // Use async version to avoid blocking main process this.resumeClaudeAsync(terminalId, sessionId).catch((error) => { - console.error('[terminal-manager] Failed to resume Claude session:', error); + console.error("[terminal-manager] Failed to resume Claude session:", error); }); - } + }, }, cols, rows @@ -356,7 +354,10 @@ export class TerminalManager { /** * Update terminal worktree config */ - setWorktreeConfig(id: string, config: import('../../shared/types').TerminalWorktreeConfig | undefined): void { + setWorktreeConfig( + id: string, + config: import("../../shared/types").TerminalWorktreeConfig | undefined + ): void { const terminal = this.terminals.get(id); if (terminal) { terminal.worktreeConfig = config; diff --git a/apps/frontend/src/main/terminal/types.ts b/apps/frontend/src/main/terminal/types.ts index b8ef101230..72230251c2 100644 --- a/apps/frontend/src/main/terminal/types.ts +++ b/apps/frontend/src/main/terminal/types.ts @@ -1,6 +1,11 @@ -import type * as pty from '@lydell/node-pty'; -import type { BrowserWindow } from 'electron'; -import type { TerminalWorktreeConfig } from '../../shared/types'; +import type * as pty from "@lydell/node-pty"; +import type { BrowserWindow } from "electron"; +import type { TerminalWorktreeConfig } from "../../shared/types"; + +/** + * Shell type for terminal + */ +export type ShellType = "powershell" | "cmd" | "bash" | "zsh" | "fish" | "unknown"; /** * Terminal process tracking @@ -19,6 +24,8 @@ export interface TerminalProcess { worktreeConfig?: TerminalWorktreeConfig; /** Whether this terminal has a pending Claude resume that should be triggered on activation */ pendingClaudeResume?: boolean; + /** Shell type for this terminal (used to generate appropriate command syntax) */ + shellType?: ShellType; } /** diff --git a/apps/frontend/src/main/utils/windows-paths.ts b/apps/frontend/src/main/utils/windows-paths.ts index 00ceaf0525..e5ede0a697 100644 --- a/apps/frontend/src/main/utils/windows-paths.ts +++ b/apps/frontend/src/main/utils/windows-paths.ts @@ -8,12 +8,12 @@ * Follows the same pattern as homebrew-python.ts for platform-specific detection. */ -import { existsSync } from 'fs'; -import { access, constants } from 'fs/promises'; -import { execFileSync, execFile } from 'child_process'; -import { promisify } from 'util'; -import path from 'path'; -import os from 'os'; +import { existsSync } from "fs"; +import { access, constants } from "fs/promises"; +import { execFileSync, execFile } from "child_process"; +import { promisify } from "util"; +import path from "path"; +import os from "os"; const execFileAsync = promisify(execFile); @@ -24,26 +24,29 @@ export interface WindowsToolPaths { } export const WINDOWS_GIT_PATHS: WindowsToolPaths = { - toolName: 'Git', - executable: 'git.exe', + toolName: "Git", + executable: "git.exe", patterns: [ - '%PROGRAMFILES%\\Git\\cmd', - '%PROGRAMFILES(X86)%\\Git\\cmd', - '%LOCALAPPDATA%\\Programs\\Git\\cmd', - '%USERPROFILE%\\scoop\\apps\\git\\current\\cmd', - '%PROGRAMFILES%\\Git\\bin', - '%PROGRAMFILES(X86)%\\Git\\bin', - '%PROGRAMFILES%\\Git\\mingw64\\bin', + "%PROGRAMFILES%\\Git\\cmd", + "%PROGRAMFILES(X86)%\\Git\\cmd", + "%LOCALAPPDATA%\\Programs\\Git\\cmd", + "%USERPROFILE%\\scoop\\apps\\git\\current\\cmd", + "%PROGRAMFILES%\\Git\\bin", + "%PROGRAMFILES(X86)%\\Git\\bin", + "%PROGRAMFILES%\\Git\\mingw64\\bin", ], }; export function isSecurePath(pathStr: string): boolean { const dangerousPatterns = [ - /[;&|`${}[\]<>!"^]/, // Shell metacharacters (parentheses removed - safe when quoted) - /%[^%]+%/, // Windows environment variable expansion (e.g., %PATH%) - /\.\.\//, // Unix directory traversal - /\.\.\\/, // Windows directory traversal - /[\r\n]/, // Newlines (command injection) + // Shell metacharacters that are NOT safe even in quoted paths + // Note: & and | are safe when properly quoted (prepareShellCommand handles this) + // Note: () are safe when quoted (removed from this check) + /[;`${}[\]<>!"^]/, + /%[^%]+%/, // Windows environment variable expansion (e.g., %PATH%) + /\.\.\//, // Unix directory traversal + /\.\.\\/, // Windows directory traversal + /[\r\n]/, // Newlines (command injection) ]; for (const pattern of dangerousPatterns) { @@ -57,11 +60,11 @@ export function isSecurePath(pathStr: string): boolean { export function expandWindowsPath(pathPattern: string): string | null { const envVars: Record = { - '%PROGRAMFILES%': process.env.ProgramFiles || 'C:\\Program Files', - '%PROGRAMFILES(X86)%': process.env['ProgramFiles(x86)'] || 'C:\\Program Files (x86)', - '%LOCALAPPDATA%': process.env.LOCALAPPDATA, - '%APPDATA%': process.env.APPDATA, - '%USERPROFILE%': process.env.USERPROFILE || os.homedir(), + "%PROGRAMFILES%": process.env.ProgramFiles || "C:\\Program Files", + "%PROGRAMFILES(X86)%": process.env["ProgramFiles(x86)"] || "C:\\Program Files (x86)", + "%LOCALAPPDATA%": process.env.LOCALAPPDATA, + "%APPDATA%": process.env.APPDATA, + "%USERPROFILE%": process.env.USERPROFILE || os.homedir(), }; let expandedPath = pathPattern; @@ -86,10 +89,10 @@ export function expandWindowsPath(pathPattern: string): string | null { export function getWindowsExecutablePaths( toolPaths: WindowsToolPaths, - logPrefix: string = '[Windows Paths]' + logPrefix: string = "[Windows Paths]" ): string[] { // Only run on Windows - if (process.platform !== 'win32') { + if (process.platform !== "win32") { return []; } @@ -134,9 +137,9 @@ export function getWindowsExecutablePaths( */ export function findWindowsExecutableViaWhere( executable: string, - logPrefix: string = '[Windows Where]' + logPrefix: string = "[Windows Where]" ): string | null { - if (process.platform !== 'win32') { + if (process.platform !== "win32") { return null; } @@ -149,19 +152,19 @@ export function findWindowsExecutableViaWhere( try { // Use 'where' command to find the executable // where.exe is a built-in Windows command that finds executables - const result = execFileSync('where.exe', [executable], { - encoding: 'utf-8', + const result = execFileSync("where.exe", [executable], { + encoding: "utf-8", timeout: 5000, windowsHide: true, }).trim(); // 'where' returns multiple paths separated by newlines if found in multiple locations // Prefer paths with .cmd or .exe extensions (executable files) - const paths = result.split(/\r?\n/).filter(p => p.trim()); + const paths = result.split(/\r?\n/).filter((p) => p.trim()); if (paths.length > 0) { // Prefer .cmd, .bat, or .exe extensions, otherwise take first path - const foundPath = (paths.find(p => /\.(cmd|bat|exe)$/i.test(p)) || paths[0]).trim(); + const foundPath = (paths.find((p) => /\.(cmd|bat|exe)$/i.test(p)) || paths[0]).trim(); // Validate the path exists and is secure if (existsSync(foundPath) && isSecurePath(foundPath)) { @@ -183,10 +186,10 @@ export function findWindowsExecutableViaWhere( */ export async function getWindowsExecutablePathsAsync( toolPaths: WindowsToolPaths, - logPrefix: string = '[Windows Paths]' + logPrefix: string = "[Windows Paths]" ): Promise { // Only run on Windows - if (process.platform !== 'win32') { + if (process.platform !== "win32") { return []; } @@ -237,9 +240,9 @@ export async function getWindowsExecutablePathsAsync( */ export async function findWindowsExecutableViaWhereAsync( executable: string, - logPrefix: string = '[Windows Where]' + logPrefix: string = "[Windows Where]" ): Promise { - if (process.platform !== 'win32') { + if (process.platform !== "win32") { return null; } @@ -252,19 +255,22 @@ export async function findWindowsExecutableViaWhereAsync( try { // Use 'where' command to find the executable // where.exe is a built-in Windows command that finds executables - const { stdout } = await execFileAsync('where.exe', [executable], { - encoding: 'utf-8', + const { stdout } = await execFileAsync("where.exe", [executable], { + encoding: "utf-8", timeout: 5000, windowsHide: true, }); // 'where' returns multiple paths separated by newlines if found in multiple locations // Prefer paths with .cmd, .bat, or .exe extensions (executable files) - const paths = stdout.trim().split(/\r?\n/).filter(p => p.trim()); + const paths = stdout + .trim() + .split(/\r?\n/) + .filter((p) => p.trim()); if (paths.length > 0) { // Prefer .cmd, .bat, or .exe extensions, otherwise take first path - const foundPath = (paths.find(p => /\.(cmd|bat|exe)$/i.test(p)) || paths[0]).trim(); + const foundPath = (paths.find((p) => /\.(cmd|bat|exe)$/i.test(p)) || paths[0]).trim(); // Validate the path exists and is secure try { diff --git a/apps/frontend/src/renderer/components/terminal/__tests__/useXterm.test.ts b/apps/frontend/src/renderer/components/terminal/__tests__/useXterm.test.ts index 39e0917c27..727b10fd70 100644 --- a/apps/frontend/src/renderer/components/terminal/__tests__/useXterm.test.ts +++ b/apps/frontend/src/renderer/components/terminal/__tests__/useXterm.test.ts @@ -6,78 +6,106 @@ * Unit tests for useXterm keyboard handlers * Tests terminal copy/paste keyboard shortcuts and platform detection */ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import type { Mock } from 'vitest'; -import { renderHook, act, render } from '@testing-library/react'; -import React from 'react'; -import { Terminal as XTerm } from '@xterm/xterm'; -import { useXterm } from '../useXterm'; +import { describe, it, expect, vi, beforeEach, afterEach, beforeAll, afterAll } from "vitest"; +import type { Mock } from "vitest"; +import { act, render, cleanup } from "@testing-library/react"; +import React from "react"; +import { Terminal as XTerm } from "@xterm/xterm"; +import { useXterm } from "../useXterm"; // Mock xterm.js -vi.mock('@xterm/xterm', () => ({ +vi.mock("@xterm/xterm", () => ({ Terminal: vi.fn().mockImplementation(() => ({ open: vi.fn(), loadAddon: vi.fn(), attachCustomKeyEventHandler: vi.fn(), hasSelection: vi.fn(() => false), - getSelection: vi.fn(() => ''), + getSelection: vi.fn(() => ""), paste: vi.fn(), input: vi.fn(), onData: vi.fn(), onResize: vi.fn(), dispose: vi.fn(), cols: 80, - rows: 24 - })) + rows: 24, + })), })); // Mock xterm addons -vi.mock('@xterm/addon-fit', () => ({ +vi.mock("@xterm/addon-fit", () => ({ FitAddon: vi.fn().mockImplementation(() => ({ - fit: vi.fn() - })) + fit: vi.fn(), + })), })); -vi.mock('@xterm/addon-web-links', () => ({ - WebLinksAddon: vi.fn() +vi.mock("@xterm/addon-web-links", () => ({ + WebLinksAddon: vi.fn(), })); -vi.mock('@xterm/addon-serialize', () => ({ +vi.mock("@xterm/addon-serialize", () => ({ SerializeAddon: vi.fn().mockImplementation(() => ({ - serialize: vi.fn(() => ''), - dispose: vi.fn() - })) + serialize: vi.fn(() => ""), + dispose: vi.fn(), + })), })); // Mock terminal buffer manager -vi.mock('../../../../lib/terminal-buffer-manager', () => ({ +vi.mock("../../../../lib/terminal-buffer-manager", () => ({ terminalBufferManager: { - get: vi.fn(() => ''), + get: vi.fn(() => ""), set: vi.fn(), - clear: vi.fn() - } + clear: vi.fn(), + }, })); // Mock navigator.platform for platform detection -const originalNavigatorPlatform = navigator.platform; +// Capture the full property descriptor to properly restore it later +const originalNavigatorPlatformDescriptor = Object.getOwnPropertyDescriptor( + navigator, + "platform" +) ?? { + value: navigator.platform, + writable: true, + configurable: true, + enumerable: true, +}; + +/** + * Helper function to set navigator.platform with configurable:true + * to avoid poisoning the property descriptor across tests + */ +function setNavigatorPlatform(value: string): void { + Object.defineProperty(navigator, "platform", { + value, + configurable: true, + writable: true, + }); +} // Mock requestAnimationFrame for jsdom environment (not provided by default) -global.requestAnimationFrame = vi.fn((cb: FrameRequestCallback) => setTimeout(cb, 0) as unknown as number); +// Save originals to restore in afterAll hook +const origRequestAnimationFrame = global.requestAnimationFrame; +const origCancelAnimationFrame = global.cancelAnimationFrame; +const origResizeObserver = global.ResizeObserver; /** * Helper function to set up XTerm mocks and render the hook * Reduces test boilerplate from ~100 lines to ~20 lines per test */ -async function setupMockXterm(overrides: { - hasSelection?: () => boolean; - getSelection?: () => string; - paste?: ReturnType; - input?: ReturnType; -} = {}) { +async function setupMockXterm( + overrides: { + hasSelection?: () => boolean; + getSelection?: () => string; + paste?: ReturnType; + input?: ReturnType; + } = {} +): Promise<{ + keyEventHandler: (event: KeyboardEvent) => boolean; +}> { let keyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; // Override XTerm mock to be constructable - (XTerm as unknown as Mock).mockImplementation(function() { + vi.mocked(XTerm).mockImplementation(function () { return { open: vi.fn(), loadAddon: vi.fn(), @@ -85,7 +113,7 @@ async function setupMockXterm(overrides: { keyEventHandler = handler; }), hasSelection: overrides.hasSelection ?? vi.fn(() => false), - getSelection: overrides.getSelection ?? vi.fn(() => ''), + getSelection: overrides.getSelection ?? vi.fn(() => ""), paste: overrides.paste ?? vi.fn(), input: overrides.input ?? vi.fn(), onData: vi.fn(), @@ -93,235 +121,243 @@ async function setupMockXterm(overrides: { dispose: vi.fn(), write: vi.fn(), cols: 80, - rows: 24 + rows: 24, }; }); // Setup addon mocks - const { FitAddon } = await import('@xterm/addon-fit'); - (FitAddon as unknown as Mock).mockImplementation(function() { + const { FitAddon } = await import("@xterm/addon-fit"); + vi.mocked(FitAddon).mockImplementation(function () { return { fit: vi.fn() }; }); - const { WebLinksAddon } = await import('@xterm/addon-web-links'); - (WebLinksAddon as unknown as Mock).mockImplementation(function() { + const { WebLinksAddon } = await import("@xterm/addon-web-links"); + vi.mocked(WebLinksAddon).mockImplementation(function () { return {}; }); - const { SerializeAddon } = await import('@xterm/addon-serialize'); - (SerializeAddon as unknown as Mock).mockImplementation(function() { + const { SerializeAddon } = await import("@xterm/addon-serialize"); + vi.mocked(SerializeAddon).mockImplementation(function () { return { - serialize: vi.fn(() => ''), - dispose: vi.fn() + serialize: vi.fn(() => ""), + dispose: vi.fn(), }; }); // Mock ResizeObserver - global.ResizeObserver = vi.fn().mockImplementation(function() { + global.ResizeObserver = vi.fn().mockImplementation(function () { return { observe: vi.fn(), unobserve: vi.fn(), - disconnect: vi.fn() + disconnect: vi.fn(), }; }); // Create and render test wrapper component const TestWrapper = () => { - const { terminalRef } = useXterm({ terminalId: 'test-terminal' }); - return React.createElement('div', { ref: terminalRef }); + const { terminalRef } = useXterm({ terminalId: "test-terminal" }); + return React.createElement("div", { ref: terminalRef }); }; render(React.createElement(TestWrapper)); // After rendering, keyEventHandler is guaranteed to be set by attachCustomKeyEventHandler - // Use non-null assertion since we know the hook will set it + // Fail fast with a clear error if the hook stops attaching the handler + if (!keyEventHandler) { + throw new Error("useXterm did not attach a custom key event handler"); + } return { - keyEventHandler: keyEventHandler!, - mockInstance: { - hasSelection: overrides.hasSelection, - getSelection: overrides.getSelection, - paste: overrides.paste, - input: overrides.input - } + keyEventHandler, // No non-null assertion needed - guaranteed non-null by check above }; } -describe('useXterm keyboard handlers', () => { +describe("useXterm keyboard handlers", () => { let mockClipboard: { writeText: ReturnType; readText: ReturnType; }; + beforeAll(() => { + // Set up global mocks once for all tests + // These persist across tests but are not affected by vi.clearAllMocks() + global.requestAnimationFrame = vi.fn( + (cb: FrameRequestCallback) => setTimeout(cb, 0) as unknown as number + ); + global.cancelAnimationFrame = vi.fn((id: unknown) => clearTimeout(id as number)); + + // Mock ResizeObserver + global.ResizeObserver = vi.fn().mockImplementation(function () { + return { + observe: vi.fn(), + unobserve: vi.fn(), + disconnect: vi.fn(), + }; + }); + }); + beforeEach(() => { // Clear all mocks before each test vi.clearAllMocks(); // Ensure window and navigator exist in test environment - if (typeof window === 'undefined') { + if (typeof window === "undefined") { (global as { window: unknown }).window = {}; } - if (typeof navigator === 'undefined') { + if (typeof navigator === "undefined") { (global as { navigator: unknown }).navigator = {}; } // Mock navigator.clipboard mockClipboard = { writeText: vi.fn().mockResolvedValue(undefined), - readText: vi.fn().mockResolvedValue('test clipboard content') + readText: vi.fn().mockResolvedValue("test clipboard content"), }; - Object.defineProperty(global.navigator, 'clipboard', { + Object.defineProperty(global.navigator, "clipboard", { value: mockClipboard, writable: true, - configurable: true + configurable: true, }); // Mock window.electronAPI (window as unknown as { electronAPI: unknown }).electronAPI = { - sendTerminalInput: vi.fn() + sendTerminalInput: vi.fn(), }; }); afterEach(() => { - vi.restoreAllMocks(); - // Reset navigator.platform to original value - Object.defineProperty(navigator, 'platform', { - value: originalNavigatorPlatform, - writable: true - }); + // Cleanup React DOM to prevent memory leaks + cleanup(); + // Restore the full original navigator.platform descriptor + Object.defineProperty(navigator, "platform", originalNavigatorPlatformDescriptor); + }); + + afterAll(() => { + // Note: We intentionally do NOT restore the original global shims (origRequestAnimationFrame, etc.) + // because jsdom doesn't provide these APIs anyway, and pending setTimeout callbacks from + // the component (e.g., performInitialFit) may still fire after all tests complete. + // The mocks are harmless and will be cleaned up when the test process exits. }); - describe('Platform detection', () => { - it('should enable paste shortcuts on Windows (CTRL+V)', async () => { + describe("Platform detection", () => { + it("should enable paste shortcuts on Windows (CTRL+V)", async () => { const mockPaste = vi.fn(); // Mock Windows platform - Object.defineProperty(navigator, 'platform', { - value: 'Win32', - writable: true - }); + setNavigatorPlatform("Win32"); const { keyEventHandler } = await setupMockXterm({ paste: mockPaste }); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'v', + const event = new KeyboardEvent("keydown", { + key: "v", ctrlKey: true, - shiftKey: false + shiftKey: false, }); keyEventHandler(event); - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); // Windows should enable CTRL+V paste - expect(mockPaste).toHaveBeenCalledWith('test clipboard content'); + expect(mockPaste).toHaveBeenCalledWith("test clipboard content"); }); - it('should enable paste shortcuts on Linux (both CTRL+V and CTRL+SHIFT+V)', async () => { + it("should enable paste shortcuts on Linux (both CTRL+V and CTRL+SHIFT+V)", async () => { const mockPaste = vi.fn(); // Mock Linux platform - Object.defineProperty(navigator, 'platform', { - value: 'Linux', - writable: true - }); + setNavigatorPlatform("Linux"); const { keyEventHandler } = await setupMockXterm({ paste: mockPaste }); // Test CTRL+V await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'v', + const event = new KeyboardEvent("keydown", { + key: "v", ctrlKey: true, - shiftKey: false + shiftKey: false, }); keyEventHandler(event); - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); expect(mockPaste).toHaveBeenCalledTimes(1); // Test CTRL+SHIFT+V (Linux-specific) await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'V', + const event = new KeyboardEvent("keydown", { + key: "V", ctrlKey: true, - shiftKey: true + shiftKey: true, }); keyEventHandler(event); - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); expect(mockPaste).toHaveBeenCalledTimes(2); }); - it('should enable copy shortcuts on Linux (both CTRL+C and CTRL+SHIFT+C)', async () => { + it("should enable copy shortcuts on Linux (both CTRL+C and CTRL+SHIFT+C)", async () => { const mockHasSelection = vi.fn(() => true); - const mockGetSelection = vi.fn(() => 'selected text'); + const mockGetSelection = vi.fn(() => "selected text"); // Mock Linux platform - Object.defineProperty(navigator, 'platform', { - value: 'Linux', - writable: true - }); + setNavigatorPlatform("Linux"); const { keyEventHandler } = await setupMockXterm({ hasSelection: mockHasSelection, - getSelection: mockGetSelection + getSelection: mockGetSelection, }); // Test CTRL+C (should copy) await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'c', + const event = new KeyboardEvent("keydown", { + key: "c", ctrlKey: true, - shiftKey: false + shiftKey: false, }); keyEventHandler(event); - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); expect(mockClipboard.writeText).toHaveBeenCalledTimes(1); // Test CTRL+SHIFT+C (Linux-specific, should also copy) await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'C', + const event = new KeyboardEvent("keydown", { + key: "C", ctrlKey: true, - shiftKey: true + shiftKey: true, }); keyEventHandler(event); - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); expect(mockClipboard.writeText).toHaveBeenCalledTimes(2); }); - it('should NOT enable custom paste handler on macOS (uses system Cmd+V)', async () => { + it("should NOT enable custom paste handler on macOS (uses system Cmd+V)", async () => { const mockPaste = vi.fn(); // Mock macOS platform - Object.defineProperty(navigator, 'platform', { - value: 'MacIntel', - writable: true - }); + setNavigatorPlatform("MacIntel"); const { keyEventHandler } = await setupMockXterm({ paste: mockPaste }); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'v', + const event = new KeyboardEvent("keydown", { + key: "v", ctrlKey: true, - shiftKey: false + shiftKey: false, }); keyEventHandler(event); - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); // macOS should NOT use custom CTRL+V handler (uses system Cmd+V instead) @@ -329,30 +365,30 @@ describe('useXterm keyboard handlers', () => { }); }); - describe('Smart CTRL+C behavior', () => { - it('should copy to clipboard when text is selected', async () => { + describe("Smart CTRL+C behavior", () => { + it("should copy to clipboard when text is selected", async () => { // Create mock functions that will be shared between the mock instance and our assertions const mockHasSelection = vi.fn(() => true); - const mockGetSelection = vi.fn(() => 'selected text'); + const mockGetSelection = vi.fn(() => "selected text"); const { keyEventHandler } = await setupMockXterm({ hasSelection: mockHasSelection, - getSelection: mockGetSelection + getSelection: mockGetSelection, }); await act(async () => { // Simulate CTRL+C keydown event - const event = new KeyboardEvent('keydown', { - key: 'c', + const event = new KeyboardEvent("keydown", { + key: "c", ctrlKey: true, - metaKey: false + metaKey: false, }); const handled = keyEventHandler(event); expect(handled).toBe(false); // Should prevent xterm handling // Wait for clipboard write - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); // Verify the xterm instance methods were called @@ -360,24 +396,24 @@ describe('useXterm keyboard handlers', () => { expect(mockGetSelection).toHaveBeenCalled(); // Verify clipboard.writeText was called with selected text - expect(mockClipboard.writeText).toHaveBeenCalledWith('selected text'); + expect(mockClipboard.writeText).toHaveBeenCalledWith("selected text"); }); - it('should send ^C interrupt when no text is selected', async () => { + it("should send ^C interrupt when no text is selected", async () => { const mockHasSelection = vi.fn(() => false); - const mockGetSelection = vi.fn(() => ''); + const mockGetSelection = vi.fn(() => ""); const { keyEventHandler } = await setupMockXterm({ hasSelection: mockHasSelection, - getSelection: mockGetSelection + getSelection: mockGetSelection, }); await act(async () => { // Simulate CTRL+C keydown event with no selection - const event = new KeyboardEvent('keydown', { - key: 'c', + const event = new KeyboardEvent("keydown", { + key: "c", ctrlKey: true, - metaKey: false + metaKey: false, }); const handled = keyEventHandler(event); @@ -388,43 +424,39 @@ describe('useXterm keyboard handlers', () => { expect(mockClipboard.writeText).not.toHaveBeenCalled(); }); - it('should handle both ctrlKey (Windows/Linux) and metaKey (Mac)', async () => { + it("should handle both ctrlKey (Windows/Linux) and metaKey (Mac)", async () => { const mockHasSelection = vi.fn(() => true); - const mockGetSelection = vi.fn(() => 'selected text'); + const mockGetSelection = vi.fn(() => "selected text"); const { keyEventHandler } = await setupMockXterm({ hasSelection: mockHasSelection, - getSelection: mockGetSelection + getSelection: mockGetSelection, }); // Test ctrlKey (Windows/Linux) await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'c', + const event = new KeyboardEvent("keydown", { + key: "c", ctrlKey: true, - metaKey: false + metaKey: false, }); - if (keyEventHandler) { - keyEventHandler!(event); - // Wait for clipboard write - await new Promise(resolve => setTimeout(resolve, 0)); - } + keyEventHandler(event); + // Wait for clipboard write + await new Promise((resolve) => setTimeout(resolve, 0)); }); // Test metaKey (Mac) await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'c', + const event = new KeyboardEvent("keydown", { + key: "c", ctrlKey: false, - metaKey: true + metaKey: true, }); - if (keyEventHandler) { - keyEventHandler!(event); - // Wait for clipboard write - await new Promise(resolve => setTimeout(resolve, 0)); - } + keyEventHandler(event); + // Wait for clipboard write + await new Promise((resolve) => setTimeout(resolve, 0)); }); // Both should trigger clipboard write @@ -432,82 +464,71 @@ describe('useXterm keyboard handlers', () => { }); }); - describe('CTRL+V paste behavior', () => { - it('should paste clipboard content on Windows', async () => { + describe("CTRL+V paste behavior", () => { + it("should paste clipboard content on Windows", async () => { const mockPaste = vi.fn(); // Mock Windows platform (navigator) - Object.defineProperty(navigator, 'platform', { - value: 'Win32', - writable: true - }); + setNavigatorPlatform("Win32"); const { keyEventHandler } = await setupMockXterm({ paste: mockPaste }); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'v', - ctrlKey: true + const event = new KeyboardEvent("keydown", { + key: "v", + ctrlKey: true, }); - if (keyEventHandler) { - const handled = keyEventHandler!(event); - expect(handled).toBe(false); // Should prevent literal ^V + const handled = keyEventHandler(event); + expect(handled).toBe(false); // Should prevent literal ^V - // Wait for clipboard read and paste - await new Promise(resolve => setTimeout(resolve, 0)); - } + // Wait for clipboard read and paste + await new Promise((resolve) => setTimeout(resolve, 0)); }); // Verify clipboard read and paste expect(mockClipboard.readText).toHaveBeenCalled(); - expect(mockPaste).toHaveBeenCalledWith('test clipboard content'); + expect(mockPaste).toHaveBeenCalledWith("test clipboard content"); }); - it('should paste clipboard content on Linux', async () => { + it("should paste clipboard content on Linux", async () => { const mockPaste = vi.fn(); // Mock Linux platform (navigator) - Object.defineProperty(navigator, 'platform', { - value: 'Linux', - writable: true - }); + setNavigatorPlatform("Linux"); const { keyEventHandler } = await setupMockXterm({ paste: mockPaste }); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'v', - ctrlKey: true + const event = new KeyboardEvent("keydown", { + key: "v", + ctrlKey: true, }); const handled = keyEventHandler(event); expect(handled).toBe(false); - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); expect(mockClipboard.readText).toHaveBeenCalled(); - expect(mockPaste).toHaveBeenCalledWith('test clipboard content'); + expect(mockPaste).toHaveBeenCalledWith("test clipboard content"); }); - it('should NOT paste on macOS (Cmd+V should work through existing handlers)', async () => { + it("should NOT paste on macOS (Cmd+V should work through existing handlers)", async () => { const mockPaste = vi.fn(); // Mock macOS platform (navigator) - Object.defineProperty(navigator, 'platform', { - value: 'MacIntel', - writable: true - }); + setNavigatorPlatform("MacIntel"); const { keyEventHandler } = await setupMockXterm({ paste: mockPaste }); await act(async () => { // On Mac, this would be Cmd+V which is metaKey - const event = new KeyboardEvent('keydown', { - key: 'v', + const event = new KeyboardEvent("keydown", { + key: "v", ctrlKey: true, // ctrlKey, not metaKey - metaKey: false + metaKey: false, }); // On Mac, ctrlKey+V should NOT trigger paste (only Cmd+V works) @@ -520,60 +541,52 @@ describe('useXterm keyboard handlers', () => { }); }); - describe('Linux CTRL+SHIFT+C copy shortcut', () => { - it('should copy on Linux when CTRL+SHIFT+C is pressed', async () => { + describe("Linux CTRL+SHIFT+C copy shortcut", () => { + it("should copy on Linux when CTRL+SHIFT+C is pressed", async () => { const mockHasSelection = vi.fn(() => true); - const mockGetSelection = vi.fn(() => 'selected text'); + const mockGetSelection = vi.fn(() => "selected text"); // Mock Linux platform (navigator) - Object.defineProperty(navigator, 'platform', { - value: 'Linux', - writable: true - }); + setNavigatorPlatform("Linux"); const { keyEventHandler } = await setupMockXterm({ hasSelection: mockHasSelection, - getSelection: mockGetSelection + getSelection: mockGetSelection, }); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'C', + const event = new KeyboardEvent("keydown", { + key: "C", ctrlKey: true, - shiftKey: true + shiftKey: true, }); const handled = keyEventHandler(event); expect(handled).toBe(false); - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); - expect(mockClipboard.writeText).toHaveBeenCalledWith('selected text'); + expect(mockClipboard.writeText).toHaveBeenCalledWith("selected text"); }); - it('should not trigger CTRL+SHIFT+C on Windows', async () => { + it("should not trigger CTRL+SHIFT+C on Windows", async () => { // Mock Windows platform (navigator) - Object.defineProperty(navigator, 'platform', { - value: 'Win32', - writable: true - }); + setNavigatorPlatform("Win32"); const { keyEventHandler } = await setupMockXterm({ hasSelection: vi.fn(() => false), - getSelection: vi.fn(() => '') + getSelection: vi.fn(() => ""), }); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'C', + const event = new KeyboardEvent("keydown", { + key: "C", ctrlKey: true, - shiftKey: true + shiftKey: true, }); - if (keyEventHandler) { - keyEventHandler!(event); - } + keyEventHandler(event); }); // Should not copy on Windows @@ -581,97 +594,91 @@ describe('useXterm keyboard handlers', () => { }); }); - describe('Linux CTRL+SHIFT+V paste shortcut', () => { - it('should paste on Linux when CTRL+SHIFT+V is pressed', async () => { + describe("Linux CTRL+SHIFT+V paste shortcut", () => { + it("should paste on Linux when CTRL+SHIFT+V is pressed", async () => { const mockPaste = vi.fn(); // Mock Linux platform (navigator) - Object.defineProperty(navigator, 'platform', { - value: 'Linux', - writable: true - }); + setNavigatorPlatform("Linux"); const { keyEventHandler } = await setupMockXterm({ paste: mockPaste }); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'V', + const event = new KeyboardEvent("keydown", { + key: "V", ctrlKey: true, - shiftKey: true + shiftKey: true, }); const handled = keyEventHandler(event); expect(handled).toBe(false); - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); expect(mockClipboard.readText).toHaveBeenCalled(); - expect(mockPaste).toHaveBeenCalledWith('test clipboard content'); + expect(mockPaste).toHaveBeenCalledWith("test clipboard content"); }); }); - describe('Clipboard error handling', () => { - it('should handle clipboard write errors gracefully', async () => { - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + describe("Clipboard error handling", () => { + it("should handle clipboard write errors gracefully", async () => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); const mockHasSelection = vi.fn(() => true); - const mockGetSelection = vi.fn(() => 'selected text'); + const mockGetSelection = vi.fn(() => "selected text"); // Mock clipboard write failure - mockClipboard.writeText = vi.fn().mockRejectedValue(new Error('Clipboard write failed')); + mockClipboard.writeText = vi.fn().mockRejectedValue(new Error("Clipboard write failed")); const { keyEventHandler } = await setupMockXterm({ hasSelection: mockHasSelection, - getSelection: mockGetSelection + getSelection: mockGetSelection, }); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'c', - ctrlKey: true + const event = new KeyboardEvent("keydown", { + key: "c", + ctrlKey: true, }); keyEventHandler(event); - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); // Should log error but not throw expect(consoleErrorSpy).toHaveBeenCalledWith( - '[useXterm] Failed to copy selection:', + expect.stringContaining("[useXterm] Failed to copy selection:"), expect.any(Error) ); consoleErrorSpy.mockRestore(); }); - it('should handle clipboard read errors gracefully', async () => { - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + it("should handle clipboard read errors gracefully", async () => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); const mockPaste = vi.fn(); // Mock Windows platform to enable custom paste handler - Object.defineProperty(navigator, 'platform', { - value: 'Win32', - writable: true - }); + setNavigatorPlatform("Win32"); // Mock clipboard read failure - mockClipboard.readText = vi.fn().mockRejectedValue(new Error('Clipboard read failed')); + mockClipboard.readText = vi.fn().mockRejectedValue(new Error("Clipboard read failed")); const { keyEventHandler } = await setupMockXterm({ paste: mockPaste }); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'v', - ctrlKey: true + const event = new KeyboardEvent("keydown", { + key: "v", + ctrlKey: true, }); keyEventHandler(event); - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); }); // Should log error but not throw expect(consoleErrorSpy).toHaveBeenCalledWith( - '[useXterm] Failed to read clipboard:', + expect.stringContaining("[useXterm] Failed to read clipboard:"), expect.any(Error) ); @@ -679,77 +686,71 @@ describe('useXterm keyboard handlers', () => { }); }); - describe('Existing shortcuts preservation', () => { - it('should let SHIFT+Enter pass through', async () => { + describe("Existing shortcuts preservation", () => { + it("should let SHIFT+Enter pass through", async () => { const mockInput = vi.fn(); const { keyEventHandler } = await setupMockXterm({ input: mockInput }); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'Enter', + const event = new KeyboardEvent("keydown", { + key: "Enter", shiftKey: true, ctrlKey: false, - metaKey: false + metaKey: false, }); - if (keyEventHandler) { - keyEventHandler!(event); - } + keyEventHandler(event); }); // Should send ESC+newline for multi-line input - expect(mockInput).toHaveBeenCalledWith('\x1b\n'); + expect(mockInput).toHaveBeenCalledWith("\x1b\n"); }); - it('should let Ctrl+Backspace pass through', async () => { + it("should let Ctrl+Backspace pass through", async () => { const mockInput = vi.fn(); const { keyEventHandler } = await setupMockXterm({ input: mockInput }); await act(async () => { - const event = new KeyboardEvent('keydown', { - key: 'Backspace', + const event = new KeyboardEvent("keydown", { + key: "Backspace", ctrlKey: true, - metaKey: false + metaKey: false, }); - if (keyEventHandler) { - keyEventHandler!(event); - } + keyEventHandler(event); }); // Should send Ctrl+U for delete line - expect(mockInput).toHaveBeenCalledWith('\x15'); + expect(mockInput).toHaveBeenCalledWith("\x15"); }); - it('should let Ctrl+1-9 pass through for project tab switching', async () => { + it("should let Ctrl+1-9 pass through for project tab switching", async () => { const { keyEventHandler } = await setupMockXterm(); // Test all number keys 1-9 for (let i = 1; i <= 9; i++) { act(() => { - const event = new KeyboardEvent('keydown', { + const event = new KeyboardEvent("keydown", { key: i.toString(), - ctrlKey: true + ctrlKey: true, }); - if (keyEventHandler) { - const handled = keyEventHandler!(event); - expect(handled).toBe(false); // Should bubble to window handler - } + const handled = keyEventHandler(event); + expect(handled).toBe(false); // Should bubble to window handler }); } }); - it('should let Ctrl+T and Ctrl+W pass through', async () => { + it("should let Ctrl+T and Ctrl+W pass through", async () => { const { keyEventHandler } = await setupMockXterm(); // Test Ctrl+T act(() => { - const event = new KeyboardEvent('keydown', { - key: 't', - ctrlKey: true + const event = new KeyboardEvent("keydown", { + key: "t", + ctrlKey: true, }); const handled = keyEventHandler(event); @@ -758,9 +759,9 @@ describe('useXterm keyboard handlers', () => { // Test Ctrl+W act(() => { - const event = new KeyboardEvent('keydown', { - key: 'w', - ctrlKey: true + const event = new KeyboardEvent("keydown", { + key: "w", + ctrlKey: true, }); const handled = keyEventHandler(event); @@ -769,18 +770,18 @@ describe('useXterm keyboard handlers', () => { }); }); - describe('Event type checking', () => { - it('should only handle keydown events, not keyup', async () => { + describe("Event type checking", () => { + it("should only handle keydown events, not keyup", async () => { const { keyEventHandler } = await setupMockXterm({ hasSelection: vi.fn(() => true), - getSelection: vi.fn(() => 'selected text') + getSelection: vi.fn(() => "selected text"), }); act(() => { // Test keyup event (should be ignored) - const keyupEvent = new KeyboardEvent('keyup', { - key: 'c', - ctrlKey: true + const keyupEvent = new KeyboardEvent("keyup", { + key: "c", + ctrlKey: true, }); keyEventHandler(keyupEvent); diff --git a/apps/frontend/src/shared/utils/shell-escape.ts b/apps/frontend/src/shared/utils/shell-escape.ts index 8bdd3b0dc6..c3f9a04d52 100644 --- a/apps/frontend/src/shared/utils/shell-escape.ts +++ b/apps/frontend/src/shared/utils/shell-escape.ts @@ -3,8 +3,17 @@ * * Provides safe escaping for shell command arguments to prevent command injection. * IMPORTANT: Always use these utilities when interpolating user-controlled values into shell commands. + * + * Supports multiple shell types: + * - POSIX shells (bash, zsh, fish): Single-quote escaping + * - Windows cmd.exe: Caret escaping with double quotes + * - PowerShell: Special escaping rules */ +// Re-export ShellType for convenience +import type { ShellType as ShellType_ } from "../../main/terminal/types"; +export type ShellType = ShellType_; + /** * Escape a string for safe use as a shell argument. * @@ -47,11 +56,11 @@ export function escapeShellPath(path: string): string { */ export function buildCdCommand(path: string | undefined): string { if (!path) { - return ''; + return ""; } // Windows cmd.exe uses double quotes, Unix shells use single quotes - if (process.platform === 'win32') { + if (process.platform === "win32") { // On Windows, escape cmd.exe metacharacters (& | < > ^) that could enable command injection, // then wrap in double quotes. Using escapeShellArgWindows for proper escaping. const escaped = escapeShellArgWindows(path); @@ -77,13 +86,13 @@ export function escapeShellArgWindows(arg: string): string { // " & | < > ^ need to be escaped // % is used for variable expansion const escaped = arg - .replace(/\^/g, '^^') // Escape carets first (escape char itself) - .replace(/"/g, '^"') // Escape double quotes - .replace(/&/g, '^&') // Escape ampersand (command separator) - .replace(/\|/g, '^|') // Escape pipe - .replace(//g, '^>') // Escape greater than - .replace(/%/g, '%%'); // Escape percent (variable expansion) + .replace(/\^/g, "^^") // Escape carets first (escape char itself) + .replace(/"/g, '^"') // Escape double quotes + .replace(/&/g, "^&") // Escape ampersand (command separator) + .replace(/\|/g, "^|") // Escape pipe + .replace(//g, "^>") // Escape greater than + .replace(/%/g, "%%"); // Escape percent (variable expansion) return escaped; } @@ -101,16 +110,359 @@ export function isPathSafe(path: string): boolean { // Note: This is defense-in-depth; escaping handles these, but we can log/reject const suspiciousPatterns = [ /\$\(/, // Command substitution $(...) - /`/, // Backtick command substitution - /\|/, // Pipe - /;/, // Command separator - /&&/, // AND operator + /`/, // Backtick command substitution + /\|/, // Pipe + /;/, // Command separator + /&&/, // AND operator /\|\|/, // OR operator - />/, // Output redirection - //, // Output redirection + / pattern.test(path)); + return !suspiciousPatterns.some((pattern) => pattern.test(path)); +} + +// ============================================================================ +// POWERSHELL-SPECIFIC UTILITIES +// ============================================================================ + +/** + * Escape a string for safe use as a PowerShell command argument. + * + * PowerShell uses different escaping rules than bash and cmd.exe: + * - Double quotes are used for strings with variable expansion + * - Single quotes are for literal strings (no expansion) + * - Backticks are the escape character inside double quotes + * - Special characters like `$` and ``` ` ``` need escaping inside double quotes + * + * For maximum safety and to avoid variable expansion, we use single-quoted strings. + * + * @param arg - The argument to escape + * @returns The escaped argument wrapped in single quotes for PowerShell + * + * @example + * ```typescript + * escapePowerShellArg("hello"); // 'hello' + * escapePowerShellArg("hello world"); // 'hello world' + * escapePowerShellArg("it's"); // 'it''s' + * escapePowerShellArg("C:\\Program Files"); // 'C:\\Program Files' + * ``` + */ +export function escapePowerShellArg(arg: string): string { + // In PowerShell, single quotes are literal strings - no variable expansion + // To include a single quote in a single-quoted string, double it: '' + const escaped = arg.replace(/'/g, "''"); + return `'${escaped}'`; +} + +/** + * Normalize PATH separators for the specified shell type. + * + * Converts between Windows semicolon separators (;) and Unix colon separators (:) + * based on the target shell type. This is needed because Windows may provide + * PATH values with semicolons, but bash-style shells (even on Windows via WSL + * or Git Bash) expect colons. + * + * @param pathValue - The PATH value to normalize + * @param shellType - The shell type (powershell, cmd, bash, etc.) + * @returns The PATH with normalized separators + * + * @example + * ```typescript + * normalizePathSeparators('C:\\Git\\cmd;C:\\npm', 'bash'); + * // Returns: `C:\\Git\\cmd:C:\\npm` + * + * normalizePathSeparators('/usr/bin:/usr/local/bin', 'powershell'); + * // Returns: `/usr/bin;/usr/local/bin` + * ``` + */ +export function normalizePathSeparators(pathValue: string, shellType: ShellType): string { + // Windows shells use semicolon, Unix shells use colon + const targetSeparator = shellType === "powershell" || shellType === "cmd" ? ";" : ":"; + const otherSeparator = targetSeparator === ";" ? ":" : ";"; + + // Replace other separators with target separators + return pathValue.split(otherSeparator).join(targetSeparator); +} + +/** + * Build a PATH environment variable assignment for the specified shell type. + * + * Different shells use different syntax: + * - PowerShell: `$env:PATH = 'value'` + * - cmd.exe: `PATH=value` (or `set "PATH=value"`) + * - bash/zsh/fish: `PATH='value'` or `export PATH='value'` + * + * This function also normalizes PATH separators for cross-shell compatibility. + * For example, on Windows, a PATH with semicolons will be converted to colons + * for bash-style shells. + * + * @param pathValue - The PATH value to set + * @param shellType - The shell type (powershell, cmd, bash, etc.) + * @returns The appropriate PATH assignment command for the shell + * + * @example + * ```typescript + * buildPathAssignment('C:\\Git\\cmd;C:\\npm', 'powershell'); + * // Returns: `$env:PATH = 'C:\\Git\\cmd;C:\\npm'` + * + * buildPathAssignment('C:\\Git\\cmd;C:\\npm', 'bash'); + * // Returns: `PATH='C:\\Git\\cmd:C:\\npm'` + * ``` + */ +export function buildPathAssignment(pathValue: string, shellType: ShellType): string { + // Normalize PATH separators for the target shell type + const normalizedPath = normalizePathSeparators(pathValue, shellType); + + switch (shellType) { + case "powershell": + return `$env:PATH = ${escapePowerShellArg(normalizedPath)}`; + + case "cmd": { + // cmd.exe uses: PATH=value (no quotes needed for value, but we escape special chars) + // For safety with special characters, we use set "VAR=value" + const escapedCmd = escapeShellArgWindows(normalizedPath); + return `set "PATH=${escapedCmd}"`; + } + + case "bash": + case "zsh": + case "fish": + return `PATH=${escapeShellArg(normalizedPath)}`; + + default: + // Fallback to bash-style syntax + return `PATH=${escapeShellArg(normalizedPath)}`; + } +} + +/** + * Build an environment variable assignment for the specified shell type. + * + * Different shells use different syntax: + * - PowerShell: `$env:VARNAME = 'value'` + * - cmd.exe: `set "VARNAME=value"` (variables accessed as %VARNAME%) + * - bash/zsh/fish: `VARNAME='value'` or `export VARNAME='value'` (accessed as $VARNAME) + * + * @param varName - The environment variable name + * @param value - The value to assign + * @param shellType - The shell type (powershell, cmd, bash, etc.) + * @returns The appropriate environment variable assignment for the shell + * + * @example + * ```typescript + * buildEnvVarAssignment('CLAUDE_CONFIG_DIR', 'C:\\Users\\Jane\\Config', 'powershell'); + * // Returns: `$env:CLAUDE_CONFIG_DIR = 'C:\\Users\\Jane\\Config'` + * + * buildEnvVarAssignment('CLAUDE_CONFIG_DIR', '/home/jane/config', 'bash'); + * // Returns: `CLAUDE_CONFIG_DIR='/home/jane/config'` + * ``` + */ +export function buildEnvVarAssignment( + varName: string, + value: string, + shellType: ShellType +): string { + switch (shellType) { + case "powershell": + return `$env:${varName} = ${escapePowerShellArg(value)}`; + + case "cmd": { + const escapedCmd = escapeShellArgWindows(value); + return `set "${varName}=${escapedCmd}"`; + } + + case "bash": + case "zsh": + case "fish": + return `${varName}=${escapeShellArg(value)}`; + + default: + // Fallback to bash-style syntax + return `${varName}=${escapeShellArg(value)}`; + } +} + +/** + * Build an environment variable reference for the specified shell type. + * + * Different shells use different syntax to reference environment variables: + * - PowerShell: `$env:VARNAME` + * - cmd.exe: `%VARNAME%` + * - bash/zsh/fish: `$VARNAME` + * + * @param varName - The environment variable name + * @param shellType - The shell type (powershell, cmd, bash, etc.) + * @returns The appropriate environment variable reference for the shell + * + * @example + * ```typescript + * buildEnvVarReference('CLAUDE_CONFIG_DIR', 'powershell'); + * // Returns: `$env:CLAUDE_CONFIG_DIR` + * + * buildEnvVarReference('CLAUDE_CONFIG_DIR', 'cmd'); + * // Returns: `%CLAUDE_CONFIG_DIR%` + * + * buildEnvVarReference('CLAUDE_CONFIG_DIR', 'bash'); + * // Returns: `$CLAUDE_CONFIG_DIR` + * ``` + */ +export function buildEnvVarReference(varName: string, shellType: ShellType): string { + switch (shellType) { + case "powershell": + return `$env:${varName}`; + case "cmd": + return `%${varName}%`; + case "bash": + case "zsh": + case "fish": + return `$${varName}`; + default: + // Fallback to bash-style syntax + return `$${varName}`; + } +} + +/** + * Build a command invocation for the specified shell type. + * + * On Windows, .cmd and .bat files require special handling: + * - PowerShell: Must use call operator `&` before quoted paths with spaces + * - cmd.exe: Can execute directly, quotes are fine + * - bash: Can execute directly, quotes are fine + * + * @param commandPath - The path to the command/executable + * @param args - Command arguments (optional) + * @param shellType - The shell type (powershell, cmd, bash, etc.) + * @returns The properly formatted command invocation for the shell + * + * @example + * ```typescript + * buildCommandInvocation('C:\\Users\\Jane Smith\\claude.cmd', ['setup-token'], 'powershell'); + * // Returns: `& 'C:\\Users\\Jane Smith\\claude.cmd' setup-token` + * + * buildCommandInvocation('C:\\Users\\Jane Smith\\claude.cmd', ['setup-token'], 'cmd'); + * // Returns: `"C:\\Users\\Jane Smith\\claude.cmd" setup-token` + * + * buildCommandInvocation('/usr/local/bin/claude', ['--version'], 'bash'); + * // Returns: `'/usr/local/bin/claude' --version` + * ``` + */ +export function buildCommandInvocation( + commandPath: string, + args: string[] = [], + shellType: ShellType +): string { + const hasArgs = args.length > 0; + const argsStr = hasArgs ? " " + args.join(" ") : ""; + + // Check if this is a Windows batch file that needs special handling + const isWindowsBatchFile = + process.platform === "win32" && + (commandPath.toLowerCase().endsWith(".cmd") || commandPath.toLowerCase().endsWith(".bat")); + + switch (shellType) { + case "powershell": + if (isWindowsBatchFile) { + // PowerShell requires & operator to execute .cmd/.bat files, especially with spaces + // The path must be quoted if it contains spaces + return `& ${escapePowerShellArg(commandPath)}${argsStr}`; + } + // For other executables (.exe, etc.), no call operator needed + return `${escapePowerShellArg(commandPath)}${argsStr}`; + + case "cmd": { + // cmd.exe doesn't need a call operator, just quote paths with spaces + const escapedCmd = escapeShellArgWindows(commandPath); + return `"${escapedCmd}"${argsStr}`; + } + + case "bash": + case "zsh": + case "fish": + return `${escapeShellArg(commandPath)}${argsStr}`; + + default: + // Fallback to bash-style syntax + return `${escapeShellArg(commandPath)}${argsStr}`; + } +} + +/** + * Build a cd command for the specified shell type. + * + * Different shells have different syntax: + * - PowerShell: `Set-Location` or `cd` (works the same) + * - cmd.exe: `cd /d "path"` (/d to also change drive) + * - bash/zsh/fish: `cd 'path'` + * + * @param path - The directory path + * @param shellType - The shell type (powershell, cmd, bash, etc.) + * @returns The appropriate cd command for the shell, or empty string if path is undefined + * + * @example + * ```typescript + * buildCdCommandForShell('C:\\Users\\Jane\\Projects', 'powershell'); + * // Returns: `cd 'C:\\Users\\Jane\\Projects' && ` + * + * buildCdCommandForShell('C:\\Users\\Jane\\Projects', 'cmd'); + * // Returns: `cd /d "C:\\Users\\Jane\\Projects" && ` + * + * buildCdCommandForShell('/home/jane/projects', 'bash'); + * // Returns: `cd '/home/jane/projects' && ` + * ``` + */ +export function buildCdCommandForShell(path: string | undefined, shellType: ShellType): string { + if (!path) { + return ""; + } + + switch (shellType) { + case "powershell": + return `cd ${escapePowerShellArg(path)} && `; + + case "cmd": { + // /d flag also changes drive letter (e.g., from C: to D:) + const escapedCmd = escapeShellArgWindows(path); + return `cd /d "${escapedCmd}" && `; + } + + case "bash": + case "zsh": + case "fish": + return `cd ${escapeShellArg(path)} && `; + + default: + // Fallback to bash-style syntax + return `cd ${escapeShellArg(path)} && `; + } +} + +/** + * Normalize PATH entries for the specified shell type. + * + * PATH separators differ between platforms and shells: + * - Windows (cmd.exe, PowerShell): Use semicolon (;) + * - Unix (bash, zsh, fish): Use colon (:) + * + * @param paths - Array of path entries to join + * @param shellType - The shell type (powershell, cmd, bash, etc.) + * @returns The properly joined PATH string + * + * @example + * ```typescript + * buildPathString(['C:\\Git\\cmd', 'C:\\npm'], 'powershell'); + * // Returns: `C:\\Git\\cmd;C:\\npm` + * + * buildPathString(['/usr/bin', '/usr/local/bin'], 'bash'); + * // Returns: `/usr/bin:/usr/local/bin` + * ``` + */ +export function buildPathString(paths: string[], shellType: ShellType): string { + // Windows shells use semicolon, Unix shells use colon + const separator = shellType === "powershell" || shellType === "cmd" ? ";" : ":"; + + return paths.filter(Boolean).join(separator); }