From 9031ce7b51323f201ac3454aad95dda88e8f047e Mon Sep 17 00:00:00 2001 From: Shoubhit Dash Date: Thu, 28 May 2026 15:10:17 +0530 Subject: [PATCH] fix(acp): speed up acp-next warm switches (#29713) --- packages/opencode/src/acp-next/service.ts | 20 ++- .../test/acp-next/service-session.test.ts | 164 ++++++++++++++++-- .../cli/acp-next/timing-diagnostics.test.ts | 16 +- 3 files changed, 175 insertions(+), 25 deletions(-) diff --git a/packages/opencode/src/acp-next/service.ts b/packages/opencode/src/acp-next/service.ts index 0e0d0f8352..f4614a5c8d 100644 --- a/packages/opencode/src/acp-next/service.ts +++ b/packages/opencode/src/acp-next/service.ts @@ -83,6 +83,7 @@ export function make(input: { const session = input.session ?? makeSessionService() const directoryService = input.directory ?? makeDirectoryService(input.sdk) const registeredMcp = new Map>() + const sessionSnapshots = new Map() const events = input.connection ? ACPNextEvent.start({ sdk: input.sdk, connection: input.connection, session }) : undefined @@ -149,6 +150,14 @@ export function make(input: { return snapshot }) + const configSnapshot = Effect.fn("ACPNext.configSnapshot")(function* (state: ACPNextSession.Info) { + const snapshot = sessionSnapshots.get(state.id) + if (snapshot) return snapshot + const loaded = yield* directorySnapshot(state.cwd) + sessionSnapshots.set(state.id, loaded) + return loaded + }) + const newSession = Effect.fn("ACPNext.newSession")(function* (params: NewSessionRequest) { const started = performance.now() const snapshot = yield* directorySnapshot(params.cwd) @@ -180,6 +189,7 @@ export function make(input: { variant, modeId, }) + sessionSnapshots.set(state.id, snapshot) yield* registerMcpServers(input.sdk, registeredMcp, params.cwd, state.id, params.mcpServers) yield* sendAvailableCommands(input.connection, state.id, snapshot) @@ -220,6 +230,7 @@ export function make(input: { variant: restored.variant ?? selectVariant(snapshot, model), modeId: restored.modeId ?? (snapshot.availableModes.length > 0 ? snapshot.defaultModeID : undefined), }) + sessionSnapshots.set(state.id, snapshot) yield* registerMcpServers(input.sdk, registeredMcp, params.cwd, state.id, params.mcpServers) yield* sendAvailableCommands(input.connection, state.id, snapshot) @@ -290,6 +301,7 @@ export function make(input: { variant: restored.variant ?? selectVariant(snapshot, model), modeId: restored.modeId ?? (snapshot.availableModes.length > 0 ? snapshot.defaultModeID : undefined), }) + sessionSnapshots.set(state.id, snapshot) yield* registerMcpServers(input.sdk, registeredMcp, params.cwd, state.id, params.mcpServers ?? []) yield* sendAvailableCommands(input.connection, state.id, snapshot) @@ -307,6 +319,7 @@ export function make(input: { const closeSession = Effect.fn("ACPNext.closeSession")(function* (params: CloseSessionRequest) { const removed = yield* session.remove(params.sessionId) registeredMcp.delete(params.sessionId) + sessionSnapshots.delete(params.sessionId) if (!removed) return {} yield* request( @@ -350,6 +363,7 @@ export function make(input: { variant: restored.variant ?? selectVariant(snapshot, model), modeId: restored.modeId ?? (snapshot.availableModes.length > 0 ? snapshot.defaultModeID : undefined), }) + sessionSnapshots.set(state.id, snapshot) yield* registerMcpServers(input.sdk, registeredMcp, params.cwd, state.id, params.mcpServers ?? []) yield* sendAvailableCommands(input.connection, state.id, snapshot) @@ -369,7 +383,7 @@ export function make(input: { params: SetSessionConfigOptionRequest, ) { const current = yield* session.get(params.sessionId) - const snapshot = yield* directorySnapshot(current.cwd) + const snapshot = yield* configSnapshot(current) if (typeof params.value !== "string") { return yield* new ACPNextError.InvalidConfigOptionError({ configId: params.configId }) } @@ -424,7 +438,7 @@ export function make(input: { const setSessionMode = Effect.fn("ACPNext.setSessionMode")(function* (params: SetSessionModeRequest) { const current = yield* session.get(params.sessionId) - const snapshot = yield* directorySnapshot(current.cwd) + const snapshot = yield* configSnapshot(current) if (!snapshot.availableModes.some((mode) => mode.id === params.modeId)) { return yield* new ACPNextError.InvalidModeError({ mode: params.modeId }) } @@ -434,7 +448,7 @@ export function make(input: { const setSessionModel = Effect.fn("ACPNext.setSessionModel")(function* (params: SetSessionModelRequest) { const current = yield* session.get(params.sessionId) - const snapshot = yield* directorySnapshot(current.cwd) + const snapshot = yield* configSnapshot(current) const selected = yield* parseSelectedModel(snapshot, params.modelId) yield* session .setVariant( diff --git a/packages/opencode/test/acp-next/service-session.test.ts b/packages/opencode/test/acp-next/service-session.test.ts index 2ea31a96c8..8ad243b047 100644 --- a/packages/opencode/test/acp-next/service-session.test.ts +++ b/packages/opencode/test/acp-next/service-session.test.ts @@ -707,24 +707,35 @@ describe("ACP next service sessions", () => { expect(results.map((error) => error.code)).toEqual([-32602, -32602, -32602, -32602]) }) - it("does not reload providers or commands when switching effort from a warm snapshot", async () => { - let providersCalls = 0 - let commandCalls = 0 + it("does not refetch providers modes or commands when switching effort from session snapshot", async () => { + const calls = { + providers: 0, + agents: 0, + commands: 0, + skills: 0, + mcpAdds: 0, + } const sdk = { config: { providers: () => { - providersCalls++ + calls.providers++ return Promise.resolve({ data: { providers: [provider], default: { test: modelID } } }) }, get: () => Promise.resolve({ data: {} }), }, app: { - agents: () => Promise.resolve({ data: [{ name: "build", mode: "primary", permission: [], options: {} }] }), - skills: () => Promise.resolve({ data: [] }), + agents: () => { + calls.agents++ + return Promise.resolve({ data: [{ name: "build", mode: "primary", permission: [], options: {} }] }) + }, + skills: () => { + calls.skills++ + return Promise.resolve({ data: [] }) + }, }, command: { list: () => { - commandCalls++ + calls.commands++ return Promise.resolve({ data: [] }) }, }, @@ -733,14 +744,16 @@ describe("ACP next service sessions", () => { list: () => Promise.resolve({ data: [] }), }, mcp: { - add: () => Promise.resolve({ data: {} }), + add: () => { + calls.mcpAdds++ + return Promise.resolve({ data: {} }) + }, }, } as unknown as OpencodeClient const service = ACPNextService.make({ sdk }) const session = await Effect.runPromise(service.newSession({ cwd: "/workspace", mcpServers: [] })) - expect(providersCalls).toBe(1) - expect(commandCalls).toBe(1) + expect(calls).toEqual({ providers: 1, agents: 1, commands: 1, skills: 1, mcpAdds: 0 }) await Effect.runPromise( service.setSessionConfigOption({ @@ -750,8 +763,135 @@ describe("ACP next service sessions", () => { }), ) - expect(providersCalls).toBe(1) - expect(commandCalls).toBe(1) + expect(calls).toEqual({ providers: 1, agents: 1, commands: 1, skills: 1, mcpAdds: 0 }) + }) + + it("switches model against the warm provider snapshot without refetching", async () => { + const calls = { + providers: 0, + agents: 0, + commands: 0, + skills: 0, + } + const sdk = { + config: { + providers: () => { + calls.providers++ + return Promise.resolve({ data: { providers: [provider], default: { test: modelID } } }) + }, + get: () => Promise.resolve({ data: {} }), + }, + app: { + agents: () => { + calls.agents++ + return Promise.resolve({ data: [{ name: "build", mode: "primary", permission: [], options: {} }] }) + }, + skills: () => { + calls.skills++ + return Promise.resolve({ data: [] }) + }, + }, + command: { + list: () => { + calls.commands++ + return Promise.resolve({ data: [] }) + }, + }, + session: { + create: () => Promise.resolve({ data: { id: "ses_model_fast" } }), + list: () => Promise.resolve({ data: [] }), + }, + mcp: { + add: () => Promise.resolve({ data: {} }), + }, + } as unknown as OpencodeClient + const service = ACPNextService.make({ sdk }) + const session = await Effect.runPromise(service.newSession({ cwd: "/workspace", mcpServers: [] })) + const updated = await Effect.runPromise( + service.setSessionConfigOption({ + sessionId: session.sessionId, + configId: "model", + value: "test/second-model", + }), + ) + + expect(select(updated, "model")?.currentValue).toBe("test/second-model") + expect(calls).toEqual({ providers: 1, agents: 1, commands: 1, skills: 1 }) + }) + + it("reuses the warm directory snapshot for a second new session in the same cwd", async () => { + const calls = { + providers: 0, + config: 0, + agents: 0, + commands: 0, + skills: 0, + sessionList: 0, + messages: 0, + creates: 0, + } + const sdk = { + config: { + providers: () => { + calls.providers++ + return Promise.resolve({ data: { providers: [provider], default: { test: modelID } } }) + }, + get: () => { + calls.config++ + return Promise.resolve({ data: {} }) + }, + }, + app: { + agents: () => { + calls.agents++ + return Promise.resolve({ data: [{ name: "build", mode: "primary", permission: [], options: {} }] }) + }, + skills: () => { + calls.skills++ + return Promise.resolve({ data: [] }) + }, + }, + command: { + list: () => { + calls.commands++ + return Promise.resolve({ data: [] }) + }, + }, + session: { + create: () => { + calls.creates++ + return Promise.resolve({ data: { id: `ses_warm_${calls.creates}` } }) + }, + list: () => { + calls.sessionList++ + return Promise.resolve({ data: [] }) + }, + messages: () => { + calls.messages++ + return Promise.resolve({ data: [] }) + }, + }, + mcp: { + add: () => Promise.resolve({ data: {} }), + }, + } as unknown as OpencodeClient + const service = ACPNextService.make({ sdk }) + + const first = await Effect.runPromise(service.newSession({ cwd: "/workspace", mcpServers: [] })) + const second = await Effect.runPromise(service.newSession({ cwd: "/workspace", mcpServers: [] })) + + expect(first.sessionId).toBe("ses_warm_1") + expect(second.sessionId).toBe("ses_warm_2") + expect(calls).toEqual({ + providers: 1, + config: 1, + agents: 1, + commands: 1, + skills: 1, + sessionList: 0, + messages: 0, + creates: 2, + }) }) it("normal text prompt sends model variant mode and converted parts", async () => { diff --git a/packages/opencode/test/cli/acp-next/timing-diagnostics.test.ts b/packages/opencode/test/cli/acp-next/timing-diagnostics.test.ts index c70a82d6b5..610c286ac0 100644 --- a/packages/opencode/test/cli/acp-next/timing-diagnostics.test.ts +++ b/packages/opencode/test/cli/acp-next/timing-diagnostics.test.ts @@ -42,7 +42,7 @@ describe("opencode acp-next verifier timing diagnostics", () => { ) cliIt.live( - "warm new session timing diagnostic stays below generous threshold", + "warm new session stays below verifier threshold", ({ home, llm, opencode }) => Effect.gen(function* () { const acp = yield* createAcpNextClient( @@ -57,15 +57,13 @@ describe("opencode acp-next verifier timing diagnostics", () => { const durationMs = Math.round(performance.now() - started) expect(session.sessionId).toBeTruthy() - // TODO: replace this diagnostic assertion with finalFastPathThresholdMs. - expect(durationMs).toBeLessThan(diagnosticFastPathThresholdMs) - expect(finalFastPathThresholdMs).toBe(100) + expect(durationMs).toBeLessThan(finalFastPathThresholdMs) }), 60_000, ) cliIt.live( - "model switch timing diagnostic updates currentValue below generous threshold", + "model switch updates currentValue below verifier threshold", ({ home, llm, opencode }) => Effect.gen(function* () { const acp = yield* createAcpNextClient( @@ -89,14 +87,13 @@ describe("opencode acp-next verifier timing diagnostics", () => { const durationMs = Math.round(performance.now() - started) expect(expectSelectOption(updated.configOptions, "model").currentValue).toBe(nextModel) - // TODO: replace this diagnostic assertion with finalFastPathThresholdMs. - expect(durationMs).toBeLessThan(diagnosticFastPathThresholdMs) + expect(durationMs).toBeLessThan(finalFastPathThresholdMs) }), 60_000, ) cliIt.live( - "effort switch timing diagnostic updates currentValue below generous threshold", + "effort switch updates currentValue below verifier threshold", ({ home, llm, opencode }) => Effect.gen(function* () { const acp = yield* createAcpNextClient( @@ -118,8 +115,7 @@ describe("opencode acp-next verifier timing diagnostics", () => { const durationMs = Math.round(performance.now() - started) expect(expectSelectOption(updated.configOptions, "effort").currentValue).toBe(nextEffort) - // TODO: replace this diagnostic assertion with finalFastPathThresholdMs. - expect(durationMs).toBeLessThan(diagnosticFastPathThresholdMs) + expect(durationMs).toBeLessThan(finalFastPathThresholdMs) }), 60_000, )