fix(acp): speed up acp-next warm switches (#29713)
This commit is contained in:
@@ -83,6 +83,7 @@ export function make(input: {
|
|||||||
const session = input.session ?? makeSessionService()
|
const session = input.session ?? makeSessionService()
|
||||||
const directoryService = input.directory ?? makeDirectoryService(input.sdk)
|
const directoryService = input.directory ?? makeDirectoryService(input.sdk)
|
||||||
const registeredMcp = new Map<string, Set<string>>()
|
const registeredMcp = new Map<string, Set<string>>()
|
||||||
|
const sessionSnapshots = new Map<string, Directory.Snapshot>()
|
||||||
const events = input.connection
|
const events = input.connection
|
||||||
? ACPNextEvent.start({ sdk: input.sdk, connection: input.connection, session })
|
? ACPNextEvent.start({ sdk: input.sdk, connection: input.connection, session })
|
||||||
: undefined
|
: undefined
|
||||||
@@ -149,6 +150,14 @@ export function make(input: {
|
|||||||
return snapshot
|
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 newSession = Effect.fn("ACPNext.newSession")(function* (params: NewSessionRequest) {
|
||||||
const started = performance.now()
|
const started = performance.now()
|
||||||
const snapshot = yield* directorySnapshot(params.cwd)
|
const snapshot = yield* directorySnapshot(params.cwd)
|
||||||
@@ -180,6 +189,7 @@ export function make(input: {
|
|||||||
variant,
|
variant,
|
||||||
modeId,
|
modeId,
|
||||||
})
|
})
|
||||||
|
sessionSnapshots.set(state.id, snapshot)
|
||||||
|
|
||||||
yield* registerMcpServers(input.sdk, registeredMcp, params.cwd, state.id, params.mcpServers)
|
yield* registerMcpServers(input.sdk, registeredMcp, params.cwd, state.id, params.mcpServers)
|
||||||
yield* sendAvailableCommands(input.connection, state.id, snapshot)
|
yield* sendAvailableCommands(input.connection, state.id, snapshot)
|
||||||
@@ -220,6 +230,7 @@ export function make(input: {
|
|||||||
variant: restored.variant ?? selectVariant(snapshot, model),
|
variant: restored.variant ?? selectVariant(snapshot, model),
|
||||||
modeId: restored.modeId ?? (snapshot.availableModes.length > 0 ? snapshot.defaultModeID : undefined),
|
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* registerMcpServers(input.sdk, registeredMcp, params.cwd, state.id, params.mcpServers)
|
||||||
yield* sendAvailableCommands(input.connection, state.id, snapshot)
|
yield* sendAvailableCommands(input.connection, state.id, snapshot)
|
||||||
@@ -290,6 +301,7 @@ export function make(input: {
|
|||||||
variant: restored.variant ?? selectVariant(snapshot, model),
|
variant: restored.variant ?? selectVariant(snapshot, model),
|
||||||
modeId: restored.modeId ?? (snapshot.availableModes.length > 0 ? snapshot.defaultModeID : undefined),
|
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* registerMcpServers(input.sdk, registeredMcp, params.cwd, state.id, params.mcpServers ?? [])
|
||||||
yield* sendAvailableCommands(input.connection, state.id, snapshot)
|
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 closeSession = Effect.fn("ACPNext.closeSession")(function* (params: CloseSessionRequest) {
|
||||||
const removed = yield* session.remove(params.sessionId)
|
const removed = yield* session.remove(params.sessionId)
|
||||||
registeredMcp.delete(params.sessionId)
|
registeredMcp.delete(params.sessionId)
|
||||||
|
sessionSnapshots.delete(params.sessionId)
|
||||||
if (!removed) return {}
|
if (!removed) return {}
|
||||||
|
|
||||||
yield* request(
|
yield* request(
|
||||||
@@ -350,6 +363,7 @@ export function make(input: {
|
|||||||
variant: restored.variant ?? selectVariant(snapshot, model),
|
variant: restored.variant ?? selectVariant(snapshot, model),
|
||||||
modeId: restored.modeId ?? (snapshot.availableModes.length > 0 ? snapshot.defaultModeID : undefined),
|
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* registerMcpServers(input.sdk, registeredMcp, params.cwd, state.id, params.mcpServers ?? [])
|
||||||
yield* sendAvailableCommands(input.connection, state.id, snapshot)
|
yield* sendAvailableCommands(input.connection, state.id, snapshot)
|
||||||
@@ -369,7 +383,7 @@ export function make(input: {
|
|||||||
params: SetSessionConfigOptionRequest,
|
params: SetSessionConfigOptionRequest,
|
||||||
) {
|
) {
|
||||||
const current = yield* session.get(params.sessionId)
|
const current = yield* session.get(params.sessionId)
|
||||||
const snapshot = yield* directorySnapshot(current.cwd)
|
const snapshot = yield* configSnapshot(current)
|
||||||
if (typeof params.value !== "string") {
|
if (typeof params.value !== "string") {
|
||||||
return yield* new ACPNextError.InvalidConfigOptionError({ configId: params.configId })
|
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 setSessionMode = Effect.fn("ACPNext.setSessionMode")(function* (params: SetSessionModeRequest) {
|
||||||
const current = yield* session.get(params.sessionId)
|
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)) {
|
if (!snapshot.availableModes.some((mode) => mode.id === params.modeId)) {
|
||||||
return yield* new ACPNextError.InvalidModeError({ mode: 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 setSessionModel = Effect.fn("ACPNext.setSessionModel")(function* (params: SetSessionModelRequest) {
|
||||||
const current = yield* session.get(params.sessionId)
|
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)
|
const selected = yield* parseSelectedModel(snapshot, params.modelId)
|
||||||
yield* session
|
yield* session
|
||||||
.setVariant(
|
.setVariant(
|
||||||
|
|||||||
@@ -707,24 +707,35 @@ describe("ACP next service sessions", () => {
|
|||||||
expect(results.map((error) => error.code)).toEqual([-32602, -32602, -32602, -32602])
|
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 () => {
|
it("does not refetch providers modes or commands when switching effort from session snapshot", async () => {
|
||||||
let providersCalls = 0
|
const calls = {
|
||||||
let commandCalls = 0
|
providers: 0,
|
||||||
|
agents: 0,
|
||||||
|
commands: 0,
|
||||||
|
skills: 0,
|
||||||
|
mcpAdds: 0,
|
||||||
|
}
|
||||||
const sdk = {
|
const sdk = {
|
||||||
config: {
|
config: {
|
||||||
providers: () => {
|
providers: () => {
|
||||||
providersCalls++
|
calls.providers++
|
||||||
return Promise.resolve({ data: { providers: [provider], default: { test: modelID } } })
|
return Promise.resolve({ data: { providers: [provider], default: { test: modelID } } })
|
||||||
},
|
},
|
||||||
get: () => Promise.resolve({ data: {} }),
|
get: () => Promise.resolve({ data: {} }),
|
||||||
},
|
},
|
||||||
app: {
|
app: {
|
||||||
agents: () => Promise.resolve({ data: [{ name: "build", mode: "primary", permission: [], options: {} }] }),
|
agents: () => {
|
||||||
skills: () => Promise.resolve({ data: [] }),
|
calls.agents++
|
||||||
|
return Promise.resolve({ data: [{ name: "build", mode: "primary", permission: [], options: {} }] })
|
||||||
|
},
|
||||||
|
skills: () => {
|
||||||
|
calls.skills++
|
||||||
|
return Promise.resolve({ data: [] })
|
||||||
|
},
|
||||||
},
|
},
|
||||||
command: {
|
command: {
|
||||||
list: () => {
|
list: () => {
|
||||||
commandCalls++
|
calls.commands++
|
||||||
return Promise.resolve({ data: [] })
|
return Promise.resolve({ data: [] })
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@@ -733,14 +744,16 @@ describe("ACP next service sessions", () => {
|
|||||||
list: () => Promise.resolve({ data: [] }),
|
list: () => Promise.resolve({ data: [] }),
|
||||||
},
|
},
|
||||||
mcp: {
|
mcp: {
|
||||||
add: () => Promise.resolve({ data: {} }),
|
add: () => {
|
||||||
|
calls.mcpAdds++
|
||||||
|
return Promise.resolve({ data: {} })
|
||||||
|
},
|
||||||
},
|
},
|
||||||
} as unknown as OpencodeClient
|
} as unknown as OpencodeClient
|
||||||
const service = ACPNextService.make({ sdk })
|
const service = ACPNextService.make({ sdk })
|
||||||
const session = await Effect.runPromise(service.newSession({ cwd: "/workspace", mcpServers: [] }))
|
const session = await Effect.runPromise(service.newSession({ cwd: "/workspace", mcpServers: [] }))
|
||||||
|
|
||||||
expect(providersCalls).toBe(1)
|
expect(calls).toEqual({ providers: 1, agents: 1, commands: 1, skills: 1, mcpAdds: 0 })
|
||||||
expect(commandCalls).toBe(1)
|
|
||||||
|
|
||||||
await Effect.runPromise(
|
await Effect.runPromise(
|
||||||
service.setSessionConfigOption({
|
service.setSessionConfigOption({
|
||||||
@@ -750,8 +763,135 @@ describe("ACP next service sessions", () => {
|
|||||||
}),
|
}),
|
||||||
)
|
)
|
||||||
|
|
||||||
expect(providersCalls).toBe(1)
|
expect(calls).toEqual({ providers: 1, agents: 1, commands: 1, skills: 1, mcpAdds: 0 })
|
||||||
expect(commandCalls).toBe(1)
|
})
|
||||||
|
|
||||||
|
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 () => {
|
it("normal text prompt sends model variant mode and converted parts", async () => {
|
||||||
|
|||||||
@@ -42,7 +42,7 @@ describe("opencode acp-next verifier timing diagnostics", () => {
|
|||||||
)
|
)
|
||||||
|
|
||||||
cliIt.live(
|
cliIt.live(
|
||||||
"warm new session timing diagnostic stays below generous threshold",
|
"warm new session stays below verifier threshold",
|
||||||
({ home, llm, opencode }) =>
|
({ home, llm, opencode }) =>
|
||||||
Effect.gen(function* () {
|
Effect.gen(function* () {
|
||||||
const acp = yield* createAcpNextClient(
|
const acp = yield* createAcpNextClient(
|
||||||
@@ -57,15 +57,13 @@ describe("opencode acp-next verifier timing diagnostics", () => {
|
|||||||
const durationMs = Math.round(performance.now() - started)
|
const durationMs = Math.round(performance.now() - started)
|
||||||
|
|
||||||
expect(session.sessionId).toBeTruthy()
|
expect(session.sessionId).toBeTruthy()
|
||||||
// TODO: replace this diagnostic assertion with finalFastPathThresholdMs.
|
expect(durationMs).toBeLessThan(finalFastPathThresholdMs)
|
||||||
expect(durationMs).toBeLessThan(diagnosticFastPathThresholdMs)
|
|
||||||
expect(finalFastPathThresholdMs).toBe(100)
|
|
||||||
}),
|
}),
|
||||||
60_000,
|
60_000,
|
||||||
)
|
)
|
||||||
|
|
||||||
cliIt.live(
|
cliIt.live(
|
||||||
"model switch timing diagnostic updates currentValue below generous threshold",
|
"model switch updates currentValue below verifier threshold",
|
||||||
({ home, llm, opencode }) =>
|
({ home, llm, opencode }) =>
|
||||||
Effect.gen(function* () {
|
Effect.gen(function* () {
|
||||||
const acp = yield* createAcpNextClient(
|
const acp = yield* createAcpNextClient(
|
||||||
@@ -89,14 +87,13 @@ describe("opencode acp-next verifier timing diagnostics", () => {
|
|||||||
const durationMs = Math.round(performance.now() - started)
|
const durationMs = Math.round(performance.now() - started)
|
||||||
|
|
||||||
expect(expectSelectOption(updated.configOptions, "model").currentValue).toBe(nextModel)
|
expect(expectSelectOption(updated.configOptions, "model").currentValue).toBe(nextModel)
|
||||||
// TODO: replace this diagnostic assertion with finalFastPathThresholdMs.
|
expect(durationMs).toBeLessThan(finalFastPathThresholdMs)
|
||||||
expect(durationMs).toBeLessThan(diagnosticFastPathThresholdMs)
|
|
||||||
}),
|
}),
|
||||||
60_000,
|
60_000,
|
||||||
)
|
)
|
||||||
|
|
||||||
cliIt.live(
|
cliIt.live(
|
||||||
"effort switch timing diagnostic updates currentValue below generous threshold",
|
"effort switch updates currentValue below verifier threshold",
|
||||||
({ home, llm, opencode }) =>
|
({ home, llm, opencode }) =>
|
||||||
Effect.gen(function* () {
|
Effect.gen(function* () {
|
||||||
const acp = yield* createAcpNextClient(
|
const acp = yield* createAcpNextClient(
|
||||||
@@ -118,8 +115,7 @@ describe("opencode acp-next verifier timing diagnostics", () => {
|
|||||||
const durationMs = Math.round(performance.now() - started)
|
const durationMs = Math.round(performance.now() - started)
|
||||||
|
|
||||||
expect(expectSelectOption(updated.configOptions, "effort").currentValue).toBe(nextEffort)
|
expect(expectSelectOption(updated.configOptions, "effort").currentValue).toBe(nextEffort)
|
||||||
// TODO: replace this diagnostic assertion with finalFastPathThresholdMs.
|
expect(durationMs).toBeLessThan(finalFastPathThresholdMs)
|
||||||
expect(durationMs).toBeLessThan(diagnosticFastPathThresholdMs)
|
|
||||||
}),
|
}),
|
||||||
60_000,
|
60_000,
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user