Skip to content

Commit 50b4ad8

Browse files
smagnusonexxeln
andauthored
fix(acp): honor session/cancel by aborting the running turn (anomalyco#30145)
Co-authored-by: Shoubhit Dash <shoubhit2005@gmail.com>
1 parent fd2278e commit 50b4ad8

2 files changed

Lines changed: 49 additions & 44 deletions

File tree

packages/opencode/src/acp/service.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -330,25 +330,34 @@ export function make(input: {
330330
}
331331
})
332332

333-
const closeSession = Effect.fn("ACP.closeSession")(function* (params: CloseSessionRequest) {
334-
const removed = yield* session.remove(params.sessionId)
335-
registeredMcp.delete(params.sessionId)
336-
sessionSnapshots.delete(params.sessionId)
337-
if (!removed) return {}
338-
333+
const abortBackingSession = Effect.fn("ACP.abortBackingSession")(function* (current: ACPSession.Info) {
339334
yield* request(
340-
() => input.sdk.session.abort({ directory: removed.cwd, sessionID: params.sessionId }, { throwOnError: true }),
335+
() => input.sdk.session.abort({ directory: current.cwd, sessionID: current.id }, { throwOnError: true }),
341336
"session",
342337
).pipe(
343338
Effect.catch((error) =>
344339
Effect.sync(() => {
345-
log.error("failed to abort session while closing ACP session", { error, sessionID: params.sessionId })
340+
log.error("failed to abort ACP backing session", { error, sessionID: current.id })
346341
}),
347342
),
348343
)
344+
})
345+
346+
const closeSession = Effect.fn("ACP.closeSession")(function* (params: CloseSessionRequest) {
347+
const removed = yield* session.remove(params.sessionId)
348+
registeredMcp.delete(params.sessionId)
349+
sessionSnapshots.delete(params.sessionId)
350+
if (!removed) return {}
351+
352+
yield* abortBackingSession(removed)
349353
return {}
350354
})
351355

356+
const cancel = Effect.fn("ACP.cancel")(function* (params: CancelNotification) {
357+
const current = yield* session.get(params.sessionId)
358+
yield* abortBackingSession(current)
359+
})
360+
352361
const forkSession = Effect.fn("ACP.forkSession")(function* (params: ForkSessionRequest) {
353362
const snapshot = yield* directorySnapshot(params.cwd)
354363
const forked = yield* request(
@@ -563,9 +572,7 @@ export function make(input: {
563572
yield* sendUsageUpdate(input.usage, input.sdk, input.connection, current.id, current.cwd)
564573
return promptResponse(undefined, params.messageId)
565574
}),
566-
cancel: Effect.fn("ACP.cancel")(function* (_input: CancelNotification) {
567-
return yield* new ACPError.UnsupportedOperationError({ method: "session/cancel" })
568-
}),
575+
cancel,
569576
}
570577
}
571578

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

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,9 @@ import type {
1212
} from "@agentclientprotocol/sdk"
1313
import type { OpencodeClient } from "@opencode-ai/sdk/v2"
1414
import { ProviderV2 } from "@opencode-ai/core/provider"
15-
import { Effect, ManagedRuntime } from "effect"
15+
import { Effect } from "effect"
1616
import * as ACPService from "@/acp/service"
1717
import * as ACPError from "@/acp/error"
18-
import { ACPSession } from "@/acp/session"
1918
import { UsageService } from "@/acp/usage"
2019
import type { Provider } from "@/provider/provider"
2120

@@ -142,7 +141,10 @@ const provider: Provider.Info = {
142141
}
143142

144143
describe("ACP service sessions", () => {
145-
const makeService = (messages: readonly { info: unknown; parts: readonly unknown[] }[] = []) => {
144+
const makeService = (
145+
messages: readonly { info: unknown; parts: readonly unknown[] }[] = [],
146+
options?: { abort?: (input: { sessionID: string }) => Promise<{ data: boolean }> },
147+
) => {
146148
const updates: SessionNotification[] = []
147149
const mcpAdds: string[] = []
148150
const aborts: string[] = []
@@ -220,10 +222,12 @@ describe("ACP service sessions", () => {
220222
summarizes.push(input)
221223
return Promise.resolve({ data: true })
222224
},
223-
abort: (input: { sessionID: string }) => {
224-
aborts.push(input.sessionID)
225-
return Promise.resolve({ data: true })
226-
},
225+
abort:
226+
options?.abort ??
227+
((input: { sessionID: string }) => {
228+
aborts.push(input.sessionID)
229+
return Promise.resolve({ data: true })
230+
}),
227231
fork: (input: { sessionID: string }) => {
228232
forks.push(input.sessionID)
229233
return Promise.resolve({ data: { id: `fork_${input.sessionID}` } })
@@ -381,34 +385,28 @@ describe("ACP service sessions", () => {
381385
expect(await Effect.runPromise(service.closeSession({ sessionId: "missing" }))).toEqual({})
382386
})
383387

384-
it("does not fail close when backing abort fails", async () => {
385-
const sessionService = ManagedRuntime.make(ACPSession.defaultLayer).runSync(
386-
ACPSession.Service.use((service) => Effect.succeed(service)),
388+
it("cancel aborts the backing session and keeps the ACP session", async () => {
389+
const { service, aborts } = makeService()
390+
const created = await Effect.runPromise(service.newSession({ cwd: "/workspace", mcpServers: [] }))
391+
392+
await Effect.runPromise(service.cancel({ sessionId: created.sessionId }))
393+
394+
// The running turn was aborted via the core session API.
395+
expect(aborts).toEqual([created.sessionId])
396+
// Unlike closeSession, the ACP session is still present afterwards so
397+
// the client can keep prompting.
398+
const stillUsable = await Effect.runPromise(
399+
service.setSessionConfigOption({ sessionId: created.sessionId, configId: "effort", value: "high" }),
387400
)
388-
const { service } = makeService()
389-
const sdk = {
390-
config: {
391-
providers: () => Promise.resolve({ data: { providers: [provider], default: { test: modelID } } }),
392-
get: () => Promise.resolve({ data: {} }),
393-
},
394-
app: {
395-
agents: () => Promise.resolve({ data: [{ name: "build", mode: "primary", permission: [], options: {} }] }),
396-
skills: () => Promise.resolve({ data: [] }),
397-
},
398-
command: {
399-
list: () => Promise.resolve({ data: [] }),
400-
},
401-
session: {
402-
abort: () => Promise.reject(new Error("nope")),
403-
},
404-
mcp: {
405-
add: () => Promise.resolve({ data: {} }),
406-
},
407-
} as unknown as OpencodeClient
408-
const closing = ACPService.make({ sdk, session: sessionService })
409-
await Effect.runPromise(sessionService.create({ id: "ses_close", cwd: "/workspace" }))
401+
expect(stillUsable).toBeDefined()
402+
})
410403

411-
expect(await Effect.runPromise(closing.closeSession({ sessionId: "ses_close" }))).toEqual({})
404+
it("does not fail cancel or close when the backing abort fails", async () => {
405+
const { service } = makeService([], { abort: () => Promise.reject(new Error("nope")) })
406+
const created = await Effect.runPromise(service.newSession({ cwd: "/workspace", mcpServers: [] }))
407+
408+
await Effect.runPromise(service.cancel({ sessionId: created.sessionId }))
409+
expect(await Effect.runPromise(service.closeSession({ sessionId: created.sessionId }))).toEqual({})
412410
expect(await Effect.runPromise(service.closeSession({ sessionId: "missing" }))).toEqual({})
413411
})
414412

0 commit comments

Comments
 (0)