Skip to content

Commit 56743dc

Browse files
authored
fix(acp): share acp-next session state (#29253)
1 parent 00ea47a commit 56743dc

3 files changed

Lines changed: 150 additions & 46 deletions

File tree

packages/opencode/src/acp-next/directory.ts

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { InstanceStore } from "@/project/instance-store"
55
import { ModelID, ProviderID } from "@/provider/schema"
66
import { Provider } from "@/provider/provider"
77
import { Context, Effect, Layer, SynchronizedRef } from "effect"
8+
import type * as ACPNextError from "./error"
89

910
export type ModelOption = {
1011
readonly providerID: ProviderID
@@ -38,12 +39,12 @@ export type Snapshot = {
3839
}
3940

4041
export interface LoaderInterface {
41-
readonly load: (directory: string) => Effect.Effect<Snapshot>
42+
readonly load: (directory: string) => Effect.Effect<Snapshot, ACPNextError.Error>
4243
}
4344

4445
export interface Interface {
45-
readonly get: (directory: string) => Effect.Effect<Snapshot>
46-
readonly refresh: (directory: string) => Effect.Effect<Snapshot>
46+
readonly get: (directory: string) => Effect.Effect<Snapshot, ACPNextError.Error>
47+
readonly refresh: (directory: string) => Effect.Effect<Snapshot, ACPNextError.Error>
4748
readonly variants: (snapshot: Snapshot, model: DefaultModel) => ModelVariants | undefined
4849
}
4950

@@ -141,15 +142,25 @@ export const layer = Layer.effect(
141142
Service,
142143
Effect.gen(function* () {
143144
const loader = yield* Loader
144-
const snapshots = yield* SynchronizedRef.make(new Map<string, Effect.Effect<Snapshot>>())
145+
const snapshots = yield* SynchronizedRef.make(new Map<string, Effect.Effect<Snapshot, ACPNextError.Error>>())
145146

146147
const cached = Effect.fnUntraced(function* (directory: string) {
147148
return yield* SynchronizedRef.modifyEffect(
148149
snapshots,
149150
Effect.fnUntraced(function* (items) {
150151
const current = items.get(directory)
151152
if (current) return [current, items] as const
152-
const next = yield* Effect.cached(loader.load(directory))
153+
const next = yield* Effect.cached(
154+
loader.load(directory).pipe(
155+
Effect.tapError(() =>
156+
SynchronizedRef.update(snapshots, (state) => {
157+
const next = new Map(state)
158+
next.delete(directory)
159+
return next
160+
}),
161+
),
162+
),
163+
)
153164
return [next, new Map(items).set(directory, next)] as const
154165
}),
155166
)
@@ -163,7 +174,17 @@ export const layer = Layer.effect(
163174
return yield* SynchronizedRef.modifyEffect(
164175
snapshots,
165176
Effect.fnUntraced(function* (items) {
166-
const next = yield* Effect.cached(loader.load(directory))
177+
const next = yield* Effect.cached(
178+
loader.load(directory).pipe(
179+
Effect.tapError(() =>
180+
SynchronizedRef.update(snapshots, (state) => {
181+
const next = new Map(state)
182+
next.delete(directory)
183+
return next
184+
}),
185+
),
186+
),
187+
)
167188
return [next, new Map(items).set(directory, next)] as const
168189
}),
169190
).pipe(Effect.flatten)

packages/opencode/src/acp-next/service.ts

Lines changed: 74 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@ import {
1616
} from "@agentclientprotocol/sdk"
1717
import { InstallationVersion } from "@opencode-ai/core/installation/version"
1818
import type { OpencodeClient } from "@opencode-ai/sdk/v2"
19-
import { Context, Effect } from "effect"
19+
import { Context, Effect, Layer, ManagedRuntime } from "effect"
2020
import * as ACPNextError from "./error"
2121
import { buildConfigOptions } from "./config-option"
2222
import { Directory } from "./directory"
23+
import { ACPNextSession } from "./session"
2324
import { ModelID, ProviderID } from "@/provider/schema"
2425
import { Provider } from "@/provider/provider"
2526
import type { Command } from "@/command"
@@ -42,9 +43,11 @@ export class Service extends Context.Service<Service, Interface>()("@opencode/AC
4243
export function make(input: {
4344
sdk: OpencodeClient
4445
connection?: Pick<AgentSideConnection, "sessionUpdate">
46+
directory?: Directory.Interface
47+
session?: ACPNextSession.Interface
4548
}): Interface {
46-
const sessions = new Map<string, SessionState>()
47-
const directories = new Map<string, Promise<Directory.Snapshot>>()
49+
const session = input.session ?? makeSessionService()
50+
const directoryService = input.directory ?? makeDirectoryService(input.sdk)
4851
const registeredMcp = new Map<string, Set<string>>()
4952

5053
const initialize = Effect.fn("ACPNext.initialize")(function* (params: InitializeRequest) {
@@ -92,16 +95,8 @@ export function make(input: {
9295
return {}
9396
})
9497

95-
const directorySnapshot = Effect.fn("ACPNext.directorySnapshot")(function* (directory: string) {
96-
const cached = directories.get(directory)
97-
if (cached) return yield* request(() => cached, "directory")
98-
99-
const promise = loadDirectorySnapshot(input.sdk, directory).catch((error: unknown) => {
100-
directories.delete(directory)
101-
throw fromUnknownError(error, "directory")
102-
})
103-
directories.set(directory, promise)
104-
return yield* request(() => promise, "directory")
98+
const directorySnapshot = Effect.fn("ACPNext.directorySnapshot")(function* (cwd: string) {
99+
return yield* directoryService.get(cwd)
105100
})
106101

107102
const newSession = Effect.fn("ACPNext.newSession")(function* (params: NewSessionRequest) {
@@ -125,7 +120,7 @@ export function make(input: {
125120
),
126121
"session",
127122
)
128-
const state = storeSession(sessions, {
123+
const state = yield* session.create({
129124
id: created.id,
130125
cwd: params.cwd,
131126
mcpServers: params.mcpServers,
@@ -134,12 +129,16 @@ export function make(input: {
134129
modeId,
135130
})
136131

137-
yield* registerMcpServers(input.sdk, registeredMcp, params.cwd, params.mcpServers)
132+
yield* registerMcpServers(input.sdk, registeredMcp, params.cwd, state.id, params.mcpServers)
138133
yield* sendAvailableCommands(input.connection, state.id, snapshot)
139134

140135
return {
141136
sessionId: state.id,
142-
configOptions: configOptions(snapshot, state),
137+
configOptions: configOptions(snapshot, {
138+
model: state.model ?? selected,
139+
variant: state.variant,
140+
modeId: state.modeId,
141+
}),
143142
}
144143
})
145144

@@ -159,7 +158,7 @@ export function make(input: {
159158
)
160159
const restored = restoreFromMessages(messages.map((item) => item.info))
161160
const model = restored.model ?? selectDefaultModel(snapshot)
162-
const state = storeSession(sessions, {
161+
const state = yield* session.load({
163162
id: params.sessionId,
164163
cwd: params.cwd,
165164
mcpServers: params.mcpServers,
@@ -168,12 +167,16 @@ export function make(input: {
168167
modeId: restored.modeId ?? (snapshot.availableModes.length > 0 ? snapshot.defaultModeID : undefined),
169168
})
170169

171-
yield* registerMcpServers(input.sdk, registeredMcp, params.cwd, params.mcpServers)
170+
yield* registerMcpServers(input.sdk, registeredMcp, params.cwd, state.id, params.mcpServers)
172171
yield* sendAvailableCommands(input.connection, state.id, snapshot)
173172

174173
return {
175174
sessionId: state.id,
176-
configOptions: configOptions(snapshot, state),
175+
configOptions: configOptions(snapshot, {
176+
model: state.model ?? model,
177+
variant: state.variant,
178+
modeId: state.modeId,
179+
}),
177180
}
178181
})
179182

@@ -191,10 +194,28 @@ export function make(input: {
191194
}
192195
}
193196

194-
type SessionState = {
195-
readonly id: string
196-
readonly cwd: string
197-
readonly mcpServers: readonly McpServer[]
197+
function makeSessionService() {
198+
return ManagedRuntime.make(ACPNextSession.defaultLayer).runSync(
199+
ACPNextSession.Service.use((service) => Effect.succeed(service)),
200+
)
201+
}
202+
203+
function makeDirectoryService(sdk: OpencodeClient) {
204+
return ManagedRuntime.make(
205+
Directory.layer.pipe(
206+
Layer.provide(
207+
Layer.succeed(
208+
Directory.Loader,
209+
Directory.Loader.of({
210+
load: (directory) => request(() => loadDirectorySnapshot(sdk, directory), "directory"),
211+
}),
212+
),
213+
),
214+
),
215+
).runSync(Directory.Service.use((service) => Effect.succeed(service)))
216+
}
217+
218+
type ConfigState = {
198219
readonly model: Directory.DefaultModel
199220
readonly variant?: string
200221
readonly modeId?: string
@@ -340,15 +361,7 @@ function selectVariant(snapshot: Directory.Snapshot, model: Directory.DefaultMod
340361
return Object.keys(variants)[0]
341362
}
342363

343-
function storeSession(sessions: Map<string, SessionState>, state: SessionState) {
344-
sessions.set(state.id, {
345-
...state,
346-
mcpServers: [...state.mcpServers],
347-
})
348-
return sessions.get(state.id)!
349-
}
350-
351-
function configOptions(snapshot: Directory.Snapshot, session: SessionState) {
364+
function configOptions(snapshot: Directory.Snapshot, session: ConfigState) {
352365
return buildConfigOptions({
353366
providers: Object.values(snapshot.providers),
354367
currentModel: session.model,
@@ -384,35 +397,47 @@ function registerMcpServers(
384397
sdk: OpencodeClient,
385398
registered: Map<string, Set<string>>,
386399
directory: string,
400+
sessionId: string,
387401
servers: readonly McpServer[],
388402
) {
389-
const current = registered.get(directory) ?? new Set<string>()
390-
registered.set(directory, current)
403+
const current = registered.get(sessionId) ?? new Set<string>()
404+
registered.set(sessionId, current)
405+
const pending = new Set<string>()
391406

392407
return Effect.all(
393-
Array.from(new Map(servers.map((server) => [server.name, server])).values())
394-
.filter((server) => !current.has(server.name))
395-
.map((server) =>
408+
servers
409+
.map((server) => ({ server, config: mcpConfig(server) }))
410+
.filter((entry) => {
411+
const key = mcpRegistrationKey(entry.server.name, entry.config)
412+
if (current.has(key) || pending.has(key)) return false
413+
pending.add(key)
414+
return true
415+
})
416+
.map((entry) =>
396417
request(
397418
() =>
398419
sdk.mcp.add(
399420
{
400421
directory,
401-
name: server.name,
402-
config: mcpConfig(server),
422+
name: entry.server.name,
423+
config: entry.config,
403424
},
404425
{ throwOnError: true },
405426
),
406427
"mcp",
407428
).pipe(
408-
Effect.tap(() => Effect.sync(() => current.add(server.name))),
429+
Effect.tap(() => Effect.sync(() => current.add(mcpRegistrationKey(entry.server.name, entry.config)))),
409430
Effect.ignore,
410431
),
411432
),
412433
{ concurrency: "unbounded" },
413434
).pipe(Effect.asVoid)
414435
}
415436

437+
function mcpRegistrationKey(name: string, config: ReturnType<typeof mcpConfig>) {
438+
return `${name}:${stableStringify(config)}`
439+
}
440+
416441
function mcpConfig(server: McpServer) {
417442
if ("type" in server) {
418443
return {
@@ -428,6 +453,15 @@ function mcpConfig(server: McpServer) {
428453
}
429454
}
430455

456+
function stableStringify(value: unknown): string {
457+
if (Array.isArray(value)) return `[${value.map(stableStringify).join(",")}]`
458+
if (!value || typeof value !== "object") return JSON.stringify(value)
459+
return `{${Object.entries(value)
460+
.toSorted(([a], [b]) => a.localeCompare(b))
461+
.map(([key, item]) => `${JSON.stringify(key)}:${stableStringify(item)}`)
462+
.join(",")}}`
463+
}
464+
431465
function restoreFromMessages(messages: readonly MessageInfo[]) {
432466
const user = messages.findLast(
433467
(message) => message.role === "user" && message.model?.providerID && message.model.modelID,

packages/opencode/test/acp-next/service-session.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,55 @@ describe("ACP next service sessions", () => {
282282
expect(providersCalls).toBe(2)
283283
})
284284

285+
it("registers same-name MCP servers again for different sessions or configs", async () => {
286+
const adds: unknown[] = []
287+
let nextSession = 0
288+
const sdk = {
289+
config: {
290+
providers: () => Promise.resolve({ data: { providers: [provider], default: { test: modelID } } }),
291+
get: () => Promise.resolve({ data: {} }),
292+
},
293+
app: {
294+
agents: () => Promise.resolve({ data: [{ name: "build", mode: "primary", permission: [], options: {} }] }),
295+
skills: () => Promise.resolve({ data: [] }),
296+
},
297+
command: {
298+
list: () => Promise.resolve({ data: [] }),
299+
},
300+
session: {
301+
create: () => {
302+
nextSession++
303+
return Promise.resolve({ data: { id: `ses_${nextSession}` } })
304+
},
305+
list: () => Promise.resolve({ data: [] }),
306+
},
307+
mcp: {
308+
add: (input: unknown) => {
309+
adds.push(input)
310+
return Promise.resolve({ data: {} })
311+
},
312+
},
313+
} as unknown as OpencodeClient
314+
const service = ACPNextService.make({ sdk })
315+
316+
await Effect.runPromise(
317+
service.newSession({
318+
cwd: "/workspace",
319+
mcpServers: [{ name: "tools", command: "node", args: ["one.js"], env: [] }],
320+
}),
321+
)
322+
await Effect.runPromise(
323+
service.newSession({
324+
cwd: "/workspace",
325+
mcpServers: [{ name: "tools", command: "node", args: ["two.js"], env: [] }],
326+
}),
327+
)
328+
329+
expect(adds).toHaveLength(2)
330+
expect(JSON.stringify(adds[0])).toContain("one.js")
331+
expect(JSON.stringify(adds[1])).toContain("two.js")
332+
})
333+
285334
it("uses the configured model as the new session default", async () => {
286335
const sdk = {
287336
config: {

0 commit comments

Comments
 (0)