Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
43 changes: 43 additions & 0 deletions apps/server/src/provider/Layers/CursorAdapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
36 changes: 36 additions & 0 deletions apps/server/src/provider/acp/AcpAdapterSupport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'");
}
});
});
5 changes: 3 additions & 2 deletions apps/server/src/provider/acp/AcpAdapterSupport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
});
}
Expand Down
20 changes: 20 additions & 0 deletions apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<number> = [];
Expand Down
42 changes: 36 additions & 6 deletions apps/server/src/provider/acp/AcpSessionRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -354,6 +355,8 @@ export const make = (
);
const stoppingRef = yield* Ref.make(false);
const stderrFailure = yield* Deferred.make<never, EffectAcpErrors.AcpError>();
const stderrTailRef = yield* Ref.make("");
const stderrDrained = yield* Deferred.make<void>();
const runtimeClosed = yield* Deferred.make<void>();
const promptSerializationSemaphore = yield* Semaphore.make(1);
const promptDispatchSemaphore = yield* Semaphore.make(1);
Expand All @@ -374,22 +377,45 @@ export const make = (
}
});

const enrichProcessExitWithStderr = (
error: EffectAcpErrors.AcpError,
): Effect.Effect<EffectAcpErrors.AcpError> =>
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) =>
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -466,6 +495,7 @@ export const make = (
),
),
),
Effect.ensuring(Deferred.succeed(stderrDrained, undefined)),
Effect.ignore,
Effect.forkIn(runtimeScope),
);
Expand Down
56 changes: 56 additions & 0 deletions apps/server/src/provider/acp/AcpStderr.test.ts
Original file line number Diff line number Diff line change
@@ -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");
});
});
38 changes: 38 additions & 0 deletions apps/server/src/provider/acp/AcpStderr.ts
Original file line number Diff line number Diff line change
@@ -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);
Comment thread
juliusmarminge marked this conversation as resolved.
}

/** 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]");
Comment thread
juliusmarminge marked this conversation as resolved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return result.trim();
}
8 changes: 5 additions & 3 deletions packages/effect-acp/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,15 @@ export class AcpProcessExitedError extends Schema.TaggedError<AcpProcessExitedEr
{
code: Schema.optional(Schema.Number),
pid: Schema.optionalKey(Schema.Int),
stderr: Schema.optionalKey(Schema.String),
Comment thread
juliusmarminge marked this conversation as resolved.
cause: Schema.optional(Schema.Defect()),
},
) {
override get message() {
return this.code === undefined
? "ACP process exited"
: `ACP process exited with code ${this.code}`;
const base =
this.code === undefined ? "ACP process exited" : `ACP process exited with code ${this.code}`;
const excerpt = this.stderr?.trim();
return excerpt && excerpt.length > 0 ? `${base}\n${excerpt}` : base;
}
}

Expand Down
Loading