From d8bf307d22663a3c458dc521a014fd21bdb60b98 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 20 Jun 2026 11:37:50 -0700 Subject: [PATCH] [codex] Enrich process runner errors (#3268) Co-authored-by: codex --- .../ServerEnvironmentLabel.test.ts | 2 +- apps/server/src/processRunner.test.ts | 92 ++++++++++++++++++- apps/server/src/processRunner.ts | 65 +++++++++---- 3 files changed, 135 insertions(+), 24 deletions(-) diff --git a/apps/server/src/environment/ServerEnvironmentLabel.test.ts b/apps/server/src/environment/ServerEnvironmentLabel.test.ts index bc30bd0ce1953c285bd9f37c575b5e8dc795153f..4bc9647fba5d72d5743be7c8a946cf0301b81f0e 100644 --- a/apps/server/src/environment/ServerEnvironmentLabel.test.ts +++ b/apps/server/src/environment/ServerEnvironmentLabel.test.ts @@ -138,7 +138,7 @@ describe("resolveServerEnvironmentLabel", () => { Effect.fail( new ProcessRunner.ProcessSpawnError({ command: "scutil", - args: ["--get", "ComputerName"], + argumentCount: 2, cause: new Error("spawn scutil ENOENT"), }), ), diff --git a/apps/server/src/processRunner.test.ts b/apps/server/src/processRunner.test.ts index 2c9d9f9503829f19ccb6c9e2c37e6cc955016c0e..e264ba7849da8eebc32aadc5d1c8211782cd8899 100644 --- a/apps/server/src/processRunner.test.ts +++ b/apps/server/src/processRunner.test.ts @@ -4,6 +4,7 @@ import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; +import * as PlatformError from "effect/PlatformError"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; import { TestClock } from "effect/testing"; @@ -55,7 +56,9 @@ function makeHandle(input: { } function makeSpawner( - f: (command: ChildProcessCommand) => Effect.Effect, + f: ( + command: ChildProcessCommand, + ) => Effect.Effect, ) { return ChildProcessSpawner.make((command) => f(asChildProcessCommand(command))); } @@ -159,6 +162,44 @@ describe("runProcess", () => { ); }); + it.effect("preserves resolved spawn context and cause", () => + Effect.gen(function* () { + const cause = PlatformError.systemError({ + _tag: "PermissionDenied", + module: "ChildProcessSpawner", + method: "spawn", + pathOrDescriptor: "/actual/fake", + }); + const spawner = makeSpawner(() => Effect.fail(cause)); + + const error = yield* runWith(spawner)({ + command: "fake", + args: ["--flag", "secret-token-value"], + cwd: "/logical", + spawnCwd: "/actual", + }).pipe(Effect.flip); + + expect(error._tag).toBe("ProcessSpawnError"); + if (error._tag !== "ProcessSpawnError") { + return expect.fail("Expected ProcessSpawnError"); + } + expect(error).toMatchObject({ + command: "fake", + argumentCount: 2, + cwd: "/logical", + spawnCwd: "/actual", + resolvedCommand: "fake", + resolvedArgumentCount: 2, + shell: false, + }); + expect(error.cause).toBe(cause); + expect(error.message).toBe("Failed to spawn process 'fake' in '/actual'"); + expect(error).not.toHaveProperty("args"); + expect(error).not.toHaveProperty("resolvedArgs"); + expect(error.message).not.toContain("secret-token-value"); + }), + ); + it.effect("fails when output exceeds max buffer in default mode", () => Effect.gen(function* () { const spawner = makeSpawner(() => Effect.succeed(makeHandle({ stdout: "x".repeat(2048) }))); @@ -169,7 +210,39 @@ describe("runProcess", () => { maxOutputBytes: 128, }).pipe(Effect.flip); - expect(error).toBeInstanceOf(ProcessRunner.ProcessOutputLimitError); + expect(error._tag).toBe("ProcessOutputLimitError"); + if (error._tag !== "ProcessOutputLimitError") { + return expect.fail("Expected ProcessOutputLimitError"); + } + expect(error).toMatchObject({ + stream: "stdout", + maxBytes: 128, + observedBytes: 2048, + }); + expect(error.message).toBe( + "Process 'fake' stdout produced 2048 bytes, exceeding the 128 byte limit", + ); + }), + ); + + it.effect("accepts output at the byte limit followed by an empty chunk", () => + Effect.gen(function* () { + const output = new TextEncoder().encode("exactly"); + const spawner = makeSpawner(() => + Effect.succeed( + makeHandle({ + stdout: Stream.make(output, new Uint8Array()), + }), + ), + ); + + const result = yield* runWith(spawner)({ + command: "fake", + args: ["exact-limit"], + maxOutputBytes: output.byteLength, + }); + + expect(result.stdout).toBe("exactly"); }), ); @@ -272,6 +345,8 @@ describe("runProcess", () => { const errorFiber = yield* runWith(spawner)({ command: "fake", args: ["sleep"], + cwd: "/logical", + spawnCwd: "/actual", timeout: "50 millis", }).pipe(Effect.flip, Effect.forkScoped); @@ -279,7 +354,18 @@ describe("runProcess", () => { yield* TestClock.adjust(Duration.millis(50)); const error = yield* Fiber.join(errorFiber); - expect(error).toBeInstanceOf(ProcessRunner.ProcessTimeoutError); + expect(error._tag).toBe("ProcessTimeoutError"); + if (error._tag !== "ProcessTimeoutError") { + return expect.fail("Expected ProcessTimeoutError"); + } + expect(error).toMatchObject({ + command: "fake", + argumentCount: 1, + cwd: "/logical", + spawnCwd: "/actual", + timeoutMs: 50, + }); + expect(error.message).toBe("Process 'fake' in '/actual' timed out after 50ms"); }), ); diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts index 5f01fcc344be8fc45b83ce4af809580d0bb59457..c1ee2b2cb0c944415d3cbb81d594b978065fbef9 100644 --- a/apps/server/src/processRunner.ts +++ b/apps/server/src/processRunner.ts @@ -45,23 +45,29 @@ export interface ProcessRunOutput { const ProcessInvocationFields = { command: Schema.String, - args: Schema.Array(Schema.String), + argumentCount: Schema.Number, cwd: Schema.optional(Schema.String), + spawnCwd: Schema.optional(Schema.String), }; const formatProcessInvocation = (input: { readonly command: string; - readonly args: ReadonlyArray; readonly cwd?: string | undefined; + readonly spawnCwd?: string | undefined; }): string => { - const command = [input.command, ...input.args].join(" "); - return input.cwd === undefined ? `'${command}'` : `'${command}' in '${input.cwd}'`; + const executionCwd = input.spawnCwd ?? input.cwd; + return executionCwd === undefined + ? `'${input.command}'` + : `'${input.command}' in '${executionCwd}'`; }; export class ProcessSpawnError extends Schema.TaggedErrorClass()( "ProcessSpawnError", { ...ProcessInvocationFields, + resolvedCommand: Schema.optional(Schema.String), + resolvedArgumentCount: Schema.optional(Schema.Number), + shell: Schema.optional(Schema.Boolean), cause: Schema.Defect(), }, ) { @@ -74,6 +80,7 @@ export class ProcessStdinError extends Schema.TaggedErrorClass; readonly cwd?: string | undefined; + readonly spawnCwd?: string | undefined; readonly streamName: "stdout" | "stderr"; readonly stream: Stream.Stream; readonly maxOutputBytes: number; @@ -174,8 +185,9 @@ const collectText = Effect.fn("processRunner.collectText")(function* (input: { (cause) => new ProcessReadError({ command: input.command, - args: input.args, + argumentCount: input.args.length, cwd: input.cwd, + spawnCwd: input.spawnCwd, stream: input.streamName, cause, }), @@ -203,14 +215,16 @@ const collectText = Effect.fn("processRunner.collectText")(function* (input: { () => ({ chunks: [], bytes: 0 }), (state, chunk) => { const remainingBytes = input.maxOutputBytes - state.bytes; - if (remainingBytes <= 0 || chunk.byteLength > remainingBytes) { + if (chunk.byteLength > remainingBytes) { return Effect.fail( new ProcessOutputLimitError({ command: input.command, - args: input.args, + argumentCount: input.args.length, cwd: input.cwd, + spawnCwd: input.spawnCwd, stream: input.streamName, maxBytes: input.maxOutputBytes, + observedBytes: state.bytes + chunk.byteLength, }), ); } @@ -259,8 +273,9 @@ function finalizeRunProcess( return Effect.fail( new ProcessTimeoutError({ command: input.command, - args: input.args, + argumentCount: input.args.length, cwd: input.cwd, + spawnCwd: input.spawnCwd, timeoutMs: Duration.toMillis(timeout), }), ); @@ -300,23 +315,30 @@ const runProcessCore = Effect.fn("processRunner.runProcessCore")(function* ( (cause) => new ProcessSpawnError({ command: input.command, - args: input.args, + argumentCount: input.args.length, cwd: input.cwd, + spawnCwd: input.spawnCwd, + resolvedCommand: spawnCommand.command, + resolvedArgumentCount: spawnCommand.args.length, + shell: spawnCommand.shell, cause, }), ), ); + const stdin = input.stdin; const writeStdin = - input.stdin === undefined + stdin === undefined ? Effect.void - : Stream.run(Stream.encodeText(Stream.make(input.stdin)), child.stdin).pipe( + : Stream.run(Stream.encodeText(Stream.make(stdin)), child.stdin).pipe( Effect.mapError( (cause) => new ProcessStdinError({ command: input.command, - args: input.args, + argumentCount: input.args.length, cwd: input.cwd, + spawnCwd: input.spawnCwd, + stdinBytes: Buffer.byteLength(stdin), cause, }), ), @@ -328,6 +350,7 @@ const runProcessCore = Effect.fn("processRunner.runProcessCore")(function* ( command: input.command, args: input.args, cwd: input.cwd, + spawnCwd: input.spawnCwd, streamName: "stdout", stream: child.stdout, maxOutputBytes, @@ -338,6 +361,7 @@ const runProcessCore = Effect.fn("processRunner.runProcessCore")(function* ( command: input.command, args: input.args, cwd: input.cwd, + spawnCwd: input.spawnCwd, streamName: "stderr", stream: child.stderr, maxOutputBytes, @@ -354,8 +378,9 @@ const runProcessCore = Effect.fn("processRunner.runProcessCore")(function* ( (cause) => new ProcessReadError({ command: input.command, - args: input.args, + argumentCount: input.args.length, cwd: input.cwd, + spawnCwd: input.spawnCwd, stream: "exitCode", cause, }), -- 2.54.0