diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts index 80cf2a1f4761..6a42b7c67ae1 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts @@ -36,6 +36,7 @@ import { makeDrainableWorker } from "@t3tools/shared/DrainableWorker"; import { resolveThreadWorkspaceCwd } from "../../checkpointing/Utils.ts"; import { increment, orchestrationEventsProcessedTotal } from "../../observability/Metrics.ts"; import { + ProviderAdapterProcessError, ProviderAdapterRequestError, ProviderAdapterValidationError, ProviderWorkspaceMissingError, @@ -64,6 +65,7 @@ import { import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; import { VcsStatusBroadcaster } from "../../vcs/VcsStatusBroadcaster.ts"; import { GitWorkflowService } from "../../git/GitWorkflowService.ts"; +const isProviderAdapterProcessError = Schema.is(ProviderAdapterProcessError); const isProviderAdapterRequestError = Schema.is(ProviderAdapterRequestError); const isProviderAdapterValidationError = Schema.is(ProviderAdapterValidationError); const isProviderWorkspaceMissingError = Schema.is(ProviderWorkspaceMissingError); @@ -373,6 +375,9 @@ const make = Effect.gen(function* () { if (isProviderAdapterRequestError(failReason?.error)) { return failReason.error.detail; } + if (isProviderAdapterProcessError(failReason?.error)) { + return failReason.error.detail; + } if (isProviderAdapterValidationError(failReason?.error)) { return failReason.error.issue; } diff --git a/apps/server/src/provider/Layers/CursorAdapter.test.ts b/apps/server/src/provider/Layers/CursorAdapter.test.ts index 4c3ad4b04551..6b6aeff17f35 100644 --- a/apps/server/src/provider/Layers/CursorAdapter.test.ts +++ b/apps/server/src/provider/Layers/CursorAdapter.test.ts @@ -543,6 +543,49 @@ cursorAdapterTestLayer("CursorAdapterLive", (it) => { }), ); + it.effect("surfaces cursor-agent cli.json schema stderr instead of a closed-session error", () => + Effect.gen(function* () { + const adapter = yield* CursorAdapter; + const settings = yield* ServerSettingsService; + const workspace = yield* Effect.promise(() => + NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "cursor-cli-json-")), + ); + const wrapperPath = writeFakeCli({ + directory: workspace, + name: "fake-cursor-agent", + source: [ + "process.stderr.write(`Invalid project config at ${process.cwd()}/.cursor/cli.json: schema validation failed. [`", + " + JSON.stringify({", + ' code: "unrecognized_keys",', + ' keys: ["approvalMode", "sandbox"],', + " path: [],", + " message: \"Unrecognized key(s) in object: 'approvalMode', 'sandbox'\",", + ' }) + "]\\n");', + "process.exit(1);", + ].join("\n"), + }); + yield* settings.updateSettings({ providers: { cursor: { binaryPath: wrapperPath } } }); + + const error = yield* adapter + .startSession({ + threadId: ThreadId.make("cursor-cli-json-schema"), + provider: ProviderDriverKind.make("cursor"), + cwd: workspace, + runtimeMode: "full-access", + }) + .pipe(Effect.flip); + + assert.equal(error._tag, "ProviderAdapterProcessError"); + assert.include(error.message, "cli.json"); + assert.include(error.message, "Unrecognized key"); + assert.notInclude(error.message, "adapter thread is closed"); + if (error._tag === "ProviderAdapterProcessError") { + assert.include(error.detail, "approvalMode"); + assert.include(error.detail, "sandbox"); + } + }), + ); + it.effect("maps app plan mode onto the ACP plan session mode", () => Effect.gen(function* () { const adapter = yield* CursorAdapter; diff --git a/apps/server/src/provider/acp/AcpAdapterSupport.test.ts b/apps/server/src/provider/acp/AcpAdapterSupport.test.ts index 0aebe0ca6d82..60f971edb499 100644 --- a/apps/server/src/provider/acp/AcpAdapterSupport.test.ts +++ b/apps/server/src/provider/acp/AcpAdapterSupport.test.ts @@ -25,4 +25,40 @@ describe("AcpAdapterSupport", () => { expect(error._tag).toBe("ProviderAdapterRequestError"); expect(error.message).toContain("Invalid params"); }); + + it("maps ACP process exits without stderr to a process error instead of a closed session", () => { + const error = mapAcpToAdapterError( + ProviderDriverKind.make("cursor"), + "thread-1" as never, + "session/start", + new EffectAcpErrors.AcpProcessExitedError({ code: 1 }), + ); + + expect(error._tag).toBe("ProviderAdapterProcessError"); + expect(error.message).not.toContain("adapter thread is closed"); + if (error._tag === "ProviderAdapterProcessError") { + expect(error.detail).toBe("ACP process exited with code 1"); + } + }); + + it("maps ACP process exits to a process error whose detail includes stderr", () => { + const error = mapAcpToAdapterError( + ProviderDriverKind.make("cursor"), + "thread-1" as never, + "session/start", + new EffectAcpErrors.AcpProcessExitedError({ + code: 1, + stderr: + "Invalid project config at ~/.cursor/cli.json: schema validation failed. Unrecognized key(s): 'approvalMode', 'sandbox'", + }), + ); + + expect(error._tag).toBe("ProviderAdapterProcessError"); + expect(error.message).toContain("cli.json"); + expect(error.message).toContain("Unrecognized key"); + expect(error.message).not.toContain("adapter thread is closed"); + if (error._tag === "ProviderAdapterProcessError") { + expect(error.detail).toContain("Unrecognized key(s): 'approvalMode', 'sandbox'"); + } + }); }); diff --git a/apps/server/src/provider/acp/AcpAdapterSupport.ts b/apps/server/src/provider/acp/AcpAdapterSupport.ts index cde110e6dd99..b90ce88e83dc 100644 --- a/apps/server/src/provider/acp/AcpAdapterSupport.ts +++ b/apps/server/src/provider/acp/AcpAdapterSupport.ts @@ -7,8 +7,8 @@ import * as Schema from "effect/Schema"; import * as EffectAcpErrors from "effect-acp/errors"; import { + ProviderAdapterProcessError, ProviderAdapterRequestError, - ProviderAdapterSessionClosedError, type ProviderAdapterError, } from "../Errors.ts"; const isAcpProcessExitedError = Schema.is(EffectAcpErrors.AcpProcessExitedError); @@ -21,9 +21,10 @@ export function mapAcpToAdapterError( error: EffectAcpErrors.AcpError, ): ProviderAdapterError { if (isAcpProcessExitedError(error)) { - return new ProviderAdapterSessionClosedError({ + return new ProviderAdapterProcessError({ provider, threadId, + detail: error.message, cause: error, }); } diff --git a/apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts b/apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts index 57323e2675e0..45a5cadb9a31 100644 --- a/apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts +++ b/apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts @@ -368,6 +368,26 @@ describe("AcpSessionRuntime", () => { }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); + it.effect("attaches child stderr when the ACP process exits before initialize", () => + Effect.gen(function* () { + const runtime = yield* AcpSessionRuntime.make({ + ...mockRuntimeOptions, + spawn: { + command: process.execPath, + args: [ + "-e", + "process.stderr.write(\"Invalid project config at /tmp/project/.cursor/cli.json: schema validation failed. Unrecognized key(s): 'approvalMode', 'sandbox'\\n\"); process.exit(1);", + ], + }, + }); + const error = yield* runtime.start().pipe(Effect.flip); + expect(error._tag).toBe("AcpProcessExitedError"); + expect(error.message).toContain("cli.json"); + expect(error.message).toContain("Unrecognized key"); + expect(error.message).not.toContain("ACP process exited with code 1\nACP process exited"); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); + it.effect("drains large stderr output and keeps auth-sized logging chunks", () => Effect.gen(function* () { const lengths: Array = []; diff --git a/apps/server/src/provider/acp/AcpSessionRuntime.ts b/apps/server/src/provider/acp/AcpSessionRuntime.ts index 70106bd16783..77517c44ea9b 100644 --- a/apps/server/src/provider/acp/AcpSessionRuntime.ts +++ b/apps/server/src/provider/acp/AcpSessionRuntime.ts @@ -22,6 +22,7 @@ import type * as EffectAcpSchema from "effect-acp/schema"; import type * as EffectAcpProtocol from "effect-acp/protocol"; import { resolveSpawnCommand } from "@t3tools/shared/shell"; +import { appendAcpStderrTail, sanitizeAcpStderrExcerpt } from "./AcpStderr.ts"; import { collectSessionConfigOptionValues, decideToolCallUpdateEmission, @@ -354,6 +355,8 @@ export const make = ( ); const stoppingRef = yield* Ref.make(false); const stderrFailure = yield* Deferred.make(); + const stderrTailRef = yield* Ref.make(""); + const stderrDrained = yield* Deferred.make(); const runtimeClosed = yield* Deferred.make(); const promptSerializationSemaphore = yield* Semaphore.make(1); const promptDispatchSemaphore = yield* Semaphore.make(1); @@ -374,22 +377,45 @@ export const make = ( } }); + const enrichProcessExitWithStderr = ( + error: EffectAcpErrors.AcpError, + ): Effect.Effect => + error._tag !== "AcpProcessExitedError" || (error.stderr?.trim().length ?? 0) > 0 + ? Effect.succeed(error) + : Deferred.await(stderrDrained).pipe( + Effect.timeout("250 millis"), + Effect.ignore, + Effect.andThen(Ref.get(stderrTailRef)), + Effect.map((tail) => { + const stderr = sanitizeAcpStderrExcerpt(tail); + return stderr.length === 0 + ? error + : new EffectAcpErrors.AcpProcessExitedError({ + ...(error.code !== undefined ? { code: error.code } : {}), + ...(error.pid !== undefined ? { pid: error.pid } : {}), + stderr, + ...(error.cause !== undefined ? { cause: error.cause } : {}), + }); + }), + ); + const recordTermination = Effect.fn("AcpSessionRuntime.recordTermination")(function* ( error: EffectAcpErrors.AcpError, ) { if (yield* Ref.get(stoppingRef)) { return; } + const enriched = yield* enrichProcessExitWithStderr(error); const firstTermination = yield* Ref.modify(terminationErrorRef, (current) => Option.isSome(current) ? ([false, current] as const) - : ([true, Option.some(error)] as const), + : ([true, Option.some(enriched)] as const), ); if (!firstTermination) { return; } yield* closeActiveAssistantSegment({ queue: eventQueue, assistantSegmentRef }); - yield* Queue.offer(eventQueue, { _tag: "ConnectionTerminated", error }); + yield* Queue.offer(eventQueue, { _tag: "ConnectionTerminated", error: enriched }); }); const logRequest = (event: AcpSessionRequestLogEvent) => @@ -406,6 +432,9 @@ export const make = ( ? Effect.raceFirst(effect, Deferred.await(stderrFailure)) : effect ).pipe( + Effect.catch((error) => + enrichProcessExitWithStderr(error).pipe(Effect.flatMap(Effect.fail)), + ), Effect.tap((result) => logRequest({ method, @@ -453,10 +482,10 @@ export const make = ( yield* child.stderr.pipe( Stream.decodeText(), Stream.runForEach((chunk) => - (options.onStderr - ? options.onStderr(chunk.slice(-maxStderrChunkLength)) - : Effect.void - ).pipe( + Ref.update(stderrTailRef, (current) => appendAcpStderrTail(current, chunk)).pipe( + Effect.andThen( + options.onStderr ? options.onStderr(chunk.slice(-maxStderrChunkLength)) : Effect.void, + ), Effect.catch((error) => Effect.gen(function* () { yield* Deferred.fail(stderrFailure, error); @@ -466,6 +495,7 @@ export const make = ( ), ), ), + Effect.ensuring(Deferred.succeed(stderrDrained, undefined)), Effect.ignore, Effect.forkIn(runtimeScope), ); diff --git a/apps/server/src/provider/acp/AcpStderr.test.ts b/apps/server/src/provider/acp/AcpStderr.test.ts new file mode 100644 index 000000000000..e88b59bdb0e1 --- /dev/null +++ b/apps/server/src/provider/acp/AcpStderr.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { + ACP_STDERR_TAIL_MAX_CHARS, + appendAcpStderrTail, + sanitizeAcpStderrExcerpt, +} from "./AcpStderr.ts"; + +describe("AcpStderr", () => { + it("keeps a bounded tail of stderr chunks", () => { + const prefix = "x".repeat(ACP_STDERR_TAIL_MAX_CHARS); + expect(appendAcpStderrTail(prefix, "abc")).toBe(`${prefix.slice(3)}abc`); + }); + + it("redacts home paths, pairing URLs, and tokens from stderr excerpts", () => { + const excerpt = sanitizeAcpStderrExcerpt( + [ + "Invalid project config at /Users/ada/.cursor/cli.json", + "Authorization: Bearer secret-token-value", + "Visit http://localhost:5733/pair#token=ABCDEF for pairing", + "key=sk-abcdefghijklmnopqrstuv", + ].join("\n"), + { HOME: "/Users/ada" }, + ); + + expect(excerpt).toContain("Invalid project config at ~/.cursor/cli.json"); + expect(excerpt).toContain("Bearer [redacted]"); + expect(excerpt).toContain("[pairing-url]"); + expect(excerpt).toContain("[redacted]"); + expect(excerpt).not.toContain("secret-token-value"); + expect(excerpt).not.toContain("ABCDEF"); + expect(excerpt).not.toContain("sk-abcdefghijklmnopqrstuv"); + }); + + it("redacts hyphenated OpenAI project keys and header credentials", () => { + const excerpt = sanitizeAcpStderrExcerpt( + [ + "openai=sk-proj-abcdefghijklmnopqrstuvwxyz012345", + "svc=sk-svcacct-abcdefghijklmnopqrstuvwxyz012345", + "anthropic=sk-ant-api03-abcdefghijklmnopqrstuvwxyz012345", + "Authorization: Basic dXNlcjpwYXNz", + "x-api-key: ant-api-key-value", + ].join("\n"), + ); + + expect(excerpt).toContain("[redacted]"); + expect(excerpt).toContain("Authorization: Basic [redacted]"); + expect(excerpt).toContain("x-api-key: [redacted]"); + expect(excerpt).not.toContain("sk-proj-"); + expect(excerpt).not.toContain("sk-svcacct-"); + expect(excerpt).not.toContain("sk-ant-api03-"); + expect(excerpt).not.toContain("abcdefghijklmnopqrstuvwxyz012345"); + expect(excerpt).not.toContain("dXNlcjpwYXNz"); + expect(excerpt).not.toContain("ant-api-key-value"); + }); +}); diff --git a/apps/server/src/provider/acp/AcpStderr.ts b/apps/server/src/provider/acp/AcpStderr.ts new file mode 100644 index 000000000000..52f68bb4c19a --- /dev/null +++ b/apps/server/src/provider/acp/AcpStderr.ts @@ -0,0 +1,38 @@ +// @effect-diagnostics nodeBuiltinImport:off -- The excerpt sanitizer masks the home directory, which only the Node os module can resolve. +import * as NodeOS from "node:os"; + +/** Last few KiB of ACP child stderr kept for startup / exit diagnostics. */ +export const ACP_STDERR_TAIL_MAX_CHARS = 4_096; + +const PAIRING_URL_PATTERN = /https?:\/\/[^\s]*\/pair#[^\s]*/gi; +const BEARER_TOKEN_PATTERN = /\bBearer\s+[A-Za-z0-9._\-+=/]+/gi; +const BASIC_AUTH_PATTERN = /\bAuthorization:\s*Basic\s+\S+/gi; +const API_KEY_HEADER_PATTERN = /\bx-api-key:\s*\S+/gi; +const SECRET_TOKEN_PATTERN = + /\b(?:sk-[A-Za-z0-9][A-Za-z0-9-]{7,}|ghp_[A-Za-z0-9]+|xox[a-zA-Z]-[A-Za-z0-9-]+)\b/g; + +export function appendAcpStderrTail(current: string, chunk: string): string { + const next = `${current}${chunk}`; + return next.length <= ACP_STDERR_TAIL_MAX_CHARS ? next : next.slice(-ACP_STDERR_TAIL_MAX_CHARS); +} + +/** Bounded, redacted excerpt safe to put on user-facing adapter errors. */ +export function sanitizeAcpStderrExcerpt( + text: string, + environment: NodeJS.ProcessEnv = process.env, +): string { + let result = text.replaceAll("\0", ""); + const homes = [environment.HOME, environment.USERPROFILE, NodeOS.homedir()].filter( + (value): value is string => typeof value === "string" && value.length > 1, + ); + for (const home of new Set(homes)) { + result = result.split(home).join("~"); + } + result = result + .replace(PAIRING_URL_PATTERN, "[pairing-url]") + .replace(BEARER_TOKEN_PATTERN, "Bearer [redacted]") + .replace(BASIC_AUTH_PATTERN, "Authorization: Basic [redacted]") + .replace(API_KEY_HEADER_PATTERN, "x-api-key: [redacted]") + .replace(SECRET_TOKEN_PATTERN, "[redacted]"); + return result.trim(); +} diff --git a/packages/effect-acp/src/errors.ts b/packages/effect-acp/src/errors.ts index a2e343d67664..01f296bb8cb6 100644 --- a/packages/effect-acp/src/errors.ts +++ b/packages/effect-acp/src/errors.ts @@ -95,13 +95,15 @@ export class AcpProcessExitedError extends Schema.TaggedError 0 ? `${base}\n${excerpt}` : base; } }