Skip to content

Commit 7210610

Browse files
AndyMik90claude
andauthored
Fix/windows issues (#471)
* fix(windows): Claude CLI detection failing on Windows On Windows, npm installs CLI tools as .cmd batch wrappers alongside bash scripts for Git Bash/Cygwin. Two issues prevented detection: 1. findExecutable() checked for extensionless files first, finding the unusable bash script before the .cmd wrapper 2. execFileSync() cannot execute .cmd files without shell: true Changes: - Reorder extension search to prioritize .exe/.cmd over extensionless - Add shell: true when validating .cmd/.bat files on Windows Both changes are Windows-specific and don't affect macOS behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(windows): terminal shortcuts and invoke Claude not working - Fix Ctrl+T/W shortcuts not working inside terminals on Windows xterm.js was capturing Ctrl+T as ^T character instead of letting it bubble up to window handler. Added T and W to custom key handler bypass list in useXterm.ts - Fix "Invoke Claude" button failing on Windows with path error buildCdCommand() was using single quotes which cmd.exe doesn't recognize. Now uses platform-appropriate quoting (double quotes on Windows, single quotes on Unix) - Improve terminal auto-naming to skip common commands Expanded skip list to include claude, git, npm, node, python, and other shell/dev commands that don't represent meaningful work. Terminal naming should come from actual task descriptions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(windows): reduce installer size by 75% (~300MB savings) Strip unnecessary files from Python site-packages during bundling: - Remove googleapiclient/discovery_cache/documents (92MB cached API docs) - Remove claude_agent_sdk/_bundled (224MB bundled Claude CLI) - Remove pythonwin directory (9MB Windows IDE) - Remove .chm help files (2.6MB) The Claude Agent SDK will fall back to the system-installed Claude Code CLI, which is already a prerequisite for Auto-Claude (required for 'claude setup-token'). Site-packages reduced from 446MB to 111MB (75% reduction). Expected installer size: ~100MB instead of ~206MB. * fix(frontend): sync project tabs with settings by removing project on tab close Closing a project tab now removes the project from the app entirely, keeping tabs and settings dropdown in sync. Files remain on disk so users can re-add projects later. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(settings): remove redundant Claude Auth project settings tab Claude authentication is already available in the app-level Integrations section, making this project-level tab redundant. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(onboarding): add one-click Ollama installation for Windows/macOS/Linux When users select Ollama as their embedding provider during onboarding but don't have it installed, they now see an "Install Ollama" button that opens their preferred terminal with the official install command. Changes: - Add checkOllamaInstalled() to detect if Ollama binary exists on system - Add installOllama() to open terminal with platform-specific install: - Windows: winget install --id Ollama.Ollama - macOS/Linux: curl -fsSL https://ollama.com/install.sh | sh - Update OllamaModelSelector to show install UI when Ollama not found - Fix Windows terminal handling for commands with pipes (PowerShell) - Use fire-and-forget pattern so UI doesn't hang waiting for terminal - Add i18n translations (English & French) for install UI The install uses the user's preferred terminal from the DevTools onboarding step, respecting their configuration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(ipc-handlers): enhance task status persistence in implementation plan - Added functionality to persist task status updates to the implementation plan file, preventing inconsistencies between UI and file state during task refresh. - Implemented error handling for file read/write operations to ensure robustness in status updates. - Updated task execution handlers to reflect changes in task status, including transitions to 'human_review' and 'backlog'. - Introduced critical comments to clarify the importance of status persistence in maintaining accurate task states. This update improves the reliability of task status management across the application. * fix(security): address PR review findings for Windows issues - Fix command injection vulnerability in buildCdCommand by using escapeShellArgWindows() to properly escape cmd.exe metacharacters - Add confirmation dialog before removing project on tab close - Add documentation explaining why skipCommands list exists - Add security comment for shell: true usage in Claude CLI validation - Remove unused EnvironmentSettings component (dead code cleanup) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(security): address follow-up PR review findings - Fix command injection in Windows terminal by escaping PowerShell metacharacters (backticks, double quotes, dollar signs) - Fix Git Bash to use passed command parameter instead of hardcoded value - Add shared plan-file-utils with mutex locking for thread-safe updates - Refactor agent-events-handlers and execution-handlers to use shared persistence utility, eliminating code duplication 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(security): strengthen shell escaping and document sync limitations - Add escaping for parentheses, semicolons, ampersands in PowerShell - Add escaping for semicolons, pipes, exclamation marks in Git Bash - Add comprehensive documentation warning about persistPlanStatusSync bypassing the async locking mechanism Note: The CRITICAL finding about EnvironmentSettings in SectionRouter.tsx is a false positive - grep confirms no such import exists in the file. Line 5 imports SecuritySettings, not EnvironmentSettings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address code scanning and TypeScript errors - Fix TypeScript error in plan-file-utils.ts (generic type assertion) - Add security comment for ollamaPath explaining hardcoded paths - Remove useless isLoading conditional in OllamaModelSelector.tsx Note: The EnvironmentSettings import finding remains a false positive - grep confirms no such import exists in SectionRouter.tsx. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(ui): restore Retry button disabled state for defensive programming Restored disabled={isLoading} and animate-spin on Retry buttons. FALSE POSITIVES in review (verified via grep/sed): - CRITICAL "EnvironmentSettings import at line 5": Line 5 is actually `import { SecuritySettings }` - no EnvironmentSettings import exists - LOW "Dead code escape functions": Functions ARE used at lines 268, 275 in openTerminalWithCommand for PowerShell and Git Bash escaping The review tool appears to be using cached/stale file data. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(security): address CodeQL security alerts - Fix 6 HIGH TOCTOU race conditions by removing existsSync checks and using try/catch with ENOENT detection instead - Fix MEDIUM command injection by using execFileSync instead of execSync for Ollama path detection (avoids shell interpretation) - Fix unused imports in execution-handlers.ts - Remove useless isLoading conditional and add explanatory comment about React batching behavior preventing double-clicks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(build): remove TOCTOU race condition in download-python script Replace existsSync check with try/catch pattern to avoid race condition between existence check and file operations. Handle ENOENT as no-op. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address PR review issues for error handling and i18n - Add error handling with toast notification for project removal in App.tsx - Replace hardcoded strings with i18n translation keys in ProjectSettingsContent.tsx - Add translation keys for projectSettings.noProjectSelected (en/fr) - Add removeProject.error translation key to dialogs.json (en/fr) - Add errors.unknownError translation key to common.json (en/fr) - Refactor terminal command escaping in claude-code-handlers.ts - Remove unused imports in execution-handlers.ts - Add explanatory comment for React batching in OllamaModelSelector.tsx 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(app): replace toast with inline error display in remove project dialog Toast function doesn't exist in this codebase. Use inline error display with AlertCircle icon to show removal errors in the dialog itself. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 52a4fcc commit 7210610

30 files changed

Lines changed: 1138 additions & 418 deletions

apps/frontend/scripts/download-python.cjs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ const STRIP_PATTERNS = {
4646
'.pytest_cache',
4747
'.mypy_cache',
4848
'__pypackages__',
49+
// Windows-specific bloat
50+
'pythonwin', // PyWin32 IDE - not needed (9MB)
4951
],
5052
// File extensions to remove
5153
extensions: [
@@ -68,6 +70,7 @@ const STRIP_PATTERNS = {
6870
'.gitignore',
6971
'.gitattributes',
7072
'.editorconfig',
73+
'.chm', // Windows help files - not needed
7174
],
7275
// Specific files to remove
7376
files: [
@@ -97,6 +100,12 @@ const STRIP_PATTERNS = {
97100
'conftest.py',
98101
'pytest.ini',
99102
],
103+
// Specific paths within packages to remove (relative to package directory)
104+
// Format: 'package_name/subpath' - removes the entire subpath
105+
packagePaths: [
106+
'googleapiclient/discovery_cache/documents', // Cached Google API discovery docs (92MB!)
107+
'claude_agent_sdk/_bundled', // Bundled Claude CLI (224MB!) - users have it installed separately
108+
],
100109
// Packages that should NEVER be bundled (too large, specialized)
101110
// If these appear in dependencies, warn and skip
102111
blockedPackages: [
@@ -455,6 +464,32 @@ function stripSitePackages(sitePackagesDir) {
455464
const sizeBefore = getDirectorySize(sitePackagesDir);
456465
let removedCount = 0;
457466

467+
// First, remove specific package paths (e.g., googleapiclient/discovery_cache/documents)
468+
// Use try/catch instead of existsSync to avoid TOCTOU race conditions
469+
if (STRIP_PATTERNS.packagePaths) {
470+
for (const pkgPath of STRIP_PATTERNS.packagePaths) {
471+
const fullPath = path.join(sitePackagesDir, pkgPath);
472+
try {
473+
// Get size first (may throw ENOENT if path doesn't exist)
474+
let pathSize = 0;
475+
try {
476+
pathSize = getDirectorySize(fullPath);
477+
} catch {
478+
// Path doesn't exist or can't get size - skip
479+
continue;
480+
}
481+
fs.rmSync(fullPath, { recursive: true, force: true });
482+
console.log(`[download-python] Removed ${pkgPath} (${formatBytes(pathSize)})`);
483+
removedCount++;
484+
} catch (err) {
485+
// ENOENT means file was already gone - not an error
486+
if (err.code !== 'ENOENT') {
487+
console.warn(`[download-python] Failed to remove ${pkgPath}: ${err.message}`);
488+
}
489+
}
490+
}
491+
}
492+
458493
function shouldRemoveDir(name) {
459494
return STRIP_PATTERNS.dirs.includes(name.toLowerCase());
460495
}

apps/frontend/src/main/cli-tool-manager.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -674,10 +674,19 @@ class CLIToolManager {
674674
*/
675675
private validateClaude(claudeCmd: string): ToolValidation {
676676
try {
677+
// On Windows, .cmd files need shell: true to execute properly.
678+
// SECURITY NOTE: shell: true is safe here because:
679+
// 1. claudeCmd comes from internal path detection (user config or known system paths)
680+
// 2. Only '--version' is passed as an argument (no user input)
681+
// If claudeCmd origin ever changes to accept user input, use escapeShellArgWindows.
682+
const needsShell = process.platform === 'win32' &&
683+
(claudeCmd.endsWith('.cmd') || claudeCmd.endsWith('.bat'));
684+
677685
const version = execFileSync(claudeCmd, ['--version'], {
678686
encoding: 'utf-8',
679687
timeout: 5000,
680688
windowsHide: true,
689+
shell: needsShell,
681690
}).trim();
682691

683692
// Claude CLI version output format: "claude-code version X.Y.Z" or similar

apps/frontend/src/main/env-utils.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,10 @@ export function findExecutable(command: string): string | null {
157157
const pathSeparator = process.platform === 'win32' ? ';' : ':';
158158
const pathDirs = (env.PATH || '').split(pathSeparator);
159159

160-
// On Windows, also check with common extensions
160+
// On Windows, check Windows-native extensions first (.exe, .cmd) before
161+
// extensionless files (which are typically bash/sh scripts for Git Bash/Cygwin)
161162
const extensions = process.platform === 'win32'
162-
? ['', '.exe', '.cmd', '.bat', '.ps1']
163+
? ['.exe', '.cmd', '.bat', '.ps1', '']
163164
: [''];
164165

165166
for (const dir of pathDirs) {

apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import type { BrowserWindow } from 'electron';
22
import path from 'path';
3-
import { existsSync, readFileSync, writeFileSync } from 'fs';
43
import { IPC_CHANNELS, getSpecsDir, AUTO_BUILD_PATHS } from '../../shared/constants';
54
import type {
65
SDKRateLimitInfo,
@@ -15,6 +14,7 @@ import { titleGenerator } from '../title-generator';
1514
import { fileWatcher } from '../file-watcher';
1615
import { projectStore } from '../project-store';
1716
import { notificationService } from '../notification-service';
17+
import { persistPlanStatusSync, getPlanPath } from './task/plan-file-utils';
1818

1919

2020
/**
@@ -92,6 +92,15 @@ export function registerAgenteventsHandlers(
9292

9393
if (task && project) {
9494
const taskTitle = task.title || task.specId;
95+
const planPath = getPlanPath(project, task);
96+
97+
// Use shared utility for persisting status (prevents race conditions)
98+
const persistStatus = (status: TaskStatus) => {
99+
const persisted = persistPlanStatusSync(planPath, status);
100+
if (persisted) {
101+
console.log(`[Task ${taskId}] Persisted status to plan: ${status}`);
102+
}
103+
};
95104

96105
if (code === 0) {
97106
notificationService.notifyReviewNeeded(taskTitle, project.id, taskId);
@@ -105,6 +114,7 @@ export function registerAgenteventsHandlers(
105114

106115
if (isActiveStatus && !hasIncompleteSubtasks) {
107116
console.log(`[Task ${taskId}] Fallback: Moving to human_review (process exited successfully)`);
117+
persistStatus('human_review');
108118
mainWindow.webContents.send(
109119
IPC_CHANNELS.TASK_STATUS_CHANGE,
110120
taskId,
@@ -113,6 +123,7 @@ export function registerAgenteventsHandlers(
113123
}
114124
} else {
115125
notificationService.notifyTaskFailed(taskTitle, project.id, taskId);
126+
persistStatus('human_review');
116127
mainWindow.webContents.send(
117128
IPC_CHANNELS.TASK_STATUS_CHANGE,
118129
taskId,
@@ -148,6 +159,26 @@ export function registerAgenteventsHandlers(
148159
taskId,
149160
newStatus
150161
);
162+
163+
// CRITICAL: Persist status to plan file to prevent flip-flop on task list refresh
164+
// When getTasks() is called, it reads status from the plan file. Without persisting,
165+
// the status in the file might differ from the UI, causing inconsistent state.
166+
// Uses shared utility with locking to prevent race conditions.
167+
try {
168+
const projects = projectStore.getProjects();
169+
for (const p of projects) {
170+
const tasks = projectStore.getTasks(p.id);
171+
const task = tasks.find((t) => t.id === taskId || t.specId === taskId);
172+
if (task) {
173+
const planPath = getPlanPath(p, task);
174+
persistPlanStatusSync(planPath, newStatus);
175+
break;
176+
}
177+
}
178+
} catch (err) {
179+
// Ignore persistence errors - UI will still work, just might flip on refresh
180+
console.warn('[execution-progress] Could not persist status:', err);
181+
}
151182
}
152183
}
153184
});

0 commit comments

Comments
 (0)