From 6ecafd2914a10e0442c2c93846d21e20bc68a37e Mon Sep 17 00:00:00 2001 From: Andro Marces Date: Sun, 26 Jul 2026 01:56:01 +0800 Subject: [PATCH 1/2] fix(cli): make the Windows test gate runnable and green pnpm --filter @codor/cli test could not spawn vitest on Windows: the wrapper (scripts/test-env.mjs) spawned bare command names directly, but child_process.spawn cannot execute a node_modules/.bin .CMD shim without shell:true. Resolve a bare bin name to its real entrypoint and run it under node directly instead; a caller-supplied path (e.g. process.execPath, used by the wrapper's own spec) is spawned unchanged. That unblocked the CLI suite's five pre-existing Windows failures, each with its own root cause: - runtime-install.spec.ts / setup-tailscale.spec.ts: POSIX-literal fixture paths didn't match what packageRuntimePaths()/resolveTailscale() actually produce after resolve() drive-qualifies them on Windows; build the fixtures the same way the code under test does. - index.spec.ts: a real Windows path was substituted unescaped into a JSON fixture, producing invalid JSON; JSON-escape it, and use the raw (unescaped) path for the actual file write. - index.spec.ts: a two-cycle real server bootstrap test routinely exceeds vitest's 5s default on Windows; extend its timeout. - artifact.spec.ts: Windows has no POSIX executable permission bit, so skip that one assertion there instead of asserting a property NTFS doesn't track. --- packages/cli/src/artifact.spec.ts | 7 +++++- packages/cli/src/index.spec.ts | 11 +++++--- packages/cli/src/runtime-install.spec.ts | 7 ++++-- packages/cli/src/setup-tailscale.spec.ts | 7 +++++- scripts/test-env.mjs | 32 +++++++++++++++++++++++- 5 files changed, 56 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/artifact.spec.ts b/packages/cli/src/artifact.spec.ts index eba8082b..c5fd5580 100644 --- a/packages/cli/src/artifact.spec.ts +++ b/packages/cli/src/artifact.spec.ts @@ -154,7 +154,12 @@ describe('built public artifact', () => { expect(readFileSync(join(cliRoot, 'runtime', 'web', 'index.html'), 'utf8')).toContain(''); expect(readFileSync(join(cliRoot, 'packaging', 'systemd', 'codor.service'), 'utf8')) .toContain('ExecStart='); - expect(statSync(join(outDir, 'bin', 'codor.mjs')).mode & 0o111).not.toBe(0); + // Windows has no POSIX executable permission bit — writeFileSync's `mode` + // option cannot set one there, so this property only exists to check on + // platforms where the filesystem tracks it. + if (process.platform !== 'win32') { + expect(statSync(join(outDir, 'bin', 'codor.mjs')).mode & 0o111).not.toBe(0); + } }); it('contains no source, specs, fixtures, symlinks, or workspace protocols', () => { diff --git a/packages/cli/src/index.spec.ts b/packages/cli/src/index.spec.ts index bbd00a49..f7f7aa47 100644 --- a/packages/cli/src/index.spec.ts +++ b/packages/cli/src/index.spec.ts @@ -1264,12 +1264,14 @@ describe('@codor/cli', () => { const fixtures = fileURLToPath(new URL('../fixtures/', import.meta.url)); const claudeTranscript = join(dir, 'claude-transcript.jsonl'); writeFileSync( - JSON.stringify(claudeTranscript).slice(1, -1), + claudeTranscript, readFileSync(join(fixtures, 'claude-transcript.jsonl'), 'utf8'), ); + // JSON-escape the path before embedding it in a JSON string value — a raw + // Windows path's single backslashes are not valid JSON escape sequences. const claudeRaw = readFileSync(join(fixtures, 'claude-stop.json'), 'utf8').replace( 'CLAUDE_TRANSCRIPT_PATH', - claudeTranscript, + JSON.stringify(claudeTranscript).slice(1, -1), ); expect(parseMirrorHook('claude', claudeRaw)).toMatchObject({ type: 'mirror_turn', @@ -1314,6 +1316,9 @@ describe('@codor/cli', () => { // harn:assume empty-database-desk-uses-service-home ref=bootstrap-service-home-regression // harn:assume empty-database-desk-seeds-tutorial-atomically ref=bootstrap-tutorial-regression + // Two full startCodor/close cycles plus native module load routinely exceed + // vitest's 5s default on Windows, where process and filesystem overhead is + // higher than on POSIX; this does not indicate a hang (observed ~6s). it('bootstraps one home-rooted Desk and one durable Tutorial message', async () => { const homeDir = join(dir, 'service-home'); mkdirSync(homeDir); @@ -1407,7 +1412,7 @@ describe('@codor/cli', () => { } finally { await restarted.close(); } - }); + }, 15000); // harn:end empty-database-desk-seeds-tutorial-atomically // harn:end empty-database-desk-uses-service-home diff --git a/packages/cli/src/runtime-install.spec.ts b/packages/cli/src/runtime-install.spec.ts index 883f1116..24c872d3 100644 --- a/packages/cli/src/runtime-install.spec.ts +++ b/packages/cli/src/runtime-install.spec.ts @@ -1,6 +1,6 @@ import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join, sep } from 'node:path'; +import { join, resolve, sep } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; @@ -12,7 +12,10 @@ import { } from './runtime-install.js'; import { type RuntimePaths } from './runtime-paths.js'; -const HOME = '/home/u'; +// Built through `resolve()`, the same call the code under test makes on a +// durable install root, so this fixture already carries a Windows drive +// prefix where the implementation's own normalization would add one. +const HOME = resolve(sep, 'home', 'u'); const DATA = join(HOME, '.codor'); const LOCATION = join(DATA, 'runtime'); diff --git a/packages/cli/src/setup-tailscale.spec.ts b/packages/cli/src/setup-tailscale.spec.ts index 69d25568..2cb995b7 100644 --- a/packages/cli/src/setup-tailscale.spec.ts +++ b/packages/cli/src/setup-tailscale.spec.ts @@ -1,3 +1,5 @@ +import { resolve, sep } from 'node:path'; + import { describe, expect, it } from 'vitest'; import { configureTailscaleServe, resolveTailscale, tailscaleServeSupported } from './setup.js'; @@ -6,7 +8,10 @@ const APP_CLI = '/Applications/Tailscale.app/Contents/MacOS/Tailscale'; describe('resolveTailscale', () => { it('returns the PATH hit when present', () => { - expect(resolveTailscale(() => '/usr/bin/tailscale', 'darwin', () => true)).toBe('/usr/bin/tailscale'); + // resolveTailscale normalizes the PATH hit through resolve(); build the + // fixture the same way so the expectation matches on every platform. + const pathHit = resolve(sep, 'usr', 'bin', 'tailscale'); + expect(resolveTailscale(() => pathHit, 'darwin', () => true)).toBe(pathHit); }); it('falls back to the macOS app location when PATH misses but the app exists', () => { diff --git a/scripts/test-env.mjs b/scripts/test-env.mjs index e7ba61d2..944b8a0c 100755 --- a/scripts/test-env.mjs +++ b/scripts/test-env.mjs @@ -1,6 +1,10 @@ #!/usr/bin/env node import { spawn } from 'node:child_process'; +import { createRequire } from 'node:module'; +import { dirname, isAbsolute, join } from 'node:path'; + +const require = createRequire(import.meta.url); const [command, ...args] = process.argv.slice(2); if (command === undefined) { @@ -8,6 +12,20 @@ if (command === undefined) { process.exit(2); } +// Resolve a bare package bin name (e.g. "vitest") to its real entrypoint and +// run it under `node` directly, bypassing the platform-specific +// node_modules/.bin shim (a .CMD file on Windows, which child_process.spawn +// refuses to execute without `shell: true`). A command that already looks +// like a path (e.g. an absolute executable path a caller resolved itself) is +// spawned unchanged below. +function resolveBinEntry(name) { + const packageJsonPath = require.resolve(`${name}/package.json`); + const pkg = require(packageJsonPath); + const bin = typeof pkg.bin === 'string' ? pkg.bin : pkg.bin?.[name]; + if (bin === undefined) throw new Error(`"${name}" declares no "${name}" bin entry (${packageJsonPath})`); + return join(dirname(packageJsonPath), bin); +} + // harn:assume cli-tests-delete-inherited-codor-environment ref=cli-test-environment-wrapper const env = { ...process.env }; const removed = Object.keys(env).filter((key) => key.startsWith('CODOR_')).sort(); @@ -16,7 +34,19 @@ if (removed.length > 0) { process.stderr.write(`test-env: removed ${String(removed.length)} inherited CODOR_ variable(s): ${removed.join(', ')}\n`); } -const child = spawn(command, args, { stdio: 'inherit', env }); +const looksLikePath = command.includes('/') || command.includes('\\') || isAbsolute(command); +let spawnCommand = command; +let spawnArgs = args; +if (!looksLikePath) { + try { + spawnCommand = process.execPath; + spawnArgs = [resolveBinEntry(command), ...args]; + } catch (error) { + process.stderr.write(`test-env: failed to resolve ${command}: ${error.message}\n`); + process.exit(1); + } +} +const child = spawn(spawnCommand, spawnArgs, { stdio: 'inherit', env }); child.on('error', (error) => { process.stderr.write(`test-env: failed to spawn ${command}: ${error.message}\n`); process.exitCode = 1; From ee7f6a6fa039dd2b34f0bda5a2a0d032da514734 Mon Sep 17 00:00:00 2001 From: Andro Marces Date: Sun, 26 Jul 2026 02:09:14 +0800 Subject: [PATCH 2/2] fix(cli): restore bare-executable fallback in test-env.mjs Codex review of PR 11 caught a compatibility regression: resolving a bare command through its package.json bin entry was made mandatory, so a bare non-package executable (node, git, etc.) now failed instead of spawning directly as it did before. Only the codebase's one caller (bare `vitest`) happened to still work, masking the narrowed contract. Make resolution a preference: fall back to spawning the bare command unchanged when it names no resolvable package bin entry. Add regression coverage for both the bin-name-resolution path (bare `vitest`) and the generic-executable fallback path (bare `node`). --- packages/cli/src/artifact.spec.ts | 17 ++++++++++++++ scripts/test-env.mjs | 38 +++++++++++++++---------------- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/artifact.spec.ts b/packages/cli/src/artifact.spec.ts index c5fd5580..b3261c38 100644 --- a/packages/cli/src/artifact.spec.ts +++ b/packages/cli/src/artifact.spec.ts @@ -215,4 +215,21 @@ describe('CLI test environment isolation', () => { expect(String(failure.stderr)).not.toContain('never-print-this-value'); } }); + + it('spawns a bare npm bin name by resolving its real entrypoint', () => { + // vitest in node_modules/.bin is a .CMD shim on Windows; this proves the + // wrapper resolves and runs it rather than spawning the bare name as-is. + const output = execFileSync(process.execPath, [wrapper, 'vitest', '--version'], { encoding: 'utf8' }); + expect(output).toMatch(/^vitest\//); + }); + + it('falls back to spawning a bare non-package executable directly', () => { + // `node` names no npm package in this workspace; the wrapper must still + // spawn it directly rather than requiring a package.json bin entry. + const output = execFileSync(process.execPath, [wrapper, 'node', '-e', 'process.stdout.write("ok")'], { + encoding: 'utf8', + env: poisoned, + }); + expect(output).toBe('ok'); + }); }); diff --git a/scripts/test-env.mjs b/scripts/test-env.mjs index 944b8a0c..552c6061 100755 --- a/scripts/test-env.mjs +++ b/scripts/test-env.mjs @@ -12,18 +12,25 @@ if (command === undefined) { process.exit(2); } -// Resolve a bare package bin name (e.g. "vitest") to its real entrypoint and -// run it under `node` directly, bypassing the platform-specific +// Resolve a bare package bin name (e.g. "vitest") to its real entrypoint so +// it can run under `node` directly, bypassing the platform-specific // node_modules/.bin shim (a .CMD file on Windows, which child_process.spawn -// refuses to execute without `shell: true`). A command that already looks -// like a path (e.g. an absolute executable path a caller resolved itself) is -// spawned unchanged below. +// refuses to execute without `shell: true`). Returns undefined for a bare +// command that names no such package (e.g. a system executable like `node` +// or `git`) — the caller falls back to spawning it directly, exactly as +// before this resolution existed. A command that already looks like a path +// (e.g. an absolute executable path a caller resolved itself) is spawned +// unchanged below and never reaches this function. function resolveBinEntry(name) { - const packageJsonPath = require.resolve(`${name}/package.json`); + let packageJsonPath; + try { + packageJsonPath = require.resolve(`${name}/package.json`); + } catch { + return undefined; + } const pkg = require(packageJsonPath); const bin = typeof pkg.bin === 'string' ? pkg.bin : pkg.bin?.[name]; - if (bin === undefined) throw new Error(`"${name}" declares no "${name}" bin entry (${packageJsonPath})`); - return join(dirname(packageJsonPath), bin); + return bin === undefined ? undefined : join(dirname(packageJsonPath), bin); } // harn:assume cli-tests-delete-inherited-codor-environment ref=cli-test-environment-wrapper @@ -35,17 +42,10 @@ if (removed.length > 0) { } const looksLikePath = command.includes('/') || command.includes('\\') || isAbsolute(command); -let spawnCommand = command; -let spawnArgs = args; -if (!looksLikePath) { - try { - spawnCommand = process.execPath; - spawnArgs = [resolveBinEntry(command), ...args]; - } catch (error) { - process.stderr.write(`test-env: failed to resolve ${command}: ${error.message}\n`); - process.exit(1); - } -} +const binEntry = looksLikePath ? undefined : resolveBinEntry(command); +const [spawnCommand, spawnArgs] = binEntry === undefined + ? [command, args] + : [process.execPath, [binEntry, ...args]]; const child = spawn(spawnCommand, spawnArgs, { stdio: 'inherit', env }); child.on('error', (error) => { process.stderr.write(`test-env: failed to spawn ${command}: ${error.message}\n`);