Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 21 additions & 17 deletions apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
*/

/**
* Integration tests for terminal copy/paste functionality
* Tests xterm.js selection API integration with clipboard operations
* 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';
Expand Down Expand Up @@ -66,8 +66,18 @@ describe('Terminal copy/paste integration', () => {
};

beforeEach(() => {
vi.useFakeTimers();
vi.clearAllMocks();

// Mock requestAnimationFrame to return an ID but not schedule real timers
// Using setTimeout causes infinite loops with vi.runAllTimersAsync() because
// requestAnimationFrame callbacks (like performInitialFit) schedule more timers
let rafId = 0;
global.requestAnimationFrame = vi.fn(() => {
return ++rafId;
});
global.cancelAnimationFrame = vi.fn();

// Mock ResizeObserver
global.ResizeObserver = vi.fn().mockImplementation(function() {
return {
Expand All @@ -77,14 +87,6 @@ describe('Terminal copy/paste integration', () => {
};
});

// Mock requestAnimationFrame for xterm.js integration tests
global.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => {
// Synchronously execute the callback to avoid timing issues in tests
// Use globalThis instead of window for cross-platform compatibility
callback.call(globalThis, 0);
return 0;
}) as unknown as Mock;

// Mock navigator.clipboard
mockClipboard = {
writeText: vi.fn().mockResolvedValue(undefined),
Expand All @@ -103,6 +105,8 @@ describe('Terminal copy/paste integration', () => {
});

afterEach(() => {
vi.runOnlyPendingTimers();
vi.useRealTimers();
vi.restoreAllMocks();
});

Expand Down Expand Up @@ -168,8 +172,8 @@ describe('Terminal copy/paste integration', () => {

if (keyEventHandler) {
keyEventHandler(event);
// Wait for clipboard write
await new Promise(resolve => setTimeout(resolve, 0));
// Advance timers to allow clipboard write to complete
await vi.runAllTimersAsync();
}
});

Expand Down Expand Up @@ -341,7 +345,7 @@ describe('Terminal copy/paste integration', () => {
if (keyEventHandler) {
keyEventHandler(event);
// Wait for clipboard read and paste
await new Promise(resolve => setTimeout(resolve, 0));
await vi.runAllTimersAsync();
}
});

Expand Down Expand Up @@ -421,7 +425,7 @@ describe('Terminal copy/paste integration', () => {
if (keyEventHandler) {
keyEventHandler(event);
// Wait for clipboard read
await new Promise(resolve => setTimeout(resolve, 0));
await vi.runAllTimersAsync();
}
});

Expand Down Expand Up @@ -513,7 +517,7 @@ describe('Terminal copy/paste integration', () => {
if (keyEventHandler) {
keyEventHandler(copyEvent);
// Wait for clipboard write
await new Promise(resolve => setTimeout(resolve, 0));
await vi.runAllTimersAsync();
}

// Copy should not send input to terminal
Expand All @@ -528,7 +532,7 @@ describe('Terminal copy/paste integration', () => {
if (keyEventHandler) {
keyEventHandler(pasteEvent);
// Wait for clipboard read
await new Promise(resolve => setTimeout(resolve, 0));
await vi.runAllTimersAsync();
}

// Paste should use xterm.paste(), not xterm.input()
Expand Down Expand Up @@ -713,7 +717,7 @@ describe('Terminal copy/paste integration', () => {
if (keyEventHandler) {
keyEventHandler(pasteEvent);
// Wait for clipboard error
await new Promise(resolve => setTimeout(resolve, 0));
await vi.runAllTimersAsync();
}
});

Expand Down
12 changes: 10 additions & 2 deletions apps/frontend/src/main/cli-tool-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,11 @@ class CLIToolManager {
}
const cmdExe = process.env.ComSpec
|| path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'cmd.exe');
const cmdLine = `""${unquotedCmd}" --version"`;
// Use chcp 65001 to set UTF-8 code page, then run the command with proper quoting.
// This prevents garbled error messages when paths contain special characters.
// Note: We use simple double-quote wrapping since the command doesn't start with
// a quote (chcp prefix), so cmd.exe /s won't strip outer quotes.
const cmdLine = `chcp 65001 >nul && "${unquotedCmd}" --version`;
const execOptions: ExecFileSyncOptionsWithVerbatim = {
encoding: 'utf-8',
timeout: 5000,
Expand Down Expand Up @@ -1096,7 +1100,11 @@ class CLIToolManager {
}
const cmdExe = process.env.ComSpec
|| path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'cmd.exe');
const cmdLine = `""${unquotedCmd}" --version"`;
// Use chcp 65001 to set UTF-8 code page, then run the command with proper quoting.
// This prevents garbled error messages when paths contain special characters.
// Note: We use simple double-quote wrapping since the command doesn't start with
// a quote (chcp prefix), so cmd.exe /s won't strip outer quotes.
const cmdLine = `chcp 65001 >nul && "${unquotedCmd}" --version`;
const execOptions: ExecFileAsyncOptionsWithVerbatim = {
encoding: 'utf-8',
timeout: 5000,
Expand Down
15 changes: 12 additions & 3 deletions apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@ async function validateClaudeCliAsync(cliPath: string): Promise<[boolean, string
// Get cmd.exe path from environment or use default
const cmdExe = process.env.ComSpec
|| path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'cmd.exe');
// Use double-quoted command line for paths with spaces
const cmdLine = `""${cliPath}" --version"`;
// Use chcp 65001 to set UTF-8 code page for proper error message encoding.
// Note: Using simple quoting because /s flag only strips outer quotes when
// command STARTS with a quote - since we prepend 'chcp', use standard quoting.
const cmdLine = `chcp 65001 >nul && "${cliPath}" --version`;
const execOptions: ExecFileAsyncOptionsWithVerbatim = {
encoding: 'utf-8',
timeout: 5000,
Expand Down Expand Up @@ -147,7 +149,14 @@ async function scanClaudeInstallations(activePath: string | null): Promise<Claud
const result = await execFileAsync('where', ['claude'], { timeout: 5000 });
const paths = result.stdout.trim().split('\n').filter(p => p.trim());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For better robustness, it's recommended to split the output of the where command using a regular expression that handles both Windows (\r\n) and Unix-style (\n) line endings. While the current implementation with split('\n') and trim() in the loop might work in most cases, using split(/\r?\n/) is safer and more explicit. This also makes the code consistent with how where output is parsed in apps/frontend/src/main/utils/windows-paths.ts.

Suggested change
const paths = result.stdout.trim().split('\n').filter(p => p.trim());
const paths = result.stdout.trim().split(/\r?\n/).filter(p => p.trim());

for (const p of paths) {
await addInstallation(p.trim(), 'system-path');
const trimmed = p.trim();
// On Windows, skip extensionless paths - they're not directly executable.
// Only consider .cmd, .exe, .bat files which are the actual executables.
if (!/\.(cmd|exe|bat)$/i.test(trimmed)) {
console.log('[Claude Code] Skipping non-executable path:', trimmed);
continue;
}
await addInstallation(trimmed, 'system-path');
}
} else {
const result = await execFileAsync('which', ['-a', 'claude'], { timeout: 5000 });
Expand Down
18 changes: 10 additions & 8 deletions apps/frontend/src/main/utils/windows-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,13 @@ export function findWindowsExecutableViaWhere(
}).trim();

// 'where' returns multiple paths separated by newlines if found in multiple locations
// Prefer paths with .cmd or .exe extensions (executable files)
// Only consider paths with .cmd, .bat, or .exe extensions - extensionless paths
// on Windows are not directly executable and cause ENOENT errors.
const paths = result.split(/\r?\n/).filter(p => p.trim());
const executablePaths = paths.filter(p => /\.(cmd|bat|exe)$/i.test(p));

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();
if (executablePaths.length > 0) {
const foundPath = executablePaths[0].trim();

// Validate the path exists and is secure
if (existsSync(foundPath) && isSecurePath(foundPath)) {
Expand Down Expand Up @@ -259,12 +260,13 @@ export async function findWindowsExecutableViaWhereAsync(
});

// 'where' returns multiple paths separated by newlines if found in multiple locations
// Prefer paths with .cmd, .bat, or .exe extensions (executable files)
// Only consider paths with .cmd, .bat, or .exe extensions - extensionless paths
// on Windows are not directly executable and cause ENOENT errors.
const paths = stdout.trim().split(/\r?\n/).filter(p => p.trim());
const executablePaths = paths.filter(p => /\.(cmd|bat|exe)$/i.test(p));

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();
if (executablePaths.length > 0) {
const foundPath = executablePaths[0].trim();

// Validate the path exists and is secure
try {
Expand Down