diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index ca2df6c1e..56db2ec15 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -43,6 +43,7 @@ import { import { GenericACPAgentClient } from "./generic-acp-agent.js"; import { transformPiModels } from "./pi/agent.js"; import type { AgentStreamEvent } from "../agent-sdk-types.js"; +import type { AgentCapabilityFlags, AgentPersistenceHandle } from "../agent-sdk-types.js"; import { createTestLogger } from "../../../test-utils/test-logger.js"; import { asInternals } from "../../test-utils/class-mocks.js"; import * as spawnUtils from "../../../utils/spawn.js"; @@ -2457,3 +2458,117 @@ describe("ACPAgentClient probe cleanup", () => { expect(child.stderr.destroyed).toBe(true); }); }); + +describe("ACP session/load invariant — cwd and mcpServers always passed", () => { + /** + * Shared factory: creates an ACPAgentSession subclass whose spawnProcess + * returns stubbed ACP internals so tests can inspect connection method calls + * without spawning real processes. Each call produces fresh vi.fn() stubs. + */ + function makeTestSession(args: { + capabilities?: AgentCapabilityFlags; + handle: AgentPersistenceHandle; + loadSession?: ReturnType; + unstableResumeSession?: ReturnType; + }) { + const loadSession = + args.loadSession ?? + vi.fn().mockResolvedValue({ + sessionId: "session-1", + modes: null, + models: null, + configOptions: [], + }); + const unstableResumeSession = + args.unstableResumeSession ?? + vi.fn().mockResolvedValue({ + sessionId: "session-1", + modes: null, + models: null, + configOptions: [], + }); + + class TestSession extends ACPAgentSession { + protected override async spawnProcess(): Promise { + return { + child: createProbeChildStub(), + connection: { + prompt: vi.fn(), + loadSession, + unstable_resumeSession: unstableResumeSession, + } as unknown as ClientSideConnection, + initialize: { agentCapabilities: args.capabilities ?? {} }, + } as SpawnedACPProcess; + } + } + + // Pass handle through the typed constructor option (no private-field casts). + const session = new TestSession( + { provider: "claude-acp", cwd: "/tmp/paseo-acp-test" }, + { + provider: "claude-acp", + logger: createTestLogger(), + defaultCommand: ["claude", "--acp"], + defaultModes: [], + capabilities: { + supportsStreaming: true, + supportsSessionPersistence: true, + supportsDynamicModes: true, + supportsMcpServers: true, + supportsReasoningStream: true, + supportsToolInvocations: true, + ...args.capabilities, + }, + handle: args.handle, + }, + ); + + return { session, loadSession, unstableResumeSession }; + } + + test("loadSession is always called with sessionId, cwd, and mcpServers even when mcpServers is empty", async () => { + const { session, loadSession } = makeTestSession({ + capabilities: { loadSession: true, supportsMcpServers: true }, + handle: { sessionId: "session-1", provider: "claude-acp" }, + }); + + await session.initializeResumedSession(); + + expect(loadSession).toHaveBeenCalledWith({ + sessionId: "session-1", + cwd: "/tmp/paseo-acp-test", + mcpServers: [], + }); + }); + + test("loadSession is always called with mcpServers even when supportsMcpServers is false", async () => { + const { session, loadSession } = makeTestSession({ + capabilities: { loadSession: true, supportsMcpServers: false }, + handle: { sessionId: "session-1", provider: "claude-acp" }, + }); + + await session.initializeResumedSession(); + + // Even with supportsMcpServers=false, mcpServers: [] must still be passed + expect(loadSession).toHaveBeenCalledWith({ + sessionId: "session-1", + cwd: "/tmp/paseo-acp-test", + mcpServers: [], + }); + }); + + test("unstable_resumeSession is always called with sessionId, cwd, and mcpServers", async () => { + const { session, unstableResumeSession } = makeTestSession({ + capabilities: { sessionCapabilities: { resume: {} } }, + handle: { sessionId: "session-1", provider: "claude-acp" }, + }); + + await session.initializeResumedSession(); + + expect(unstableResumeSession).toHaveBeenCalledWith({ + sessionId: "session-1", + cwd: "/tmp/paseo-acp-test", + mcpServers: [], + }); + }); +}); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 00016a61a..d2fe5aadb 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -1333,6 +1333,13 @@ export class ACPAgentSession implements AgentSession, ACPClient { await this.applyConfiguredOverrides(); } + /** + * IMPORTANT: Some ACP providers (e.g., Devin CLI) require all three params + * (sessionId, cwd, mcpServers) to be present in session/load or + * unstable_resumeSession — even when mcpServers is an empty array — and + * return "Invalid params" if any are omitted. Never drop cwd or mcpServers + * from these calls regardless of capabilities. + */ async initializeResumedSession(): Promise { const handle = this.initialHandle; if (!handle) { diff --git a/packages/server/src/server/agent/providers/pi/agent.ts b/packages/server/src/server/agent/providers/pi/agent.ts index cd4fbddc8..affcac79d 100644 --- a/packages/server/src/server/agent/providers/pi/agent.ts +++ b/packages/server/src/server/agent/providers/pi/agent.ts @@ -89,7 +89,7 @@ const PASEO_PI_TREE_EXTENSION_COMMAND = "paseo_tree"; const PASEO_PI_CAPTURE_EXTENSION_COMMAND = "paseo_capture_entries"; const PASEO_PI_ENTRY_CAPTURE_MARKER = "PASEO_ENTRY_CAPTURE"; const PASEO_PI_COMMAND_RESULT_MARKER = "PASEO_COMMAND_RESULT"; -const PASEO_PI_EXTENSION_RESULT_TIMEOUT_MS = 10_000; +const DEFAULT_PI_EXTENSION_RESULT_TIMEOUT_MS = 30_000; const QUESTION_RESPONSE_HEADER = "Response"; const QUESTION_COMMENT_HEADER = "Comment"; const PI_ASK_USER_FREEFORM_SENTINEL = "✏️ Type custom response..."; @@ -98,6 +98,7 @@ const COMBINED_ASK_USER_METADATA = "ask_user_select_optional_comment"; export const PiProviderParamsSchema = z .object({ sessionDir: z.string().min(1).optional(), + extensionTimeoutMs: z.number().int().positive().default(DEFAULT_PI_EXTENSION_RESULT_TIMEOUT_MS), }) .strict(); @@ -187,6 +188,7 @@ interface PiRpcAgentSessionOptions { initialState: PiSessionState; capabilities: AgentCapabilityFlags; cleanup?: () => void; + extensionTimeoutMs?: number; } interface PiResumeConfig { @@ -1011,6 +1013,7 @@ export class PiRpcAgentSession implements AgentSession { normalizePiThinkingOption(options.config.thinkingOptionId) ?? this.state.thinkingLevel ?? null; + this.extensionTimeoutMs = options.extensionTimeoutMs ?? DEFAULT_PI_EXTENSION_RESULT_TIMEOUT_MS; this.runtimeSession.onEvent((event) => { this.handleRuntimeEvent(event); @@ -1020,6 +1023,7 @@ export class PiRpcAgentSession implements AgentSession { private readonly runtimeSession: PiRuntimeSession; private readonly config: AgentSessionConfig; private readonly cleanup?: () => void; + private readonly extensionTimeoutMs: number; get id(): string | null { return this.state.sessionId; @@ -1419,7 +1423,7 @@ export class PiRpcAgentSession implements AgentSession { const timer = setTimeout(() => { this.pendingExtensionResults.delete(requestId); reject(new Error(`Pi extension result timed out for request ${requestId}`)); - }, PASEO_PI_EXTENSION_RESULT_TIMEOUT_MS); + }, this.extensionTimeoutMs); this.pendingExtensionResults.set(requestId, { resolve, reject, timer }); }); } @@ -1911,6 +1915,7 @@ export class PiRpcAgentClient implements AgentClient { initialState: await runtimeSession.getState(), capabilities: withPiMcpCapability(mcpConfig !== null), cleanup: combineCleanup([mcpConfig?.cleanup, paseoExtension.cleanup]), + extensionTimeoutMs: this.providerParams.extensionTimeoutMs, }); } catch (error) { await runtimeSession.close().catch(() => undefined); @@ -1961,6 +1966,7 @@ export class PiRpcAgentClient implements AgentClient { initialState: await runtimeSession.getState(), capabilities: withPiMcpCapability(mcpConfig !== null), cleanup: combineCleanup([mcpConfig?.cleanup, paseoExtension.cleanup]), + extensionTimeoutMs: this.providerParams.extensionTimeoutMs, }); } catch (error) { await runtimeSession.close().catch(() => undefined);