From 8f7692aa12ec556dfd7b7244a66c58868b692f10 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 30 Jul 2026 18:27:16 -0700 Subject: [PATCH] Fix packaged skills CLI runtime ownership (#11627) * fix(cli): make packaged skills runtime self-contained * fix(cli): address packaged skills review feedback * ci(cli): smoke packaged skills on Windows --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .github/workflows/pr.yml | 54 +++- config/electron-builder.config.cjs | 18 +- .../scripts/electron-builder-config.test.mjs | 33 ++- .../scripts/pr-workflow-parallelism.test.mjs | 3 +- .../skills-cli-package-workflow.test.mjs | 31 +++ config/scripts/smoke-packaged-cli.mjs | 40 ++- config/scripts/verify-skills-cli-runtime.cjs | 229 +++++++++++++++++ .../verify-skills-cli-runtime.test.mjs | 230 ++++++++++++++++++ config/tsconfig.cli.json | 2 - electron.vite.config.ts | 12 +- package.json | 6 +- src/cli/handlers/account.test.ts | 4 +- src/cli/handlers/account.ts | 7 +- src/cli/handlers/skills.ts | 11 +- src/cli/main-module-bundle-parity.test.ts | 8 +- src/cli/skills.test.ts | 6 +- src/main/codex-cli/command.ts | 227 +---------------- .../ipc/local-agent-install-dir-detection.ts | 19 +- src/main/ipc/preflight.test.ts | 2 +- src/main/ipc/tui-agent-detection-commands.ts | 78 +----- src/main/win32-utils.test.ts | 24 +- src/main/win32-utils.ts | 69 +----- .../local-agent-install-dir-detection.ts | 17 ++ src/shared/node-cli-command-resolution.ts | 218 +++++++++++++++++ src/shared/tui-agent-detection-commands.ts | 77 ++++++ src/shared/windows-batch-spawn.ts | 43 ++++ 26 files changed, 1033 insertions(+), 435 deletions(-) create mode 100644 config/scripts/skills-cli-package-workflow.test.mjs create mode 100644 config/scripts/verify-skills-cli-runtime.cjs create mode 100644 config/scripts/verify-skills-cli-runtime.test.mjs create mode 100644 src/shared/local-agent-install-dir-detection.ts create mode 100644 src/shared/node-cli-command-resolution.ts create mode 100644 src/shared/tui-agent-detection-commands.ts create mode 100644 src/shared/windows-batch-spawn.ts diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 4888dd6f3b3..2f0e19980a6 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -273,6 +273,55 @@ jobs: - name: Smoke packaged CLI run: node config/scripts/smoke-packaged-cli.mjs --app-dir=dist/linux-unpacked + package_windows: + name: package (windows) + runs-on: windows-2022 + timeout-minutes: 30 + + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + persist-credentials: false + + - name: Setup pnpm + uses: pnpm/action-setup@v6 + with: + run_install: false + + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version-file: package.json + cache: pnpm + + - name: Cache electron-builder downloads + uses: actions/cache@v5 + with: + path: | + ~\AppData\Local\electron\Cache + ~\AppData\Local\electron-builder\Cache + key: electron-builder-windows-${{ hashFiles('pnpm-lock.yaml') }} + restore-keys: | + electron-builder-windows- + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Build package inputs + run: pnpm run build:release + + - name: Prepare Electron native runtime + run: node config/scripts/ensure-native-runtime.mjs --runtime=electron + + - name: Package unpacked app + env: + ORCA_REUSE_PREPARED_NATIVE_RUNTIME: '1' + run: pnpm exec electron-builder --config config/electron-builder.config.cjs --dir + + - name: Smoke packaged CLI + run: node config/scripts/smoke-packaged-cli.mjs --app-dir=dist/win-unpacked + # Why: regression specs under tests/e2e/** used to merge green without ever # running — e2e.yml only fired on schedule/release (#10518). Path-filter so # ordinary PRs stay light; any E2E suite change still gets a full shard run. @@ -331,6 +380,7 @@ jobs: - shell_contracts - test - package + - package_windows runs-on: ubuntu-latest steps: @@ -350,6 +400,7 @@ jobs: SHELL_CONTRACTS: ${{ needs.shell_contracts.result }} TEST: ${{ needs.test.result }} PACKAGE: ${{ needs.package.result }} + PACKAGE_WINDOWS: ${{ needs.package_windows.result }} run: | for result in \ "$STATIC_ANALYSIS" \ @@ -357,7 +408,8 @@ jobs: "$GIT_COMPATIBILITY" \ "$SHELL_CONTRACTS" \ "$TEST" \ - "$PACKAGE"; do + "$PACKAGE" \ + "$PACKAGE_WINDOWS"; do if [ "$result" != "success" ]; then exit 1 fi diff --git a/config/electron-builder.config.cjs b/config/electron-builder.config.cjs index a2d385bd698..e7a80704c14 100644 --- a/config/electron-builder.config.cjs +++ b/config/electron-builder.config.cjs @@ -14,6 +14,7 @@ const { const { verifyLinuxGlibcFloor } = require('./scripts/verify-linux-glibc-floor.cjs') const { writeMacBuildCompatibility } = require('./scripts/mac-build-compatibility.cjs') const { verifyPackagedPluginResources } = require('./scripts/verify-packaged-plugin-resources.cjs') +const { verifySkillsCliRuntime } = require('./scripts/verify-skills-cli-runtime.cjs') const isMacRelease = process.env.ORCA_MAC_RELEASE === '1' const isLinuxArm64Release = process.env.ORCA_LINUX_ARM64_RELEASE === '1' @@ -143,16 +144,12 @@ module.exports = { 'out/main/claude/**', 'out/main/claude-accounts/keychain.js', 'out/main/codex/**', - 'out/main/codex-cli/command.js', 'out/main/copilot/**', 'out/main/cursor/**', 'out/main/droid/**', 'out/main/gemini/**', 'out/main/grok/**', 'out/main/hermes/**', - 'out/main/ipc/local-agent-install-dir-detection.js', - 'out/main/ipc/tui-agent-detection-commands.js', - 'out/main/win32-utils.js', 'out/main/daemon-entry.js', 'out/main/plugin-host-entry.js', 'out/main/computer-sidecar.js', @@ -183,7 +180,7 @@ module.exports = { ) : join(context.appOutDir, 'resources') if (!existsSync(resourcesDir)) { - return + throw new Error(`Missing packaged resources directory: ${resourcesDir}`) } if (context.electronPlatformName === 'darwin') { const architectureByEnum = { 1: 'x64', 3: 'arm64' } @@ -213,7 +210,16 @@ module.exports = { // arm64=3, universal=4 (universal contains the host slice, so run it). const archEnumByNodeArch = { ia32: 0, x64: 1, armv7l: 2, arm64: 3 } const hostArchEnum = archEnumByNodeArch[process.arch] - if (context.arch === hostArchEnum || context.arch === 4) { + const canExecuteTargetArch = context.arch === hostArchEnum || context.arch === 4 + verifySkillsCliRuntime(join(resourcesDir, 'app.asar.unpacked', 'out'), resourcesDir, { + executeCommands: canExecuteTargetArch + }) + if (!canExecuteTargetArch) { + console.log( + `[verify-skills-cli-runtime] skipped command probes on cross-arch slice (target ${context.arch}, host ${process.arch})` + ) + } + if (canExecuteTargetArch) { verifyPackagedDaemonEntryBoots(resourcesDir) } else { // Why: a cross-arch slice can't be booted by the host Node, but the diff --git a/config/scripts/electron-builder-config.test.mjs b/config/scripts/electron-builder-config.test.mjs index f90d6e49b5a..7d761c70af7 100644 --- a/config/scripts/electron-builder-config.test.mjs +++ b/config/scripts/electron-builder-config.test.mjs @@ -158,11 +158,7 @@ describe('electron-builder config', () => { 'out/package.json', 'out/cli/**', 'out/shared/**', - 'out/main/claude-accounts/keychain.js', - 'out/main/codex-cli/command.js', - 'out/main/ipc/local-agent-install-dir-detection.js', - 'out/main/ipc/tui-agent-detection-commands.js', - 'out/main/win32-utils.js' + 'out/main/claude-accounts/keychain.js' ]) ) }) @@ -486,6 +482,20 @@ describe('electron-builder config', () => { } }) + it('fails when the packaged resources directory is missing', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-electron-builder-config-')) + try { + await expect( + electronBuilderConfig.afterPack({ + appOutDir: root, + electronPlatformName: 'win32' + }) + ).rejects.toThrow(/Missing packaged resources directory/) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + it.skipIf(process.platform === 'win32')( 'marks packaged Unix CLI launchers executable', async () => { @@ -509,6 +519,19 @@ describe('electron-builder config', () => { 'console.error("Usage: daemon-entry "); process.exit(1)\n', 'utf8' ) + const unpackedCliDir = join(resourcesDir, 'app.asar.unpacked', 'out', 'cli') + await mkdir(join(unpackedCliDir, 'handlers'), { recursive: true }) + await writeFile(join(unpackedCliDir, 'handlers', 'skills.js'), '', 'utf8') + await writeFile( + join(unpackedCliDir, 'index.js'), + [ + 'const args = process.argv.slice(2)', + "if (args[1] === 'list') console.log(JSON.stringify({ topics: [{ name: 'orca-cli' }, { name: 'computer-use' }] }))", + "else if (args[1] === 'get') console.log(`---\\nname: ${args[2]}\\n---`)", + 'else console.log(JSON.stringify({ executed: false }))' + ].join('\n'), + 'utf8' + ) await writeFile(launcherPath, '#!/usr/bin/env bash\n', { encoding: 'utf8', mode: 0o644 }) await electronBuilderConfig.afterPack({ diff --git a/config/scripts/pr-workflow-parallelism.test.mjs b/config/scripts/pr-workflow-parallelism.test.mjs index 6c758a74fe1..dbece23cafd 100644 --- a/config/scripts/pr-workflow-parallelism.test.mjs +++ b/config/scripts/pr-workflow-parallelism.test.mjs @@ -177,7 +177,8 @@ describe('PR workflow parallelism', () => { 'git_compatibility', 'shell_contracts', 'test', - 'package' + 'package', + 'package_windows' ]) }) }) diff --git a/config/scripts/skills-cli-package-workflow.test.mjs b/config/scripts/skills-cli-package-workflow.test.mjs new file mode 100644 index 00000000000..72e6cf25829 --- /dev/null +++ b/config/scripts/skills-cli-package-workflow.test.mjs @@ -0,0 +1,31 @@ +import { readFileSync } from 'node:fs' +import { parse } from 'yaml' +import { describe, expect, it } from 'vitest' + +const workflow = parse(readFileSync('.github/workflows/pr.yml', 'utf8')) + +describe('packaged skills CLI PR gates', () => { + it('builds and executes the Windows packaged CLI', () => { + const job = workflow.jobs.package_windows + const buildStep = job.steps.find((step) => step.name === 'Build package inputs') + const prepareStep = job.steps.find((step) => step.name === 'Prepare Electron native runtime') + const packageStep = job.steps.find((step) => step.name === 'Package unpacked app') + const smokeStep = job.steps.find((step) => step.name === 'Smoke packaged CLI') + + expect(job['runs-on']).toBe('windows-2022') + expect(buildStep.run).toBe('pnpm run build:release') + expect(prepareStep.run).toBe('node config/scripts/ensure-native-runtime.mjs --runtime=electron') + expect(packageStep.run).toContain('electron-builder') + expect(packageStep.run).toContain('--dir') + expect(packageStep.env.ORCA_REUSE_PREPARED_NATIVE_RUNTIME).toBe('1') + expect(smokeStep.run).toBe( + 'node config/scripts/smoke-packaged-cli.mjs --app-dir=dist/win-unpacked' + ) + + const aggregateStep = workflow.jobs.verify.steps.find( + (step) => step.name === 'Require successful checks' + ) + expect(aggregateStep.env.PACKAGE_WINDOWS).toBe('${{ needs.package_windows.result }}') + expect(aggregateStep.run).toContain('"$PACKAGE_WINDOWS"') + }) +}) diff --git a/config/scripts/smoke-packaged-cli.mjs b/config/scripts/smoke-packaged-cli.mjs index 269118d867e..4cecf49ad4b 100644 --- a/config/scripts/smoke-packaged-cli.mjs +++ b/config/scripts/smoke-packaged-cli.mjs @@ -3,6 +3,7 @@ import { execFile } from 'node:child_process' import { tmpdir } from 'node:os' import { basename, join, resolve } from 'node:path' import { promisify } from 'node:util' +import assert from 'node:assert/strict' const execFileAsync = promisify(execFile) @@ -37,10 +38,41 @@ const copiedAppDir = join(tempRoot, basename(appDir)) try { await cp(appDir, copiedAppDir, { recursive: true, verbatimSymlinks: true }) const cliPath = getPackagedCliPath(copiedAppDir) - await execFileAsync(cliPath, ['--help'], { - env: { ...process.env, NODE_PATH: '' } - }) - console.log(`[packaged-cli-smoke] ${cliPath} --help succeeded outside the repo`) + const env = { ...process.env, NODE_PATH: '' } + delete env.ORCA_CLI_CWD + const run = (args) => + execFileAsync(cliPath, args, { + env, + killSignal: 'SIGKILL', + maxBuffer: 16 * 1024 * 1024, + timeout: 30_000 + }) + + await run(['--help']) + const list = JSON.parse((await run(['skills', 'list', '--json'])).stdout) + assert(list.topics.some((topic) => topic.name === 'orca-cli')) + assert.match((await run(['skills', 'get', 'orca-cli'])).stdout, /name: orca-cli/) + assert.match((await run(['skills', 'get', 'computer-use'])).stdout, /name: computer-use/) + const install = JSON.parse( + ( + await run([ + 'skills', + 'install', + '--skill', + 'orca-cli', + '--agent', + 'codex', + '--dry-run', + '--json' + ]) + ).stdout + ) + const update = JSON.parse( + (await run(['skills', 'update', '--skill', 'orca-cli', '--dry-run', '--json'])).stdout + ) + assert.equal(install.executed, false) + assert.equal(update.executed, false) + console.log(`[packaged-cli-smoke] help and skills commands passed via ${cliPath}`) } finally { await rm(tempRoot, { recursive: true, force: true }) } diff --git a/config/scripts/verify-skills-cli-runtime.cjs b/config/scripts/verify-skills-cli-runtime.cjs new file mode 100644 index 00000000000..0a76078d318 --- /dev/null +++ b/config/scripts/verify-skills-cli-runtime.cjs @@ -0,0 +1,229 @@ +const { existsSync, readFileSync, realpathSync } = require('node:fs') +const { builtinModules, createRequire, isBuiltin } = require('node:module') +const { dirname, isAbsolute, join, relative, resolve, sep } = require('node:path') +const { spawnSync } = require('node:child_process') +const ts = require('typescript-api') + +const BUILTINS = new Set(builtinModules.flatMap((name) => [name, `node:${name}`])) +const CLI_COMMAND_TIMEOUT_MS = 30_000 + +function artifactPath(outDir, file) { + return relative(outDir, file).split(sep).join('/') +} + +function runtimeImportSpecifiers(source, file) { + const sourceFile = ts.createSourceFile( + file, + source, + ts.ScriptTarget.Latest, + false, + ts.ScriptKind.JS + ) + const specifiers = [] + + function visit(node) { + if (ts.isCallExpression(node) && node.arguments.length > 0) { + const [argument] = node.arguments + const expression = node.expression + const isRequire = ts.isIdentifier(expression) && expression.text === 'require' + const isRequireResolve = + ts.isPropertyAccessExpression(expression) && + ts.isIdentifier(expression.expression) && + expression.expression.text === 'require' && + expression.name.text === 'resolve' + const isDynamicImport = expression.kind === ts.SyntaxKind.ImportKeyword + + if ((isRequire || isRequireResolve || isDynamicImport) && ts.isStringLiteralLike(argument)) { + specifiers.push(argument.text) + } + } + ts.forEachChild(node, visit) + } + + visit(sourceFile) + return specifiers +} + +function isOutsideRoot(root, target) { + const pathFromRoot = relative(root, target) + return isAbsolute(pathFromRoot) || pathFromRoot === '..' || pathFromRoot.startsWith(`..${sep}`) +} + +function isOptionalPackageImport(artifactRoot, importer, specifier) { + if (specifier.startsWith('.') || isAbsolute(specifier)) { + return false + } + const segments = specifier.split('/') + const packageName = specifier.startsWith('@') ? segments.slice(0, 2).join('/') : segments[0] + let directory = realpathSync(dirname(importer)) + + while (!isOutsideRoot(artifactRoot, directory)) { + const packageJson = join(directory, 'package.json') + if (existsSync(packageJson)) { + try { + const manifest = JSON.parse(readFileSync(packageJson, 'utf8')) + return ( + Object.hasOwn(manifest.optionalDependencies ?? {}, packageName) || + manifest.peerDependenciesMeta?.[packageName]?.optional === true + ) + } catch { + return false + } + } + if (directory === artifactRoot) { + break + } + directory = dirname(directory) + } + return false +} + +function resolveRuntimeImport(outDir, artifactRoot, importer, specifier) { + if (BUILTINS.has(specifier) || isBuiltin(specifier)) { + return null + } + let resolved + try { + resolved = createRequire(importer).resolve(specifier) + } catch (error) { + if (isOptionalPackageImport(artifactRoot, importer, specifier)) { + return null + } + const detail = error instanceof Error ? error.message : String(error) + throw new Error( + `[verify-skills-cli-runtime] missing runtime import "${specifier}" from ` + + `${artifactPath(outDir, importer)}: ${detail}` + ) + } + if (isOutsideRoot(artifactRoot, resolved)) { + throw new Error( + `[verify-skills-cli-runtime] import "${specifier}" from ` + + `${artifactPath(outDir, importer)} resolved outside ${artifactRoot}: ${resolved}` + ) + } + return resolved +} + +function collectRuntimeClosure(outDir, artifactRoot = dirname(outDir)) { + outDir = realpathSync(outDir) + artifactRoot = realpathSync(artifactRoot) + if (isOutsideRoot(artifactRoot, outDir)) { + throw new Error(`[verify-skills-cli-runtime] ${outDir} is outside ${artifactRoot}`) + } + const entry = resolve(outDir, 'cli', 'index.js') + if (!existsSync(entry)) { + throw new Error(`[verify-skills-cli-runtime] missing entry ${entry}`) + } + const pending = [entry] + const visited = new Set() + + while (pending.length > 0) { + const file = pending.pop() + if (!file || visited.has(file)) { + continue + } + visited.add(file) + const source = readFileSync(file, 'utf8') + for (const specifier of runtimeImportSpecifiers(source, file)) { + const resolved = resolveRuntimeImport(outDir, artifactRoot, file, specifier) + if (resolved && !isOutsideRoot(artifactRoot, resolved) && /\.(?:c|m)?js$/.test(resolved)) { + pending.push(resolved) + } + } + } + + return [...visited].sort() +} + +function runCli(outDir, args, timeoutMs = CLI_COMMAND_TIMEOUT_MS) { + const entry = resolve(outDir, 'cli', 'index.js') + const env = { ...process.env, NODE_PATH: '' } + delete env.ORCA_CLI_CWD + const result = spawnSync(process.execPath, [entry, ...args], { + cwd: dirname(outDir), + encoding: 'utf8', + env, + killSignal: 'SIGKILL', + maxBuffer: 16 * 1024 * 1024, + timeout: timeoutMs + }) + if (result.error || result.signal || result.status !== 0) { + const detail = [ + result.error?.message, + result.signal ? `terminated by ${result.signal}` : null, + result.stdout, + result.stderr + ] + .filter(Boolean) + .join('\n') + throw new Error( + `[verify-skills-cli-runtime] ${args.join(' ')} exited ${String(result.status)}\n${detail}` + ) + } + return result.stdout +} + +function parseJson(label, output) { + try { + return JSON.parse(output) + } catch { + throw new Error(`[verify-skills-cli-runtime] ${label} emitted invalid JSON:\n${output}`) + } +} + +function verifySkillsCliRuntime(outDir, artifactRoot = dirname(outDir), options = {}) { + const absoluteOutDir = resolve(outDir) + const closure = collectRuntimeClosure(absoluteOutDir, resolve(artifactRoot)) + if (options.executeCommands === false) { + return { closureFiles: closure.length, commands: 0 } + } + const list = parseJson('skills list', runCli(absoluteOutDir, ['skills', 'list', '--json'])) + const topicNames = new Set(list.topics?.map((topic) => topic.name)) + for (const topic of ['orca-cli', 'computer-use']) { + if (!topicNames.has(topic)) { + throw new Error(`[verify-skills-cli-runtime] skills list omitted ${topic}`) + } + const guide = runCli(absoluteOutDir, ['skills', 'get', topic]) + if (!guide.includes(`name: ${topic}`)) { + throw new Error(`[verify-skills-cli-runtime] skills get ${topic} returned the wrong guide`) + } + } + + const install = parseJson( + 'skills install --dry-run', + runCli(absoluteOutDir, [ + 'skills', + 'install', + '--skill', + 'orca-cli', + '--agent', + 'codex', + '--dry-run', + '--json' + ]) + ) + const update = parseJson( + 'skills update --dry-run', + runCli(absoluteOutDir, ['skills', 'update', '--skill', 'orca-cli', '--dry-run', '--json']) + ) + if (install.executed !== false || update.executed !== false) { + throw new Error('[verify-skills-cli-runtime] a dry-run reported execution') + } + + return { closureFiles: closure.length, commands: 5 } +} + +if (require.main === module) { + try { + const result = verifySkillsCliRuntime(process.argv[2] ?? 'out') + console.log( + `[verify-skills-cli-runtime] ${result.closureFiles} closure files and ` + + `${result.commands} commands passed` + ) + } catch (error) { + console.error(error instanceof Error ? error.message : error) + process.exitCode = 1 + } +} + +module.exports = { collectRuntimeClosure, runCli, verifySkillsCliRuntime } diff --git a/config/scripts/verify-skills-cli-runtime.test.mjs b/config/scripts/verify-skills-cli-runtime.test.mjs new file mode 100644 index 00000000000..f60adfd38ac --- /dev/null +++ b/config/scripts/verify-skills-cli-runtime.test.mjs @@ -0,0 +1,230 @@ +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { realpathSync } from 'node:fs' +import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { join, relative } from 'node:path' +import { describe, expect, it } from 'vitest' + +const require = createRequire(import.meta.url) +const { + collectRuntimeClosure, + runCli, + verifySkillsCliRuntime +} = require('./verify-skills-cli-runtime.cjs') + +async function writeSkillsCliFixture(outDir, handlerSource) { + const cliDir = join(outDir, 'cli') + const handlerDir = join(cliDir, 'handlers') + await mkdir(handlerDir, { recursive: true }) + await writeFile(join(cliDir, 'index.js'), "require('./handlers/skills')\n", 'utf8') + await writeFile(join(handlerDir, 'skills.js'), handlerSource, 'utf8') +} + +describe('skills CLI runtime closure', () => { + it('runs after Electron composes the final output', async () => { + const packageJson = JSON.parse( + await readFile(new URL('../../package.json', import.meta.url), 'utf8') + ) + for (const scriptName of ['build:desktop', 'build:release']) { + const script = packageJson.scripts[scriptName] + expect(script.indexOf('build:electron-vite')).toBeLessThan( + script.indexOf('verify:built-skills-cli') + ) + } + }) + + it('reports the missing final-artifact import and its owner', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-cli-closure-')) + try { + await writeSkillsCliFixture(root, "require('../../main/codex-cli/command')\n") + + expect(() => collectRuntimeClosure(root)).toThrow( + /missing runtime import "\.\.\/\.\.\/main\/codex-cli\/command" from cli\/handlers\/skills\.js/ + ) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('walks static and dynamic relative imports', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-cli-closure-')) + try { + const sharedDir = join(root, 'shared') + await mkdir(sharedDir, { recursive: true }) + await writeSkillsCliFixture( + root, + "require('../../shared/first.js'); import('../../shared/second.js')\n" + ) + await writeFile(join(sharedDir, 'first.js'), '', 'utf8') + await writeFile(join(sharedDir, 'second.js'), '', 'utf8') + + expect( + collectRuntimeClosure(root) + .map((file) => relative(realpathSync(root), file)) + .sort() + ).toEqual(['cli/handlers/skills.js', 'cli/index.js', 'shared/first.js', 'shared/second.js']) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('ignores import-shaped text in comments and strings', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-cli-closure-')) + try { + await writeSkillsCliFixture( + root, + [ + "// require('../../missing-comment.js')", + 'const message = "import(\'../../missing-string.js\')"', + "const template = `require.resolve('../../missing-template.js')`" + ].join('\n') + ) + + expect(collectRuntimeClosure(root)).toHaveLength(2) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('can inspect a cross-arch artifact without executing it', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-cli-closure-')) + try { + await writeSkillsCliFixture(root, '') + + expect(verifySkillsCliRuntime(root, undefined, { executeCommands: false })).toEqual({ + closureFiles: 2, + commands: 0 + }) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('bounds command execution time', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-cli-closure-')) + try { + await writeSkillsCliFixture(root, 'setInterval(() => {}, 1_000)\n') + + expect(() => runCli(root, [], 50)).toThrow(/ETIMEDOUT|terminated by SIGKILL/) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('rejects bare imports resolved outside the artifact', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-cli-closure-')) + try { + const artifactRoot = join(root, 'artifact') + const outDir = join(artifactRoot, 'out') + const externalPackageDir = join(root, 'node_modules', 'external-package') + await mkdir(externalPackageDir, { recursive: true }) + await writeSkillsCliFixture(outDir, "require('external-package')\n") + await writeFile( + join(externalPackageDir, 'package.json'), + JSON.stringify({ main: 'index.js' }), + 'utf8' + ) + await writeFile(join(externalPackageDir, 'index.js'), '', 'utf8') + + expect(() => collectRuntimeClosure(outDir, artifactRoot)).toThrow( + /external-package.*resolved outside/s + ) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('rejects package dependencies resolved outside the artifact', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-cli-closure-')) + try { + const artifactRoot = join(root, 'artifact') + const outDir = join(artifactRoot, 'out') + const packageDir = join(artifactRoot, 'node_modules', 'inside-package') + const externalPackageDir = join(root, 'node_modules', 'ancestor-dependency') + await writeSkillsCliFixture(outDir, "require('inside-package')\n") + await mkdir(packageDir, { recursive: true }) + await mkdir(externalPackageDir, { recursive: true }) + await writeFile( + join(packageDir, 'package.json'), + JSON.stringify({ main: 'index.js' }), + 'utf8' + ) + await writeFile(join(packageDir, 'index.js'), "require('ancestor-dependency')\n", 'utf8') + await writeFile( + join(externalPackageDir, 'package.json'), + JSON.stringify({ main: 'index.js' }), + 'utf8' + ) + await writeFile(join(externalPackageDir, 'index.js'), '', 'utf8') + + expect(() => collectRuntimeClosure(outDir, artifactRoot)).toThrow( + /ancestor-dependency.*resolved outside/s + ) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('allows absent dependencies declared optional by their package', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-cli-closure-')) + try { + const artifactRoot = join(root, 'artifact') + const outDir = join(artifactRoot, 'out') + const packageDir = join(artifactRoot, 'node_modules', 'inside-package') + await writeSkillsCliFixture(outDir, "require('inside-package')\n") + await mkdir(packageDir, { recursive: true }) + await writeFile( + join(packageDir, 'package.json'), + JSON.stringify({ + main: 'index.js', + peerDependencies: { 'optional-native': '*' }, + peerDependenciesMeta: { 'optional-native': { optional: true } } + }), + 'utf8' + ) + await writeFile( + join(packageDir, 'index.js'), + "try { require('optional-native') } catch {}\n", + 'utf8' + ) + + expect(collectRuntimeClosure(outDir, artifactRoot)).toHaveLength(3) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('rejects optional dependencies resolved only outside the artifact', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-cli-closure-')) + try { + const artifactRoot = join(root, 'artifact') + const outDir = join(artifactRoot, 'out') + const packageDir = join(artifactRoot, 'node_modules', 'inside-package') + const externalPackageDir = join(root, 'node_modules', 'optional-native') + await writeSkillsCliFixture(outDir, "require('inside-package')\n") + await mkdir(packageDir, { recursive: true }) + await mkdir(externalPackageDir, { recursive: true }) + await writeFile( + join(packageDir, 'package.json'), + JSON.stringify({ + main: 'index.js', + optionalDependencies: { 'optional-native': '*' } + }), + 'utf8' + ) + await writeFile(join(packageDir, 'index.js'), "require('optional-native')\n", 'utf8') + await writeFile( + join(externalPackageDir, 'package.json'), + JSON.stringify({ main: 'index.js' }), + 'utf8' + ) + await writeFile(join(externalPackageDir, 'index.js'), '', 'utf8') + + expect(() => collectRuntimeClosure(outDir, artifactRoot)).toThrow( + /optional-native.*resolved outside/s + ) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) +}) diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 6bbfa8814f3..f4247281300 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -12,8 +12,6 @@ "../src/main/agent-hooks/local-agent-cli-presence.ts", "../src/main/agent-hooks/managed-agent-hook-controls.ts", "../src/main/agent-hooks/managed-agent-hook-registry.ts", - "../src/main/ipc/local-agent-install-dir-detection.ts", - "../src/main/ipc/tui-agent-detection-commands.ts", "../src/main/amp/hook-service.ts", "../src/main/antigravity/hook-service.ts", "../src/main/claude/hook-settings.ts", diff --git a/electron.vite.config.ts b/electron.vite.config.ts index 562d6e9db91..4a4cd6c8eba 100644 --- a/electron.vite.config.ts +++ b/electron.vite.config.ts @@ -260,16 +260,8 @@ export const electronViteConfig: UserConfig = { 'agent-hooks/managed-agent-hook-controls': resolve( 'src/main/agent-hooks/managed-agent-hook-controls.ts' ), - 'ipc/local-agent-install-dir-detection': resolve( - 'src/main/ipc/local-agent-install-dir-detection.ts' - ), - 'ipc/tui-agent-detection-commands': resolve( - 'src/main/ipc/tui-agent-detection-commands.ts' - ), - // Why: same rule — `orca account add` / `account list` import these. - 'claude-accounts/keychain': resolve('src/main/claude-accounts/keychain.ts'), - 'codex-cli/command': resolve('src/main/codex-cli/command.ts'), - 'win32-utils': resolve('src/main/win32-utils.ts') + // Why: account import mutates the user's macOS Keychain from the CLI. + 'claude-accounts/keychain': resolve('src/main/claude-accounts/keychain.ts') }, // Why: Rolldown's SSR default is ESM, but Electron and sidecar launchers // consume these stable CommonJS paths. diff --git a/package.json b/package.json index e7effafc8fc..4c1e8474517 100644 --- a/package.json +++ b/package.json @@ -58,19 +58,21 @@ "smoke:computer": "node config/scripts/computer-use-smoke.mjs", "verify:computer-native": "node config/scripts/verify-computer-native.mjs", "verify:cli-bin": "node config/scripts/verify-cli-bin.mjs", + "verify:built-skills-cli": "node config/scripts/verify-skills-cli-runtime.cjs out", "verify:localization-catalog": "node config/scripts/verify-localization-catalog.mjs", "sync:localization-catalog": "node config/scripts/verify-localization-catalog.mjs --fix", "verify:localization-extraction": "node config/scripts/verify-localization-extraction.mjs", "verify:localization-coverage": "node config/scripts/audit-localization-coverage.mjs --check", "audit:localization": "node config/scripts/audit-localization-coverage.mjs", "build:cli": "tsc -p config/tsconfig.cli.json --outDir out --composite false --incremental false && node config/scripts/verify-cli-bin.mjs --fix-executable --fix-package-json && node config/scripts/install-dev-cli.mjs", + "test:repro:skills-cli-runtime": "pnpm run build:cli && pnpm run build:electron-vite && pnpm run verify:built-skills-cli", "build:electron-vite": "node config/scripts/run-electron-vite-build.mjs", "build:electron-vite:parallel": "node config/scripts/run-electron-vite-targets-in-parallel.mjs", "build:web": "node config/scripts/run-vite-web-build.mjs && node config/scripts/verify-web-build.mjs", "build:web-from-renderer": "node config/scripts/project-renderer-web-client.mjs && node config/scripts/verify-web-build.mjs", - "build:desktop": "pnpm run typecheck && pnpm run build:relay && pnpm run build:cli && pnpm run build:electron-vite && pnpm run build:web-from-renderer", + "build:desktop": "pnpm run typecheck && pnpm run build:relay && pnpm run build:cli && pnpm run build:electron-vite && pnpm run verify:built-skills-cli && pnpm run build:web-from-renderer", "build": "pnpm run build:desktop && pnpm run build:native", - "build:release": "pnpm run build:relay && pnpm run build:native && pnpm run verify:computer-native && pnpm run build:cli && pnpm run build:electron-vite && pnpm run build:web-from-renderer", + "build:release": "pnpm run build:relay && pnpm run build:native && pnpm run verify:computer-native && pnpm run build:cli && pnpm run build:electron-vite && pnpm run verify:built-skills-cli && pnpm run build:web-from-renderer", "postinstall": "node config/scripts/rebuild-native-deps.mjs", "rebuild:electron": "node config/scripts/rebuild-native-deps.mjs", "rebuild:node": "pnpm rebuild node-pty", diff --git a/src/cli/handlers/account.test.ts b/src/cli/handlers/account.test.ts index 97f7bee9c92..d39a5f33293 100644 --- a/src/cli/handlers/account.test.ts +++ b/src/cli/handlers/account.test.ts @@ -41,7 +41,7 @@ vi.mock('../../main/claude-accounts/keychain', () => ({ readActiveClaudeKeychainCredentialsStrict: readKeychainMock, writeActiveClaudeKeychainCredentials: writeKeychainMock })) -vi.mock('../../main/codex-cli/command', () => ({ +vi.mock('../../shared/node-cli-command-resolution', () => ({ getVersionManagerBinPaths: getVersionManagerBinPathsMock, resolveCliCommand: resolveCliCommandMock })) @@ -49,7 +49,7 @@ vi.mock('../../main/codex-cli/command', () => ({ import { ACCOUNT_HANDLERS } from './account' import type { HandlerContext } from '../dispatch' import type { RuntimeClient } from '../runtime-client' -import { getCmdExePath } from '../../main/win32-utils' +import { getCmdExePath } from '../../shared/windows-batch-spawn' import { ACCOUNT_IMPORT_RUNTIME_CAPABILITY } from '../../shared/protocol-version' function successfulChild(): EventEmitter { diff --git a/src/cli/handlers/account.ts b/src/cli/handlers/account.ts index 0a7a3212d66..0ad19aa240d 100644 --- a/src/cli/handlers/account.ts +++ b/src/cli/handlers/account.ts @@ -12,8 +12,11 @@ import { readActiveClaudeKeychainCredentialsStrict, writeActiveClaudeKeychainCredentials } from '../../main/claude-accounts/keychain' -import { getVersionManagerBinPaths, resolveCliCommand } from '../../main/codex-cli/command' -import { getSpawnArgsForWindows } from '../../main/win32-utils' +import { + getVersionManagerBinPaths, + resolveCliCommand +} from '../../shared/node-cli-command-resolution' +import { getSpawnArgsForWindows } from '../../shared/windows-batch-spawn' import { ACCOUNT_IMPORT_RUNTIME_CAPABILITY } from '../../shared/protocol-version' import type { RuntimeStatus } from '../../shared/runtime-types' import type { ClaudeRateLimitAccountsState, CodexRateLimitAccountsState } from '../../shared/types' diff --git a/src/cli/handlers/skills.ts b/src/cli/handlers/skills.ts index 26e9ea5a734..7c911ea1751 100644 --- a/src/cli/handlers/skills.ts +++ b/src/cli/handlers/skills.ts @@ -3,14 +3,17 @@ import type { CommandHandler } from '../dispatch' import { RuntimeClientError } from '../runtime-client' import { delimiter, dirname } from 'node:path' import { getRepeatedStringFlag } from '../flags' -import { resolveCliCommand } from '../../main/codex-cli/command' -import { detectCommandsInInstallDirs } from '../../main/ipc/local-agent-install-dir-detection' +import { resolveCliCommand } from '../../shared/node-cli-command-resolution' +import { detectCommandsInInstallDirs } from '../../shared/local-agent-install-dir-detection' import { getTuiAgentDetectionProbeCommands, KNOWN_TUI_AGENT_DETECTION_COMMANDS, resolveDetectedTuiAgentIds -} from '../../main/ipc/tui-agent-detection-commands' -import { getSpawnArgsForWindows, UnsafeWindowsBatchArgumentsError } from '../../main/win32-utils' +} from '../../shared/tui-agent-detection-commands' +import { + getSpawnArgsForWindows, + UnsafeWindowsBatchArgumentsError +} from '../../shared/windows-batch-spawn' import { isSkillsCliAgentKeyShaped, toSkillsCliAgentKeys } from '../../shared/skills-cli-agent-keys' import { buildAgentFeatureSkillInstallArgs, diff --git a/src/cli/main-module-bundle-parity.test.ts b/src/cli/main-module-bundle-parity.test.ts index 8d00749f65b..a109fcb9fdb 100644 --- a/src/cli/main-module-bundle-parity.test.ts +++ b/src/cli/main-module-bundle-parity.test.ts @@ -41,8 +41,8 @@ function findElectronViteMainEntries(): Set { describe('CLI imports of main-process modules', () => { // Why: electron-vite cleans out/main and emits only its declared entries, so a // `src/main/*` module the CLI imports but the config omits is deleted by the - // build that runs after `build:cli` — the command then dies at require time - // with "Cannot find module". Nothing else catches this before packaging. + // build that runs after `build:cli` — keep source-level feedback ahead of the + // final-artifact runtime verifier. it('has an electron-vite entry for every main module the CLI imports', () => { const entries = findElectronViteMainEntries() const missing = findMainImports().filter(({ module }) => !entries.has(module)) @@ -52,7 +52,7 @@ describe('CLI imports of main-process modules', () => { it('finds the imports it is meant to guard', () => { // Why: a broken matcher would make the guard above vacuously pass. - expect(findMainImports().length).toBeGreaterThanOrEqual(4) - expect(findElectronViteMainEntries().size).toBeGreaterThanOrEqual(4) + expect(findMainImports().length).toBeGreaterThanOrEqual(2) + expect(findElectronViteMainEntries().size).toBeGreaterThanOrEqual(2) }) }) diff --git a/src/cli/skills.test.ts b/src/cli/skills.test.ts index 1cdee5aad4b..11caa30d704 100644 --- a/src/cli/skills.test.ts +++ b/src/cli/skills.test.ts @@ -1,7 +1,7 @@ import { EventEmitter } from 'node:events' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { delimiter } from 'node:path' -import type * as CodexCliCommandModule from '../main/codex-cli/command' +import type * as CodexCliCommandModule from '../shared/node-cli-command-resolution' const { detectCommandsMock, @@ -19,12 +19,12 @@ const { // Why: agent detection probes the real machine, so pin it or every install // assertion depends on what the test runner happens to have installed. -vi.mock('../main/ipc/local-agent-install-dir-detection', () => ({ +vi.mock('../shared/local-agent-install-dir-detection', () => ({ detectCommandsInInstallDirs: detectCommandsMock })) // Why: override only the npx lookup so the real Windows .cmd rail still runs. -vi.mock('../main/codex-cli/command', async (importOriginal) => ({ +vi.mock('../shared/node-cli-command-resolution', async (importOriginal) => ({ ...(await importOriginal()), resolveCliCommand: resolveCliCommandMock })) diff --git a/src/main/codex-cli/command.ts b/src/main/codex-cli/command.ts index 177b7e0ea9c..61e08eb457e 100644 --- a/src/main/codex-cli/command.ts +++ b/src/main/codex-cli/command.ts @@ -1,226 +1 @@ -import { accessSync, constants, existsSync, readdirSync, statSync } from 'node:fs' -import { homedir } from 'node:os' -import { delimiter, dirname, join } from 'node:path' - -type ResolveCommandOptions = { - pathEnv?: string | null - platform?: NodeJS.Platform - homePath?: string -} - -function getExecutableNames(platform: NodeJS.Platform, commandName: string): string[] { - if (platform === 'win32') { - return [`${commandName}.cmd`, `${commandName}.exe`, `${commandName}.bat`, commandName] - } - - return [commandName] -} - -function splitPath(pathEnv: string | null | undefined): string[] { - if (!pathEnv) { - return [] - } - - return pathEnv - .split(delimiter) - .map((entry) => entry.trim()) - .filter(Boolean) -} - -function parseVersionSegment(raw: string): number[] { - return raw - .replace(/^v/i, '') - .split('.') - .map((segment) => Number.parseInt(segment, 10)) - .map((segment) => (Number.isFinite(segment) ? segment : 0)) -} - -function compareVersionDesc(left: string, right: string): number { - const leftParts = parseVersionSegment(left) - const rightParts = parseVersionSegment(right) - const length = Math.max(leftParts.length, rightParts.length) - - for (let index = 0; index < length; index += 1) { - const delta = (rightParts[index] ?? 0) - (leftParts[index] ?? 0) - if (delta !== 0) { - return delta - } - } - - return right.localeCompare(left) -} - -function findFirstExecutable( - platform: NodeJS.Platform, - directories: string[], - executableNames: string[] -): string | null { - for (const directory of directories) { - for (const executableName of executableNames) { - const candidate = join(directory, executableName) - if (isRunnableCommand(platform, candidate)) { - return candidate - } - } - } - - return null -} - -function isRunnableCommand(platform: NodeJS.Platform, candidate: string): boolean { - try { - const stats = statSync(candidate) - if (!stats.isFile()) { - return false - } - if (platform === 'win32') { - return true - } - // Why: GUI fallback probing should skip placeholders/directories so spawn - // can continue to a runnable CLI instead of failing later with EACCES/EISDIR. - accessSync(candidate, constants.X_OK) - return true - } catch { - return false - } -} - -function getBaseVersionManagerDirectories(platform: NodeJS.Platform, homePath: string): string[] { - const directories = [ - join(homePath, '.volta', 'bin'), - join(homePath, '.asdf', 'shims'), - join(homePath, '.fnm', 'aliases', 'default', 'bin'), - // Why: mise (formerly rtx) exposes managed tool binaries via a shims - // directory, similar to asdf. Without this, users who installed node - // or CLI tools through mise can't be found by the fallback probe. - join(homePath, '.local', 'share', 'mise', 'shims') - ] - - if (platform === 'win32') { - // Why: Anthropic's native Windows installer places claude.exe here, and - // GUI-launched Orca may not inherit the user's PATH entry for it. - directories.push(join(homePath, '.local', 'bin')) - directories.push(join(homePath, 'AppData', 'Roaming', 'npm')) - directories.push(join(homePath, 'AppData', 'Local', 'pnpm')) - directories.push(join(homePath, 'AppData', 'Local', 'Yarn', 'bin')) - } else { - directories.push(join(homePath, '.local', 'bin')) - // Why: pnpm uses platform-specific global bin directories that differ from - // npm's ~/.local/bin. macOS follows the ~/Library convention while Linux - // uses the XDG-compatible ~/.local/share path. Without these, users who - // installed via `pnpm add -g` can't be found by the fallback probe. - if (platform === 'darwin') { - directories.push(join(homePath, 'Library', 'pnpm')) - } else { - directories.push(join(homePath, '.local', 'share', 'pnpm')) - } - directories.push(join(homePath, '.yarn', 'bin')) - } - - // Why: bun uses ~/.bun/bin on all platforms for globally installed packages. - directories.push(join(homePath, '.bun', 'bin')) - - return directories -} - -function getNvmVersionDirectories(homePath: string): string[] { - const nvmVersionsDir = join(homePath, '.nvm', 'versions', 'node') - if (!existsSync(nvmVersionsDir)) { - return [] - } - - return readdirSync(nvmVersionsDir, { withFileTypes: true }) - .filter((entry) => entry.isDirectory()) - .map((entry) => entry.name) - .sort(compareVersionDesc) - .map((entry) => join(nvmVersionsDir, entry, 'bin')) -} - -function getVersionManagerDirectories( - platform: NodeJS.Platform, - homePath: string, - executableNames: string[] -): string[] { - const directories = getBaseVersionManagerDirectories(platform, homePath) - - // Why: GUI-launched Electron apps do not inherit shell init from nvm, so - // command resolution probes the newest installed Node versions explicitly. - const firstNvmMatch = findFirstExecutable( - platform, - getNvmVersionDirectories(homePath), - executableNames - ) - if (firstNvmMatch) { - directories.unshift(dirname(firstNvmMatch)) - } - - return directories -} - -export function resolveCliCommand( - commandName: string, - options: ResolveCommandOptions = {} -): string { - const platform = options.platform ?? process.platform - const executableNames = getExecutableNames(platform, commandName) - const pathEnv = options.pathEnv ?? process.env.PATH ?? process.env.Path ?? null - const pathCandidate = findFirstExecutable(platform, splitPath(pathEnv), executableNames) - if (pathCandidate) { - return pathCandidate - } - - const homePath = options.homePath ?? homedir() - const versionManagerCandidate = findFirstExecutable( - platform, - getVersionManagerDirectories(platform, homePath, executableNames), - executableNames - ) - return versionManagerCandidate ?? commandName -} - -export function resolveCliCommands( - commandNames: readonly string[], - options: ResolveCommandOptions = {} -): Map { - const platform = options.platform ?? process.platform - const pathEnv = options.pathEnv ?? process.env.PATH ?? process.env.Path ?? null - const pathDirectories = splitPath(pathEnv) - const homePath = options.homePath ?? homedir() - // Why: agent detection probes many CLIs at once; compute expensive install - // directories, especially nvm versions, once per detection pass. - const installDirectories = [ - ...getNvmVersionDirectories(homePath), - ...getBaseVersionManagerDirectories(platform, homePath) - ] - const resolved = new Map() - - for (const commandName of new Set(commandNames)) { - const executableNames = getExecutableNames(platform, commandName) - const pathCandidate = findFirstExecutable(platform, pathDirectories, executableNames) - const installCandidate = - pathCandidate ?? findFirstExecutable(platform, installDirectories, executableNames) - resolved.set(commandName, installCandidate ?? commandName) - } - - return resolved -} - -export function resolveCodexCommand(options: ResolveCommandOptions = {}): string { - return resolveCliCommand('codex', options) -} - -export function resolveClaudeCommand(options: ResolveCommandOptions = {}): string { - return resolveCliCommand('claude', options) -} - -// Why: GUI-launched Electron apps inherit a minimal PATH that excludes Node -// version manager directories. CLI tools like codex/claude are Node scripts -// with #!/usr/bin/env node shebangs — they need `node` in PATH to execute, -// not just to be *found*. This function returns the version manager bin paths -// so the caller can augment process.env.PATH at startup. -export function getVersionManagerBinPaths(options: ResolveCommandOptions = {}): string[] { - const platform = options.platform ?? process.platform - const homePath = options.homePath ?? homedir() - const nodeNames = getExecutableNames(platform, 'node') - return getVersionManagerDirectories(platform, homePath, nodeNames) -} +export * from '../../shared/node-cli-command-resolution' diff --git a/src/main/ipc/local-agent-install-dir-detection.ts b/src/main/ipc/local-agent-install-dir-detection.ts index 1364b5bc7d7..7493affdb47 100644 --- a/src/main/ipc/local-agent-install-dir-detection.ts +++ b/src/main/ipc/local-agent-install-dir-detection.ts @@ -1,18 +1 @@ -import path from 'node:path' -import { resolveCliCommands } from '../codex-cli/command' - -// Why: local agent detection may run before shell-PATH hydration, but the -// fallback must stay bounded because it runs on the main process. -export function detectCommandsInInstallDirs(commands: readonly string[]): Set { - if (commands.length === 0) { - return new Set() - } - try { - const resolvedCommands = resolveCliCommands(commands) - return new Set( - commands.filter((command) => path.isAbsolute(resolvedCommands.get(command) ?? command)) - ) - } catch { - return new Set() - } -} +export { detectCommandsInInstallDirs } from '../../shared/local-agent-install-dir-detection' diff --git a/src/main/ipc/preflight.test.ts b/src/main/ipc/preflight.test.ts index fead0c97c86..c959bda20a1 100644 --- a/src/main/ipc/preflight.test.ts +++ b/src/main/ipc/preflight.test.ts @@ -54,7 +54,7 @@ vi.mock('../startup/hydrate-shell-path', () => ({ mergePathSegments: mergePathSegmentsMock })) -vi.mock('../codex-cli/command', () => ({ +vi.mock('../../shared/node-cli-command-resolution', () => ({ resolveCliCommands: resolveCliCommandsMock })) diff --git a/src/main/ipc/tui-agent-detection-commands.ts b/src/main/ipc/tui-agent-detection-commands.ts index dc5d5909543..88d65c1fa34 100644 --- a/src/main/ipc/tui-agent-detection-commands.ts +++ b/src/main/ipc/tui-agent-detection-commands.ts @@ -1,77 +1 @@ -import type { TuiAgent } from '../../shared/types' -import { - getTuiAgentDetectCommands, - TUI_AGENT_CONFIG, - type TuiAgentConfig, - type TuiAgentDetectionRuntime -} from '../../shared/tui-agent-config' - -export type TuiAgentDetectionCommand = { - id: TuiAgent - cmd: string - requiredCommands?: readonly string[] - unsupportedRuntimes?: readonly TuiAgentDetectionRuntime[] -} - -export const KNOWN_TUI_AGENT_DETECTION_COMMANDS = buildTuiAgentDetectionCommands() - -function buildTuiAgentDetectionCommands(): TuiAgentDetectionCommand[] { - return Object.entries(TUI_AGENT_CONFIG).flatMap(([id, config]) => - getTuiAgentDetectCommands(config).map((cmd) => - buildTuiAgentDetectionCommand(id as TuiAgent, cmd, config) - ) - ) -} - -function buildTuiAgentDetectionCommand( - id: TuiAgent, - cmd: string, - config: TuiAgentConfig -): TuiAgentDetectionCommand { - return { - id, - cmd, - ...(config.detectRequiredCommands?.length - ? { requiredCommands: config.detectRequiredCommands } - : {}), - ...(config.detectUnsupportedRuntimes?.length - ? { unsupportedRuntimes: config.detectUnsupportedRuntimes } - : {}) - } -} - -export function getTuiAgentDetectionProbeCommands( - commands: readonly TuiAgentDetectionCommand[], - runtime: TuiAgentDetectionRuntime -): string[] { - return [ - ...new Set( - commands - .filter((command) => !isDetectionUnsupportedInRuntime(command, runtime)) - .flatMap((command) => [command.cmd, ...(command.requiredCommands ?? [])]) - ) - ] -} - -export function resolveDetectedTuiAgentIds( - commands: readonly TuiAgentDetectionCommand[], - foundCommands: ReadonlySet, - runtime: TuiAgentDetectionRuntime -): TuiAgent[] { - const detected = commands - .filter( - (command) => - !isDetectionUnsupportedInRuntime(command, runtime) && - foundCommands.has(command.cmd) && - (command.requiredCommands ?? []).every((required) => foundCommands.has(required)) - ) - .map(({ id }) => id) - return [...new Set(detected)] -} - -export function isDetectionUnsupportedInRuntime( - command: TuiAgentDetectionCommand, - runtime: TuiAgentDetectionRuntime -): boolean { - return command.unsupportedRuntimes?.includes(runtime) === true -} +export * from '../../shared/tui-agent-detection-commands' diff --git a/src/main/win32-utils.test.ts b/src/main/win32-utils.test.ts index abf0b0964be..3f719e5ae34 100644 --- a/src/main/win32-utils.test.ts +++ b/src/main/win32-utils.test.ts @@ -119,17 +119,29 @@ describe('getSpawnArgsForWindows', () => { it('rejects unsafe args for .cmd scripts on win32', () => { withPlatform('win32', () => { - expect(() => getSpawnArgsForWindows('C:\\tools\\agent.cmd', ['hello & goodbye'])).toThrow( - 'UNSAFE_WINDOWS_BATCH_ARGUMENTS' - ) + for (const argument of ['hello & goodbye', 'close)', '(open']) { + expect(() => getSpawnArgsForWindows('C:\\tools\\agent.cmd', [argument])).toThrow( + 'UNSAFE_WINDOWS_BATCH_ARGUMENTS' + ) + } }) }) it('rejects unsafe command paths for .cmd scripts on win32', () => { withPlatform('win32', () => { - expect(() => getSpawnArgsForWindows('C:\\bad&path\\agent.cmd', ['login'])).toThrow( - 'UNSAFE_WINDOWS_BATCH_ARGUMENTS' - ) + for (const command of ['C:\\bad&path\\agent.cmd', 'C:\\bad(path\\agent.cmd']) { + expect(() => getSpawnArgsForWindows(command, ['login'])).toThrow( + 'UNSAFE_WINDOWS_BATCH_ARGUMENTS' + ) + } + }) + }) + + it('allows punctuation that is not a cmd command operator', () => { + withPlatform('win32', () => { + expect( + getSpawnArgsForWindows('C:\\tools\\agent.cmd', ['package,name;version']).spawnArgs + ).toEqual(['/d', '/c', 'C:\\tools\\agent.cmd', 'package,name;version']) }) }) }) diff --git a/src/main/win32-utils.ts b/src/main/win32-utils.ts index 4edc47b93a1..21f3a75853d 100644 --- a/src/main/win32-utils.ts +++ b/src/main/win32-utils.ts @@ -2,6 +2,14 @@ import { execFile, execFileSync, type ExecFileOptionsWithStringEncoding } from ' import { delimiter, join, win32 } from 'node:path' import { existsSync } from 'node:fs' +export { + getCmdExePath, + getSpawnArgsForWindows, + isWindowsBatchScript, + WINDOWS_BATCH_UNSAFE_ARGUMENTS_ERROR, + UnsafeWindowsBatchArgumentsError +} from '../shared/windows-batch-spawn' + function execFileWithoutBlocking( command: string, args: string[], @@ -38,20 +46,6 @@ export function getRegExePath(env: NodeJS.ProcessEnv = process.env): string { return win32.join(root, 'System32', 'reg.exe') } -/** - * Full path to cmd.exe, respecting the ComSpec convention used elsewhere in - * the codebase (hooks.ts, repo.ts, ssh-connection-utils.ts). - * Falls back to SystemRoot-based path if ComSpec is unset. - */ -export function getCmdExePath(): string { - return process.env.ComSpec || `${process.env.SystemRoot ?? 'C:\\Windows'}\\System32\\cmd.exe` -} - -/** Whether a resolved command path points to a Windows batch script (.cmd/.bat). */ -export function isWindowsBatchScript(commandPath: string): boolean { - return process.platform === 'win32' && /\.(cmd|bat)$/i.test(commandPath) -} - export function resolveWindowsCommand( command: string, env: NodeJS.ProcessEnv = process.env @@ -79,19 +73,6 @@ export function resolveWindowsCommand( return command } -export const WINDOWS_BATCH_UNSAFE_ARGUMENTS_ERROR = 'UNSAFE_WINDOWS_BATCH_ARGUMENTS' - -export class UnsafeWindowsBatchArgumentsError extends Error { - constructor() { - super(WINDOWS_BATCH_UNSAFE_ARGUMENTS_ERROR) - this.name = 'UnsafeWindowsBatchArgumentsError' - } -} - -function hasUnsafeWindowsBatchSyntax(value: string): boolean { - return /[&|<>^"%!\r\n]/.test(value) -} - /** Check whether an error is a Windows permission error (EACCES or EPERM). */ export function isPermissionError(error: unknown): boolean { return ( @@ -235,37 +216,3 @@ export async function grantDirAclAsync(dirPath: string): Promise { } ) } - -/** - * Resolve spawn parameters for a command that may be a Windows batch script. - * - * Why: Node's spawn() cannot execute .cmd/.bat files directly without - * shell:true, but shell:true with an args array triggers DEP0190 because - * args are concatenated, not escaped. Routing through cmd.exe /c explicitly - * avoids the deprecation warning while passing args correctly. - * - * Why /d: disables per-machine/user AutoRun registry commands so a background - * spawn cannot inherit surprising side effects from the user's shell config. - * - * SAFETY: when the .cmd/.bat branch is taken, cmd.exe re-parses the command - * line. Args with cmd metacharacters are rejected instead of escaped because - * the agent prompt may contain arbitrary staged diff text. - */ -export function getSpawnArgsForWindows( - command: string, - args: string[] -): { spawnCmd: string; spawnArgs: string[] } { - if (isWindowsBatchScript(command)) { - for (const value of [command, ...args]) { - if (hasUnsafeWindowsBatchSyntax(value)) { - throw new UnsafeWindowsBatchArgumentsError() - } - } - - // Why: when Node passes a pre-quoted command line as one argv entry, - // cmd.exe sees literal escaped quotes on Windows and refuses to run .cmd - // shims. Separate argv entries let Node quote spaces without breaking cmd. - return { spawnCmd: getCmdExePath(), spawnArgs: ['/d', '/c', command, ...args] } - } - return { spawnCmd: command, spawnArgs: args } -} diff --git a/src/shared/local-agent-install-dir-detection.ts b/src/shared/local-agent-install-dir-detection.ts new file mode 100644 index 00000000000..9b29a142f9e --- /dev/null +++ b/src/shared/local-agent-install-dir-detection.ts @@ -0,0 +1,17 @@ +import path from 'node:path' +import { resolveCliCommands } from './node-cli-command-resolution' + +// Why: detection may precede shell-PATH hydration, but the fallback stays bounded. +export function detectCommandsInInstallDirs(commands: readonly string[]): Set { + if (commands.length === 0) { + return new Set() + } + try { + const resolvedCommands = resolveCliCommands(commands) + return new Set( + commands.filter((command) => path.isAbsolute(resolvedCommands.get(command) ?? command)) + ) + } catch { + return new Set() + } +} diff --git a/src/shared/node-cli-command-resolution.ts b/src/shared/node-cli-command-resolution.ts new file mode 100644 index 00000000000..44e85c4dd08 --- /dev/null +++ b/src/shared/node-cli-command-resolution.ts @@ -0,0 +1,218 @@ +import { accessSync, constants, existsSync, readdirSync, statSync } from 'node:fs' +import { homedir } from 'node:os' +import { delimiter, dirname, join } from 'node:path' + +type ResolveCommandOptions = { + pathEnv?: string | null + platform?: NodeJS.Platform + homePath?: string +} + +function getExecutableNames(platform: NodeJS.Platform, commandName: string): string[] { + if (platform === 'win32') { + return [`${commandName}.cmd`, `${commandName}.exe`, `${commandName}.bat`, commandName] + } + + return [commandName] +} + +function splitPath(pathEnv: string | null | undefined): string[] { + if (!pathEnv) { + return [] + } + + return pathEnv + .split(delimiter) + .map((entry) => entry.trim()) + .filter(Boolean) +} + +function parseVersionSegment(raw: string): number[] { + return raw + .replace(/^v/i, '') + .split('.') + .map((segment) => Number.parseInt(segment, 10)) + .map((segment) => (Number.isFinite(segment) ? segment : 0)) +} + +function compareVersionDesc(left: string, right: string): number { + const leftParts = parseVersionSegment(left) + const rightParts = parseVersionSegment(right) + const length = Math.max(leftParts.length, rightParts.length) + + for (let index = 0; index < length; index += 1) { + const delta = (rightParts[index] ?? 0) - (leftParts[index] ?? 0) + if (delta !== 0) { + return delta + } + } + + return right.localeCompare(left) +} + +function findFirstExecutable( + platform: NodeJS.Platform, + directories: string[], + executableNames: string[] +): string | null { + for (const directory of directories) { + for (const executableName of executableNames) { + const candidate = join(directory, executableName) + if (isRunnableCommand(platform, candidate)) { + return candidate + } + } + } + + return null +} + +function isRunnableCommand(platform: NodeJS.Platform, candidate: string): boolean { + try { + const stats = statSync(candidate) + if (!stats.isFile()) { + return false + } + if (platform === 'win32') { + return true + } + // Why: GUI fallback probing should skip placeholders/directories so spawn + // can continue to a runnable CLI instead of failing later with EACCES/EISDIR. + accessSync(candidate, constants.X_OK) + return true + } catch { + return false + } +} + +function getBaseVersionManagerDirectories(platform: NodeJS.Platform, homePath: string): string[] { + const directories = [ + join(homePath, '.volta', 'bin'), + join(homePath, '.asdf', 'shims'), + join(homePath, '.fnm', 'aliases', 'default', 'bin'), + // Why: mise (formerly rtx) exposes managed tool binaries via a shims + // directory, similar to asdf. + join(homePath, '.local', 'share', 'mise', 'shims') + ] + + if (platform === 'win32') { + // Why: Anthropic's native Windows installer places claude.exe here, and + // GUI-launched Orca may not inherit the user's PATH entry for it. + directories.push(join(homePath, '.local', 'bin')) + directories.push(join(homePath, 'AppData', 'Roaming', 'npm')) + directories.push(join(homePath, 'AppData', 'Local', 'pnpm')) + directories.push(join(homePath, 'AppData', 'Local', 'Yarn', 'bin')) + } else { + directories.push(join(homePath, '.local', 'bin')) + // Why: pnpm uses platform-specific global bin directories that differ from + // npm's ~/.local/bin. + if (platform === 'darwin') { + directories.push(join(homePath, 'Library', 'pnpm')) + } else { + directories.push(join(homePath, '.local', 'share', 'pnpm')) + } + directories.push(join(homePath, '.yarn', 'bin')) + } + + directories.push(join(homePath, '.bun', 'bin')) + return directories +} + +function getNvmVersionDirectories(homePath: string): string[] { + const nvmVersionsDir = join(homePath, '.nvm', 'versions', 'node') + if (!existsSync(nvmVersionsDir)) { + return [] + } + + return readdirSync(nvmVersionsDir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .sort(compareVersionDesc) + .map((entry) => join(nvmVersionsDir, entry, 'bin')) +} + +function getVersionManagerDirectories( + platform: NodeJS.Platform, + homePath: string, + executableNames: string[] +): string[] { + const directories = getBaseVersionManagerDirectories(platform, homePath) + const firstNvmMatch = findFirstExecutable( + platform, + getNvmVersionDirectories(homePath), + executableNames + ) + if (firstNvmMatch) { + directories.unshift(dirname(firstNvmMatch)) + } + return directories +} + +export function resolveCliCommand( + commandName: string, + options: ResolveCommandOptions = {} +): string { + const platform = options.platform ?? process.platform + const executableNames = getExecutableNames(platform, commandName) + const pathEnv = options.pathEnv ?? process.env.PATH ?? process.env.Path ?? null + const pathCandidate = findFirstExecutable(platform, splitPath(pathEnv), executableNames) + if (pathCandidate) { + return pathCandidate + } + + const homePath = options.homePath ?? homedir() + const nvmCandidate = findFirstExecutable( + platform, + getNvmVersionDirectories(homePath), + executableNames + ) + const versionManagerCandidate = + nvmCandidate ?? + findFirstExecutable( + platform, + getBaseVersionManagerDirectories(platform, homePath), + executableNames + ) + return versionManagerCandidate ?? commandName +} + +export function resolveCliCommands( + commandNames: readonly string[], + options: ResolveCommandOptions = {} +): Map { + const platform = options.platform ?? process.platform + const pathEnv = options.pathEnv ?? process.env.PATH ?? process.env.Path ?? null + const pathDirectories = splitPath(pathEnv) + const homePath = options.homePath ?? homedir() + const installDirectories = [ + ...getNvmVersionDirectories(homePath), + ...getBaseVersionManagerDirectories(platform, homePath) + ] + const resolved = new Map() + + for (const commandName of new Set(commandNames)) { + const executableNames = getExecutableNames(platform, commandName) + const pathCandidate = findFirstExecutable(platform, pathDirectories, executableNames) + const installCandidate = + pathCandidate ?? findFirstExecutable(platform, installDirectories, executableNames) + resolved.set(commandName, installCandidate ?? commandName) + } + + return resolved +} + +export function resolveCodexCommand(options: ResolveCommandOptions = {}): string { + return resolveCliCommand('codex', options) +} + +export function resolveClaudeCommand(options: ResolveCommandOptions = {}): string { + return resolveCliCommand('claude', options) +} + +// Why: Node-script CLIs need their version-manager sibling `node` on PATH. +export function getVersionManagerBinPaths(options: ResolveCommandOptions = {}): string[] { + const platform = options.platform ?? process.platform + const homePath = options.homePath ?? homedir() + const nodeNames = getExecutableNames(platform, 'node') + return getVersionManagerDirectories(platform, homePath, nodeNames) +} diff --git a/src/shared/tui-agent-detection-commands.ts b/src/shared/tui-agent-detection-commands.ts new file mode 100644 index 00000000000..b0ca1b443b8 --- /dev/null +++ b/src/shared/tui-agent-detection-commands.ts @@ -0,0 +1,77 @@ +import type { TuiAgent } from './types' +import { + getTuiAgentDetectCommands, + TUI_AGENT_CONFIG, + type TuiAgentConfig, + type TuiAgentDetectionRuntime +} from './tui-agent-config' + +export type TuiAgentDetectionCommand = { + id: TuiAgent + cmd: string + requiredCommands?: readonly string[] + unsupportedRuntimes?: readonly TuiAgentDetectionRuntime[] +} + +export const KNOWN_TUI_AGENT_DETECTION_COMMANDS = buildTuiAgentDetectionCommands() + +function buildTuiAgentDetectionCommands(): TuiAgentDetectionCommand[] { + return Object.entries(TUI_AGENT_CONFIG).flatMap(([id, config]) => + getTuiAgentDetectCommands(config).map((cmd) => + buildTuiAgentDetectionCommand(id as TuiAgent, cmd, config) + ) + ) +} + +function buildTuiAgentDetectionCommand( + id: TuiAgent, + cmd: string, + config: TuiAgentConfig +): TuiAgentDetectionCommand { + return { + id, + cmd, + ...(config.detectRequiredCommands?.length + ? { requiredCommands: config.detectRequiredCommands } + : {}), + ...(config.detectUnsupportedRuntimes?.length + ? { unsupportedRuntimes: config.detectUnsupportedRuntimes } + : {}) + } +} + +export function getTuiAgentDetectionProbeCommands( + commands: readonly TuiAgentDetectionCommand[], + runtime: TuiAgentDetectionRuntime +): string[] { + return [ + ...new Set( + commands + .filter((command) => !isDetectionUnsupportedInRuntime(command, runtime)) + .flatMap((command) => [command.cmd, ...(command.requiredCommands ?? [])]) + ) + ] +} + +export function resolveDetectedTuiAgentIds( + commands: readonly TuiAgentDetectionCommand[], + foundCommands: ReadonlySet, + runtime: TuiAgentDetectionRuntime +): TuiAgent[] { + const detected = commands + .filter( + (command) => + !isDetectionUnsupportedInRuntime(command, runtime) && + foundCommands.has(command.cmd) && + (command.requiredCommands ?? []).every((required) => foundCommands.has(required)) + ) + .map(({ id }) => id) + return [...new Set(detected)] +} + +export function isDetectionUnsupportedInRuntime( + command: TuiAgentDetectionCommand, + runtime: TuiAgentDetectionRuntime +): boolean { + return command.unsupportedRuntimes?.includes(runtime) === true +} diff --git a/src/shared/windows-batch-spawn.ts b/src/shared/windows-batch-spawn.ts new file mode 100644 index 00000000000..cd20dff5c79 --- /dev/null +++ b/src/shared/windows-batch-spawn.ts @@ -0,0 +1,43 @@ +import { win32 } from 'node:path' + +/** Full path to cmd.exe for GUI and service-launched processes. */ +export function getCmdExePath(): string { + return ( + process.env.ComSpec || + win32.join(process.env.SystemRoot ?? 'C:\\Windows', 'System32', 'cmd.exe') + ) +} + +export function isWindowsBatchScript(commandPath: string): boolean { + return process.platform === 'win32' && /\.(cmd|bat)$/i.test(commandPath) +} + +export const WINDOWS_BATCH_UNSAFE_ARGUMENTS_ERROR = 'UNSAFE_WINDOWS_BATCH_ARGUMENTS' + +export class UnsafeWindowsBatchArgumentsError extends Error { + constructor() { + super(WINDOWS_BATCH_UNSAFE_ARGUMENTS_ERROR) + this.name = 'UnsafeWindowsBatchArgumentsError' + } +} + +function hasUnsafeWindowsBatchSyntax(value: string): boolean { + return /[&|<>^"%!()\r\n]/.test(value) +} + +export function getSpawnArgsForWindows( + command: string, + args: string[] +): { spawnCmd: string; spawnArgs: string[] } { + if (isWindowsBatchScript(command)) { + for (const value of [command, ...args]) { + if (hasUnsafeWindowsBatchSyntax(value)) { + throw new UnsafeWindowsBatchArgumentsError() + } + } + + // Why: separate argv entries let Node quote spaces without breaking cmd. + return { spawnCmd: getCmdExePath(), spawnArgs: ['/d', '/c', command, ...args] } + } + return { spawnCmd: command, spawnArgs: args } +}