diff --git a/src/host/README.md b/src/host/README.md index 93b0afff..56b5c74e 100644 --- a/src/host/README.md +++ b/src/host/README.md @@ -62,7 +62,7 @@ - `allocateSession` throws `INVALID_DIMENSIONS` for non-positive `cols` or `rows`, `STORAGE_READ_ERROR` when `cwd` is not a directory and `INVALID_INPUT` when the shell path is not a file. An empty `command` defaults to the shell path. It creates the session directory and calls `launch` only after these checks pass. [lifecycle.ts:allocateSession] - `buildReplayInput` validates the manifest with `SessionRecordSchema` and requires its `sessionId` to match. It validates the events with `validateEventRecords` unless `trustValidated` is set. `targetSeq` is -1 for no events, else from 0 to the last `seq`. [replay.ts:buildReplayInput] - `RpcServer.listen` removes a stale socket file (no answer within 1,000 ms, `ECONNREFUSED` or `ENOENT`) and throws when another server is listening. It asserts a socket path of at most 104 characters. [rpcServer.ts:RpcServer] -- `RpcServer` and `sendRpc` cap a message at 1,048,576 characters. The server validates the request with `RpcRequestSchema`, the params and result with `RpcMethodSchemas`, and the response with `RpcResponseSchema` (all from `src/protocol/messages.ts`). A handler that throws a `CliError` gets an error response with its code, message and, when it has any, its `details`. It leaves `details` out when they do not serialize to a JSON object (a `BigInt`, a circular object, a `toJSON` that returns something else) or take more than 65,536 characters. It cuts an error message to 65,536 characters and ends it with `[message truncated]`. So an error response stays under the 1,048,576-character cap and keeps the error's code. `sendRpc` rebuilds that `CliError` with the same details. Any other thrown error becomes `RPC_ERROR` without details. If building or sending the response throws, the server logs the error and answers `RPC_ERROR` "RPC request handling failed unexpectedly.", so the host keeps running. Unknown methods get an error response. [rpcServer.ts:RpcServer] [rpcClient.ts:sendRpc] +- `RpcServer` and `sendRpc` cap a message at 1,048,576 characters. The server validates the request with `RpcRequestSchema`, the params and result with `RpcMethodSchemas`, and the response with `RpcResponseSchema` (all from `src/protocol/messages.ts`). A handler that throws a `CliError` gets an error response with its code, message and, when it has any, its `details`. It leaves `details` out when they do not serialize to a JSON object (a `BigInt`, a circular object, a `toJSON` that returns something else) or take more than 65,536 characters. It cuts an error message to 65,536 characters, without splitting a surrogate pair, and ends it with `[message truncated]`. A request id over 128 characters fails `RpcRequestSchema`, and the error response uses the id `unknown`. So an error response stays under the 1,048,576-character cap and keeps the error's code. `sendRpc` rebuilds that `CliError` with the same details. Any other thrown error becomes `RPC_ERROR` without details. If building or sending the response throws, the server logs the error and answers `RPC_ERROR` "RPC request handling failed unexpectedly.", so the host keeps running. Unknown methods get an error response. [rpcServer.ts:RpcServer] [rpcClient.ts:sendRpc] - `sendRpc` waits 5,000 ms by default. It requires the response `id` to match the request. It maps `ECONNREFUSED`, `ENOENT` and `ECONNRESET` to `HOST_UNREACHABLE`, a socket timeout to `HOST_TIMEOUT` and other transport errors to `RPC_ERROR`. [rpcClient.ts:sendRpc] - `HostRendererManager` runs backend operations one at a time. `withBackend` requires the replay input `sessionId` to match, and recreates the backend when the initial size changes. [renderer.ts:HostRendererManager] - A waited `run` calls `EventLog.startAppend`, `registerWaitedRun` and `pty.write` with no `await` in between, then awaits `written`. When `registerWaitedRun` or `pty.write` throws, it waits for `written` to settle and rethrows that error, so a failed write never becomes an unhandled rejection. So no other input reaches the PTY or the log between the command's `seq` and its write, and the marker is registered before the command can print. [hostMain.ts:runHost] diff --git a/src/host/rpcServer.ts b/src/host/rpcServer.ts index 8af7c18a..461c8d23 100644 --- a/src/host/rpcServer.ts +++ b/src/host/rpcServer.ts @@ -4,6 +4,7 @@ import net from 'node:net'; import { CliError } from '../cli/errors.js'; import { ERROR_CODES } from '../protocol/errors.js'; import { + MAX_RPC_REQUEST_ID_CHARS, RpcMethodSchemas, RpcRequestSchema, RpcResponseSchema, @@ -23,9 +24,12 @@ const MAX_RPC_ERROR_DETAILS_CHARS = 65_536; const MAX_RPC_ERROR_MESSAGE_CHARS = 65_536; const TRUNCATED_MESSAGE_SUFFIX = ' [message truncated]'; // JSON writes a control character as 6 characters (\u0001), so an error -// response with a message and details at their caps still fits in one buffer. +// response with an id, a message and details at their caps still fits in one +// buffer. invariant( - 6 * MAX_RPC_ERROR_MESSAGE_CHARS + MAX_RPC_ERROR_DETAILS_CHARS + 1_024 < + 6 * (MAX_RPC_REQUEST_ID_CHARS + MAX_RPC_ERROR_MESSAGE_CHARS) + + MAX_RPC_ERROR_DETAILS_CHARS + + 1_024 < MAX_RPC_BUFFER_BYTES, 'RPC error message and details caps must fit in one RPC response.', ); @@ -122,7 +126,11 @@ function extractRequestId(value: unknown): string { if (value !== null && typeof value === 'object' && 'id' in value) { const id = value.id; - if (typeof id === 'string' && id.length > 0) { + if ( + typeof id === 'string' && + id.length > 0 && + id.length <= MAX_RPC_REQUEST_ID_CHARS + ) { return id; } } @@ -139,9 +147,16 @@ function requestIdFromLine(line: string): string { } function sendableMessage(message: string): string { - return message.length <= MAX_RPC_ERROR_MESSAGE_CHARS - ? message - : `${message.slice(0, MAX_RPC_ERROR_MESSAGE_CHARS - TRUNCATED_MESSAGE_SUFFIX.length)}${TRUNCATED_MESSAGE_SUFFIX}`; + if (message.length <= MAX_RPC_ERROR_MESSAGE_CHARS) { + return message; + } + let end = MAX_RPC_ERROR_MESSAGE_CHARS - TRUNCATED_MESSAGE_SUFFIX.length; + const lastKept = message.charCodeAt(end - 1); + // Do not keep the first half of a surrogate pair without its second half. + if (lastKept >= 0xd800 && lastKept <= 0xdbff) { + end -= 1; + } + return `${message.slice(0, end)}${TRUNCATED_MESSAGE_SUFFIX}`; } function buildErrorResponse(id: string, message: string): RpcResponse { diff --git a/src/protocol/README.md b/src/protocol/README.md index 04d6fd22..b8404970 100644 --- a/src/protocol/README.md +++ b/src/protocol/README.md @@ -34,7 +34,7 @@ - `InputRunEventPayloadSchema` requires `marker` when `noWait` is false and rejects it when `noWait` is true. [schemas.ts:InputRunEventPayloadSchema] - `WaitForRenderParamsSchema` requires at least one of `text`, `regex`, `screenStableMs`, `cursorRow` or `cursorCol`. `text` and `regex` are mutually exclusive, and scope `cursor-line` needs `text` or `regex`. [schemas.ts:WaitForRenderParamsSchema] - `RecordDiffResultSchema` checks that `identical` matches the equality of the two screen hashes. An identical result must have an empty diff and equal side row counts. A side with `capturedAtSeq` -1 must hash to a blank screen. For a non-identical result, the rows rebuilt from the diff must hash to each side's `screenHash`. [schemas.ts:RecordDiffResultSchema] -- `RpcRequestSchema` requires non-empty `id` and `method`. It does not restrict `method` to `RpcMethodSchema`. [messages.ts:RpcRequestSchema] +- `RpcRequestSchema` requires non-empty `id` and `method`. The `id` takes at most `MAX_RPC_REQUEST_ID_CHARS` (128) characters, so the id that the server echoes in a response stays small. It does not restrict `method` to `RpcMethodSchema`. [messages.ts:RpcRequestSchema] - `RpcErrorSchema` requires non-empty `code` and `message`, and allows an optional `details` record. A CLI from before `details` was added rejects an error response that has it, so that CLI reports `RPC_ERROR` (`retryable: true`) for such an error from a newer host, and retrying does not help. A newer CLI reads an older host's error responses. [messages.ts:RpcErrorSchema] - `RpcMethodSchemas` uses `satisfies Record`, so TypeScript requires one entry per method. [messages.ts:RpcMethodSchemas] - `RunParamsSchema` defaults `noWait` to `false`. [messages.ts:RunParamsSchema] diff --git a/src/protocol/messages.ts b/src/protocol/messages.ts index d67134d8..1a1657e4 100644 --- a/src/protocol/messages.ts +++ b/src/protocol/messages.ts @@ -69,9 +69,13 @@ const EmptyObjectSchema = z.object({}).strict(); const NonEmptyStringSchema = z.string().min(1); const DurationSchema = z.number().int().positive(); +// The client sends a UUID (36 characters). The cap bounds the id that the +// server echoes in every response. +export const MAX_RPC_REQUEST_ID_CHARS = 128; + export const RpcRequestSchema = z .object({ - id: NonEmptyStringSchema, + id: z.string().min(1).max(MAX_RPC_REQUEST_ID_CHARS), method: NonEmptyStringSchema, params: z.record(z.string(), z.unknown()).optional(), }) diff --git a/test/unit/host/rpcServer.test.ts b/test/unit/host/rpcServer.test.ts index d7fa7cf3..22736e8a 100644 --- a/test/unit/host/rpcServer.test.ts +++ b/test/unit/host/rpcServer.test.ts @@ -62,6 +62,51 @@ describe('RpcServer request abort handling', () => { }); }); +describe('RpcServer request ids', () => { + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), 'agent-tty-rpc-server-')); + }); + + afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); + tempDir = ''; + }); + + it('rejects a request id over 128 characters without echoing it', async () => { + const socketFile = join(tempDir, 'rpc.sock'); + const inspect = vi.fn(() => Promise.resolve({})); + const server = new RpcServer(socketFile, { inspect }); + await server.listen(); + const client = net.connect({ path: socketFile }); + client.setEncoding('utf8'); + let received = ''; + client.on('data', (chunk: string) => { + received += chunk; + }); + const ended = new Promise((resolve, reject) => { + client.once('end', resolve); + client.once('error', reject); + }); + + try { + client.write( + `${JSON.stringify({ id: 'x'.repeat(129), method: 'inspect', params: {} })}\n`, + ); + await ended; + + expect(JSON.parse(received)).toMatchObject({ + id: 'unknown', + ok: false, + error: { code: 'RPC_ERROR' }, + }); + expect(inspect).not.toHaveBeenCalled(); + } finally { + client.destroy(); + await server.close(); + } + }); +}); + describe('RpcServer CliError responses', () => { beforeEach(async () => { tempDir = await mkdtemp(join(tmpdir(), 'agent-tty-rpc-server-')); @@ -193,6 +238,14 @@ describe('RpcServer CliError responses', () => { expect(message.endsWith('[message truncated]')).toBe(true); }); + it('does not cut a surrogate pair in half when it shortens a message', async () => { + const error = await rejectionFrom(new Error(`a${'😀'.repeat(40_000)}`)); + + expect((error as CliError).message).toBe( + `a${'😀'.repeat(32_757)} [message truncated]`, + ); + }); + it('answers RPC_ERROR when building the error response throws', async () => { const consoleError = vi .spyOn(console, 'error') diff --git a/test/unit/protocol/messages.test.ts b/test/unit/protocol/messages.test.ts index 7258a272..4cfb6fdb 100644 --- a/test/unit/protocol/messages.test.ts +++ b/test/unit/protocol/messages.test.ts @@ -292,6 +292,14 @@ describe('RPC message schemas', () => { expect(result.success).toBe(true); }); + it('accepts a request id of up to 128 characters', () => { + const request = (id: string) => + RpcRequestSchema.safeParse({ id, method: 'inspect' }).success; + + expect(request('x'.repeat(128))).toBe(true); + expect(request('x'.repeat(129))).toBe(false); + }); + it('rejects a request with a non-object params payload', () => { const result = RpcRequestSchema.safeParse({ id: 'request-1',