From 75f9e6bcec94576cebdafe14d0d998b9bc53125a Mon Sep 17 00:00:00 2001 From: iliya Date: Sat, 25 Apr 2026 17:19:30 +0300 Subject: [PATCH 1/2] feat: add teammate runtime compatibility notices and test coverage --- scripts/dev-with-runtime.mjs | 8 +- .../codexAppServer/CodexBinaryResolver.ts | 2 +- .../__tests__/CodexBinaryResolver.test.ts | 80 ++++ src/main/utils/childProcess.ts | 123 +++++- .../runtime/ProviderRuntimeSettingsDialog.tsx | 17 +- .../team/dialogs/CreateTeamDialog.tsx | 64 +++- .../team/dialogs/LaunchTeamDialog.tsx | 84 ++++- .../team/dialogs/TeamModelSelector.tsx | 5 +- .../TeammateRuntimeCompatibilityNotice.tsx | 60 +++ .../dialogs/teammateRuntimeCompatibility.tsx | 357 ++++++++++++++++++ .../team/members/LeadModelRow.test.tsx | 5 +- src/renderer/hooks/useKeyboardShortcuts.ts | 20 + src/renderer/index.css | 14 + src/renderer/index.html | 4 + src/renderer/utils/pathDisplay.ts | 39 +- test/main/utils/childProcess.test.ts | 132 ++++++- .../ProviderRuntimeSettingsDialog.test.ts | 74 +++- .../TeamModelSelectorDisabledState.test.ts | 11 +- .../team/dialogs/LaunchTeamDialog.test.ts | 43 ++- .../teammateRuntimeCompatibility.test.ts | 159 ++++++++ .../hooks/useKeyboardShortcuts.test.ts | 28 ++ test/renderer/utils/pathDisplay.test.ts | 37 ++ 22 files changed, 1312 insertions(+), 54 deletions(-) create mode 100644 src/main/services/infrastructure/codexAppServer/__tests__/CodexBinaryResolver.test.ts create mode 100644 src/renderer/components/team/dialogs/TeammateRuntimeCompatibilityNotice.tsx create mode 100644 src/renderer/components/team/dialogs/teammateRuntimeCompatibility.tsx create mode 100644 test/renderer/components/team/dialogs/teammateRuntimeCompatibility.test.ts create mode 100644 test/renderer/hooks/useKeyboardShortcuts.test.ts create mode 100644 test/renderer/utils/pathDisplay.test.ts diff --git a/scripts/dev-with-runtime.mjs b/scripts/dev-with-runtime.mjs index ebd5db07..a42857b7 100644 --- a/scripts/dev-with-runtime.mjs +++ b/scripts/dev-with-runtime.mjs @@ -27,6 +27,11 @@ function shouldUseWindowsShell(cmd) { return false; } + const extension = path.extname(cmd).toLowerCase(); + if (extension === '.cmd' || extension === '.bat') { + return true; + } + const commandName = path.basename(cmd).toLowerCase(); return WINDOWS_SHELL_COMMANDS.has(commandName); } @@ -502,7 +507,8 @@ async function resolveRuntimeCli() { runOrExit(runtimePackageManager, ['run', 'build:dev'], { cwd: runtimeRepoRoot }); - const runtimeCliPath = path.join(runtimeRepoRoot, 'cli-dev'); + const runtimeCliName = process.platform === 'win32' ? 'cli-dev.cmd' : 'cli-dev'; + const runtimeCliPath = path.join(runtimeRepoRoot, runtimeCliName); return { binaryPath: runtimeCliPath, versionText: readBinaryVersion(runtimeCliPath), diff --git a/src/main/services/infrastructure/codexAppServer/CodexBinaryResolver.ts b/src/main/services/infrastructure/codexAppServer/CodexBinaryResolver.ts index 71627c13..5a108abf 100644 --- a/src/main/services/infrastructure/codexAppServer/CodexBinaryResolver.ts +++ b/src/main/services/infrastructure/codexAppServer/CodexBinaryResolver.ts @@ -40,7 +40,7 @@ function expandWindowsExtensions(candidate: string): string[] { return [candidate]; } - return [candidate, ...pathext.map((ext) => `${candidate}${ext.toLowerCase()}`)]; + return [...pathext.map((ext) => `${candidate}${ext.toLowerCase()}`), candidate]; } async function verifyBinary(candidate: string): Promise { diff --git a/src/main/services/infrastructure/codexAppServer/__tests__/CodexBinaryResolver.test.ts b/src/main/services/infrastructure/codexAppServer/__tests__/CodexBinaryResolver.test.ts new file mode 100644 index 00000000..ec2aeaef --- /dev/null +++ b/src/main/services/infrastructure/codexAppServer/__tests__/CodexBinaryResolver.test.ts @@ -0,0 +1,80 @@ +// @vitest-environment node +import { constants as fsConstants } from 'node:fs'; +import type { PathLike } from 'node:fs'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const accessMock = vi.fn<(filePath: PathLike, mode?: number) => Promise>(); + +vi.mock('node:fs/promises', () => ({ + access: (filePath: PathLike, mode?: number) => accessMock(filePath, mode), +})); + +const originalPlatform = process.platform; +const originalPath = process.env.PATH; +const originalPathExt = process.env.PATHEXT; +const originalCodexCliPath = process.env.CODEX_CLI_PATH; + +function setPlatform(value: NodeJS.Platform): void { + Object.defineProperty(process, 'platform', { + value, + configurable: true, + writable: true, + }); +} + +describe('CodexBinaryResolver', () => { + beforeEach(() => { + vi.resetModules(); + vi.clearAllMocks(); + setPlatform('win32'); + process.env.PATHEXT = '.EXE;.CMD;.BAT;.COM'; + delete process.env.CODEX_CLI_PATH; + }); + + afterEach(() => { + setPlatform(originalPlatform); + process.env.PATH = originalPath; + process.env.PATHEXT = originalPathExt; + process.env.CODEX_CLI_PATH = originalCodexCliPath; + }); + + it('prefers the Windows command shim over the extensionless POSIX shim on PATH', async () => { + const binDir = 'C:\\Program Files\\nodejs'; + const extensionless = path.join(binDir, 'codex'); + const cmdShim = path.join(binDir, 'codex.cmd'); + process.env.PATH = binDir; + + accessMock.mockImplementation((filePath, mode) => { + expect(mode).toBe(fsConstants.X_OK); + if (filePath === extensionless || filePath === cmdShim) { + return Promise.resolve(); + } + return Promise.reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + }); + + const { CodexBinaryResolver } = await import('../CodexBinaryResolver'); + CodexBinaryResolver.clearCache(); + + await expect(CodexBinaryResolver.resolve()).resolves.toBe(cmdShim); + }); + + it('expands an explicit extensionless override to the Windows command shim first', async () => { + const extensionless = 'C:\\Program Files\\nodejs\\codex'; + const cmdShim = 'C:\\Program Files\\nodejs\\codex.cmd'; + process.env.CODEX_CLI_PATH = extensionless; + + accessMock.mockImplementation((filePath) => { + if (filePath === extensionless || filePath === cmdShim) { + return Promise.resolve(); + } + return Promise.reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + }); + + const { CodexBinaryResolver } = await import('../CodexBinaryResolver'); + CodexBinaryResolver.clearCache(); + + await expect(CodexBinaryResolver.resolve()).resolves.toBe(cmdShim); + }); +}); diff --git a/src/main/utils/childProcess.ts b/src/main/utils/childProcess.ts index 384731d6..b4faa4f5 100644 --- a/src/main/utils/childProcess.ts +++ b/src/main/utils/childProcess.ts @@ -7,6 +7,7 @@ import { spawn, type SpawnOptions, } from 'child_process'; +import { existsSync, readFileSync } from 'fs'; import path from 'path'; /** @@ -79,14 +80,95 @@ function containsNonAscii(str: string): boolean { } /** - * On Windows, creating a process whose *path* contains non-ASCII - * characters will often fail with `spawn EINVAL`. Detect that case so - * callers can automatically fall back to launching via a shell. + * On Windows, batch launchers need cmd.exe, and creating a process whose + * path contains non-ASCII characters will often fail with `spawn EINVAL`. + * Detect both cases so callers can launch through a shell when needed. */ function needsShell(binaryPath: string): boolean { if (process.platform !== 'win32') return false; if (!binaryPath) return false; - return containsNonAscii(binaryPath); + const extension = path.extname(binaryPath).toLowerCase(); + return extension === '.cmd' || extension === '.bat' || containsNonAscii(binaryPath); +} + +interface DirectWindowsLauncher { + command: string; + argsPrefix: string[]; +} + +function isWindowsBatchLauncher(binaryPath: string): boolean { + const extension = path.extname(binaryPath).toLowerCase(); + return extension === '.cmd' || extension === '.bat'; +} + +function resolveCmdPathTemplate(template: string, launcherDir: string): string { + const dirWithSep = launcherDir.endsWith(path.sep) ? launcherDir : `${launcherDir}${path.sep}`; + return path.resolve( + template + .replace(/%SCRIPT_DIR%/gi, dirWithSep) + .replace(/%~dp0/gi, dirWithSep) + .replace(/%dp0%/gi, dirWithSep) + ); +} + +function resolveGeneratedBunLauncher( + content: string, + launcherDir: string +): DirectWindowsLauncher | null { + if (!/\bbun\s+"%TARGET%"\s+%\*/i.test(content)) { + return null; + } + const targetMatch = /set\s+"TARGET=([^"]+)"/i.exec(content); + const targetTemplate = targetMatch?.[1]; + if (!targetTemplate) { + return null; + } + + const target = resolveCmdPathTemplate(targetTemplate, launcherDir); + if (!existsSync(target)) { + return null; + } + return { command: 'bun', argsPrefix: [target] }; +} + +function resolveNpmNodeShim(content: string, launcherDir: string): DirectWindowsLauncher | null { + const scriptMatch = /"%_prog%"\s+"([^"]+\.(?:cjs|mjs|js))"\s+%\*/i.exec(content); + const scriptTemplate = scriptMatch?.[1]; + if (!scriptTemplate) { + return null; + } + + const scriptPath = resolveCmdPathTemplate(scriptTemplate, launcherDir); + if (!existsSync(scriptPath)) { + return null; + } + + const localNode = path.join(launcherDir, 'node.exe'); + return { + command: existsSync(localNode) ? localNode : 'node', + argsPrefix: [scriptPath], + }; +} + +/** + * Some Windows launchers are thin wrappers around a real JS entrypoint. + * Running that entrypoint directly with an argv array avoids cmd.exe's + * percent expansion, which cannot safely represent args like `%PATH%`. + */ +function resolveDirectWindowsLauncher(binaryPath: string): DirectWindowsLauncher | null { + if (process.platform !== 'win32' || !isWindowsBatchLauncher(binaryPath)) { + return null; + } + + try { + const content = readFileSync(binaryPath, 'utf8'); + const launcherDir = path.dirname(binaryPath); + return ( + resolveGeneratedBunLauncher(content, launcherDir) ?? resolveNpmNodeShim(content, launcherDir) + ); + } catch { + return null; + } } /** @@ -94,17 +176,25 @@ function needsShell(binaryPath: string): boolean { * * cmd.exe rules: * - Double-quote args containing spaces or special characters - * - Inside double quotes, escape literal `"` as `""` - * - `%` is expanded as env var even inside double quotes — escape as `%%` + * - Inside double quotes, escape literal `"` as `\"` for the target argv parser + * - Double trailing backslashes so they do not escape the closing quote + * - `%` is expanded as env var even inside double quotes. Keep it outside + * quoted chunks and escape it as `^%`. * - `^`, `&`, `|`, `<`, `>` are safe inside double quotes * * Our callers only pass controlled strings (binary paths, CLI flags), * NOT arbitrary user input. */ +function quoteCmdChunk(chunk: string): string { + const escaped = chunk + .replace(/(\\*)"/g, (_match, backslashes: string) => `${backslashes}${backslashes}\\"`) + .replace(/(\\+)$/g, '$1$1'); + return `"${escaped}"`; +} + function quoteArg(arg: string): string { if (/[^A-Za-z0-9_\-/.]/.test(arg)) { - const escaped = arg.replace(/%/g, '%%').replace(/"/g, '""'); - return `"${escaped}"`; + return arg.split('%').map(quoteCmdChunk).join('^%'); } return arg; } @@ -172,6 +262,15 @@ export async function execCli( } const target = binaryPath; const opts = withCliEnv(options); + const directLauncher = resolveDirectWindowsLauncher(target); + if (directLauncher) { + const result = await execFileAsync( + directLauncher.command, + [...directLauncher.argsPrefix, ...args], + opts + ); + return { stdout: String(result.stdout), stderr: String(result.stderr) }; + } // attempt the normal execFile path first if (!needsShell(target)) { @@ -209,6 +308,14 @@ export function spawnCli( options: SpawnOptions = {} ): ReturnType { const opts = withCliEnv(options); + const directLauncher = resolveDirectWindowsLauncher(binaryPath); + if (directLauncher) { + const directOpts = { ...opts }; + delete directOpts.shell; + return trackCliProcess( + spawn(directLauncher.command, [...directLauncher.argsPrefix, ...args], directOpts) + ); + } if (process.platform === 'win32' && needsShell(binaryPath)) { const cmd = [binaryPath, ...args].map(quoteArg).join(' '); diff --git a/src/renderer/components/runtime/ProviderRuntimeSettingsDialog.tsx b/src/renderer/components/runtime/ProviderRuntimeSettingsDialog.tsx index c625526c..7fe5396b 100644 --- a/src/renderer/components/runtime/ProviderRuntimeSettingsDialog.tsx +++ b/src/renderer/components/runtime/ProviderRuntimeSettingsDialog.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useState } from 'react'; +import { useEffect, useMemo, useRef, useState } from 'react'; import { formatCodexCreditsValue, @@ -561,6 +561,7 @@ export const ProviderRuntimeSettingsDialog = ({ const [runtimeSaving, setRuntimeSaving] = useState(false); const [pendingConnectionAction, setPendingConnectionAction] = useState(null); + const apiKeyInputRef = useRef(null); const apiKeys = useStore((s) => s.apiKeys); const apiKeysLoading = useStore((s) => s.apiKeysLoading); @@ -799,6 +800,19 @@ export const ProviderRuntimeSettingsDialog = ({ configuredAuthMode !== 'api_key' && selectedProvider.statusMessage !== 'Checking...' && (!selectedProvider?.authenticated || hasSubscriptionSession || configuredAuthMode === 'oauth'); + + useEffect(() => { + if (!showApiKeyForm) { + return; + } + + const frame = window.requestAnimationFrame(() => { + apiKeyInputRef.current?.focus({ preventScroll: true }); + }); + + return () => window.cancelAnimationFrame(frame); + }, [selectedProvider?.providerId, showApiKeyForm]); + let connectionStatusLabel: string | null = null; if (selectedProvider) { if (!hideConnectionMethodMeta && selectedProvider.authenticated) { @@ -1733,6 +1747,7 @@ export const ProviderRuntimeSettingsDialog = ({ {apiKeyConfig.name} s.cliStatusLoading); const bootstrapCliStatus = useStore((s) => s.bootstrapCliStatus); const fetchCliStatus = useStore((s) => s.fetchCliStatus); + const openDashboard = useStore((s) => s.openDashboard); const loadingCliStatus = useMemo( () => !cliStatus && cliStatusLoading && multimodelEnabled @@ -543,6 +549,7 @@ export const CreateTeamDialog = ({ () => (syncModelsWithLead ? members.map(clearMemberModelOverrides) : members), [members, syncModelsWithLead] ); + const tmuxRuntime = useTmuxRuntimeReadiness(open && canCreate); const selectedMemberProviders = useMemo(() => { if (!multimodelEnabled) { @@ -582,6 +589,14 @@ export const CreateTeamDialog = ({ ), [effectiveCliStatus?.providers] ); + const selectedProviderBackendId = useMemo( + () => + resolveUiOwnedProviderBackendId( + selectedProviderId, + runtimeProviderStatusById.get(selectedProviderId) + ), + [runtimeProviderStatusById, selectedProviderId] + ); const runtimeBackendSummaryByProviderRef = useRef(runtimeBackendSummaryByProvider); const prepareChecksRef = useRef([]); const prepareModelResultsCacheRef = useRef( @@ -1136,6 +1151,31 @@ export const CreateTeamDialog = ({ ), [limitContext, runtimeProviderStatusById, selectedModel, selectedProviderId] ); + const teammateRuntimeCompatibility = useMemo( + () => + analyzeTeammateRuntimeCompatibility({ + leadProviderId: selectedProviderId, + leadProviderBackendId: selectedProviderBackendId, + members: effectiveMemberDrafts, + soloTeam: soloTeam || !canCreate, + extraCliArgs: launchTeam ? customArgs : undefined, + tmuxStatus: tmuxRuntime.status, + tmuxStatusLoading: tmuxRuntime.loading, + tmuxStatusError: tmuxRuntime.error, + }), + [ + customArgs, + effectiveMemberDrafts, + launchTeam, + canCreate, + selectedProviderBackendId, + selectedProviderId, + soloTeam, + tmuxRuntime.error, + tmuxRuntime.loading, + tmuxRuntime.status, + ] + ); const anthropicRuntimeSelection = useMemo( () => selectedProviderId === 'anthropic' @@ -1279,11 +1319,7 @@ export const CreateTeamDialog = ({ cwd: effectiveCwd, prompt: prompt.trim() || undefined, providerId: selectedProviderId, - providerBackendId: - resolveUiOwnedProviderBackendId( - selectedProviderId, - runtimeProviderStatusById.get(selectedProviderId) - ) ?? undefined, + providerBackendId: selectedProviderBackendId ?? undefined, model: effectiveModel, effort: (selectedEffort as EffortLevel) || undefined, fastMode: @@ -1304,7 +1340,7 @@ export const CreateTeamDialog = ({ effectiveCwd, prompt, selectedProviderId, - runtimeProviderStatusById, + selectedProviderBackendId, effectiveModel, selectedEffort, selectedFastMode, @@ -1388,7 +1424,8 @@ export const CreateTeamDialog = ({ isNameTakenByExistingTeam || isNameProvisioning || !requestValidation.valid || - !!modelValidationError; + !!modelValidationError || + teammateRuntimeCompatibility.blocksSubmission; const internalArgs = useMemo(() => { const args: string[] = []; @@ -1528,6 +1565,10 @@ export const CreateTeamDialog = ({ setLocalError(modelValidationError); return; } + if (teammateRuntimeCompatibility.blocksSubmission) { + setLocalError(teammateRuntimeCompatibility.message); + return; + } setFieldErrors({}); setLocalError(null); setIsSubmitting(true); @@ -1661,6 +1702,14 @@ export const CreateTeamDialog = ({

) : null} + { + onClose(); + openDashboard(); + }} + /> +
@@ -1735,6 +1784,7 @@ export const CreateTeamDialog = ({ onTeammateWorktreeDefaultChange={setTeammateWorktreeDefault} disableGeminiOption={isGeminiUiFrozen()} leadModelIssueText={leadModelIssueText} + memberWarningById={teammateRuntimeCompatibility.memberWarningById} memberModelIssueById={memberModelIssueById} headerTop={
diff --git a/src/renderer/components/team/dialogs/LaunchTeamDialog.tsx b/src/renderer/components/team/dialogs/LaunchTeamDialog.tsx index 935d9260..4dcf1792 100644 --- a/src/renderer/components/team/dialogs/LaunchTeamDialog.tsx +++ b/src/renderer/components/team/dialogs/LaunchTeamDialog.tsx @@ -128,6 +128,11 @@ import { shouldHideProvisioningProviderStatusList, updateProviderCheck, } from './ProvisioningProviderStatusList'; +import { + analyzeTeammateRuntimeCompatibility, + useTmuxRuntimeReadiness, +} from './teammateRuntimeCompatibility'; +import { TeammateRuntimeCompatibilityNotice } from './TeammateRuntimeCompatibilityNotice'; import { computeEffectiveTeamModel, formatTeamModelSummary, @@ -451,6 +456,7 @@ export const LaunchTeamDialog = (props: LaunchTeamDialogProps): React.JSX.Elemen () => (syncModelsWithLead ? membersDrafts.map(clearMemberModelOverrides) : membersDrafts), [membersDrafts, syncModelsWithLead] ); + const tmuxRuntime = useTmuxRuntimeReadiness(open && isLaunchMode); const selectedMemberProviders = useMemo( () => !multimodelEnabled @@ -842,6 +848,46 @@ export const LaunchTeamDialog = (props: LaunchTeamDialogProps): React.JSX.Elemen ) ?? '', [limitContext, runtimeProviderStatusById, selectedModel, selectedProviderId] ); + const selectedProviderBackendId = useMemo( + () => + resolveUiOwnedProviderBackendId( + selectedProviderId, + runtimeProviderStatusById.get(selectedProviderId) + ) ?? + migrateProviderBackendId( + selectedProviderId, + previousLaunchParams?.providerBackendId ?? savedLaunchProviderBackendId + ) ?? + undefined, + [ + previousLaunchParams?.providerBackendId, + runtimeProviderStatusById, + savedLaunchProviderBackendId, + selectedProviderId, + ] + ); + const teammateRuntimeCompatibility = useMemo( + () => + analyzeTeammateRuntimeCompatibility({ + leadProviderId: selectedProviderId, + leadProviderBackendId: selectedProviderBackendId, + members: isLaunchMode ? effectiveMemberDrafts : [], + extraCliArgs: isLaunchMode ? customArgs : undefined, + tmuxStatus: tmuxRuntime.status, + tmuxStatusLoading: tmuxRuntime.loading, + tmuxStatusError: tmuxRuntime.error, + }), + [ + customArgs, + effectiveMemberDrafts, + isLaunchMode, + selectedProviderBackendId, + selectedProviderId, + tmuxRuntime.error, + tmuxRuntime.loading, + tmuxRuntime.status, + ] + ); const anthropicRuntimeSelection = useMemo( () => selectedProviderId === 'anthropic' @@ -1218,6 +1264,15 @@ export const LaunchTeamDialog = (props: LaunchTeamDialogProps): React.JSX.Elemen } return warnings; }, [effectiveMemberDrafts, runtimeChangeNoteByKey]); + const combinedMemberRuntimeWarningById = useMemo(() => { + const warnings: Record = { ...memberRuntimeWarningById }; + for (const [memberId, warning] of Object.entries( + teammateRuntimeCompatibility.memberWarningById + )) { + warnings[memberId] = warnings[memberId] ? `${warnings[memberId]} ${warning}` : warning; + } + return warnings; + }, [memberRuntimeWarningById, teammateRuntimeCompatibility.memberWarningById]); // --------------------------------------------------------------------------- // Launch-only effects @@ -1823,6 +1878,10 @@ export const LaunchTeamDialog = (props: LaunchTeamDialogProps): React.JSX.Elemen setLocalError(modelValidationError); return; } + if (isLaunchMode && teammateRuntimeCompatibility.blocksSubmission) { + setLocalError(teammateRuntimeCompatibility.message); + return; + } if (isLaunchMode && !effectiveCwd) { setLocalError('Select working directory (cwd)'); return; @@ -1862,10 +1921,7 @@ export const LaunchTeamDialog = (props: LaunchTeamDialogProps): React.JSX.Elemen selectedProviderId, runtimeProviderStatusById.get(selectedProviderId) ) ?? - migrateProviderBackendId( - selectedProviderId, - previousLaunchParams?.providerBackendId ?? savedLaunchProviderBackendId - ) ?? + selectedProviderBackendId ?? undefined, model: computeEffectiveTeamModel( selectedModel, @@ -1902,10 +1958,7 @@ export const LaunchTeamDialog = (props: LaunchTeamDialogProps): React.JSX.Elemen selectedProviderId, runtimeProviderStatusById.get(selectedProviderId) ) ?? - migrateProviderBackendId( - selectedProviderId, - previousLaunchParams?.providerBackendId ?? savedLaunchProviderBackendId - ) ?? + selectedProviderBackendId ?? undefined; const scheduleModel = computeEffectiveTeamModel( selectedModel, @@ -2000,7 +2053,8 @@ export const LaunchTeamDialog = (props: LaunchTeamDialogProps): React.JSX.Elemen validationErrors.length > 0 || !!modelValidationError || hasInvalidLaunchMemberNames || - hasDuplicateLaunchMemberNames + hasDuplicateLaunchMemberNames || + teammateRuntimeCompatibility.blocksSubmission : isSubmitting || validationErrors.length > 0 || !!modelValidationError; // --------------------------------------------------------------------------- @@ -2130,6 +2184,16 @@ export const LaunchTeamDialog = (props: LaunchTeamDialogProps): React.JSX.Elemen
) : null} + {isLaunchMode ? ( + { + closeDialog(); + openDashboard(); + }} + /> + ) : null} +
{/* ═══════════════════════════════════════════════════════════════════ Schedule-only: Team selector (standalone mode) @@ -2360,7 +2424,7 @@ export const LaunchTeamDialog = (props: LaunchTeamDialogProps): React.JSX.Elemen teammateWorktreeDefault={teammateWorktreeDefault} onTeammateWorktreeDefaultChange={setTeammateWorktreeDefault} leadWarningText={leadRuntimeWarningText} - memberWarningById={memberRuntimeWarningById} + memberWarningById={combinedMemberRuntimeWarningById} leadModelIssueText={leadModelIssueText} memberModelIssueById={memberModelIssueById} softDeleteMembers diff --git a/src/renderer/components/team/dialogs/TeamModelSelector.tsx b/src/renderer/components/team/dialogs/TeamModelSelector.tsx index 216058f3..6a315294 100644 --- a/src/renderer/components/team/dialogs/TeamModelSelector.tsx +++ b/src/renderer/components/team/dialogs/TeamModelSelector.tsx @@ -59,8 +59,9 @@ const PROVIDERS: ProviderDef[] = [ ]; const OPENCODE_UI_DISABLED_REASON = 'OpenCode team launch is not ready.'; -export const OPENCODE_TEAM_LEAD_DISABLED_REASON = 'OpenCode is not available for team lead.'; -export const OPENCODE_TEAM_LEAD_DISABLED_BADGE_LABEL = 'not teamlead'; +export const OPENCODE_TEAM_LEAD_DISABLED_REASON = + 'OpenCode is teammate-only in this phase. Use Anthropic, Codex, or Gemini as the team lead, then add OpenCode as a teammate.'; +export const OPENCODE_TEAM_LEAD_DISABLED_BADGE_LABEL = 'side lane'; export function getTeamModelLabel(model: string): string { return getCatalogTeamModelLabel(model) ?? model; diff --git a/src/renderer/components/team/dialogs/TeammateRuntimeCompatibilityNotice.tsx b/src/renderer/components/team/dialogs/TeammateRuntimeCompatibilityNotice.tsx new file mode 100644 index 00000000..73e6b9e4 --- /dev/null +++ b/src/renderer/components/team/dialogs/TeammateRuntimeCompatibilityNotice.tsx @@ -0,0 +1,60 @@ +import React from 'react'; + +import { Button } from '@renderer/components/ui/button'; +import { AlertTriangle, Info } from 'lucide-react'; + +import type { TeammateRuntimeCompatibility } from './teammateRuntimeCompatibility'; + +interface TeammateRuntimeCompatibilityNoticeProps { + readonly analysis: TeammateRuntimeCompatibility; + readonly onOpenDashboard?: () => void; +} + +export const TeammateRuntimeCompatibilityNotice = ({ + analysis, + onOpenDashboard, +}: TeammateRuntimeCompatibilityNoticeProps): React.JSX.Element | null => { + if (!analysis.visible) { + return null; + } + const Icon = analysis.checking ? Info : AlertTriangle; + return ( +
+
+ +
+

{analysis.title}

+

{analysis.message}

+ {analysis.tmuxDetail ? ( +

{analysis.tmuxDetail}

+ ) : null} + {analysis.details.length > 0 ? ( +
    + {analysis.details.map((detail) => ( +
  • {detail}
    • + ))} +
+ ) : null} + {onOpenDashboard ? ( + + ) : null} +
+
+
+ ); +}; diff --git a/src/renderer/components/team/dialogs/teammateRuntimeCompatibility.tsx b/src/renderer/components/team/dialogs/teammateRuntimeCompatibility.tsx new file mode 100644 index 00000000..2b4913ab --- /dev/null +++ b/src/renderer/components/team/dialogs/teammateRuntimeCompatibility.tsx @@ -0,0 +1,357 @@ +import { useCallback, useEffect, useMemo, useState } from 'react'; + +import { api } from '@renderer/api'; +import { parseCliArgs } from '@shared/utils/cliArgsParser'; +import { migrateProviderBackendId } from '@shared/utils/providerBackend'; +import { normalizeOptionalTeamProviderId } from '@shared/utils/teamProvider'; + +import type { TmuxStatus } from '@features/tmux-installer/contracts'; +import type { TeamProviderId } from '@shared/types'; + +type TeammateRuntimeIssueReason = + | 'mixed-provider' + | 'codex-native-runtime' + | 'explicit-tmux-mode' + | 'opencode-led-mixed-unsupported'; + +interface RuntimeMemberInput { + id?: string; + name: string; + providerId?: TeamProviderId; + providerBackendId?: string | null; + removedAt?: number | string | null; +} + +interface RuntimeIssue { + reason: TeammateRuntimeIssueReason; + memberId?: string; + memberName?: string; + memberProviderId?: TeamProviderId; +} + +export interface TeammateRuntimeCompatibility { + visible: boolean; + blocksSubmission: boolean; + checking: boolean; + title: string; + message: string; + details: string[]; + tmuxDetail: string | null; + memberWarningById: Record; +} + +interface AnalyzeTeammateRuntimeCompatibilityInput { + leadProviderId: TeamProviderId; + leadProviderBackendId?: string | null; + members: readonly RuntimeMemberInput[]; + soloTeam?: boolean; + extraCliArgs?: string; + tmuxStatus: TmuxStatus | null; + tmuxStatusLoading: boolean; + tmuxStatusError: string | null; +} + +export interface TmuxRuntimeReadiness { + status: TmuxStatus | null; + loading: boolean; + error: string | null; + refresh: () => Promise; +} + +const PROVIDER_LABELS: Record = { + anthropic: 'Anthropic', + codex: 'Codex', + gemini: 'Gemini', + opencode: 'OpenCode', +}; + +function getProviderLabel(providerId: TeamProviderId): string { + return PROVIDER_LABELS[providerId] ?? providerId; +} + +function getExplicitTeammateMode( + rawExtraCliArgs: string | undefined +): 'auto' | 'tmux' | 'in-process' | null { + const tokens = parseCliArgs(rawExtraCliArgs); + for (let index = 0; index < tokens.length; index += 1) { + const token = tokens[index]; + // eslint-disable-next-line security/detect-possible-timing-attacks -- parsing UI CLI flags, not comparing secrets + if (token === '--teammate-mode') { + const value = tokens[index + 1]; + return value === 'auto' || value === 'tmux' || value === 'in-process' ? value : null; + } + if (token.startsWith('--teammate-mode=')) { + const value = token.slice('--teammate-mode='.length); + return value === 'auto' || value === 'tmux' || value === 'in-process' ? value : null; + } + } + return null; +} + +function isTmuxRuntimeReady(status: TmuxStatus | null): boolean { + return status?.effective.available === true && status.effective.runtimeReady === true; +} + +function getTmuxDetail(status: TmuxStatus | null, error: string | null): string | null { + if (error) { + return error; + } + return status?.effective.detail ?? status?.wsl?.statusDetail ?? status?.error ?? null; +} + +function summarizeIssueNames( + issues: readonly RuntimeIssue[], + reason: TeammateRuntimeIssueReason +): string { + const names = issues + .filter((issue) => issue.reason === reason) + .map((issue) => issue.memberName) + .filter((name): name is string => Boolean(name)); + if (names.length === 0) { + return ''; + } + if (names.length <= 3) { + return names.join(', '); + } + return `${names.slice(0, 3).join(', ')} and ${names.length - 3} more`; +} + +export function analyzeTeammateRuntimeCompatibility({ + leadProviderId, + leadProviderBackendId, + members, + soloTeam = false, + extraCliArgs, + tmuxStatus, + tmuxStatusLoading, + tmuxStatusError, +}: AnalyzeTeammateRuntimeCompatibilityInput): TeammateRuntimeCompatibility { + const activeMembers = soloTeam + ? [] + : members.filter((member) => member.removedAt == null && member.name.trim().length > 0); + const explicitTeammateMode = getExplicitTeammateMode(extraCliArgs); + const leadBackendId = migrateProviderBackendId(leadProviderId, leadProviderBackendId); + const issues: RuntimeIssue[] = []; + + if (explicitTeammateMode === 'tmux' && activeMembers.length > 0) { + issues.push({ reason: 'explicit-tmux-mode' }); + } + + for (const member of activeMembers) { + const memberProviderId = normalizeOptionalTeamProviderId(member.providerId) ?? leadProviderId; + const memberName = member.name.trim(); + if (memberProviderId !== leadProviderId) { + if (leadProviderId !== 'opencode' && memberProviderId === 'opencode') { + continue; + } + if (leadProviderId === 'opencode') { + issues.push({ + reason: 'opencode-led-mixed-unsupported', + memberId: member.id, + memberName, + memberProviderId, + }); + continue; + } + issues.push({ + reason: 'mixed-provider', + memberId: member.id, + memberName, + memberProviderId, + }); + continue; + } + + const memberBackendId = migrateProviderBackendId( + memberProviderId, + member.providerBackendId ?? leadBackendId + ); + if (memberProviderId === 'codex' && memberBackendId === 'codex-native') { + issues.push({ + reason: 'codex-native-runtime', + memberId: member.id, + memberName, + memberProviderId, + }); + } + } + + if (issues.length === 0) { + return { + visible: false, + blocksSubmission: false, + checking: false, + title: '', + message: '', + details: [], + tmuxDetail: null, + memberWarningById: {}, + }; + } + + const tmuxReady = isTmuxRuntimeReady(tmuxStatus); + const hasOpenCodeLeadMixedUnsupported = issues.some( + (issue) => issue.reason === 'opencode-led-mixed-unsupported' + ); + if (tmuxReady && !hasOpenCodeLeadMixedUnsupported) { + return { + visible: false, + blocksSubmission: false, + checking: false, + title: '', + message: '', + details: [], + tmuxDetail: null, + memberWarningById: {}, + }; + } + + const checking = !hasOpenCodeLeadMixedUnsupported && tmuxStatusLoading && !tmuxStatus; + const blocksSubmission = true; + const hasMixedProviders = issues.some((issue) => issue.reason === 'mixed-provider'); + const hasCodexNative = issues.some((issue) => issue.reason === 'codex-native-runtime'); + const hasExplicitTmux = issues.some((issue) => issue.reason === 'explicit-tmux-mode'); + const details: string[] = []; + const memberWarningById: Record = {}; + + if (hasMixedProviders) { + const names = summarizeIssueNames(issues, 'mixed-provider'); + details.push( + names + ? `Mixed providers: ${names} use a different provider than the ${getProviderLabel(leadProviderId)} lead.` + : 'Mixed providers require teammate processes.' + ); + } + if (hasOpenCodeLeadMixedUnsupported) { + const names = summarizeIssueNames(issues, 'opencode-led-mixed-unsupported'); + details.push( + names + ? `OpenCode-led mixed team: ${names} use a non-OpenCode provider.` + : 'OpenCode-led mixed teams are not supported in this phase.' + ); + } + if (hasCodexNative) { + const names = summarizeIssueNames(issues, 'codex-native-runtime'); + details.push( + names + ? `Codex native teammates: ${names} must run through separate Codex processes.` + : 'Codex native teammates must run through separate Codex processes.' + ); + } + if (hasExplicitTmux) { + details.push('Custom CLI args force --teammate-mode tmux.'); + } + if (hasOpenCodeLeadMixedUnsupported) { + details.push( + 'Fix: keep the team lead on Anthropic, Codex, or Gemini when mixing OpenCode with other providers.' + ); + } else { + details.push( + hasCodexNative && !hasMixedProviders + ? 'Fix: install tmux/WSL tmux, use Solo team, or choose a same-provider runtime that supports in-process teammates.' + : 'Fix: install tmux/WSL tmux, use Solo team, or keep every teammate on the same non-Codex-native provider as the lead.' + ); + } + + for (const issue of issues) { + if (!issue.memberId || !issue.memberName) { + continue; + } + if (issue.reason === 'mixed-provider') { + memberWarningById[issue.memberId] = + `${issue.memberName} uses ${getProviderLabel(issue.memberProviderId ?? leadProviderId)}. ` + + `Without tmux, teammates must use the same provider as the ${getProviderLabel(leadProviderId)} lead.`; + } else if (issue.reason === 'codex-native-runtime') { + memberWarningById[issue.memberId] = + `${issue.memberName} uses Codex native. Codex native teammates require a separate process, which currently needs tmux.`; + } else if (issue.reason === 'opencode-led-mixed-unsupported') { + memberWarningById[issue.memberId] = + `${issue.memberName} uses ${getProviderLabel(issue.memberProviderId ?? leadProviderId)}. ` + + 'OpenCode cannot be the team lead when mixing providers in this phase.'; + } + } + + return { + visible: blocksSubmission || checking, + blocksSubmission, + checking, + title: checking + ? 'Checking tmux runtime for teammate support' + : hasOpenCodeLeadMixedUnsupported + ? 'OpenCode cannot lead mixed-provider teams' + : hasCodexNative && !hasMixedProviders + ? 'Codex teammates need tmux before they can run' + : 'This team needs tmux before it can run', + message: checking + ? 'Some teammates require separate processes. The app is checking whether tmux is available.' + : hasOpenCodeLeadMixedUnsupported + ? 'OpenCode teammates can run as secondary runtime lanes under an Anthropic, Codex, or Gemini lead, but OpenCode-led mixed teams are not supported in this phase.' + : hasCodexNative && !hasMixedProviders + ? 'The Codex lead can run without tmux, but Codex native teammates cannot use the in-process teammate adapter. They must start as separate Codex processes, and this path currently needs tmux.' + : 'tmux is not ready on this machine. Same-provider in-process teammates can run without tmux, but this team has teammates that require separate processes.', + details, + tmuxDetail: getTmuxDetail(tmuxStatus, tmuxStatusError), + memberWarningById, + }; +} + +export function useTmuxRuntimeReadiness(enabled: boolean): TmuxRuntimeReadiness { + const [status, setStatus] = useState(null); + const [loading, setLoading] = useState(enabled); + const [error, setError] = useState(null); + + const refresh = useCallback(async () => { + if (!enabled) { + setLoading(false); + return; + } + setLoading(true); + setError(null); + try { + if (typeof api.tmux?.getStatus !== 'function') { + throw new Error('tmux status API is not available. Restart the app.'); + } + const nextStatus = await api.tmux.getStatus(); + setStatus(nextStatus); + } catch (nextError) { + setError(nextError instanceof Error ? nextError.message : 'Failed to load tmux status'); + setStatus(null); + } finally { + setLoading(false); + } + }, [enabled]); + + useEffect(() => { + if (!enabled) { + setStatus(null); + setError(null); + setLoading(false); + return; + } + void refresh(); + }, [enabled, refresh]); + + useEffect(() => { + if (!enabled) { + return undefined; + } + if (typeof api.tmux?.onProgress !== 'function') { + return undefined; + } + return api.tmux.onProgress(() => { + void refresh(); + }); + }, [enabled, refresh]); + + const effectiveLoading = enabled && (loading || (!status && !error)); + + return useMemo( + () => ({ + status, + loading: effectiveLoading, + error, + refresh, + }), + [effectiveLoading, error, refresh, status] + ); +} diff --git a/src/renderer/components/team/members/LeadModelRow.test.tsx b/src/renderer/components/team/members/LeadModelRow.test.tsx index 04ed479a..21173533 100644 --- a/src/renderer/components/team/members/LeadModelRow.test.tsx +++ b/src/renderer/components/team/members/LeadModelRow.test.tsx @@ -20,8 +20,9 @@ vi.mock('@renderer/components/team/dialogs/LimitContextCheckbox', () => ({ vi.mock('@renderer/components/team/dialogs/TeamModelSelector', () => ({ getProviderScopedTeamModelLabel: (_providerId: string, model: string) => model || 'Default', getTeamProviderLabel: (providerId: string) => providerId, - OPENCODE_TEAM_LEAD_DISABLED_BADGE_LABEL: 'not teamlead', - OPENCODE_TEAM_LEAD_DISABLED_REASON: 'OpenCode is not available for team lead.', + OPENCODE_TEAM_LEAD_DISABLED_BADGE_LABEL: 'side lane', + OPENCODE_TEAM_LEAD_DISABLED_REASON: + 'OpenCode is teammate-only in this phase. Use Anthropic, Codex, or Gemini as the team lead, then add OpenCode as a teammate.', TeamModelSelector: () => React.createElement('div', null, 'team-model-selector'), })); diff --git a/src/renderer/hooks/useKeyboardShortcuts.ts b/src/renderer/hooks/useKeyboardShortcuts.ts index 007a01aa..d1c4d10a 100644 --- a/src/renderer/hooks/useKeyboardShortcuts.ts +++ b/src/renderer/hooks/useKeyboardShortcuts.ts @@ -16,6 +16,22 @@ import { useStore } from '../store'; const logger = createLogger('Hook:KeyboardShortcuts'); +export function isEditableShortcutTarget(target: EventTarget | null): boolean { + if (!(target instanceof Element)) { + return false; + } + + const editableElement = target.closest( + 'input, textarea, select, [role="textbox"], [contenteditable]' + ); + if (!editableElement) { + return false; + } + + const contentEditable = editableElement.getAttribute('contenteditable'); + return contentEditable == null || contentEditable.toLowerCase() !== 'false'; +} + export function useKeyboardShortcuts(): void { const { openTabs, @@ -77,6 +93,10 @@ export function useKeyboardShortcuts(): void { useEffect(() => { function handleKeyDown(event: KeyboardEvent): void { + if (isEditableShortcutTarget(event.target)) { + return; + } + // Check if Cmd (macOS) or Ctrl (Windows/Linux) is pressed const isMod = event.metaKey || event.ctrlKey; // Layout-independent key (uses event.code for letters/symbols) diff --git a/src/renderer/index.css b/src/renderer/index.css index 455760a8..1c3fb8ef 100644 --- a/src/renderer/index.css +++ b/src/renderer/index.css @@ -790,6 +790,20 @@ body.theme-transitioning { -webkit-app-region: no-drag; } +button, +input, +textarea, +select, +a[href], +[role='button'], +[role='combobox'], +[role='menuitem'], +[role='tab'], +[role='textbox'], +[contenteditable='true'] { + -webkit-app-region: no-drag; +} + /* Prevent drag region bleed-through on Windows: all fixed overlays must be no-drag */ .fixed { diff --git a/src/renderer/index.html b/src/renderer/index.html index dd89a9fa..687d6619 100644 --- a/src/renderer/index.html +++ b/src/renderer/index.html @@ -2,6 +2,10 @@ + = 65 && drive <= 90) || (drive >= 97 && drive <= 122)) && input[1] === ':'; - return hasDriveLetter && input.startsWith('\\Users\\', 2); + return hasDriveLetter && input.slice(2, 9).toLowerCase() === '\\users\\'; } export function resolveAbsolutePath(filePath: string, projectRoot?: string): string { let p = filePath; // Resolve ~ using home dir inferred from projectRoot - if (p.startsWith('~/') && projectRoot) { + if ((p.startsWith('~/') || p.startsWith('~\\')) && projectRoot) { const homeDir = inferHomeDir(projectRoot); if (homeDir) { - p = homeDir + p.slice(1); + p = joinDisplayPath(homeDir, p.slice(2)); } } // Make relative paths absolute by prepending projectRoot - if (projectRoot && !p.startsWith('/') && !p.startsWith('~') && !/^[A-Z]:[/\\]/.test(p)) { - p = projectRoot.replace(/[/\\]$/, '') + '/' + p; + if (projectRoot && !p.startsWith('/') && !p.startsWith('~') && !isWindowsAbsolutePath(p)) { + p = joinDisplayPath(projectRoot, p); } return p; diff --git a/test/main/utils/childProcess.test.ts b/test/main/utils/childProcess.test.ts index cc43bdd1..b2bf5f54 100644 --- a/test/main/utils/childProcess.test.ts +++ b/test/main/utils/childProcess.test.ts @@ -1,4 +1,8 @@ // @vitest-environment node +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs'; +import { tmpdir } from 'os'; +import path from 'path'; + import { describe, it, expect, beforeEach, afterEach, vi, type Mock } from 'vitest'; // Mock the entire child_process module so that we can inspect how our helpers @@ -31,6 +35,30 @@ function setPlatform(value: string) { // restore platform after tests const originalPlatform = process.platform; +function createGeneratedBunLauncher(): { dir: string; launcher: string; target: string } { + const dir = mkdtempSync(path.join(tmpdir(), 'cat-cli-launcher-')); + const targetDir = path.join(dir, 'dist'); + mkdirSync(targetDir, { recursive: true }); + const target = path.join(targetDir, 'cli.js'); + writeFileSync(target, 'console.log("ok")', 'utf8'); + const launcher = path.join(dir, 'cli-dev.cmd'); + writeFileSync( + launcher, + [ + '@echo off', + 'setlocal', + 'set "SCRIPT_DIR=%~dp0"', + 'set "TARGET=%SCRIPT_DIR%dist\\cli.js"', + ':run_target', + 'bun "%TARGET%" %*', + 'exit /b %ERRORLEVEL%', + '', + ].join('\r\n'), + 'utf8' + ); + return { dir, launcher, target }; +} + describe('cli child process helpers', () => { beforeEach(() => { vi.resetAllMocks(); @@ -79,6 +107,37 @@ describe('cli child process helpers', () => { expect(result).toBe(fake); }); + it('uses shell directly for Windows cmd launchers', () => { + setPlatform('win32'); + const fake = {} as any; + const spawnMock = child.spawn as unknown as Mock; + spawnMock.mockReturnValue(fake); + + const result = spawnCli('C:\\runtime\\cli-dev.cmd', ['--version']); + expect(spawnMock).toHaveBeenCalledTimes(1); + expect(spawnMock.mock.calls[0][0]).toContain('cli-dev.cmd'); + expect(spawnMock.mock.calls[0][1]).toMatchObject({ shell: true }); + expect(result).toBe(fake); + }); + + it('runs generated Bun cmd launchers directly to preserve percent args', () => { + setPlatform('win32'); + const fake = {} as any; + const spawnMock = child.spawn as unknown as Mock; + spawnMock.mockReturnValue(fake); + const { dir, launcher, target } = createGeneratedBunLauncher(); + try { + const result = spawnCli(launcher, ['--model', 'test%PATH%"arg']); + expect(spawnMock).toHaveBeenCalledTimes(1); + expect(spawnMock.mock.calls[0][0]).toBe('bun'); + expect(spawnMock.mock.calls[0][1]).toEqual([target, '--model', 'test%PATH%"arg']); + expect(spawnMock.mock.calls[0][2]).not.toHaveProperty('shell'); + expect(result).toBe(fake); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it('uses shell directly when path contains non-ASCII on windows', () => { setPlatform('win32'); const fake = {} as any; @@ -170,6 +229,44 @@ describe('cli child process helpers', () => { expect(result.stdout).toBe('ok'); }); + it('skips straight to shell for Windows cmd launchers', async () => { + setPlatform('win32'); + const execFileMock = child.execFile as unknown as Mock; + const execMock = child.exec as unknown as Mock; + execMock.mockImplementation((_cmd: string, _opts: unknown, cb: ExecCallback) => { + cb(null, '0.0.8', ''); + return {} as any; + }); + + const result = await execCli('C:\\runtime\\cli-dev.cmd', ['--version']); + expect(execFileMock).not.toHaveBeenCalled(); + expect(execMock).toHaveBeenCalled(); + expect(result.stdout).toBe('0.0.8'); + }); + + it('executes generated Bun cmd launchers directly to preserve percent args', async () => { + setPlatform('win32'); + const execFileMock = child.execFile as unknown as Mock; + const execMock = child.exec as unknown as Mock; + execFileMock.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, cb: ExecCallback) => { + cb(null, 'ok', ''); + return {} as any; + } + ); + const { dir, launcher, target } = createGeneratedBunLauncher(); + try { + const result = await execCli(launcher, ['--model', 'test%PATH%"arg']); + expect(execFileMock).toHaveBeenCalledTimes(1); + expect(execFileMock.mock.calls[0][0]).toBe('bun'); + expect(execFileMock.mock.calls[0][1]).toEqual([target, '--model', 'test%PATH%"arg']); + expect(execMock).not.toHaveBeenCalled(); + expect(result.stdout).toBe('ok'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it('skips straight to shell when path contains non-ASCII on windows', async () => { setPlatform('win32'); const execFileMock = child.execFile as unknown as Mock; @@ -198,12 +295,35 @@ describe('cli child process helpers', () => { await execCli('C:\\Users\\Алексей\\bin\\claude.cmd', ['--model', 'test%PATH%"arg']); const shellCmd = execMock.mock.calls[0][0] as string; - // %PATH% must become %%PATH%% to prevent cmd.exe env var expansion - expect(shellCmd).toContain('%%PATH%%'); - // double quote inside arg must become "" (cmd.exe escaping) - expect(shellCmd).toContain('""arg'); - // should NOT contain \" (Unix-style escaping) - expect(shellCmd).not.toContain('\\"'); + // Keep % outside quoted chunks so cmd.exe does not expand it as an env var. + expect(shellCmd).toContain('^%"PATH"^%'); + expect(shellCmd).not.toContain('%PATH%'); + expect(shellCmd).not.toContain('%%PATH%%'); + // Quotes inside JSON-like args must survive cmd.exe and the target argv parser. + expect(shellCmd).toContain('\\"arg'); + expect(shellCmd).not.toContain('""arg'); + }); + + it('keeps inline settings JSON as one argv-safe argument for Windows cmd launchers', async () => { + setPlatform('win32'); + const execMock = child.exec as unknown as Mock; + execMock.mockImplementation((_cmd: string, _opts: unknown, cb: ExecCallback) => { + cb(null, 'ok', ''); + return {} as any; + }); + + await execCli('C:\\runtime\\cli-dev.cmd', [ + '--settings', + '{"codex":{"forced_login_method":"chatgpt"}}', + 'runtime', + 'status', + '--json', + '--provider', + 'codex', + ]); + const shellCmd = execMock.mock.calls[0][0] as string; + expect(shellCmd).toContain('"{\\"codex\\":{\\"forced_login_method\\":\\"chatgpt\\"}}"'); + expect(shellCmd).not.toContain('{""codex"":'); }); it('shell: true cannot be overridden by caller options', () => { diff --git a/test/renderer/components/runtime/ProviderRuntimeSettingsDialog.test.ts b/test/renderer/components/runtime/ProviderRuntimeSettingsDialog.test.ts index ab517ea9..7e1bf142 100644 --- a/test/renderer/components/runtime/ProviderRuntimeSettingsDialog.test.ts +++ b/test/renderer/components/runtime/ProviderRuntimeSettingsDialog.test.ts @@ -119,8 +119,9 @@ vi.mock('@renderer/components/ui/dialog', () => ({ })); vi.mock('@renderer/components/ui/input', () => ({ - Input: (props: React.InputHTMLAttributes) => - React.createElement('input', props), + Input: React.forwardRef>( + (props, ref) => React.createElement('input', { ...props, ref }) + ), })); vi.mock('@renderer/components/ui/label', () => ({ @@ -438,6 +439,16 @@ function findButtonByText(container: HTMLElement, text: string): HTMLButtonEleme return button; } +function setInputValue(input: HTMLInputElement, value: string): void { + const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set; + if (!setter) { + throw new Error('HTMLInputElement value setter not found'); + } + + setter.call(input, value); + input.dispatchEvent(new Event('input', { bubbles: true })); +} + describe('ProviderRuntimeSettingsDialog', () => { beforeEach(() => { vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true); @@ -563,6 +574,65 @@ describe('ProviderRuntimeSettingsDialog', () => { expect(onRefreshProvider).toHaveBeenCalledWith('anthropic'); }); + it('accepts and saves a typed Anthropic API key from provider settings', async () => { + const host = document.createElement('div'); + document.body.appendChild(host); + const root = createRoot(host); + const onRefreshProvider = vi.fn(() => Promise.resolve(undefined)); + + await act(async () => { + root.render( + React.createElement(ProviderRuntimeSettingsDialog, { + open: true, + onOpenChange: vi.fn(), + providers: [ + createAnthropicProvider({ + authenticated: false, + authMethod: null, + apiKeyConfigured: false, + apiKeySource: null, + apiKeySourceLabel: null, + }), + ], + initialProviderId: 'anthropic', + onSelectBackend: vi.fn(), + onRefreshProvider, + }) + ); + await Promise.resolve(); + }); + + await act(async () => { + findButtonByText(host, 'Set API key').click(); + await Promise.resolve(); + }); + + const input = host.querySelector('#anthropic-api-key') as HTMLInputElement | null; + expect(input).not.toBeNull(); + + await act(async () => { + setInputValue(input!, 'sk-ant-test-key'); + await Promise.resolve(); + }); + + expect(input!.value).toBe('sk-ant-test-key'); + + await act(async () => { + findButtonByText(host, 'Save key').click(); + await Promise.resolve(); + }); + + expect(storeState.saveApiKey).toHaveBeenCalledWith( + expect.objectContaining({ + envVarName: 'ANTHROPIC_API_KEY', + name: 'Anthropic API Key', + scope: 'user', + value: 'sk-ant-test-key', + }) + ); + expect(onRefreshProvider).toHaveBeenCalledWith('anthropic'); + }); + it('shows native-only Codex connection copy and API-key management without login actions', async () => { const host = document.createElement('div'); document.body.appendChild(host); diff --git a/test/renderer/components/team/TeamModelSelectorDisabledState.test.ts b/test/renderer/components/team/TeamModelSelectorDisabledState.test.ts index d0edde5c..b63b5f0e 100644 --- a/test/renderer/components/team/TeamModelSelectorDisabledState.test.ts +++ b/test/renderer/components/team/TeamModelSelectorDisabledState.test.ts @@ -813,10 +813,11 @@ describe('TeamModelSelector disabled Codex models', () => { value: '', onValueChange: () => undefined, providerDisabledReasonById: { - opencode: 'OpenCode is not available for team lead.', + opencode: + 'OpenCode is teammate-only in this phase. Use Anthropic, Codex, or Gemini as the team lead, then add OpenCode as a teammate.', }, providerDisabledBadgeLabelById: { - opencode: 'not teamlead', + opencode: 'side lane', }, }) ); @@ -827,8 +828,10 @@ describe('TeamModelSelector disabled Codex models', () => { button.textContent?.includes('OpenCode') ); expect(openCodeButton?.hasAttribute('disabled')).toBe(true); - expect(openCodeButton?.getAttribute('title')).toBe('OpenCode is not available for team lead.'); - expect(openCodeButton?.textContent).toContain('not teamlead'); + expect(openCodeButton?.getAttribute('title')).toBe( + 'OpenCode is teammate-only in this phase. Use Anthropic, Codex, or Gemini as the team lead, then add OpenCode as a teammate.' + ); + expect(openCodeButton?.textContent).toContain('side lane'); await act(async () => { openCodeButton?.dispatchEvent(new MouseEvent('click', { bubbles: true })); diff --git a/test/renderer/components/team/dialogs/LaunchTeamDialog.test.ts b/test/renderer/components/team/dialogs/LaunchTeamDialog.test.ts index 68d0c90d..de4af5c8 100644 --- a/test/renderer/components/team/dialogs/LaunchTeamDialog.test.ts +++ b/test/renderer/components/team/dialogs/LaunchTeamDialog.test.ts @@ -44,6 +44,44 @@ vi.mock('@renderer/api', () => ({ replaceMembers: vi.fn(async () => {}), prepareProvisioning: vi.fn(async () => ({})), }, + tmux: { + getStatus: vi.fn(() => + Promise.resolve({ + platform: 'win32', + nativeSupported: false, + checkedAt: '2026-04-25T00:00:00.000Z', + host: { + available: false, + version: null, + binaryPath: null, + error: null, + }, + effective: { + available: true, + location: 'wsl', + version: '3.4', + binaryPath: '/usr/bin/tmux', + runtimeReady: true, + detail: 'tmux is ready', + }, + error: null, + autoInstall: { + supported: false, + strategy: 'manual', + packageManagerLabel: null, + requiresTerminalInput: false, + requiresAdmin: false, + requiresRestart: false, + mayOpenExternalWindow: false, + reasonIfUnsupported: null, + manualHints: [], + }, + wsl: null, + wslPreference: null, + }) + ), + onProgress: vi.fn(() => vi.fn()), + }, }, })); @@ -313,8 +351,9 @@ vi.mock('@renderer/components/team/dialogs/TeamModelSelector', () => ({ computeEffectiveTeamModel: (model: string) => model || undefined, formatTeamModelSummary: (providerId: string, model: string, effort?: string) => [providerId, model, effort].filter(Boolean).join(' '), - OPENCODE_TEAM_LEAD_DISABLED_BADGE_LABEL: 'not teamlead', - OPENCODE_TEAM_LEAD_DISABLED_REASON: 'OpenCode is not available for team lead.', + OPENCODE_TEAM_LEAD_DISABLED_BADGE_LABEL: 'side lane', + OPENCODE_TEAM_LEAD_DISABLED_REASON: + 'OpenCode is teammate-only in this phase. Use Anthropic, Codex, or Gemini as the team lead, then add OpenCode as a teammate.', })); vi.mock('@renderer/components/team/dialogs/EffortLevelSelector', () => ({ diff --git a/test/renderer/components/team/dialogs/teammateRuntimeCompatibility.test.ts b/test/renderer/components/team/dialogs/teammateRuntimeCompatibility.test.ts new file mode 100644 index 00000000..db9ba6d9 --- /dev/null +++ b/test/renderer/components/team/dialogs/teammateRuntimeCompatibility.test.ts @@ -0,0 +1,159 @@ +import { describe, expect, it } from 'vitest'; + +import { analyzeTeammateRuntimeCompatibility } from '@renderer/components/team/dialogs/teammateRuntimeCompatibility'; + +import type { TmuxStatus } from '@features/tmux-installer/contracts'; + +function buildTmuxStatus(ready: boolean): TmuxStatus { + return { + platform: 'win32', + nativeSupported: false, + checkedAt: '2026-04-25T00:00:00.000Z', + host: { + available: false, + version: null, + binaryPath: null, + error: null, + }, + effective: { + available: ready, + location: ready ? 'wsl' : null, + version: ready ? '3.4' : null, + binaryPath: ready ? '/usr/bin/tmux' : null, + runtimeReady: ready, + detail: ready ? 'tmux is ready' : 'tmux is not available', + }, + error: null, + autoInstall: { + supported: false, + strategy: 'manual', + packageManagerLabel: null, + requiresTerminalInput: false, + requiresAdmin: false, + requiresRestart: false, + mayOpenExternalWindow: false, + reasonIfUnsupported: null, + manualHints: [], + }, + wsl: null, + wslPreference: null, + }; +} + +describe('analyzeTeammateRuntimeCompatibility', () => { + it('allows same-provider non-Codex teammates without tmux', () => { + const result = analyzeTeammateRuntimeCompatibility({ + leadProviderId: 'anthropic', + members: [{ id: 'alice', name: 'alice', providerId: 'anthropic' }], + tmuxStatus: buildTmuxStatus(false), + tmuxStatusLoading: false, + tmuxStatusError: null, + }); + + expect(result.blocksSubmission).toBe(false); + expect(result.visible).toBe(false); + expect(result.memberWarningById).toEqual({}); + }); + + it('blocks mixed-provider teammates when tmux is unavailable', () => { + const result = analyzeTeammateRuntimeCompatibility({ + leadProviderId: 'anthropic', + members: [{ id: 'bob', name: 'bob', providerId: 'codex' }], + tmuxStatus: buildTmuxStatus(false), + tmuxStatusLoading: false, + tmuxStatusError: null, + }); + + expect(result.blocksSubmission).toBe(true); + expect(result.details.join('\n')).toContain('Mixed providers'); + expect(result.memberWarningById.bob).toContain('same provider as the Anthropic lead'); + }); + + it('allows OpenCode secondary-lane teammates without tmux under a non-OpenCode lead', () => { + const result = analyzeTeammateRuntimeCompatibility({ + leadProviderId: 'anthropic', + members: [{ id: 'bob', name: 'bob', providerId: 'opencode' }], + tmuxStatus: buildTmuxStatus(false), + tmuxStatusLoading: false, + tmuxStatusError: null, + }); + + expect(result.blocksSubmission).toBe(false); + expect(result.visible).toBe(false); + expect(result.memberWarningById).toEqual({}); + }); + + it('blocks OpenCode-led mixed teams independently of tmux readiness', () => { + const result = analyzeTeammateRuntimeCompatibility({ + leadProviderId: 'opencode', + members: [{ id: 'bob', name: 'bob', providerId: 'anthropic' }], + tmuxStatus: buildTmuxStatus(true), + tmuxStatusLoading: false, + tmuxStatusError: null, + }); + + expect(result.blocksSubmission).toBe(true); + expect(result.title).toBe('OpenCode cannot lead mixed-provider teams'); + expect(result.message).toContain('OpenCode-led mixed teams are not supported'); + expect(result.memberWarningById.bob).toContain('OpenCode cannot be the team lead'); + }); + + it('blocks same-provider Codex native teammates when tmux is unavailable', () => { + const result = analyzeTeammateRuntimeCompatibility({ + leadProviderId: 'codex', + leadProviderBackendId: 'codex-native', + members: [{ id: 'jack', name: 'jack', providerId: 'codex' }], + tmuxStatus: buildTmuxStatus(false), + tmuxStatusLoading: false, + tmuxStatusError: null, + }); + + expect(result.blocksSubmission).toBe(true); + expect(result.title).toBe('Codex teammates need tmux before they can run'); + expect(result.message).toContain('The Codex lead can run without tmux'); + expect(result.details.join('\n')).toContain('Codex native teammates'); + expect(result.memberWarningById.jack).toContain('Codex native teammates require'); + }); + + it('allows separate-process teammate requirements when tmux is ready', () => { + const result = analyzeTeammateRuntimeCompatibility({ + leadProviderId: 'anthropic', + members: [{ id: 'bob', name: 'bob', providerId: 'codex' }], + tmuxStatus: buildTmuxStatus(true), + tmuxStatusLoading: false, + tmuxStatusError: null, + }); + + expect(result.blocksSubmission).toBe(false); + expect(result.visible).toBe(false); + }); + + it('ignores teammate runtime requirements for solo teams', () => { + const result = analyzeTeammateRuntimeCompatibility({ + leadProviderId: 'codex', + leadProviderBackendId: 'codex-native', + members: [{ id: 'jack', name: 'jack', providerId: 'codex' }], + soloTeam: true, + tmuxStatus: buildTmuxStatus(false), + tmuxStatusLoading: false, + tmuxStatusError: null, + }); + + expect(result.blocksSubmission).toBe(false); + expect(result.visible).toBe(false); + }); + + it('blocks explicit tmux teammate mode when tmux is unavailable', () => { + const result = analyzeTeammateRuntimeCompatibility({ + leadProviderId: 'anthropic', + members: [{ id: 'alice', name: 'alice', providerId: 'anthropic' }], + extraCliArgs: '--teammate-mode tmux', + tmuxStatus: buildTmuxStatus(false), + tmuxStatusLoading: false, + tmuxStatusError: null, + }); + + expect(result.blocksSubmission).toBe(true); + expect(result.details).toContain('Custom CLI args force --teammate-mode tmux.'); + }); +}); diff --git a/test/renderer/hooks/useKeyboardShortcuts.test.ts b/test/renderer/hooks/useKeyboardShortcuts.test.ts new file mode 100644 index 00000000..c4674875 --- /dev/null +++ b/test/renderer/hooks/useKeyboardShortcuts.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest'; + +import { isEditableShortcutTarget } from '@renderer/hooks/useKeyboardShortcuts'; + +describe('isEditableShortcutTarget', () => { + it('treats native form fields as editable shortcut targets', () => { + const input = document.createElement('input'); + const textarea = document.createElement('textarea'); + const select = document.createElement('select'); + + expect(isEditableShortcutTarget(input)).toBe(true); + expect(isEditableShortcutTarget(textarea)).toBe(true); + expect(isEditableShortcutTarget(select)).toBe(true); + }); + + it('treats nested contenteditable textboxes as editable shortcut targets', () => { + const textbox = document.createElement('div'); + textbox.setAttribute('role', 'textbox'); + const child = document.createElement('span'); + textbox.appendChild(child); + + expect(isEditableShortcutTarget(child)).toBe(true); + }); + + it('does not mark regular buttons as editable targets', () => { + expect(isEditableShortcutTarget(document.createElement('button'))).toBe(false); + }); +}); diff --git a/test/renderer/utils/pathDisplay.test.ts b/test/renderer/utils/pathDisplay.test.ts new file mode 100644 index 00000000..20c5976b --- /dev/null +++ b/test/renderer/utils/pathDisplay.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from 'vitest'; + +import { + formatProjectPath, + resolveAbsolutePath, + shortenDisplayPath, +} from '../../../src/renderer/utils/pathDisplay'; + +describe('pathDisplay Windows paths', () => { + it('treats lowercase drive paths as absolute', () => { + expect(resolveAbsolutePath('c:\\Users\\Alice\\repo\\src\\app.ts', 'C:\\Users\\Alice\\repo')).toBe( + 'c:\\Users\\Alice\\repo\\src\\app.ts' + ); + }); + + it('shortens project-root relative paths case-insensitively on Windows', () => { + expect(shortenDisplayPath('c:\\Users\\Alice\\repo\\src\\app.ts', 'C:\\Users\\Alice\\Repo')).toBe( + 'src\\app.ts' + ); + }); + + it('formats lowercase Windows user paths with a home marker', () => { + expect(formatProjectPath('c:\\users\\Alice\\repo')).toBe('~/repo'); + }); + + it('resolves home paths from lowercase Windows user roots', () => { + expect(resolveAbsolutePath('~/repo/src/app.ts', 'c:\\users\\Alice\\workspace')).toBe( + 'c:\\users\\Alice\\repo\\src\\app.ts' + ); + }); + + it('resolves relative paths using the project root separator', () => { + expect(resolveAbsolutePath('src/app.ts', 'C:\\Users\\Alice\\repo')).toBe( + 'C:\\Users\\Alice\\repo\\src\\app.ts' + ); + }); +}); From f2b70242263312ac37a627b24b85f13e145c601f Mon Sep 17 00:00:00 2001 From: iliya Date: Sat, 25 Apr 2026 19:12:11 +0300 Subject: [PATCH 2/2] fix(windows): support path mentions and editor launch --- src/main/http/validation.ts | 17 +++- src/main/ipc/config.ts | 80 ++++++++++++++--- src/main/utils/childProcess.ts | 6 +- .../components/chat/UserChatGroup.tsx | 37 ++++---- src/renderer/hooks/useFileSuggestions.ts | 16 ++++ src/renderer/utils/groupTransformer.ts | 86 +++++++++++++------ test/main/utils/childProcess.test.ts | 16 +++- .../renderer/hooks/useFileSuggestions.test.ts | 9 +- test/renderer/utils/fileReferences.test.ts | 21 +++++ 9 files changed, 226 insertions(+), 62 deletions(-) diff --git a/src/main/http/validation.ts b/src/main/http/validation.ts index c9d9ddcd..9b15c606 100644 --- a/src/main/http/validation.ts +++ b/src/main/http/validation.ts @@ -20,11 +20,20 @@ const logger = createLogger('HTTP:validation'); * Prevents path traversal attacks. */ function isPathContained(fullPath: string, basePath: string): boolean { - const normalizedFull = path.normalize(fullPath); - const normalizedBase = path.normalize(basePath); + const normalizedFull = normalizeForContainment(fullPath); + const normalizedBase = normalizeForContainment(basePath); return normalizedFull === normalizedBase || normalizedFull.startsWith(normalizedBase + path.sep); } +function normalizeForContainment(value: string): string { + const resolved = path.resolve(value); + return process.platform === 'win32' ? resolved.toLowerCase() : resolved; +} + +function resolveProjectPath(projectPath: string, requestedPath: string): string { + return path.isAbsolute(requestedPath) ? requestedPath : path.join(projectPath, requestedPath); +} + export function registerValidationRoutes(app: FastifyInstance): void { // Validate path app.post<{ Body: { relativePath: string; projectPath: string } }>( @@ -32,7 +41,7 @@ export function registerValidationRoutes(app: FastifyInstance): void { async (request) => { try { const { relativePath, projectPath } = request.body; - const fullPath = path.join(projectPath, relativePath); + const fullPath = resolveProjectPath(projectPath, relativePath); if (!isPathContained(fullPath, projectPath)) { logger.warn('validate-path blocked path traversal attempt:', relativePath); @@ -57,7 +66,7 @@ export function registerValidationRoutes(app: FastifyInstance): void { // Validate all mentions in parallel with async I/O const entries = await Promise.all( mentions.map(async (mention) => { - const fullPath = path.join(projectPath, mention.value); + const fullPath = resolveProjectPath(projectPath, mention.value); if (!isPathContained(fullPath, projectPath)) { return [`@${mention.value}`, false] as const; } diff --git a/src/main/ipc/config.ts b/src/main/ipc/config.ts index bcc81789..d4b40c2f 100644 --- a/src/main/ipc/config.ts +++ b/src/main/ipc/config.ts @@ -18,10 +18,11 @@ */ import { syncTelemetryFlag } from '@main/sentry'; +import { quoteWindowsCmdArg } from '@main/utils/childProcess'; import { getAutoDetectedClaudeBasePath, getClaudeBasePath } from '@main/utils/pathDecoder'; import { getErrorMessage } from '@shared/utils/errorHandling'; import { createLogger } from '@shared/utils/logger'; -import { execFile } from 'child_process'; +import { execFile, execFileSync, spawn } from 'child_process'; import { BrowserWindow, dialog, type IpcMain, type IpcMainInvokeEvent } from 'electron'; import * as fs from 'fs'; import * as path from 'path'; @@ -57,6 +58,72 @@ let onClaudeRootPathUpdated: ((claudeRootPath: string | null) => Promise | null; let onAgentLanguageUpdated: ((newLangCode: string) => Promise | void) | null = null; +function isPathLikeCommand(command: string): boolean { + return /[\\/]/.test(command) || /^[A-Za-z]:/.test(command); +} + +function resolveWindowsEditorCommand(editor: string): string { + if (process.platform !== 'win32' || isPathLikeCommand(editor)) { + return editor; + } + + try { + const whereExe = path.join(process.env.SystemRoot ?? 'C:\\Windows', 'System32', 'where.exe'); + const output = execFileSync(whereExe, [editor], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, + }); + return output.trim().split(/\r?\n/)[0] || editor; + } catch { + return editor; + } +} + +function needsWindowsShell(command: string): boolean { + if (process.platform !== 'win32') return false; + const extension = path.extname(command).toLowerCase(); + return extension === '.cmd' || extension === '.bat'; +} + +function launchExternalEditor(editor: string, configPath: string): Promise { + return new Promise((resolve, reject) => { + const resolvedEditor = resolveWindowsEditorCommand(editor); + const launchOptions = { + detached: true, + stdio: 'ignore' as const, + windowsHide: true, + }; + let child: ReturnType; + if (needsWindowsShell(resolvedEditor)) { + const command = [resolvedEditor, configPath].map(quoteWindowsCmdArg).join(' '); + // eslint-disable-next-line sonarjs/os-command -- Windows .cmd launchers require cmd.exe; editor path is resolved via where.exe and args are cmd-escaped. + child = spawn(command, { + ...launchOptions, + shell: true, + }); + } else { + child = spawn(resolvedEditor, [configPath], launchOptions); + } + + let settled = false; + function settle(fn: () => void): void { + if (settled) return; + settled = true; + clearTimeout(timer); + fn(); + } + const timer = setTimeout(() => { + child.unref(); + settle(() => resolve()); + }, 500); + + child.on('error', (err) => { + settle(() => reject(err)); + }); + }); +} + /** * Initializes config handlers with callbacks that require app-level services. */ @@ -607,16 +674,7 @@ async function handleOpenInEditor(_event: IpcMainInvokeEvent): Promise((resolve, reject) => { - const child = execFile(editor, [configPath], { timeout: 5000 }); - // If the process spawns successfully, resolve after a short delay - // (editors typically fork and the parent exits quickly) - const timer = setTimeout(() => resolve(), 500); - child.on('error', (err) => { - clearTimeout(timer); - reject(err); - }); - }); + await launchExternalEditor(editor, configPath); return { success: true }; } catch { // Editor not found, try next diff --git a/src/main/utils/childProcess.ts b/src/main/utils/childProcess.ts index b4faa4f5..30319e9c 100644 --- a/src/main/utils/childProcess.ts +++ b/src/main/utils/childProcess.ts @@ -192,13 +192,17 @@ function quoteCmdChunk(chunk: string): string { return `"${escaped}"`; } -function quoteArg(arg: string): string { +export function quoteWindowsCmdArg(arg: string): string { if (/[^A-Za-z0-9_\-/.]/.test(arg)) { return arg.split('%').map(quoteCmdChunk).join('^%'); } return arg; } +function quoteArg(arg: string): string { + return quoteWindowsCmdArg(arg); +} + /** Env vars injected into every spawned Claude CLI process. */ const CLI_ENV_DEFAULTS: Record = { CLAUDE_HOOK_JUDGE_MODE: 'true', diff --git a/src/renderer/components/chat/UserChatGroup.tsx b/src/renderer/components/chat/UserChatGroup.tsx index 4baeff2b..ef7fa9a5 100644 --- a/src/renderer/components/chat/UserChatGroup.tsx +++ b/src/renderer/components/chat/UserChatGroup.tsx @@ -8,6 +8,7 @@ import { useTabUI } from '@renderer/hooks/useTabUI'; import { useTheme } from '@renderer/hooks/useTheme'; import { useStore } from '@renderer/store'; import { selectResolvedMembersForTeamName } from '@renderer/store/slices/teamSlice'; +import { extractFileReferenceTokens } from '@renderer/utils/groupTransformer'; import { REHYPE_PLUGINS } from '@renderer/utils/markdownPlugins'; import { buildMemberColorMap } from '@renderer/utils/memberHelpers'; import { linkifyAllMentionsInMarkdown } from '@renderer/utils/mentionLinkify'; @@ -33,9 +34,6 @@ import type { UserGroup } from '@renderer/types/groups'; const logger = createLogger('Component:UserChatGroup'); -// Pattern for @paths only (file references) -const PATH_PATTERN = /@([^\s,)}\]]+)/g; - interface UserChatGroupProps { userGroup: UserGroup; } @@ -46,24 +44,22 @@ interface UserChatGroupProps { */ // eslint-disable-next-line sonarjs/function-return-type -- React child manipulation inherently returns mixed node types function highlightTextNode(text: string, validatedPaths: Record): React.ReactNode { - const pathPattern = /@[^\s,)}\]]+/g; + const pathReferences = extractFileReferenceTokens(text); const parts: React.ReactNode[] = []; let lastIndex = 0; - let match; - pathPattern.lastIndex = 0; - while ((match = pathPattern.exec(text)) !== null) { - if (match.index > lastIndex) { - parts.push(text.slice(lastIndex, match.index)); + for (const reference of pathReferences) { + if (reference.startIndex > lastIndex) { + parts.push(text.slice(lastIndex, reference.startIndex)); } - const fullMatch = match[0]; + const fullMatch = reference.raw; const isValid = validatedPaths[fullMatch] === true; if (isValid) { parts.push( parts.push(fullMatch); } - lastIndex = match.index + fullMatch.length; + lastIndex = reference.endIndex; } if (lastIndex < text.length) { @@ -456,13 +452,10 @@ const UserChatGroupInner = ({ userGroup }: Readonly): React. // Extract @path mentions from text const pathMentions = useMemo(() => { if (!textContent) return []; - const result: { value: string; raw: string }[] = []; - const pathPattern = new RegExp(PATH_PATTERN.source, PATH_PATTERN.flags); - let match; - while ((match = pathPattern.exec(textContent)) !== null) { - result.push({ value: match[1], raw: match[0] }); - } - return result; + return extractFileReferenceTokens(textContent).map((reference) => ({ + value: reference.path, + raw: reference.raw, + })); }, [textContent]); // Validate @path mentions via IPC @@ -475,7 +468,11 @@ const UserChatGroupInner = ({ userGroup }: Readonly): React. const toValidate = pathMentions.map((m) => ({ type: 'path' as const, value: m.value })); const results = await api.validateMentions(toValidate, projectPath); if (isCurrent) { - setValidatedPaths(results); + setValidatedPaths( + Object.fromEntries( + pathMentions.map((mention) => [mention.raw, results[`@${mention.value}`] === true]) + ) + ); } } catch (err) { logger.error('Path validation failed:', err); diff --git a/src/renderer/hooks/useFileSuggestions.ts b/src/renderer/hooks/useFileSuggestions.ts index 605d99db..15b8541e 100644 --- a/src/renderer/hooks/useFileSuggestions.ts +++ b/src/renderer/hooks/useFileSuggestions.ts @@ -19,6 +19,7 @@ import type { QuickOpenFile } from '@shared/types/editor'; const MAX_FILE_SUGGESTIONS = 8; const MAX_FOLDER_SUGGESTIONS = 5; +const MENTION_PATH_QUOTE_NEEDED = /[\s,)}\]"']/; export interface UseFileSuggestionsResult { suggestions: MentionSuggestion[]; @@ -35,6 +36,19 @@ interface DerivedFolder { absolutePath: string; } +export function formatFileMentionPath(relativePath: string): string { + if (!MENTION_PATH_QUOTE_NEEDED.test(relativePath)) { + return relativePath; + } + if (!relativePath.includes('"')) { + return `"${relativePath}"`; + } + if (!relativePath.includes("'")) { + return `'${relativePath}'`; + } + return `"${relativePath.replace(/"/g, '')}"`; +} + /** * Extracts unique directories from a list of file paths. * Returns directories sorted by depth (shallower first), then alphabetically. @@ -94,6 +108,7 @@ export function filterFileSuggestions(files: QuickOpenFile[], query: string): Me type: 'file', filePath: f.path, relativePath: f.relativePath, + insertText: formatFileMentionPath(f.relativePath), }); } } @@ -127,6 +142,7 @@ export function filterFolderSuggestions( type: 'folder', filePath: f.absolutePath, relativePath: f.relativePath, + insertText: formatFileMentionPath(f.relativePath), }); } } diff --git a/src/renderer/utils/groupTransformer.ts b/src/renderer/utils/groupTransformer.ts index 0a975adf..274a0699 100644 --- a/src/renderer/utils/groupTransformer.ts +++ b/src/renderer/utils/groupTransformer.ts @@ -353,17 +353,21 @@ const KNOWN_DIRS = new Set([ 'node_modules', ]); -/** - * Simple pattern for detecting @ mentions that could be file paths. - * The filtering logic in extractFileReferences determines validity. - */ -const FILE_REF_PATTERN = /@([~a-zA-Z0-9._/-]+)/g; +export type FileReferenceToken = FileReference & { + startIndex: number; + endIndex: number; +}; + +const UNQUOTED_FILE_REF_STOP = /[\s,)}\]]/; /** * Checks if a path looks like a valid file reference. * Must start with known dir, contain /, or start with ./ or ../ */ function isValidFileRef(path: string): boolean { + if (/^[A-Za-z]:[\\/]/.test(path) || path.startsWith('\\\\')) { + return true; + } // Check for relative path indicators if (isRelativePath(path)) { return true; @@ -385,6 +389,58 @@ function isValidFileRef(path: string): boolean { return false; } +function readFileRefAt(text: string, atIndex: number): FileReferenceToken | null { + const valueStart = atIndex + 1; + const firstChar = text[valueStart]; + if (!firstChar) return null; + + let path = ''; + let endIndex = valueStart; + + if (firstChar === '"' || firstChar === "'") { + const quote = firstChar; + const quotedStart = valueStart + 1; + const quotedEnd = text.indexOf(quote, quotedStart); + if (quotedEnd < 0) return null; + path = text.slice(quotedStart, quotedEnd); + endIndex = quotedEnd + 1; + } else { + while (endIndex < text.length && !UNQUOTED_FILE_REF_STOP.test(text[endIndex])) { + endIndex += 1; + } + path = text.slice(valueStart, endIndex); + } + + if (!path || !isValidFileRef(path)) return null; + return { + path, + raw: text.slice(atIndex, endIndex), + startIndex: atIndex, + endIndex, + }; +} + +export function extractFileReferenceTokens(text: string): FileReferenceToken[] { + if (!text) return []; + + const references: FileReferenceToken[] = []; + let index = 0; + while (index < text.length) { + const atIndex = text.indexOf('@', index); + if (atIndex < 0) break; + + const reference = readFileRefAt(text, atIndex); + if (reference) { + references.push(reference); + index = reference.endIndex; + } else { + index = atIndex + 1; + } + } + + return references; +} + /** * Extracts file references (@file.ts) from text. * @@ -392,25 +448,7 @@ function isValidFileRef(path: string): boolean { * @returns Array of FileReference objects */ export function extractFileReferences(text: string): FileReference[] { - if (!text) return []; - - const references: FileReference[] = []; - // Reset regex state before use - FILE_REF_PATTERN.lastIndex = 0; - let match: RegExpExecArray | null; - - while ((match = FILE_REF_PATTERN.exec(text)) !== null) { - const [fullMatch, path] = match; - // Only include if it looks like a valid file reference - if (isValidFileRef(path)) { - references.push({ - path, - raw: fullMatch, - }); - } - } - - return references; + return extractFileReferenceTokens(text).map(({ path, raw }) => ({ path, raw })); } // ============================================================================= diff --git a/test/main/utils/childProcess.test.ts b/test/main/utils/childProcess.test.ts index b2bf5f54..ff44665f 100644 --- a/test/main/utils/childProcess.test.ts +++ b/test/main/utils/childProcess.test.ts @@ -19,7 +19,12 @@ vi.mock('child_process', async (importOriginal) => { // Import after the mock call so that the mocked module is returned. import * as child from 'child_process'; -import { execCli, killTrackedCliProcesses, spawnCli } from '@main/utils/childProcess'; +import { + execCli, + killTrackedCliProcesses, + quoteWindowsCmdArg, + spawnCli, +} from '@main/utils/childProcess'; type ExecCallback = (error: Error | null, stdout: string, stderr: string) => void; @@ -68,6 +73,15 @@ describe('cli child process helpers', () => { setPlatform(originalPlatform); }); + describe('quoteWindowsCmdArg', () => { + it('keeps percent signs literal in cmd.exe command strings', () => { + const quoted = quoteWindowsCmdArg('C:\\Users\\Alice\\a%PATH%b.txt'); + expect(quoted).toContain('"C:\\Users\\Alice\\a"^%"PATH"^%"b.txt"'); + expect(quoted).not.toContain('%PATH%'); + expect(quoted).not.toContain('%%PATH%%'); + }); + }); + describe('spawnCli', () => { it('calls spawn directly when path is ascii on windows', () => { setPlatform('win32'); diff --git a/test/renderer/hooks/useFileSuggestions.test.ts b/test/renderer/hooks/useFileSuggestions.test.ts index d6966a8e..1f916937 100644 --- a/test/renderer/hooks/useFileSuggestions.test.ts +++ b/test/renderer/hooks/useFileSuggestions.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { filterFileSuggestions } from '@renderer/hooks/useFileSuggestions'; +import { filterFileSuggestions, formatFileMentionPath } from '@renderer/hooks/useFileSuggestions'; import type { QuickOpenFile } from '@shared/types/editor'; @@ -43,6 +43,7 @@ describe('filterFileSuggestions', () => { expect(results[0].type).toBe('file'); expect(results[0].filePath).toBe('/project/src/test.ts'); expect(results[0].relativePath).toBe('src/test.ts'); + expect(results[0].insertText).toBe('src/test.ts'); }); it('filters by relative path', () => { @@ -94,4 +95,10 @@ describe('filterFileSuggestions', () => { const results = filterFileSuggestions(FILES, '.ts'); expect(results[0].name).toBe('index.ts'); }); + + it('quotes inserted paths that contain spaces', () => { + expect(formatFileMentionPath('src/My Component/App.tsx')).toBe( + '"src/My Component/App.tsx"' + ); + }); }); diff --git a/test/renderer/utils/fileReferences.test.ts b/test/renderer/utils/fileReferences.test.ts index ee7e5c0c..e0f78300 100644 --- a/test/renderer/utils/fileReferences.test.ts +++ b/test/renderer/utils/fileReferences.test.ts @@ -82,6 +82,27 @@ describe('extractFileReferences', () => { expect(refs).toHaveLength(1); expect(refs[0].path).toBe('~/projects/foo'); }); + + it('accepts Windows backslash paths', () => { + const refs = extractFileReferences('Check @src\\components\\App.tsx'); + expect(refs).toHaveLength(1); + expect(refs[0].path).toBe('src\\components\\App.tsx'); + }); + + it('accepts Windows drive absolute paths', () => { + const refs = extractFileReferences('Open @C:\\Users\\Alice\\project\\src\\App.tsx'); + expect(refs).toHaveLength(1); + expect(refs[0].path).toBe('C:\\Users\\Alice\\project\\src\\App.tsx'); + }); + + it('accepts quoted paths with spaces', () => { + const refs = extractFileReferences('Open @"src/My Component/App.tsx" now'); + expect(refs).toHaveLength(1); + expect(refs[0]).toEqual({ + path: 'src/My Component/App.tsx', + raw: '@"src/My Component/App.tsx"', + }); + }); }); describe('rejects npm scoped packages', () => {