Skip to content

fix(frontend): add windowsVerbatimArguments for Windows .cmd validation (ACS-252) #1075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AndyMik90 merged 15 commits into from
Jan 15, 2026
Merged

fix(frontend): add windowsVerbatimArguments for Windows .cmd validation (ACS-252) #1075

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
5425a78
fix(frontend): add windowsVerbatimArguments for Windows .cmd validation
StillKnotKnown Jan 14, 2026
68cc956
refactor: address PR review feedback
StillKnotKnown Jan 14, 2026
28a69c9
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
StillKnotKnown Jan 14, 2026
a49b683
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
StillKnotKnown Jan 14, 2026
9fae247
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
StillKnotKnown Jan 15, 2026
8af146f
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
StillKnotKnown Jan 15, 2026
4f0d0c3
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
StillKnotKnown Jan 15, 2026
dcd7165
refactor: use top-level type imports for better consistency
StillKnotKnown Jan 15, 2026
140e13a
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
StillKnotKnown Jan 15, 2026
e26b7ac
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
StillKnotKnown Jan 15, 2026
b59cde4
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
StillKnotKnown Jan 15, 2026
08bf443
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
StillKnotKnown Jan 15, 2026
bbbb2f2
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
StillKnotKnown Jan 15, 2026
948f3fa
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
StillKnotKnown Jan 15, 2026
705cac6
Merge branch 'develop' into stillknotknown/acs-252-windows-clicking-c…
AndyMik90 Jan 15, 2026
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
10 changes: 8 additions & 2 deletions apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import semver from 'semver';

const execFileAsync = promisify(execFile);

type ExecFileAsyncOptionsWithVerbatim = import('child_process').ExecFileOptionsWithStringEncoding & {
windowsVerbatimArguments?: boolean;
};
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 14, 2026

Choose a reason for hiding this comment

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

medium

This type definition is a duplicate of the one found in apps/frontend/src/main/cli-tool-manager.ts (lines 38-40). To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, consider moving this type to a shared location. For example, you could export it from cli-tool-manager.ts and import it here, or move it to a new shared types file within the main process directory.


Copy link
Contributor

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

LGTM! Type definition correctly extends base options with Windows-specific property.

The type properly adds windowsVerbatimArguments to the exec options. Consider using the union type approach for slightly better type safety:

💡 Optional: Alternative type definition 
-type ExecFileAsyncOptionsWithVerbatim = import('child_process').ExecFileOptionsWithStringEncoding & {
-  windowsVerbatimArguments?: boolean;
-};
+import type { ExecFileOptionsWithStringEncoding } from 'child_process';
+
+type ExecFileAsyncOptionsWithVerbatim = ExecFileOptionsWithStringEncoding & {
+  windowsVerbatimArguments?: boolean;
+};

This moves the import to the top-level for consistency with other imports in the file.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
type ExecFileAsyncOptionsWithVerbatim = import('child_process').ExecFileOptionsWithStringEncoding & {
windowsVerbatimArguments?: boolean;
};
import type { ExecFileOptionsWithStringEncoding } from 'child_process';
type ExecFileAsyncOptionsWithVerbatim = ExecFileOptionsWithStringEncoding & {
windowsVerbatimArguments?: boolean;
};
🤖 Prompt for AI Agents 
In `@apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts` around lines 26
- 29, Summary: Suggest moving the child_process import to the file top-level and
switch ExecFileAsyncOptionsWithVerbatim to a union-based type for slightly
stronger type safety. Fix: add a top-level import of
ExecFileOptionsWithStringEncoding from 'child_process' (use an import type to
match other imports), then redefine the ExecFileAsyncOptionsWithVerbatim type
using a union that includes the base ExecFileOptionsWithStringEncoding and a
variant that adds windowsVerbatimArguments?: boolean so callers get the safer
discrimination; update any references to ExecFileAsyncOptionsWithVerbatim
accordingly.

// Cache for latest version (avoid hammering npm registry)
let cachedLatestVersion: { version: string; timestamp: number } | null = null;
let cachedVersionList: { versions: string[]; timestamp: number } | null = null;
Expand Down Expand Up @@ -56,12 +60,14 @@ async function validateClaudeCliAsync(cliPath: string): Promise<[boolean, string
|| path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'cmd.exe');
// Use double-quoted command line for paths with spaces
const cmdLine = `""${cliPath}" --version"`;
const result = await execFileAsync(cmdExe, ['/d', '/s', '/c', cmdLine], {
const execOptions: ExecFileAsyncOptionsWithVerbatim = {
encoding: 'utf-8',
timeout: 5000,
windowsHide: true,
windowsVerbatimArguments: true,
env,
});
};
const result = await execFileAsync(cmdExe, ['/d', '/s', '/c', cmdLine], execOptions);
stdout = result.stdout;
} else {
const result = await execFileAsync(cliPath, ['--version'], {
Expand Down
Loading