Skip to content
Open
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
// Just pass timestamp directly - this context isn't used by RAF callbacks
callback(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`;

This comment was marked as outdated.

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());
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');
Comment on lines +152 to +159
Copy link
Contributor

Choose a reason for hiding this comment

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

high

This is a great fix for handling extensionless paths from the where command on Windows. I've noticed that similar logic for finding the Claude CLI exists in apps/frontend/src/main/cli-tool-manager.ts within the detectClaude function, which in turn uses findWindowsExecutableViaWhere from apps/frontend/src/main/utils/windows-paths.ts.

The findWindowsExecutableViaWhere function appears to have a related issue where it might fall back to an extensionless path if no executable with a .cmd, .exe, or .bat extension is found first in the list from where. To ensure consistency and fix this potential bug across the application, consider making findWindowsExecutableViaWhere stricter to only return paths with valid executable extensions, similar to the logic you've added here.

}
} 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
Loading