From 515455fbc3b9d8b19c1a061cdc894a023fbc030a Mon Sep 17 00:00:00 2001 From: Christian Klotz Date: Mon, 3 Aug 2026 11:19:07 +0300 Subject: [PATCH] fix(server): sanitize unexpected protocol errors --- packages/coding-agent/src/server/backend.ts | 9 +++-- packages/coding-agent/src/server/errors.ts | 12 +++++-- .../src/server/session-runtime.ts | 14 ++++---- .../coding-agent/test/server/errors.test.ts | 35 +++++++++++++++++++ .../coding-agent/test/server/runtime.test.ts | 14 ++++---- packages/protocol/CHANGELOG.md | 1 + packages/protocol/src/schemas.ts | 1 + packages/server/CHANGELOG.md | 1 + packages/server/src/errors.ts | 10 +++++- packages/server/src/server.ts | 6 +++- packages/server/src/sessions.ts | 9 +++-- packages/server/test/conformance.test.ts | 35 +++++++++++++++++-- packages/server/test/sessions.test.ts | 2 +- 13 files changed, 115 insertions(+), 34 deletions(-) create mode 100644 packages/coding-agent/test/server/errors.test.ts diff --git a/packages/coding-agent/src/server/backend.ts b/packages/coding-agent/src/server/backend.ts index 861644152..e2ed41f7a 100644 --- a/packages/coding-agent/src/server/backend.ts +++ b/packages/coding-agent/src/server/backend.ts @@ -8,6 +8,7 @@ import { type SessionCreateOptions, type SessionMetadata, type SessionRepository, + toError, } from "@earendil-works/pi-agent-core"; import { NodeExecutionEnv } from "@earendil-works/pi-agent-core/node"; import type { Api, Model } from "@earendil-works/pi-ai"; @@ -21,7 +22,7 @@ import { import { getAgentDir } from "../config.ts"; import { ModelRuntime } from "../core/model-runtime.ts"; import { SettingsManager } from "../core/settings-manager.ts"; -import { toPiServerError } from "./errors.ts"; +import { mapKnownServerError } from "./errors.ts"; import { ServerModelResolver } from "./model-resolver.ts"; import { type SessionLease, SessionLockManager } from "./session-lock.ts"; import { CodingAgentSessionRuntime } from "./session-runtime.ts"; @@ -230,7 +231,7 @@ export class CodingAgentServerBackend< if (cleanupErrors.length > 0) { throw new AggregateError([error, ...cleanupErrors], `Failed to create and clean up session ${options.id}`); } - throw toPiServerError(error); + throw mapKnownServerError(error) ?? toError(error); } } @@ -256,7 +257,7 @@ export class CodingAgentServerBackend< return await this.createRuntime(session, model, thinkingLevel, lease, state.cwd); } catch (error) { await lease.release(); - throw toPiServerError(error); + throw mapKnownServerError(error) ?? toError(error); } } @@ -297,5 +298,3 @@ async function validateCwd(cwd: string): Promise { await env.cleanup(); } } - -export { toPiServerError } from "./errors.ts"; diff --git a/packages/coding-agent/src/server/errors.ts b/packages/coding-agent/src/server/errors.ts index f30fd8164..3969981ee 100644 --- a/packages/coding-agent/src/server/errors.ts +++ b/packages/coding-agent/src/server/errors.ts @@ -1,7 +1,7 @@ -import { AgentHarnessError, SessionError } from "@earendil-works/pi-agent-core"; +import { AgentHarnessError, SessionError, toError } from "@earendil-works/pi-agent-core"; import { PiServerError } from "@earendil-works/pi-server"; -export function toPiServerError(error: unknown): Error { +export function mapKnownServerError(error: unknown): PiServerError | undefined { if (error instanceof PiServerError) return error; if (error instanceof AgentHarnessError) { if (error.code === "busy") return new PiServerError("busy", error.message); @@ -13,5 +13,11 @@ export function toPiServerError(error: unknown): Error { if (error instanceof SessionError && error.code === "not_found") { return new PiServerError("not_found", error.message); } - return error instanceof Error ? error : new Error(String(error)); + return undefined; +} + +export function createInternalServerError(cause: unknown): PiServerError { + const error = new PiServerError("internal_error", "Internal server error"); + error.cause = toError(cause); + return error; } diff --git a/packages/coding-agent/src/server/session-runtime.ts b/packages/coding-agent/src/server/session-runtime.ts index 789467fcd..3e601aa1c 100644 --- a/packages/coding-agent/src/server/session-runtime.ts +++ b/packages/coding-agent/src/server/session-runtime.ts @@ -12,7 +12,7 @@ import { import type { ModelRuntime } from "../core/model-runtime.ts"; import type { SettingsManager } from "../core/settings-manager.ts"; import { createServerHarness } from "./create-harness.ts"; -import { toPiServerError } from "./errors.ts"; +import { createInternalServerError, mapKnownServerError } from "./errors.ts"; import type { ServerModelResolver } from "./model-resolver.ts"; import type { SessionLease } from "./session-lock.ts"; import { inspectServerSession } from "./session-state.ts"; @@ -127,7 +127,7 @@ export class CodingAgentSessionRuntime implements PiSessionRuntime { this.assertUsable(); } catch (error) { this.assertUsable(); - throw toPiServerError(error); + throw mapKnownServerError(error) ?? toError(error); } finally { if (!this.lockCompromise) this.phase = "idle"; } @@ -141,7 +141,7 @@ export class CodingAgentSessionRuntime implements PiSessionRuntime { this.assertUsable(); } catch (error) { this.assertUsable(); - throw toPiServerError(error); + throw mapKnownServerError(error) ?? toError(error); } } @@ -153,7 +153,7 @@ export class CodingAgentSessionRuntime implements PiSessionRuntime { this.assertUsable(); } catch (error) { this.assertUsable(); - throw toPiServerError(error); + throw mapKnownServerError(error) ?? toError(error); } } @@ -243,8 +243,7 @@ export class CodingAgentSessionRuntime implements PiSessionRuntime { private handleProjectionFailure(error: unknown): void { if (this.terminalError) return; - this.terminalError = new PiServerError("invalid_request", "Session protocol projection failed"); - this.terminalError.cause = error instanceof Error ? error : new Error(String(error)); + this.terminalError = createInternalServerError(error); this.terminateHarness(this.terminalError); } @@ -272,8 +271,7 @@ export class CodingAgentSessionRuntime implements PiSessionRuntime { if (this.terminalError || this.lockCompromise || this.lease.compromisedError() || this.disposed) { this.assertUsable(); } - this.terminalError = new PiServerError("invalid_request", `Session failed to ${operation}`); - this.terminalError.cause = toPiServerError(error); + this.terminalError = mapKnownServerError(error) ?? createInternalServerError(error); this.terminateHarness(this.terminalError); throw this.terminalError; } finally { diff --git a/packages/coding-agent/test/server/errors.test.ts b/packages/coding-agent/test/server/errors.test.ts new file mode 100644 index 000000000..3a51af1ef --- /dev/null +++ b/packages/coding-agent/test/server/errors.test.ts @@ -0,0 +1,35 @@ +import { AgentHarnessError, SessionError } from "@earendil-works/pi-agent-core"; +import { PiServerError } from "@earendil-works/pi-server"; +import { describe, expect, test } from "vitest"; +import { createInternalServerError, mapKnownServerError } from "../../src/server/errors.ts"; + +describe("coding-agent server error mapping", () => { + test("maps known domain errors to stable protocol errors", () => { + const existing = new PiServerError("session_locked", "already mapped"); + expect(mapKnownServerError(existing)).toBe(existing); + expect(mapKnownServerError(new AgentHarnessError("busy", "harness busy"))).toMatchObject({ + code: "busy", + message: "harness busy", + }); + expect(mapKnownServerError(new AgentHarnessError("invalid_argument", "bad argument"))).toMatchObject({ + code: "invalid_request", + message: "bad argument", + }); + const missing = new SessionError("not_found", "missing session"); + expect(mapKnownServerError(missing)).toMatchObject({ code: "not_found", message: "missing session" }); + expect(mapKnownServerError(new AgentHarnessError("session", "wrapped missing session", missing))).toMatchObject({ + code: "not_found", + message: "wrapped missing session", + }); + }); + + test("does not classify unexpected failures as protocol-safe", () => { + expect(mapKnownServerError(new Error("private detail"))).toBeUndefined(); + expect(mapKnownServerError(new AgentHarnessError("unknown", "unexpected"))).toBeUndefined(); + expect(mapKnownServerError("non-error throw")).toBeUndefined(); + + const internal = createInternalServerError("private detail"); + expect(internal).toMatchObject({ code: "internal_error", message: "Internal server error" }); + expect(internal.cause).toMatchObject({ message: "private detail" }); + }); +}); diff --git a/packages/coding-agent/test/server/runtime.test.ts b/packages/coding-agent/test/server/runtime.test.ts index b59c94698..2bbf7def3 100644 --- a/packages/coding-agent/test/server/runtime.test.ts +++ b/packages/coding-agent/test/server/runtime.test.ts @@ -237,14 +237,14 @@ describe("coding-agent session runtime", () => { try { await expect(runtime.setModel({ provider: fixture.faux.provider.id, id: "faux-plain" })).rejects.toMatchObject( { - code: "invalid_request", - message: "Session failed to set model", + code: "internal_error", + message: "Internal server error", cause: expect.objectContaining({ message: expect.stringContaining("thinking write failed") }), }, ); await expect(runtime.snapshot()).rejects.toMatchObject({ - code: "invalid_request", - message: "Session failed to set model", + code: "internal_error", + message: "Internal server error", }); } finally { appendSpy.mockRestore(); @@ -318,11 +318,11 @@ describe("coding-agent session runtime", () => { fauxAssistantMessage(fauxToolCall("read", { path: Number.NaN }), { stopReason: "toolUse" }), ]); await expect(runtime.prompt({ text: "trigger invalid input" })).rejects.toMatchObject({ - code: "invalid_request", - message: "Session protocol projection failed", + code: "internal_error", + message: "Internal server error", }); expect(projectionError?.cause).toBeInstanceOf(TypeError); - await expect(runtime.snapshot()).rejects.toMatchObject({ code: "invalid_request" }); + await expect(runtime.snapshot()).rejects.toMatchObject({ code: "internal_error" }); } finally { await runtime.dispose(); await removeServerBackendFixture(fixture); diff --git a/packages/protocol/CHANGELOG.md b/packages/protocol/CHANGELOG.md index 50b8ba8d5..b3730892b 100644 --- a/packages/protocol/CHANGELOG.md +++ b/packages/protocol/CHANGELOG.md @@ -9,3 +9,4 @@ ### Added - Added transport-neutral CBOR protocol schemas, codecs, and length-prefixed framing for remote pi sessions. +- Added the `internal_error` protocol code for unexpected server failures. diff --git a/packages/protocol/src/schemas.ts b/packages/protocol/src/schemas.ts index afd61e91b..3c4fda722 100644 --- a/packages/protocol/src/schemas.ts +++ b/packages/protocol/src/schemas.ts @@ -268,6 +268,7 @@ export const ProtocolErrorCodeSchema = Type.Union([ Type.Literal("busy"), Type.Literal("session_locked"), Type.Literal("not_found"), + Type.Literal("internal_error"), Type.Literal("invalid_request"), ]); export const ProtocolErrorSchema = StrictObject({ diff --git a/packages/server/CHANGELOG.md b/packages/server/CHANGELOG.md index d5b3aaac6..f65d370c4 100644 --- a/packages/server/CHANGELOG.md +++ b/packages/server/CHANGELOG.md @@ -10,6 +10,7 @@ ### Fixed - Hardened protocol adapters against contradictory lifecycle states, invalid identifiers and timestamps, sparse execution arrays, and additive `pi-ai` contract drift. +- Changed unexpected backend failures to report `internal_error` without exposing implementation details. ## [0.83.0] - 2026-07-29 diff --git a/packages/server/src/errors.ts b/packages/server/src/errors.ts index 6623651c8..9e50baf5e 100644 --- a/packages/server/src/errors.ts +++ b/packages/server/src/errors.ts @@ -2,7 +2,7 @@ import type { JsonValue, ProtocolErrorCode } from "@earendil-works/pi-protocol"; export type PiServerOperationErrorCode = Extract< ProtocolErrorCode, - "busy" | "session_locked" | "not_found" | "invalid_request" + "busy" | "session_locked" | "not_found" | "invalid_request" | "internal_error" >; /** A backend/runtime error that can safely cross the protocol boundary. */ @@ -38,3 +38,11 @@ export class SessionNotFoundError extends PiServerError { this.name = "SessionNotFoundError"; } } + +export class InternalServerError extends PiServerError { + constructor(cause: Error) { + super("internal_error", "Internal server error"); + this.name = "InternalServerError"; + this.cause = cause; + } +} diff --git a/packages/server/src/server.ts b/packages/server/src/server.ts index 95e7c859b..9f5e77d08 100644 --- a/packages/server/src/server.ts +++ b/packages/server/src/server.ts @@ -345,6 +345,10 @@ export class PiServer { private toProtocolError(error: unknown): ProtocolError { if (error instanceof PiServerError) { + if (error.code === "internal_error") { + this.reportError(error.cause ?? error); + return { code: "internal_error", message: "Internal server error" }; + } return error.details === undefined ? { code: error.code, message: error.message } : { code: error.code, message: error.message, details: error.details }; @@ -353,7 +357,7 @@ export class PiServer { return { code: "invalid_request", message: error.message }; } this.reportError(error); - return { code: "invalid_request", message: "Internal server error" }; + return { code: "internal_error", message: "Internal server error" }; } private reportError(error: unknown): void { diff --git a/packages/server/src/sessions.ts b/packages/server/src/sessions.ts index d6e08324a..31d2eac91 100644 --- a/packages/server/src/sessions.ts +++ b/packages/server/src/sessions.ts @@ -1,7 +1,7 @@ import { randomUUID } from "node:crypto"; import type { Command, EventEnvelope, SessionSnapshot, SessionSummary } from "@earendil-works/pi-protocol"; import type { ByteConnection, ConnectionState } from "./connection.ts"; -import { PiServerError } from "./errors.ts"; +import { InternalServerError, PiServerError } from "./errors.ts"; import type { CreateSessionOptions, PiSessionBackend, PiSessionRuntime, PiSessionRuntimeEvent } from "./types.ts"; interface LiveSession { @@ -223,9 +223,8 @@ export class LiveSessionManager { try { const snapshot = await runtime.snapshot(); if (snapshot.id !== id) { - throw new PiServerError( - "invalid_request", - `Backend returned session ${snapshot.id} for server-assigned session ${id}`, + throw new InternalServerError( + new Error(`Backend returned session ${snapshot.id} for server-assigned session ${id}`), ); } live = { @@ -283,7 +282,7 @@ export class LiveSessionManager { private async normalizedSnapshot(live: LiveSession): Promise { const snapshot = await live.runtime.snapshot(); if (snapshot.id !== live.id) { - throw new PiServerError("invalid_request", `Runtime session ID changed from ${live.id} to ${snapshot.id}`); + throw new InternalServerError(new Error(`Runtime session ID changed from ${live.id} to ${snapshot.id}`)); } return { ...snapshot, diff --git a/packages/server/test/conformance.test.ts b/packages/server/test/conformance.test.ts index 5ebba2192..c7dae5565 100644 --- a/packages/server/test/conformance.test.ts +++ b/packages/server/test/conformance.test.ts @@ -3,8 +3,14 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { encodeClientMessage, encodeFrame, PROTOCOL_VERSION } from "@earendil-works/pi-protocol"; import { afterEach, describe, expect, test } from "vitest"; -import { type PiServer, PiServerError } from "../src/index.ts"; -import { connectUnixTestClient, Deferred, type ProtocolTestClient, TestSessionBackend } from "../src/testing/index.ts"; +import { InternalServerError, type PiServer, PiServerError } from "../src/index.ts"; +import { + connectUnixTestClient, + Deferred, + type ProtocolTestClient, + TEST_TOKEN, + TestSessionBackend, +} from "../src/testing/index.ts"; import { createUnixServer, type UnixServerOptions } from "../src/transports/unix/index.ts"; const servers = new Set(); @@ -245,6 +251,29 @@ describe("Unix transport conformance", () => { expect(backend.latestRuntime("terminal")).not.toBe(runtime); }); + test("reports wrapped internal causes without exposing them to clients", async () => { + class FailingBackend extends TestSessionBackend { + private listCount = 0; + + override async listSessions() { + this.listCount += 1; + if (this.listCount > 1) throw new InternalServerError(new Error("private wrapped detail")); + return super.listSessions(); + } + } + const errors: Error[] = []; + const backend = new FailingBackend(); + const { server } = await startServer(backend, { onError: (error) => errors.push(error) }); + const client = await connect(server); + await client.hello(); + + expect(await client.request({ command: "list" })).toMatchObject({ + ok: false, + error: { code: "internal_error", message: "Internal server error" }, + }); + expect(errors).toContainEqual(expect.objectContaining({ message: "private wrapped detail" })); + }); + test("does not expose unexpected backend errors to clients", async () => { class FailingBackend extends TestSessionBackend { private listCount = 0; @@ -267,7 +296,7 @@ describe("Unix transport conformance", () => { await client.hello(); expect(await client.request({ command: "list" })).toMatchObject({ ok: false, - error: { code: "invalid_request", message: "Internal server error" }, + error: { code: "internal_error", message: "Internal server error" }, }); expect(errors).toContainEqual(expect.objectContaining({ message: "private backend detail" })); }); diff --git a/packages/server/test/sessions.test.ts b/packages/server/test/sessions.test.ts index 1f8acfb3b..e12860ef3 100644 --- a/packages/server/test/sessions.test.ts +++ b/packages/server/test/sessions.test.ts @@ -367,7 +367,7 @@ describe("PiServer Unix integration", () => { const client = await connect(server); await client.hello(); const response = await client.request({ command: "create" }); - expect(response).toMatchObject({ ok: false, error: { code: "invalid_request" } }); + expect(response).toMatchObject({ ok: false, error: { code: "internal_error" } }); expect(backend.latestRuntime("wrong-id").disposeCount).toBe(1); });