fix(server): harden Codex/Windows startup and provider resolution (#454)
* fix(server): harden Codex/Windows startup and provider resolution
Addresses #452, #443, #353, #418, #403, #221, #307, and locks in #284.
- Replace custom where.exe/which output parsing with npm `which@^5` plus a
spawn probe. findExecutable now enumerates all PATH+PATHEXT candidates
and returns the first invokable one. A WindowsApps-ACL'd codex.exe no
longer wins over a working codex.cmd (#452).
- Make default provider isAvailable() check the binary instead of always
returning true. Codex/Claude/OpenCode default isAvailable() now defer
to isCommandAvailable() so missing CLIs surface as unavailable instead
of throwing later from spawn (#221, #443).
- Gate AgentManager.resumeAgentFromPersistence on isAvailable() so a
persisted agent record with a missing binary cannot reach provider
spawn during rehydration. The daemon stays up and the agent reports
unavailable (#443, #353, #418, #403).
- Drop --path-format=absolute from rev-parse callers and validate stdout
through a shared parser that rejects multi-line output and unknown-flag
echoes. --show-toplevel is absolute by default in modern Git;
--git-common-dir is resolved against the command cwd. Fixes workspace
registration on pre-2.31 Git that echoed the unknown flag and produced
a two-line "path" (#307).
Tests:
- Real-FS executable.test.ts using temp PATH fixtures, covering the .cmd
fallback after .exe pre-spawn failure (Windows-only) plus the
null-on-no-invokable-candidate case.
- provider-availability.test.ts builds real provider clients against a
temp-dir-only PATH for Codex/Claude/OpenCode.
- bootstrap-provider-availability.test.ts builds the daemon and triggers
ensureAgentLoaded so it actually exercises resumeAgentFromPersistence.
- claude-agent.spawn.test.ts asserts shell: false reaches spawnProcess
from the Claude SDK spawn override, locking in 39b56af4 for #284.
- checkout-git-rev-parse.test.ts covers nested-checkout resolution and
the old-Git multi-line stdout case via a tightly-scoped runGitCommand
fake.
CI:
- server-tests-windows now also runs the new and modified test files so
the Windows behaviors are exercised on windows-latest.
* fix: update lockfile signatures and Nix hash
* fix(server): handle synchronous spawn UNKNOWN on Windows
On Windows, child_process.spawn() throws synchronously when invoked on
a corrupt or invalid .exe (e.g., a WindowsApps stub the current user
cannot execute, or a zero-byte file). The executable probe did not
guard the spawn call, so the synchronous throw rejected the probe
promise instead of resolving false, preventing findExecutable() from
trying the next candidate. This is the root cause of the daemon-crash
pattern in #452: codex.exe from WindowsApps would hard-fail before the
.cmd shim ever got a chance.
Wrap spawn() in try/catch and settle false on sync throw. The existing
error-event and exit-event handlers already cover async failure modes;
sync throw just needed one more guard.
Also adjust three Windows-only test comparisons that were asserting
platform-dependent string equality:
- executable.test: compare .cmd paths case-insensitively (which@5
preserves PATHEXT casing, which is uppercase in production).
- workspace-registry-model.test: expect normalizeWorkspaceId(path),
not the hardcoded POSIX form.
- checkout-git-rev-parse.test: normalize separators/case when
comparing git's Windows forward-slash output against realpathSync.
* test(server): canonicalize repo root via git on Windows to fix short-name mismatch
realpathSync on Windows preserves 8.3 short names (e.g. RUNNER~1) while git's
rev-parse --show-toplevel always returns the long-name form (runneradmin).
Use git as the canonicalizer on both sides of the assertion so the comparison
holds regardless of how Windows exposes the temp directory path.
* fix(server): Claude is always available in default mode; SDK bundles cli.js
The Phase 2 change wrongly tied Claude's default-mode isAvailable() to
isCommandAvailable("claude"). Claude's default runtime does not use an
external `claude` binary — @anthropic-ai/claude-agent-sdk ships its own
cli.js and spawnClaudeCodeProcess runs it via process.execPath. The
previous `return true` was correct; restore it.
Update provider-availability.test.ts to assert the truthful behavior:
Claude reports available even when no `claude` binary is on PATH.
Codex and OpenCode genuinely require their binaries on PATH, so their
availability checks remain unchanged.
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This commit is contained in:
Mohamed Boudra
co-authored by
github-actions[bot] <github-actions[bot]@users.noreply.github.com>
github-actions[bot] <github-actions[bot]@users.noreply.github.com>
parent
11b407c804
commit
7c50b7a080
@@ -1,54 +1,100 @@
|
||||
import { execFile } from "node:child_process";
|
||||
import { spawn, type ChildProcess } from "node:child_process";
|
||||
import { createRequire } from "node:module";
|
||||
import { existsSync } from "node:fs";
|
||||
import path, { extname } from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
import { extname } from "node:path";
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
type Which = (command: string, options: { all: true }) => Promise<string[]>;
|
||||
|
||||
function pickBestWindowsCandidate(lines: string[]): string | null {
|
||||
const candidates = lines.filter((line) => line.length > 0);
|
||||
if (candidates.length === 0) return null;
|
||||
const require = createRequire(import.meta.url);
|
||||
const which = require("which") as Which;
|
||||
const PROBE_TIMEOUT_MS = 2000;
|
||||
|
||||
const extPriority = [".exe", ".cmd", ".ps1"];
|
||||
for (const ext of extPriority) {
|
||||
const match = candidates.find((candidate) => candidate.toLowerCase().endsWith(ext));
|
||||
if (match) return match;
|
||||
}
|
||||
|
||||
return candidates[0] ?? null;
|
||||
function hasPathSeparator(value: string): boolean {
|
||||
return value.includes("/") || value.includes("\\");
|
||||
}
|
||||
|
||||
function resolveExecutableFromWhichOutput(
|
||||
name: string,
|
||||
output: string,
|
||||
source: "which",
|
||||
): string | null {
|
||||
const lines = output
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter((line) => line.length > 0);
|
||||
const candidate = lines.at(-1);
|
||||
|
||||
if (!candidate) {
|
||||
return null;
|
||||
async function enumerateCandidates(name: string): Promise<string[]> {
|
||||
let candidates: string[];
|
||||
try {
|
||||
candidates = await which(name, { all: true });
|
||||
} catch (error) {
|
||||
// `which` throws ENOENT when the command is absent from PATH.
|
||||
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
|
||||
return [];
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
|
||||
if (!path.isAbsolute(candidate)) {
|
||||
console.warn(
|
||||
`[findExecutable] Ignoring non-absolute ${source} output for '${name}': ${JSON.stringify(candidate)}`,
|
||||
);
|
||||
return null;
|
||||
}
|
||||
const seen = new Set<string>();
|
||||
return candidates.filter((candidate) => {
|
||||
if (seen.has(candidate)) {
|
||||
return false;
|
||||
}
|
||||
seen.add(candidate);
|
||||
return true;
|
||||
});
|
||||
}
|
||||
|
||||
return candidate;
|
||||
function isWindowsCommandScript(executablePath: string): boolean {
|
||||
const extension = extname(executablePath).toLowerCase();
|
||||
return process.platform === "win32" && (extension === ".cmd" || extension === ".bat");
|
||||
}
|
||||
|
||||
async function probeExecutable(executablePath: string): Promise<boolean> {
|
||||
return await new Promise((resolve) => {
|
||||
let settled = false;
|
||||
let started = false;
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
|
||||
const settle = (result: boolean) => {
|
||||
if (settled) {
|
||||
return;
|
||||
}
|
||||
settled = true;
|
||||
if (timer) {
|
||||
clearTimeout(timer);
|
||||
}
|
||||
resolve(result);
|
||||
};
|
||||
|
||||
let child: ChildProcess;
|
||||
try {
|
||||
child = spawn(executablePath, ["--version"], {
|
||||
stdio: "ignore",
|
||||
windowsHide: true,
|
||||
// Windows batch shims (.cmd/.bat) require cmd.exe; native binaries do not.
|
||||
shell: isWindowsCommandScript(executablePath),
|
||||
});
|
||||
} catch {
|
||||
settle(false);
|
||||
return;
|
||||
}
|
||||
|
||||
timer = setTimeout(() => {
|
||||
if (started) {
|
||||
child.kill();
|
||||
settle(true);
|
||||
return;
|
||||
}
|
||||
settle(false);
|
||||
}, PROBE_TIMEOUT_MS);
|
||||
timer.unref?.();
|
||||
|
||||
child.once("spawn", () => {
|
||||
started = true;
|
||||
});
|
||||
child.once("error", () => {
|
||||
// ENOENT/EACCES/EPERM/UNKNOWN here means the OS could not start the candidate.
|
||||
settle(started);
|
||||
});
|
||||
child.once("exit", () => {
|
||||
settle(started);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* On Unix we use `which`. On Windows we use `where.exe`.
|
||||
*
|
||||
* Both rely on the inherited process.env.PATH — on macOS/Linux, Electron
|
||||
* enriches it at startup via inheritLoginShellEnv(); on Windows, Electron
|
||||
* inherits the full user environment from Explorer.
|
||||
* Check a literal executable path. PATH search is handled by findExecutable().
|
||||
*/
|
||||
export function executableExists(
|
||||
executablePath: string,
|
||||
@@ -70,35 +116,18 @@ export async function findExecutable(name: string): Promise<string | null> {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (trimmed.includes("/") || trimmed.includes("\\")) {
|
||||
return executableExists(trimmed);
|
||||
if (hasPathSeparator(trimmed)) {
|
||||
return (await probeExecutable(trimmed)) ? trimmed : null;
|
||||
}
|
||||
|
||||
if (process.platform === "win32") {
|
||||
try {
|
||||
const { stdout } = await execFileAsync("where.exe", [trimmed], {
|
||||
encoding: "utf8",
|
||||
windowsHide: true,
|
||||
});
|
||||
return (
|
||||
pickBestWindowsCandidate(
|
||||
stdout
|
||||
.trim()
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim()),
|
||||
) ?? null
|
||||
);
|
||||
} catch {
|
||||
return null;
|
||||
const candidates = await enumerateCandidates(trimmed);
|
||||
for (const candidate of candidates) {
|
||||
if (await probeExecutable(candidate)) {
|
||||
return candidate;
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
const { stdout } = await execFileAsync("which", [trimmed], { encoding: "utf8" });
|
||||
return resolveExecutableFromWhichOutput(trimmed, stdout.trim(), "which");
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export async function isCommandAvailable(command: string): Promise<boolean> {
|
||||
|
||||
Reference in New Issue
Block a user