fix(server): remove wall-clock timeout for Pi compact RPC (#2181)
Pi compact is a blocking LLM summarization job that often exceeds the default 30s control-plane timeout on long sessions, causing a false UI error while the real compact continues. Wait for the RPC response, process exit, or session close instead. Also cover the no-timeout lifecycle: null timeout still rejects on close, and compact waits past 30s for a late success response. Closes #1946
This commit is contained in:
@@ -140,6 +140,29 @@ describe("JsonlRpcProcess", () => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("null timeout waits past short wall-clock limits until the response arrives", async () => {
|
||||||
|
const transport = startProcess();
|
||||||
|
|
||||||
|
try {
|
||||||
|
await expect(
|
||||||
|
transport.request({ type: "echo", value: "slow", delayMs: 80 }, null),
|
||||||
|
).resolves.toMatchObject({ value: "slow" });
|
||||||
|
} finally {
|
||||||
|
await transport.close();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test("null timeout still rejects when the process is closed", async () => {
|
||||||
|
const transport = startProcess();
|
||||||
|
await transport.request({ type: "echo", value: "ready" });
|
||||||
|
const request = transport.request({ type: "hang" }, null);
|
||||||
|
|
||||||
|
const rejection = expect(request).rejects.toThrow("JSONL RPC process is closed");
|
||||||
|
await transport.close();
|
||||||
|
|
||||||
|
await rejection;
|
||||||
|
});
|
||||||
|
|
||||||
test("rejects pending requests and publishes stderr when the child exits", async () => {
|
test("rejects pending requests and publishes stderr when the child exits", async () => {
|
||||||
const transport = startProcess();
|
const transport = startProcess();
|
||||||
const exit = nextExit(transport);
|
const exit = nextExit(transport);
|
||||||
|
|||||||
@@ -4,7 +4,14 @@ import type { Logger } from "pino";
|
|||||||
import { spawnProcess } from "../../../utils/spawn.js";
|
import { spawnProcess } from "../../../utils/spawn.js";
|
||||||
import { terminateWithTreeKill } from "../../../utils/tree-kill.js";
|
import { terminateWithTreeKill } from "../../../utils/tree-kill.js";
|
||||||
|
|
||||||
const DEFAULT_TIMEOUT_MS = 30_000;
|
/** Default wall-clock timeout for control-plane / short RPC calls. */
|
||||||
|
export const JSONL_RPC_DEFAULT_TIMEOUT_MS = 30_000;
|
||||||
|
/**
|
||||||
|
* Pass as `timeoutMs` to wait only for a response, process death, or `close()`.
|
||||||
|
* Use for long-running blocking RPCs (e.g. LLM-backed compact).
|
||||||
|
*/
|
||||||
|
export const JSONL_RPC_NO_TIMEOUT = null;
|
||||||
|
|
||||||
const STDERR_BUFFER_LIMIT = 8192;
|
const STDERR_BUFFER_LIMIT = 8192;
|
||||||
const GRACEFUL_SHUTDOWN_TIMEOUT_MS = 2_000;
|
const GRACEFUL_SHUTDOWN_TIMEOUT_MS = 2_000;
|
||||||
const FORCE_SHUTDOWN_TIMEOUT_MS = 1_000;
|
const FORCE_SHUTDOWN_TIMEOUT_MS = 1_000;
|
||||||
@@ -28,7 +35,7 @@ interface JsonlRpcResponse {
|
|||||||
interface PendingRequest {
|
interface PendingRequest {
|
||||||
resolve: (value: unknown) => void;
|
resolve: (value: unknown) => void;
|
||||||
reject: (error: Error) => void;
|
reject: (error: Error) => void;
|
||||||
timer: NodeJS.Timeout;
|
timer: NodeJS.Timeout | null;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface JsonlRpcExit {
|
export interface JsonlRpcExit {
|
||||||
@@ -116,7 +123,7 @@ export class JsonlRpcProcess {
|
|||||||
|
|
||||||
startRequest(
|
startRequest(
|
||||||
command: { type: string; [key: string]: unknown },
|
command: { type: string; [key: string]: unknown },
|
||||||
timeoutMs = DEFAULT_TIMEOUT_MS,
|
timeoutMs: number | null = JSONL_RPC_DEFAULT_TIMEOUT_MS,
|
||||||
): { id: string; promise: Promise<unknown> } {
|
): { id: string; promise: Promise<unknown> } {
|
||||||
if (this.disposed) {
|
if (this.disposed) {
|
||||||
return {
|
return {
|
||||||
@@ -127,14 +134,14 @@ export class JsonlRpcProcess {
|
|||||||
const id = `req_${this.nextRequestId}`;
|
const id = `req_${this.nextRequestId}`;
|
||||||
this.nextRequestId += 1;
|
this.nextRequestId += 1;
|
||||||
const promise = new Promise<unknown>((resolve, reject) => {
|
const promise = new Promise<unknown>((resolve, reject) => {
|
||||||
const timer = setTimeout(() => {
|
const timer = createRequestTimeout(timeoutMs, () => {
|
||||||
this.pending.delete(id);
|
this.pending.delete(id);
|
||||||
reject(
|
reject(
|
||||||
new Error(
|
new Error(
|
||||||
`${this.diagnosticName} request timed out for ${command.type}\n${this.stderrBuffer}`.trim(),
|
`${this.diagnosticName} request timed out for ${command.type}\n${this.stderrBuffer}`.trim(),
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
}, timeoutMs);
|
});
|
||||||
this.pending.set(id, { resolve, reject, timer });
|
this.pending.set(id, { resolve, reject, timer });
|
||||||
this.send({ ...command, id });
|
this.send({ ...command, id });
|
||||||
});
|
});
|
||||||
@@ -143,7 +150,7 @@ export class JsonlRpcProcess {
|
|||||||
|
|
||||||
request(
|
request(
|
||||||
command: { type: string; [key: string]: unknown },
|
command: { type: string; [key: string]: unknown },
|
||||||
timeoutMs = DEFAULT_TIMEOUT_MS,
|
timeoutMs: number | null = JSONL_RPC_DEFAULT_TIMEOUT_MS,
|
||||||
): Promise<unknown> {
|
): Promise<unknown> {
|
||||||
return this.startRequest(command, timeoutMs).promise;
|
return this.startRequest(command, timeoutMs).promise;
|
||||||
}
|
}
|
||||||
@@ -228,7 +235,9 @@ export class JsonlRpcProcess {
|
|||||||
if (!pending) {
|
if (!pending) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
clearTimeout(pending.timer);
|
if (pending.timer) {
|
||||||
|
clearTimeout(pending.timer);
|
||||||
|
}
|
||||||
this.pending.delete(response.id);
|
this.pending.delete(response.id);
|
||||||
if (!response.success) {
|
if (!response.success) {
|
||||||
pending.reject(
|
pending.reject(
|
||||||
@@ -247,9 +256,25 @@ export class JsonlRpcProcess {
|
|||||||
}
|
}
|
||||||
this.disposed = true;
|
this.disposed = true;
|
||||||
for (const pending of this.pending.values()) {
|
for (const pending of this.pending.values()) {
|
||||||
clearTimeout(pending.timer);
|
if (pending.timer) {
|
||||||
|
clearTimeout(pending.timer);
|
||||||
|
}
|
||||||
pending.reject(error);
|
pending.reject(error);
|
||||||
}
|
}
|
||||||
this.pending.clear();
|
this.pending.clear();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Schedule a request timeout, or return null when the call should wait
|
||||||
|
* indefinitely for a response, process exit, or close().
|
||||||
|
*/
|
||||||
|
function createRequestTimeout(
|
||||||
|
timeoutMs: number | null,
|
||||||
|
onTimeout: () => void,
|
||||||
|
): NodeJS.Timeout | null {
|
||||||
|
if (timeoutMs == null || !Number.isFinite(timeoutMs) || timeoutMs <= 0) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
return setTimeout(onTimeout, timeoutMs);
|
||||||
|
}
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ import type { ChildProcessWithoutNullStreams } from "node:child_process";
|
|||||||
import { EventEmitter } from "node:events";
|
import { EventEmitter } from "node:events";
|
||||||
import { PassThrough } from "node:stream";
|
import { PassThrough } from "node:stream";
|
||||||
import pino from "pino";
|
import pino from "pino";
|
||||||
import { describe, expect, test } from "vitest";
|
import { describe, expect, test, vi } from "vitest";
|
||||||
|
|
||||||
import { PiCliRuntime } from "./cli-runtime.js";
|
import { PiCliRuntime } from "./cli-runtime.js";
|
||||||
import type { PiRuntimeLaunch } from "./runtime.js";
|
import type { PiRuntimeLaunch } from "./runtime.js";
|
||||||
@@ -47,10 +47,7 @@ function createRuntime(
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
function replyToCommands(
|
function onPiCommand(child: PiChild, handler: (command: Record<string, unknown>) => void): void {
|
||||||
child: PiChild,
|
|
||||||
handler: (command: Record<string, unknown>) => unknown,
|
|
||||||
): void {
|
|
||||||
let buffer = "";
|
let buffer = "";
|
||||||
child.stdin.on("data", (chunk) => {
|
child.stdin.on("data", (chunk) => {
|
||||||
buffer += chunk.toString();
|
buffer += chunk.toString();
|
||||||
@@ -59,34 +56,68 @@ function replyToCommands(
|
|||||||
if (newlineIndex === -1) break;
|
if (newlineIndex === -1) break;
|
||||||
const line = buffer.slice(0, newlineIndex);
|
const line = buffer.slice(0, newlineIndex);
|
||||||
buffer = buffer.slice(newlineIndex + 1);
|
buffer = buffer.slice(newlineIndex + 1);
|
||||||
const command = JSON.parse(line) as Record<string, unknown>;
|
handler(JSON.parse(line) as Record<string, unknown>);
|
||||||
try {
|
|
||||||
const result = handler(command);
|
|
||||||
child.stdout.write(
|
|
||||||
`${JSON.stringify({
|
|
||||||
id: command.id,
|
|
||||||
type: "response",
|
|
||||||
command: command.type,
|
|
||||||
success: true,
|
|
||||||
data: result,
|
|
||||||
})}\n`,
|
|
||||||
);
|
|
||||||
} catch (error) {
|
|
||||||
const message = error instanceof Error ? error.message : String(error);
|
|
||||||
child.stdout.write(
|
|
||||||
`${JSON.stringify({
|
|
||||||
id: command.id,
|
|
||||||
type: "response",
|
|
||||||
command: command.type,
|
|
||||||
success: false,
|
|
||||||
error: message,
|
|
||||||
})}\n`,
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function replyToCommands(
|
||||||
|
child: PiChild,
|
||||||
|
handler: (command: Record<string, unknown>) => unknown,
|
||||||
|
): void {
|
||||||
|
onPiCommand(child, (command) => {
|
||||||
|
try {
|
||||||
|
const result = handler(command);
|
||||||
|
child.stdout.write(
|
||||||
|
`${JSON.stringify({
|
||||||
|
id: command.id,
|
||||||
|
type: "response",
|
||||||
|
command: command.type,
|
||||||
|
success: true,
|
||||||
|
data: result,
|
||||||
|
})}\n`,
|
||||||
|
);
|
||||||
|
} catch (error) {
|
||||||
|
const message = error instanceof Error ? error.message : String(error);
|
||||||
|
child.stdout.write(
|
||||||
|
`${JSON.stringify({
|
||||||
|
id: command.id,
|
||||||
|
type: "response",
|
||||||
|
command: command.type,
|
||||||
|
success: false,
|
||||||
|
error: message,
|
||||||
|
})}\n`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function capturePendingCommand(child: PiChild, type: string): Promise<Record<string, unknown>> {
|
||||||
|
return new Promise((resolve) => {
|
||||||
|
onPiCommand(child, (command) => {
|
||||||
|
if (command.type === type) {
|
||||||
|
resolve(command);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function writePiResponse(
|
||||||
|
child: PiChild,
|
||||||
|
command: Record<string, unknown>,
|
||||||
|
data: unknown = {},
|
||||||
|
): void {
|
||||||
|
child.stdout.write(
|
||||||
|
`${JSON.stringify({
|
||||||
|
id: command.id,
|
||||||
|
type: "response",
|
||||||
|
command: command.type,
|
||||||
|
success: true,
|
||||||
|
data,
|
||||||
|
})}\n`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
describe("PiCliRuntime", () => {
|
describe("PiCliRuntime", () => {
|
||||||
test("starts pi in rpc mode and resolves command responses", async () => {
|
test("starts pi in rpc mode and resolves command responses", async () => {
|
||||||
const child = createPiChild();
|
const child = createPiChild();
|
||||||
@@ -282,6 +313,51 @@ describe("PiCliRuntime", () => {
|
|||||||
await rejection;
|
await rejection;
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("compact waits beyond the default control-plane timeout for a late response", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
const child = createPiChild();
|
||||||
|
const pendingCompact = capturePendingCommand(child, "compact");
|
||||||
|
const session = await createRuntime(child).startSession({ cwd: "/workspace/project" });
|
||||||
|
|
||||||
|
try {
|
||||||
|
const compactPromise = session.compact("focus on tests");
|
||||||
|
const compactCommand = await pendingCompact;
|
||||||
|
await vi.advanceTimersByTimeAsync(35_000);
|
||||||
|
|
||||||
|
expect(compactCommand).toMatchObject({
|
||||||
|
type: "compact",
|
||||||
|
customInstructions: "focus on tests",
|
||||||
|
id: expect.any(String),
|
||||||
|
});
|
||||||
|
|
||||||
|
writePiResponse(child, compactCommand, {
|
||||||
|
summary: "done",
|
||||||
|
firstKeptEntryId: "entry-1",
|
||||||
|
tokensBefore: 120_000,
|
||||||
|
estimatedTokensAfter: 20_000,
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(compactPromise).resolves.toBeUndefined();
|
||||||
|
} finally {
|
||||||
|
vi.useRealTimers();
|
||||||
|
await session.close();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test("compact without a wall-clock timeout rejects when the session closes", async () => {
|
||||||
|
const child = createPiChild();
|
||||||
|
const pendingCompact = capturePendingCommand(child, "compact");
|
||||||
|
const session = await createRuntime(child).startSession({ cwd: "/workspace/project" });
|
||||||
|
|
||||||
|
const compactPromise = session.compact();
|
||||||
|
await pendingCompact;
|
||||||
|
|
||||||
|
const rejection = expect(compactPromise).rejects.toThrow("Pi RPC session is closed");
|
||||||
|
await session.close();
|
||||||
|
|
||||||
|
await rejection;
|
||||||
|
});
|
||||||
|
|
||||||
test("disposes the Pi process", async () => {
|
test("disposes the Pi process", async () => {
|
||||||
const child = createPiChild();
|
const child = createPiChild();
|
||||||
const session = await createRuntime(child).startSession({ cwd: "/workspace/project" });
|
const session = await createRuntime(child).startSession({ cwd: "/workspace/project" });
|
||||||
|
|||||||
@@ -2,7 +2,11 @@ import type { ChildProcessWithoutNullStreams } from "node:child_process";
|
|||||||
import type { Logger } from "pino";
|
import type { Logger } from "pino";
|
||||||
|
|
||||||
import type { ProviderRuntimeSettings } from "../../provider-launch-config.js";
|
import type { ProviderRuntimeSettings } from "../../provider-launch-config.js";
|
||||||
import { JsonlRpcProcess, type JsonlRpcLaunch } from "../jsonl-rpc-process.js";
|
import {
|
||||||
|
JSONL_RPC_NO_TIMEOUT,
|
||||||
|
JsonlRpcProcess,
|
||||||
|
type JsonlRpcLaunch,
|
||||||
|
} from "../jsonl-rpc-process.js";
|
||||||
import {
|
import {
|
||||||
buildPiLaunch,
|
buildPiLaunch,
|
||||||
type PiRuntime,
|
type PiRuntime,
|
||||||
@@ -26,6 +30,14 @@ const DEFAULT_PI_COMMAND: [string, ...string[]] = [
|
|||||||
];
|
];
|
||||||
const DEFAULT_COMMANDS_RPC_NAME = "get_commands";
|
const DEFAULT_COMMANDS_RPC_NAME = "get_commands";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Pi RPC timeout policy:
|
||||||
|
* - Control-plane / accept-and-stream (`prompt`, `get_state`, `abort`, …): default 30s
|
||||||
|
* - Long-running blocking LLM jobs (`compact`): no wall-clock timeout — complete on
|
||||||
|
* response, process death, or session close (`JsonlRpcProcess.failAll` / `close`).
|
||||||
|
*/
|
||||||
|
const PI_COMPACT_REQUEST_TIMEOUT_MS = JSONL_RPC_NO_TIMEOUT;
|
||||||
|
|
||||||
export interface PiCliRuntimeOptions {
|
export interface PiCliRuntimeOptions {
|
||||||
logger: Logger;
|
logger: Logger;
|
||||||
runtimeSettings?: ProviderRuntimeSettings;
|
runtimeSettings?: ProviderRuntimeSettings;
|
||||||
@@ -112,10 +124,16 @@ class PiCliRuntimeSession implements PiRuntimeSession {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async compact(customInstructions?: string): Promise<void> {
|
async compact(customInstructions?: string): Promise<void> {
|
||||||
await this.request({
|
// Compact is a blocking LLM summarization job; Pi only returns the RPC
|
||||||
type: "compact",
|
// response after the summary is written. A control-plane 30s timeout falsely
|
||||||
...(customInstructions ? { customInstructions } : {}),
|
// fails long sessions while the real compact continues (issue #1946).
|
||||||
});
|
await this.request(
|
||||||
|
{
|
||||||
|
type: "compact",
|
||||||
|
...(customInstructions ? { customInstructions } : {}),
|
||||||
|
},
|
||||||
|
PI_COMPACT_REQUEST_TIMEOUT_MS,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
async setAutoCompaction(enabled: boolean): Promise<void> {
|
async setAutoCompaction(enabled: boolean): Promise<void> {
|
||||||
@@ -135,7 +153,7 @@ class PiCliRuntimeSession implements PiRuntimeSession {
|
|||||||
return data.messages ?? [];
|
return data.messages ?? [];
|
||||||
}
|
}
|
||||||
|
|
||||||
async getAvailableModels(timeoutMs?: number): Promise<PiModel[]> {
|
async getAvailableModels(timeoutMs?: number | null): Promise<PiModel[]> {
|
||||||
const data = (await this.request({ type: "get_available_models" }, timeoutMs)) as {
|
const data = (await this.request({ type: "get_available_models" }, timeoutMs)) as {
|
||||||
models?: PiModel[];
|
models?: PiModel[];
|
||||||
};
|
};
|
||||||
@@ -208,7 +226,7 @@ class PiCliRuntimeSession implements PiRuntimeSession {
|
|||||||
await this.process.close(new Error("Pi RPC session is closed"));
|
await this.process.close(new Error("Pi RPC session is closed"));
|
||||||
}
|
}
|
||||||
|
|
||||||
request(command: PiRpcCommand, timeoutMs?: number): Promise<unknown> {
|
request(command: PiRpcCommand, timeoutMs?: number | null): Promise<unknown> {
|
||||||
return this.process.request(command, timeoutMs);
|
return this.process.request(command, timeoutMs);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -51,12 +51,15 @@ export interface PiRuntimeSession {
|
|||||||
abort(): Promise<void>;
|
abort(): Promise<void>;
|
||||||
getState(): Promise<PiSessionState>;
|
getState(): Promise<PiSessionState>;
|
||||||
getMessages(): Promise<PiAgentMessage[]>;
|
getMessages(): Promise<PiAgentMessage[]>;
|
||||||
getAvailableModels(timeoutMs?: number): Promise<PiModel[]>;
|
getAvailableModels(timeoutMs?: number | null): Promise<PiModel[]>;
|
||||||
setModel(provider: string, modelId: string): Promise<PiModel>;
|
setModel(provider: string, modelId: string): Promise<PiModel>;
|
||||||
setThinkingLevel(level: string): Promise<void>;
|
setThinkingLevel(level: string): Promise<void>;
|
||||||
getSessionStats(): Promise<PiSessionStats>;
|
getSessionStats(): Promise<PiSessionStats>;
|
||||||
getCommands(): Promise<PiRpcSlashCommand[]>;
|
getCommands(): Promise<PiRpcSlashCommand[]>;
|
||||||
request(command: { type: string; [key: string]: unknown }, timeoutMs?: number): Promise<unknown>;
|
request(
|
||||||
|
command: { type: string; [key: string]: unknown },
|
||||||
|
timeoutMs?: number | null,
|
||||||
|
): Promise<unknown>;
|
||||||
sendRawFrame(frame: object & { type: string }): void;
|
sendRawFrame(frame: object & { type: string }): void;
|
||||||
respondToExtensionUiRequest(
|
respondToExtensionUiRequest(
|
||||||
id: string,
|
id: string,
|
||||||
|
|||||||
@@ -227,7 +227,7 @@ export class FakePiSession implements PiRuntimeSession {
|
|||||||
return this.messages;
|
return this.messages;
|
||||||
}
|
}
|
||||||
|
|
||||||
async getAvailableModels(_timeoutMs?: number): Promise<PiModel[]> {
|
async getAvailableModels(_timeoutMs?: number | null): Promise<PiModel[]> {
|
||||||
return this.models;
|
return this.models;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user