Spawn child processes directly and drop the Deno.Command shim

This commit is contained in:
Danny Kim
2026-10-17 23:15:39 +09:00
parent 6ad859be62
commit 7d25b759fb
13 changed files with 255 additions and 361 deletions
@@ -0,0 +1,39 @@
---
description: Child processes use plain node:child_process spawn (explicit stdio, uid/gid, whole env); src/child.ts has exitCode (128+signo) and echo; no execFile maxBuffer
---
Every call site calls `spawn` from `node:child_process` itself, with
everything explicit:
- **`uid`/`gid`** of the profile for anything an agent profile runs (each
profile is its own UID, 20000+). Never drop them.
- **`env`** is the whole environment the child gets (a sanitized one for
agents); nothing from the coordinator is merged in.
- **`stdio`**: stdin is `"ignore"` so an agent CLI never waits on input
(only `codex login --with-api-key` pipes the token in). Output that is not
read is `"ignore"`; output that is read is `"pipe"` and streamed.
- No `execFile`/`exec`: their `maxBuffer` (1 MiB) kills a chatty child. Read
piped streams incrementally instead.
`src/child.ts` holds what the call sites share:
- **`exitCode(child)`**: call it right after `spawn`. It settles on `exit`
(not `close`, which a daemonized grandchild can hold open), maps a signal to
`128 + signo` as a shell does, and rejects on a failed start. The rejection
is pre-caught so it is not reported as unhandled before the caller awaits it.
- **`echo(stream, chunk)`**: writes and waits for `drain` when the buffer is
full, so echoing never outruns a slow reader.
Tests use `spawnSync` for short commands.
Other port details:
- **`src/errors.ts`** `isNotFound` checks `ENOENT` (file) or `ESRCH` (process).
- Temporary files are an exclusive `wx` create with mode `0600`.
- `src/json.ts` holds type guards used instead of `as` casts on parsed JSON.
- `process` and `Buffer` must come from `node:` imports:
`no-restricted-globals` in `scripts/project-lint.ts` enforces it
([[lint-parity-never-shrinks]]).
Related: how each agent CLI reports bad credentials is in
[[agent-cli-auth-failures]].
@@ -1,25 +0,0 @@
---
description: src/command.ts reproduces Deno.Command on child_process.spawn (stdio defaults, clearEnv, uid/gid, 128+signo); src/errors.ts maps NotFound to ENOENT/ESRCH
---
The Deno code was ported one for one; two small modules keep the old shapes
so callers barely changed:
- **`src/command.ts`** wraps `child_process.spawn` with `Deno.Command`'s API:
`output()` and `spawn()`, `clearEnv`, `uid`/`gid` (the agent isolation runs
each profile as its own UID, 20000+), and Deno's stdio defaults: stdin
inherited; stdout/stderr piped for `output()`, inherited for `spawn()`. A
signal exit maps to status `128 + signo`. Status settles on process exit,
not on stream close. Use it instead of calling `node:child_process`
directly so isolation and status handling stay uniform.
- **`src/errors.ts`** replaces `Deno.errors.NotFound` checks with an
`ENOENT`/`ESRCH` code check.
- `makeTempFile` became an exclusive `wx` create with mode `0600`.
- `src/json.ts` holds type guards used instead of `as` casts on parsed JSON.
- `process` and `Buffer` must come from `node:` imports:
`no-restricted-globals` in `scripts/project-lint.ts` maps Deno's
`no-window`/`no-process-global`/`no-node-globals` there
([[lint-parity-never-shrinks]]).
Related: how each agent CLI reports bad credentials is in
[[agent-cli-auth-failures]].
+3 -3
View File
@@ -7,10 +7,10 @@ credential cleanup. They need root and are written as
`test.skipIf(!IS_ROOT)(...)` (`IS_ROOT` from `#testing`), so they show `test.skipIf(!IS_ROOT)(...)` (`IS_ROOT` from `#testing`), so they show
as skipped instead of silently passing. as skipped instead of silently passing.
- As a normal user `vp test` passes 56 and skips 5. In CI (self-hosted - As a normal user `vp test` passes 61 and skips 5. In CI (self-hosted
runner, root) all 61 run. runner, root) all 66 run.
- When changing `isolation.ts`, `runtime.ts`, `credentials.ts`, `prepare.ts`, - When changing `isolation.ts`, `runtime.ts`, `credentials.ts`, `prepare.ts`,
or `command.ts`, run the suite as root in `node:24-bookworm` (e.g. a or `child.ts`, run the suite as root in `node:24-bookworm` (e.g. a
container with the repo mounted) before pushing; a green non-root run container with the repo mounted) before pushing; a green non-root run
proves nothing about those paths. proves nothing about those paths.
- The migration also ran an end-to-end smoke as root with fake - The migration also ran an end-to-end smoke as root with fake
+5 -5
View File
@@ -17,7 +17,7 @@ test("retries once after an invalid refresh token", async () => {
const result = await retryInvalidToken( const result = await retryInvalidToken(
() => () =>
Promise.resolve({ Promise.resolve({
status: { success: false }, code: 1,
error: attempts++ === 0 ? "ERROR: invalid_refresh_token" : "still failed", error: attempts++ === 0 ? "ERROR: invalid_refresh_token" : "still failed",
}), }),
() => { () => {
@@ -40,7 +40,7 @@ test("does not retry an unrelated failure", async () => {
() => { () => {
attempts++; attempts++;
return Promise.resolve({ return Promise.resolve({
status: { success: false }, code: 1,
error: "rate limited", error: "rate limited",
}); });
}, },
@@ -86,7 +86,7 @@ test("re-logs in after the job 9837 revoked-session stderr", async () => {
const result = await retryInvalidToken( const result = await retryInvalidToken(
() => () =>
Promise.resolve({ Promise.resolve({
status: { success: attempts++ > 0 }, code: attempts++ > 0 ? 0 : 1,
error: attempts === 1 ? REVOKED_CODEX.join("\n") : "", error: attempts === 1 ? REVOKED_CODEX.join("\n") : "",
}), }),
() => { () => {
@@ -95,9 +95,9 @@ test("re-logs in after the job 9837 revoked-session stderr", async () => {
}, },
); );
expect({ relogins, success: result.status.success }).toStrictEqual({ expect({ relogins, code: result.code }).toStrictEqual({
relogins: 1, relogins: 1,
success: true, code: 0,
}); });
}); });
+3 -3
View File
@@ -48,12 +48,12 @@ export function isClaudeAuthFailure(result: ClaudeFailureFields): boolean {
); );
} }
export async function retryInvalidToken<T extends { status: { success: boolean }; error: string }>( export async function retryInvalidToken<T extends { code: number; error: string }>(
attempt: () => Promise<T>, attempt: () => Promise<T>,
relogin: () => Promise<void>, relogin: () => Promise<void>,
): Promise<T> { ): Promise<T> {
let result = await attempt(); let result = await attempt();
if (!result.status.success && isCodexAuthFailure(result.error)) { if (result.code !== 0 && isCodexAuthFailure(result.error)) {
await relogin(); await relogin();
result = await attempt(); result = await attempt();
} }
@@ -111,7 +111,7 @@ export async function stageCodexAuth(
return stagedPath; return stagedPath;
} }
// Creates an empty private file under a fresh name, as Deno.makeTempFile did. // Creates an empty private file under a fresh name.
async function makeTempFile(directory: string): Promise<string> { async function makeTempFile(directory: string): Promise<string> {
const path = join(directory, `.tmp-${randomUUID()}`); const path = join(directory, `.tmp-${randomUUID()}`);
const handle = await open(path, "wx", 0o600); const handle = await open(path, "wx", 0o600);
+35
View File
@@ -0,0 +1,35 @@
import { spawn } from "node:child_process";
import { expect, test } from "vite-plus/test";
import { isNotFound } from "./errors.ts";
import { exitCode } from "./child.ts";
test("reports the exit code", async () => {
expect(await exitCode(spawn("/bin/sh", ["-c", "exit 0"], { stdio: "ignore" }))).toBe(0);
expect(await exitCode(spawn("/bin/sh", ["-c", "exit 3"], { stdio: "ignore" }))).toBe(3);
});
test("reports a signal as 128 plus its number", async () => {
const child = spawn("/bin/sh", ["-c", "kill -KILL $$"], { stdio: "ignore" });
expect(await exitCode(child)).toBe(137);
});
test("resolves on exit while a grandchild still holds the output open", async () => {
const child = spawn("/bin/sh", ["-c", "sleep 2 & exit 5"], { stdio: ["ignore", "pipe", "pipe"] });
const started = Date.now();
expect(await exitCode(child)).toBe(5);
expect(Date.now() - started).toBeLessThan(1500);
child.stdout.destroy();
child.stderr.destroy();
});
test("rejects when the command does not exist", async () => {
const child = spawn("/nonexistent/command", [], { stdio: "ignore" });
const code = exitCode(child);
// The rejection must stay unhandled-safe until it is awaited.
await new Promise((resolve) => {
setTimeout(resolve, 50);
});
await expect(code).rejects.toSatisfy(isNotFound);
});
+30
View File
@@ -0,0 +1,30 @@
// What the child process call sites share: the exit status, read right after
// `spawn` so the exit event cannot pass unseen while the caller reads the
// streams, and echoing output without outrunning a slow reader.
import type { ChildProcess } from "node:child_process";
import { once } from "node:events";
import { constants } from "node:os";
import type { Writable } from "node:stream";
// Resolves once the process exits, before its streams close, with its exit
// code, or with 128 plus the signal number if a signal killed it, as a shell
// reports. A process that fails to start rejects with the spawn error.
export function exitCode(child: ChildProcess): Promise<number> {
const code = new Promise<number>((resolve, reject) => {
child.once("error", reject);
child.once("exit", (exit, signal) => {
resolve(exit ?? (signal === null ? 1 : 128 + constants.signals[signal]));
});
});
// Callers await it after draining the streams; a failed start must not be
// reported as an unhandled rejection before then.
code.catch(() => {});
return code;
}
// Writes a chunk, waiting for the stream to drain when its buffer is full.
export async function echo(stream: Writable, chunk: string | Uint8Array): Promise<void> {
if (!stream.write(chunk)) {
await once(stream, "drain");
}
}
-160
View File
@@ -1,160 +0,0 @@
// A child process with the call shapes the runner was reviewed with: `output()`
// collects both streams, `spawn()` hands them out, and `clearEnv` passes only
// the given environment. Streams default as Deno's did: stdin is inherited,
// stdout and stderr are piped for `output()` and inherited for `spawn()`.
import { spawn as spawnProcess } from "node:child_process";
import type { ChildProcess, StdioOptions } from "node:child_process";
import { Buffer } from "node:buffer";
import { constants } from "node:os";
import process from "node:process";
import type { Readable, Writable } from "node:stream";
import { buffer } from "node:stream/consumers";
import { finished } from "node:stream/promises";
export type StdioMode = "inherit" | "piped" | "null";
export interface CommandOptions {
args?: string[];
cwd?: string;
env?: Record<string, string>;
clearEnv?: boolean;
uid?: number;
gid?: number;
stdin?: StdioMode;
stdout?: StdioMode;
stderr?: StdioMode;
}
export interface CommandStatus {
success: boolean;
code: number;
}
export interface CommandOutput extends CommandStatus {
stdout: Uint8Array;
stderr: Uint8Array;
}
export interface CommandChild {
status: Promise<CommandStatus>;
stdin: Writable;
stdout: Readable;
stderr: Readable;
}
const STDIO: Record<StdioMode, "inherit" | "pipe" | "ignore"> = {
inherit: "inherit",
piped: "pipe",
null: "ignore",
};
function signalNumber(signal: NodeJS.Signals): number {
return constants.signals[signal];
}
function startProcess(
command: string,
options: CommandOptions,
outputDefault: StdioMode,
): ChildProcess {
const stdio: StdioOptions = [
STDIO[options.stdin ?? "inherit"],
STDIO[options.stdout ?? outputDefault],
STDIO[options.stderr ?? outputDefault],
];
const environment = options.clearEnv === true ? options.env : { ...process.env, ...options.env };
return spawnProcess(command, options.args ?? [], {
stdio,
...(options.cwd !== undefined && { cwd: options.cwd }),
...(environment !== undefined && { env: environment }),
...(options.uid !== undefined && { uid: options.uid }),
...(options.gid !== undefined && { gid: options.gid }),
});
}
// Settles when the process exits, before its streams close, as Deno's did. A
// signal-killed process reports 128 plus the signal number, as a shell does.
function waitForStatus(child: ChildProcess): Promise<CommandStatus> {
return new Promise((resolve, reject) => {
child.once("error", reject);
child.once("exit", (code, signal) => {
const exitCode = code ?? (signal === null ? 1 : 128 + signalNumber(signal));
resolve({ success: exitCode === 0, code: exitCode });
});
});
}
function piped<T>(stream: T | null, name: string): T {
if (stream === null) {
throw new TypeError(`${name} is not piped`);
}
return stream;
}
function collect(stream: Readable | null): Promise<Uint8Array> {
return stream === null ? Promise.resolve(new Uint8Array()) : buffer(stream);
}
// A stream's chunks as bytes, whether or not it has an encoding set.
export async function* chunks(stream: Readable): AsyncGenerator<Uint8Array> {
for await (const chunk of stream) {
const value: unknown = chunk;
if (value instanceof Uint8Array) {
yield value;
} else if (typeof value === "string") {
yield Buffer.from(value);
}
}
}
export function spawn(command: string, options: CommandOptions = {}): CommandChild {
const child = startProcess(command, options, "inherit");
const status = waitForStatus(child);
// A failed start rejects `status`; the caller sees it once it awaits that.
status.catch(() => {});
return {
status,
get stdin(): Writable {
return piped(child.stdin, "stdin");
},
get stdout(): Readable {
return piped(child.stdout, "stdout");
},
get stderr(): Readable {
return piped(child.stderr, "stderr");
},
};
}
export async function output(
command: string,
options: CommandOptions = {},
): Promise<CommandOutput> {
const child = startProcess(command, options, "piped");
const [status, stdout, stderr] = await Promise.all([
waitForStatus(child),
collect(child.stdout),
collect(child.stderr),
]);
return { ...status, stdout, stderr };
}
// Writes one chunk and settles once the stream has taken it, as Deno's
// `Deno.stdout.write` did.
export async function writeChunk(stream: Writable, chunk: Uint8Array): Promise<void> {
await new Promise<void>((resolve, reject) => {
stream.write(chunk, (error) => {
if (error) {
reject(error);
} else {
resolve();
}
});
});
}
// Writes the last chunk and settles once the stream has flushed and closed.
export async function endWith(stream: Writable, chunk: string): Promise<void> {
stream.end(chunk);
await finished(stream);
}
+1 -2
View File
@@ -1,5 +1,4 @@
// Node reports a missing file as ENOENT and a missing process as ESRCH, where // A missing file or a missing process: Node reports them as ENOENT and ESRCH.
// Deno threw `Deno.errors.NotFound` for both.
export function isNotFound(error: unknown): boolean { export function isNotFound(error: unknown): boolean {
if (!(error instanceof Error) || !("code" in error)) { if (!(error instanceof Error) || !("code" in error)) {
return false; return false;
+35 -35
View File
@@ -1,14 +1,11 @@
import { spawnSync } from "node:child_process";
import type { SpawnSyncReturns } from "node:child_process";
import { expect, test } from "vite-plus/test"; import { expect, test } from "vite-plus/test";
import { chmod, chown, mkdir, readFile, rm, symlink, writeFile } from "node:fs/promises"; import { chmod, chown, mkdir, readFile, rm, symlink, writeFile } from "node:fs/promises";
import process from "node:process"; import process from "node:process";
import { isNotFound } from "./errors.ts"; import { isNotFound } from "./errors.ts";
import { import { isRunnableProfileProcess, killProfileProcesses, readProfileFile } from "./isolation.ts";
agentCommand,
isRunnableProfileProcess,
killProfileProcesses,
readProfileFile,
} from "./isolation.ts";
import type { RuntimeProfile } from "./runtime.ts"; import type { RuntimeProfile } from "./runtime.ts";
import { currentUid, IS_ROOT, IS_WINDOWS, makeTempDir } from "#testing"; import { currentUid, IS_ROOT, IS_WINDOWS, makeTempDir } from "#testing";
@@ -22,6 +19,23 @@ const profile = (uid: number, home: string): RuntimeProfile => ({
tokenFile: null, tokenFile: null,
}); });
// Runs a command as the profile's user from the root directory. Without `env`
// it inherits this process's environment.
function runAs(
selected: RuntimeProfile,
command: string,
args: string[],
env?: Record<string, string>,
): SpawnSyncReturns<string> {
return spawnSync(command, args, {
cwd: "/",
uid: selected.uid,
gid: selected.gid,
encoding: "utf8",
...(env !== undefined && { env }),
});
}
test("zombie process entries are already quiescent", () => { test("zombie process entries are already quiescent", () => {
const status = "State:\tZ (zombie)\nUid:\t20010\t20010\t20010\t20010\n"; const status = "State:\tZ (zombie)\nUid:\t20010\t20010\t20010\t20010\n";
expect(isRunnableProfileProcess(status, 20_010)).toBe(false); expect(isRunnableProfileProcess(status, 20_010)).toBe(false);
@@ -60,20 +74,14 @@ test.skipIf(!IS_ROOT)("a profile process can read only its own private files", a
await writeFile(`${root}/reviewer`, "root", { mode: 0o600 }); await writeFile(`${root}/reviewer`, "root", { mode: 0o600 });
const selected = profile(20_000, homeA); const selected = profile(20_000, homeA);
const own = await agentCommand(selected, "/bin/cat", [`${homeA}/token`], { const own = runAs(selected, "/bin/cat", [`${homeA}/token`]);
cwd: "/", const sibling = runAs(selected, "/bin/cat", [`${homeB}/token`]);
}).output(); const reviewer = runAs(selected, "/bin/cat", [`${root}/reviewer`]);
const sibling = await agentCommand(selected, "/bin/cat", [`${homeB}/token`], {
cwd: "/",
}).output();
const reviewer = await agentCommand(selected, "/bin/cat", [`${root}/reviewer`], {
cwd: "/",
}).output();
expect(new TextDecoder().decode(own.stdout)).toBe("own"); expect(own.stdout).toBe("own");
expect(own.success).toBe(true); expect(own.status).toBe(0);
expect(sibling.success).toBe(false); expect(sibling.status).not.toBe(0);
expect(reviewer.success).toBe(false); expect(reviewer.status).not.toBe(0);
} finally { } finally {
await rm(root, { recursive: true }); await rm(root, { recursive: true });
} }
@@ -81,13 +89,8 @@ test.skipIf(!IS_ROOT)("a profile process can read only its own private files", a
test.skipIf(!IS_ROOT)("cleanup leaves no runnable daemonized process", async () => { test.skipIf(!IS_ROOT)("cleanup leaves no runnable daemonized process", async () => {
const selected = profile(20_010, "/tmp"); const selected = profile(20_010, "/tmp");
const launched = await agentCommand( const launched = runAs(selected, "/bin/sh", ["-c", "sleep 60 >/dev/null 2>&1 & echo $!"]);
selected, const pid = Number(launched.stdout.trim());
"/bin/sh",
["-c", "sleep 60 >/dev/null 2>&1 & echo $!"],
{ cwd: "/" },
).output();
const pid = Number(new TextDecoder().decode(launched.stdout).trim());
expect(Number.isInteger(pid)).toBe(true); expect(Number.isInteger(pid)).toBe(true);
await killProfileProcesses(selected.uid); await killProfileProcesses(selected.uid);
@@ -99,14 +102,11 @@ test.skipIf(!IS_ROOT)("cleanup leaves no runnable daemonized process", async ()
expect(status === null || !isRunnableProfileProcess(status, selected.uid)).toBe(true); expect(status === null || !isRunnableProfileProcess(status, selected.uid)).toBe(true);
}); });
test.skipIf(!IS_ROOT)("the agent process cannot read its coordinator environment", async () => { test.skipIf(!IS_ROOT)("the agent process cannot read its coordinator environment", () => {
const selected = profile(20_020, "/tmp"); const selected = profile(20_020, "/tmp");
const result = await agentCommand(selected, "/usr/bin/cat", [`/proc/${process.pid}/environ`], { const result = runAs(selected, "/usr/bin/cat", [`/proc/${process.pid}/environ`], {
cwd: "/", PARENT_SECRET: "must-not-cross-boundary",
env: { PARENT_SECRET: "must-not-cross-boundary" }, });
clearEnv: true,
stderr: "piped", expect(result.status).not.toBe(0);
}).output();
expect(result.success).toBe(false);
}); });
-22
View File
@@ -2,31 +2,9 @@ import { lstat, readdir, readFile } from "node:fs/promises";
import process from "node:process"; import process from "node:process";
import { setTimeout as sleep } from "node:timers/promises"; import { setTimeout as sleep } from "node:timers/promises";
import { output, spawn } from "./command.ts";
import type { CommandChild, CommandOptions, CommandOutput } from "./command.ts";
import { isNotFound } from "./errors.ts"; import { isNotFound } from "./errors.ts";
import type { RuntimeProfile } from "./runtime.ts"; import type { RuntimeProfile } from "./runtime.ts";
export type AgentCommandOptions = Omit<CommandOptions, "args" | "uid" | "gid">;
export interface AgentCommand {
output(): Promise<CommandOutput>;
spawn(): CommandChild;
}
export function agentCommand(
profile: RuntimeProfile,
command: string,
args: string[],
options: AgentCommandOptions = {},
): AgentCommand {
const full: CommandOptions = { ...options, args, uid: profile.uid, gid: profile.gid };
return {
output: () => output(command, full),
spawn: () => spawn(command, full),
};
}
export function isRunnableProfileProcess(status: string, uid: number): boolean { export function isRunnableProfileProcess(status: string, uid: number): boolean {
const owner = /^Uid:\s+(\d+)/mu.exec(status)?.[1]; const owner = /^Uid:\s+(\d+)/mu.exec(status)?.[1];
const state = /^State:\s+(\w)/mu.exec(status)?.[1]; const state = /^State:\s+(\w)/mu.exec(status)?.[1];
+83 -82
View File
@@ -1,10 +1,12 @@
// Runs one ordered agent profile. Credential decryption and reviewer posting // Runs one ordered agent profile. Credential decryption and reviewer posting
// live in separate processes so neither privileged secret reaches this runner. // live in separate processes so neither privileged secret reaches this runner.
import { spawn } from "node:child_process";
import { chmod, chown, readFile, writeFile } from "node:fs/promises"; import { chmod, chown, readFile, writeFile } from "node:fs/promises";
import process from "node:process"; import process from "node:process";
import { createInterface } from "node:readline"; import { createInterface } from "node:readline";
import type { Readable } from "node:stream"; import type { Readable } from "node:stream";
import { text as readText } from "node:stream/consumers"; import { text as readText } from "node:stream/consumers";
import { finished } from "node:stream/promises";
import { setTimeout as sleep } from "node:timers/promises"; import { setTimeout as sleep } from "node:timers/promises";
import { import {
@@ -16,9 +18,8 @@ import {
stageCodexAuth, stageCodexAuth,
} from "./auth.ts"; } from "./auth.ts";
import type { ClaudeFailureFields, CodexAuthentication } from "./auth.ts"; import type { ClaudeFailureFields, CodexAuthentication } from "./auth.ts";
import { chunks, endWith, output, writeChunk } from "./command.ts"; import { echo, exitCode } from "./child.ts";
import type { CommandStatus } from "./command.ts"; import { killProfileProcesses, readProfileFile } from "./isolation.ts";
import { agentCommand, killProfileProcesses, readProfileFile } from "./isolation.ts";
import { isObject } from "./json.ts"; import { isObject } from "./json.ts";
import { import {
collectOpenCodeOutput, collectOpenCodeOutput,
@@ -98,8 +99,8 @@ async function postAuthorComment(body: string): Promise<void> {
} }
async function runRoot(command: string, args: string[]): Promise<void> { async function runRoot(command: string, args: string[]): Promise<void> {
const { success, code } = await output(command, { args, env: PROCESS_ENV, clearEnv: true }); const code = await exitCode(spawn(command, args, { env: PROCESS_ENV, stdio: "ignore" }));
if (!success) throw new Error(`${command} ${args[0]} exited with ${code}`); if (code !== 0) throw new Error(`${command} ${args[0]} exited with ${code}`);
} }
async function configureGitAuthor(): Promise<void> { async function configureGitAuthor(): Promise<void> {
@@ -122,12 +123,15 @@ async function runAgentCommand(
args: string[], args: string[],
token?: string, token?: string,
): Promise<void> { ): Promise<void> {
const { success, code } = await agentCommand(profile, command, args, { const child = spawn(command, args, {
cwd: WORKSPACE, cwd: WORKSPACE,
env: agentEnvironment(PROCESS_ENV, profile, token), env: agentEnvironment(PROCESS_ENV, profile, token),
clearEnv: true, uid: profile.uid,
}).output(); gid: profile.gid,
if (!success) throw new Error(`${command} ${args[0]} exited with ${code}`); stdio: "ignore",
});
const code = await exitCode(child);
if (code !== 0) throw new Error(`${command} ${args[0]} exited with ${code}`);
} }
async function prepareProfile(profile: RuntimeProfile): Promise<void> { async function prepareProfile(profile: RuntimeProfile): Promise<void> {
@@ -189,8 +193,7 @@ async function renderPrompt(
} }
async function runClaude(profile: RuntimeProfile, token: string, prompt: string): Promise<string> { async function runClaude(profile: RuntimeProfile, token: string, prompt: string): Promise<string> {
const claude = agentCommand( const claude = spawn(
profile,
"claude", "claude",
[ [
"--print", "--print",
@@ -208,21 +211,21 @@ async function runClaude(profile: RuntimeProfile, token: string, prompt: string)
...agentEnvironment(PROCESS_ENV, profile, token), ...agentEnvironment(PROCESS_ENV, profile, token),
IS_SANDBOX: "1", IS_SANDBOX: "1",
}, },
clearEnv: true, uid: profile.uid,
stdout: "piped", gid: profile.gid,
stderr: "piped", stdio: ["ignore", "pipe", "pipe"],
}, },
).spawn(); );
const status = exitCode(claude);
let result: ClaudeResult | undefined; let result: ClaudeResult | undefined;
let error = ""; let error = "";
const copyError = (async (): Promise<void> => { const copyError = (async (): Promise<void> => {
const decoder = new TextDecoder(); claude.stderr.setEncoding("utf8");
for await (const chunk of chunks(claude.stderr)) { for await (const chunk of claude.stderr) {
const part = decoder.decode(chunk, { stream: true }); const part = String(chunk);
error += part; error += part;
console.error(part); console.error(part);
} }
error += decoder.decode();
})(); })();
for await (const line of createInterface({ input: claude.stdout, crlfDelay: Infinity })) { for await (const line of createInterface({ input: claude.stdout, crlfDelay: Infinity })) {
if (line !== "") { if (line !== "") {
@@ -250,7 +253,7 @@ async function runClaude(profile: RuntimeProfile, token: string, prompt: string)
} }
} }
} }
const [status] = await Promise.all([claude.status, copyError]); const [code] = await Promise.all([status, copyError]);
if (result?.is_error === true) { if (result?.is_error === true) {
const failure = new Error(result.result ?? `claude ended with ${result.subtype}`); const failure = new Error(result.result ?? `claude ended with ${result.subtype}`);
if (isClaudeAuthFailure(result)) { if (isClaudeAuthFailure(result)) {
@@ -261,8 +264,8 @@ async function runClaude(profile: RuntimeProfile, token: string, prompt: string)
if (result?.result === undefined) { if (result?.result === undefined) {
throw new Error(error.trim() || `claude ended with ${result?.subtype ?? "no result"}`); throw new Error(error.trim() || `claude ended with ${result?.subtype ?? "no result"}`);
} }
if (!status.success) { if (code !== 0) {
throw new Error(error.trim() || `claude exited with ${status.code}`); throw new Error(error.trim() || `claude exited with ${code}`);
} }
return result.result; return result.result;
} }
@@ -270,7 +273,7 @@ async function runClaude(profile: RuntimeProfile, token: string, prompt: string)
// Polls every 100 ms until the login prints something, failing if it exits first. // Polls every 100 ms until the login prints something, failing if it exits first.
async function pollForInstructions( async function pollForInstructions(
shownText: () => string, shownText: () => string,
status: Promise<CommandStatus>, status: Promise<number>,
): Promise<void> { ): Promise<void> {
if (shownText().trim() !== "") { if (shownText().trim() !== "") {
return; return;
@@ -286,29 +289,28 @@ async function codexDeviceLogin(
profile: RuntimeProfile, profile: RuntimeProfile,
environment: Record<string, string>, environment: Record<string, string>,
): Promise<void> { ): Promise<void> {
const login = agentCommand(profile, "codex", ["login", "--device-auth"], { const login = spawn("codex", ["login", "--device-auth"], {
cwd: WORKSPACE, cwd: WORKSPACE,
env: environment, env: environment,
clearEnv: true, uid: profile.uid,
stdout: "piped", gid: profile.gid,
stderr: "piped", stdio: ["ignore", "pipe", "pipe"],
}).spawn(); });
const status = exitCode(login);
let shown = ""; let shown = "";
const collect = async (stream: Readable): Promise<void> => { const collect = async (stream: Readable): Promise<void> => {
const decoder = new TextDecoder(); stream.setEncoding("utf8");
for await (const chunk of chunks(stream)) { for await (const chunk of stream) {
shown += decoder.decode(chunk, { stream: true }); shown += String(chunk);
} }
shown += decoder.decode();
}; };
const drained = Promise.all([collect(login.stdout), collect(login.stderr)]); const drained = Promise.all([collect(login.stdout), collect(login.stderr)]);
const status = login.status;
await sleep(3000); await sleep(3000);
await pollForInstructions(() => shown, status); await pollForInstructions(() => shown, status);
await postAuthorComment(withProfile(shown, profile)); await postAuthorComment(withProfile(shown, profile));
const [result] = await Promise.all([status, drained]); const [code] = await Promise.all([status, drained]);
if (!result.success) { if (code !== 0) {
throw new AgentAuthError(`codex login exited with ${result.code}`); throw new AgentAuthError(`codex login exited with ${code}`);
} }
} }
@@ -317,17 +319,19 @@ async function codexApiLogin(
environment: Record<string, string>, environment: Record<string, string>,
token: string, token: string,
): Promise<void> { ): Promise<void> {
const login = agentCommand(profile, "codex", ["login", "--with-api-key"], { const login = spawn("codex", ["login", "--with-api-key"], {
cwd: WORKSPACE, cwd: WORKSPACE,
env: environment, env: environment,
clearEnv: true, uid: profile.uid,
stdin: "piped", gid: profile.gid,
stderr: "piped", stdio: ["pipe", "inherit", "pipe"],
}).spawn(); });
await endWith(login.stdin, `${token}\n`); const status = exitCode(login);
const [status, error] = await Promise.all([login.status, readText(login.stderr)]); login.stdin.end(`${token}\n`);
if (!status.success) { await finished(login.stdin);
throw new Error(error.trim() || `codex login exited with ${status.code}`); const [code, error] = await Promise.all([status, readText(login.stderr)]);
if (code !== 0) {
throw new Error(error.trim() || `codex login exited with ${code}`);
} }
} }
@@ -349,17 +353,18 @@ async function runCodex(
if (authentication.kind === "api-key") { if (authentication.kind === "api-key") {
await codexApiLogin(profile, environment, authentication.token); await codexApiLogin(profile, environment, authentication.token);
} else { } else {
const loggedIn = await agentCommand(profile, "codex", ["login", "status"], { const check = spawn("codex", ["login", "status"], {
cwd: WORKSPACE, cwd: WORKSPACE,
env: environment, env: environment,
clearEnv: true, uid: profile.uid,
}).output(); gid: profile.gid,
if (!loggedIn.success) await codexDeviceLogin(profile, environment); stdio: "ignore",
});
if ((await exitCode(check)) !== 0) await codexDeviceLogin(profile, environment);
} }
const outputPath = await makeProfileFile(profile, "result.md"); const outputPath = await makeProfileFile(profile, "result.md");
const attempt = async (): Promise<{ status: CommandStatus; error: string }> => { const attempt = async (): Promise<{ code: number; error: string }> => {
const codex = agentCommand( const codex = spawn(
profile,
"codex", "codex",
[ [
"exec", "exec",
@@ -373,20 +378,21 @@ async function runCodex(
{ {
cwd: WORKSPACE, cwd: WORKSPACE,
env: environment, env: environment,
clearEnv: true, uid: profile.uid,
stdout: "inherit", gid: profile.gid,
stderr: "piped", stdio: ["ignore", "inherit", "pipe"],
}, },
).spawn(); );
const decoder = new TextDecoder(); const status = exitCode(codex);
codex.stderr.setEncoding("utf8");
let error = ""; let error = "";
for await (const chunk of chunks(codex.stderr)) { for await (const chunk of codex.stderr) {
await writeChunk(process.stderr, chunk); const part = String(chunk);
error += decoder.decode(chunk, { stream: true }); await echo(process.stderr, part);
error += part;
} }
error += decoder.decode();
// Codex echoes the prompt, whose comments may quote auth errors. // Codex echoes the prompt, whose comments may quote auth errors.
return { status: await codex.status, error: error.replaceAll(prompt, "") }; return { code: await status, error: error.replaceAll(prompt, "") };
}; };
const result = const result =
authentication.kind === "api-key" authentication.kind === "api-key"
@@ -395,8 +401,8 @@ async function runCodex(
await runAgentCommand(profile, "codex", ["logout"]); await runAgentCommand(profile, "codex", ["logout"]);
await codexDeviceLogin(profile, environment); await codexDeviceLogin(profile, environment);
}); });
if (!result.status.success) { if (result.code !== 0) {
const failure = new Error(result.error.trim() || `codex exited with ${result.status.code}`); const failure = new Error(result.error.trim() || `codex exited with ${result.code}`);
if (isCodexAuthFailure(result.error)) { if (isCodexAuthFailure(result.error)) {
throw new AgentAuthError("codex sign-in failed", { cause: failure }); throw new AgentAuthError("codex sign-in failed", { cause: failure });
} }
@@ -410,29 +416,24 @@ async function runOpenCode(
token: string, token: string,
prompt: string, prompt: string,
): Promise<string> { ): Promise<string> {
const child = agentCommand(profile, "opencode", openCodeArgs(profile.model, prompt), { const child = spawn("opencode", openCodeArgs(profile.model, prompt), {
cwd: WORKSPACE, cwd: WORKSPACE,
env: agentEnvironment(PROCESS_ENV, profile, token), env: agentEnvironment(PROCESS_ENV, profile, token),
clearEnv: true, uid: profile.uid,
stdout: "piped", gid: profile.gid,
stderr: "piped", stdio: ["ignore", "pipe", "pipe"],
}).spawn(); });
const status = exitCode(child);
const collected = collectOpenCodeOutput( const collected = collectOpenCodeOutput(
chunks(child.stdout), child.stdout,
chunks(child.stderr), child.stderr,
async (chunk) => { (chunk) => echo(process.stdout, chunk),
await writeChunk(process.stdout, chunk); (chunk) => echo(process.stderr, chunk),
},
async (chunk) => {
await writeChunk(process.stderr, chunk);
},
); );
const [status, streams] = await Promise.all([child.status, collected]); const [code, streams] = await Promise.all([status, collected]);
const error = streams.stderr.trim(); const error = streams.stderr.trim();
if (!status.success) { if (code !== 0) {
throw ( throw openCodeError(streams.stdout) ?? new Error(error || `opencode exited with ${code}`);
openCodeError(streams.stdout) ?? new Error(error || `opencode exited with ${status.code}`)
);
} }
return parseOpenCodeEvents(streams.stdout); return parseOpenCodeEvents(streams.stdout);
} }
+17 -20
View File
@@ -1,10 +1,10 @@
import { spawnSync } from "node:child_process";
import { chmod, readdir, readFile, rm, stat, writeFile } from "node:fs/promises"; import { chmod, readdir, readFile, rm, stat, writeFile } from "node:fs/promises";
import process from "node:process"; import process from "node:process";
import { fileURLToPath } from "node:url"; import { fileURLToPath } from "node:url";
import { assert, expect, test } from "vite-plus/test"; import { assert, expect, test } from "vite-plus/test";
import { output } from "./command.ts";
import { prepareRuntime } from "./runtime.ts"; import { prepareRuntime } from "./runtime.ts";
import type { RuntimeProfile } from "./runtime.ts"; import type { RuntimeProfile } from "./runtime.ts";
import { IS_ROOT, makeTempDir } from "#testing"; import { IS_ROOT, makeTempDir } from "#testing";
@@ -13,14 +13,9 @@ const MASTER_KEY = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8";
const ENCRYPTED = "v1:ICEiIyQlJicoKSor:oV_FAgnsN3p1FyegJxwGVdyjfyPio6qDeSUwbA"; const ENCRYPTED = "v1:ICEiIyQlJicoKSor:oV_FAgnsN3p1FyegJxwGVdyjfyPio6qDeSUwbA";
// Runs `command` as the profile's user, from the root directory. // Runs `command` as the profile's user, from the root directory.
async function runAs(profile: RuntimeProfile, command: string, path: string): Promise<boolean> { function runAs(profile: RuntimeProfile, command: string, path: string): boolean {
const result = await output(command, { const result = spawnSync(command, [path], { cwd: "/", uid: profile.uid, gid: profile.gid });
args: [path], return result.status === 0;
cwd: "/",
uid: profile.uid,
gid: profile.gid,
});
return result.success;
} }
async function expectPrivate(path: string, uid: number, mode: number): Promise<void> { async function expectPrivate(path: string, uid: number, mode: number): Promise<void> {
@@ -58,16 +53,16 @@ test.skipIf(!IS_ROOT)("prepares root-only reviewer and isolated profile credenti
expect((await stat(codex.home)).uid).toStrictEqual(codex.uid); expect((await stat(codex.home)).uid).toStrictEqual(codex.uid);
expect(codex.tokenFile).toBe(null); expect(codex.tokenFile).toBe(null);
const ownToken = await output("/bin/cat", { const ownToken = spawnSync("/bin/cat", [claude.tokenFile], {
args: [claude.tokenFile],
cwd: "/", cwd: "/",
uid: claude.uid, uid: claude.uid,
gid: claude.gid, gid: claude.gid,
encoding: "utf8",
}); });
expect(ownToken.success).toBe(true); expect(ownToken.status).toBe(0);
expect(new TextDecoder().decode(ownToken.stdout)).toBe("secret-token"); expect(ownToken.stdout).toBe("secret-token");
expect(await runAs(claude, "/bin/cat", manifest.reviewerFile)).toBe(false); expect(runAs(claude, "/bin/cat", manifest.reviewerFile)).toBe(false);
expect(await runAs(claude, "/bin/ls", codex.home)).toBe(false); expect(runAs(claude, "/bin/ls", codex.home)).toBe(false);
} finally { } finally {
await rm(root, { recursive: true }); await rm(root, { recursive: true });
await rm(action, { recursive: true }); await rm(action, { recursive: true });
@@ -85,18 +80,20 @@ test.skipIf(!IS_ROOT)("removes prepared credentials when output publication fail
profiles: [{ type: "claude", token: ENCRYPTED }], profiles: [{ type: "claude", token: ENCRYPTED }],
}), }),
); );
const result = await output(process.execPath, { const result = spawnSync(
args: [fileURLToPath(new URL("prepare.ts", import.meta.url))], process.execPath,
[fileURLToPath(new URL("prepare.ts", import.meta.url))],
{
env: { env: {
ACTION_PATH: action, ACTION_PATH: action,
RUNNER_TEMP: root, RUNNER_TEMP: root,
BOT_AGENTS_MASTER_KEY: MASTER_KEY, BOT_AGENTS_MASTER_KEY: MASTER_KEY,
GITHUB_OUTPUT: action, GITHUB_OUTPUT: action,
}, },
clearEnv: true, },
}); );
expect(result.success).toBe(false); expect(result.status).not.toBe(0);
expect(await readdir(root)).toStrictEqual([]); expect(await readdir(root)).toStrictEqual([]);
} finally { } finally {
await rm(root, { recursive: true }); await rm(root, { recursive: true });