mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(server): sanitize unexpected protocol errors
This commit is contained in:
parent
2acd77b5c1
commit
515455fbc3
13 changed files with 115 additions and 34 deletions
|
|
@ -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<void> {
|
|||
await env.cleanup();
|
||||
}
|
||||
}
|
||||
|
||||
export { toPiServerError } from "./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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
35
packages/coding-agent/test/server/errors.test.ts
Normal file
35
packages/coding-agent/test/server/errors.test.ts
Normal file
|
|
@ -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" });
|
||||
});
|
||||
});
|
||||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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<SessionSnapshot> {
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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<PiServer>();
|
||||
|
|
@ -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" }));
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue