mirrors/opencode
1
0
Fork 0
mirror of https://github.com/anomalyco/opencode.git synced 2026-11-10 07:11:25 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(acp): tighten cwd, cancel, delete, and terminal auth handling (#53359)

Browse Source
This commit is contained in:
Shoubhit Dash
2026-10-06 17:15:51 +05:30
committed by GitHub
Unverified
parent f6ae986549
commit f17b15eaa5
10 changed files with 167 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/cli/src/acp/directories.ts
+8
View File
@@ -2,6 +2,7 @@ import { isAbsolute, join, resolve } from "node:path"
import { isDeepStrictEqual } from "node:util"
import type { OpenCodeClient } from "@opencode/client/effect"
import type { Permission } from "@opencode/schema/permission"
import { AbsolutePath } from "@opencode/schema/schema"
import type { Session } from "@opencode/schema/session"
import { FSUtil } from "@opencode/util/fs-util"
import { Effect, Option, Schema } from "effect"
@@ -13,6 +14,7 @@ const decodeStored = Schema.decodeUnknownOption(Schema.Array(Schema.String))
// Permission resources treat `*` and `?` as wildcards.
export const parse = Effect.fnUntraced(function* (cwd: string, directories: readonly string[] = []) {
yield* parseCwd(cwd)
const invalid = directories.find((directory) => !isAbsolute(directory) || /[*?]/.test(directory))
if (invalid !== undefined) return yield* new ACPError.InvalidAdditionalDirectoryError({ directory: invalid })
const root = FSUtil.resolve(cwd)
@@ -21,6 +23,12 @@ export const parse = Effect.fnUntraced(function* (cwd: string, directories: read
)
})
export const parseCwd = Effect.fnUntraced(function* (cwd: string) {
if (!isAbsolute(cwd))
return yield* new ACPError.InvalidRequestError({ message: `cwd must be an absolute path: ${cwd}`, field: "cwd" })
return AbsolutePath.make(cwd)
})
export function grant(directories: readonly string[]) {
if (directories.length === 0) return {}
return { permissions: rules(directories), metadata: { [key]: [...directories] } }
packages/cli/src/acp/service.ts
+5 -1
View File
@@ -109,6 +109,7 @@ export function make(input: {
description: "Run `opencode auth login` in the terminal",
name: "Login with opencode",
id: AuthMethodID,
...(params.clientCapabilities?.auth?.terminal ? { type: "terminal" as const, args: ["--login"] } : {}),
}
if (params.clientCapabilities?._meta?.["terminal-auth"] === true) {
authMethod._meta = {
@@ -164,7 +165,9 @@ export function make(input: {
listSessions: Effect.fnUntraced(function* (params) {
const page = yield* input.client.session
.list({
...(params.cwd ? { directory: AbsolutePath.make(params.cwd) } : {}),
...(params.cwd !== undefined && params.cwd !== null
? { directory: yield* ACPDirectories.parseCwd(params.cwd) }
: {}),
order: "desc",
limit: 100,
...(params.cursor ? { cursor: Schema.decodeSync(SessionsCursor)(params.cursor) } : {}),
@@ -188,6 +191,7 @@ export function make(input: {
}
}),
deleteSession: Effect.fnUntraced(function* (params) {
yield* input.turn.cancel({ sessionId: params.sessionId })
yield* ACPClient.decodeSessionID(params.sessionId).pipe(
Effect.flatMap((sessionID) => input.client.session.remove({ sessionID })),
Effect.catchTag(["ACPInvalidRequestError", "SessionNotFoundError"], () => Effect.void),
packages/cli/src/acp/turn.ts
+15 -15
View File
@@ -17,7 +17,7 @@ import {
Stream,
} from "effect"
import type { Capabilities } from "./capabilities"
import type { ACPCatalog } from "./catalog"
import { findModel, type ACPCatalog } from "./catalog"
import { ACPChild } from "./child"
import { ACPClient } from "./client"
import { currentModel } from "./config-option"
@@ -317,10 +317,12 @@ export const make = Effect.fnUntraced(function* (input: {
exit: Exit.Exit<ACPTranslate.Terminal, ACPError.Failure>,
) {
if (Exit.isFailure(exit) && !Cause.hasInterrupts(exit.cause)) return yield* Effect.failCause(exit.cause)
const failure = ACPTranslate.failure(current)
const terminal = Exit.isSuccess(exit) ? exit.value : "interrupted"
const failure = terminal === "interrupted" ? undefined : ACPTranslate.failure(current)
if (failure) return yield* failure
yield* sendUsageUpdate(attached, current)
return ACPTranslate.response(current, attached.id, Exit.isSuccess(exit) ? exit.value : "interrupted")
const cancelledWhileSettling = Exit.isFailure(yield* Effect.exit(Effect.interruptible(Effect.void)))
return ACPTranslate.response(current, attached.id, cancelledWhileSettling ? "interrupted" : terminal)
})
const sendUsageUpdate = Effect.fn("cli.acp.turn.usage")(
@@ -328,8 +330,7 @@ export const make = Effect.fnUntraced(function* (input: {
const used = state.usage ? TokenUsage.total(state.usage.last) : 0
if (!used) return
const catalog = yield* input.catalog.get(attached.cwd)
const current = currentModel(catalog, yield* Ref.get(attached.selection))
const model = catalog.models.find((item) => item.providerID === current.providerID && item.id === current.id)
const model = findModel(catalog.models, currentModel(catalog, yield* Ref.get(attached.selection)))
if (!model?.limit.context) return
const info = yield* input.client.session.get({ sessionID: attached.id }).pipe(Effect.catch(ACPClient.classify))
yield* input.connection.sessionUpdate({
@@ -347,8 +348,10 @@ export const make = Effect.fnUntraced(function* (input: {
),
)
// Forked uninterruptible: interruption reaches only `execute`, so the fiber still settles with a response.
const run = Effect.fn("cli.acp.turn.run")(function* (attached: Attached, prompt: ACPPrompt.Prepared) {
// Forked uninterruptible: interruption lands only in `execute` and the last check in `settle`, so a response follows.
const run = Effect.fn("cli.acp.turn.run")(function* (params: PromptRequest) {
const attached = yield* input.sessions.require(params.sessionId)
const prompt = yield* ACPPrompt.prepare(yield* input.catalog.get(attached.cwd), params.prompt)
const capabilities = yield* Ref.get(input.capabilities)
const state = yield* Ref.make(ACPTranslate.initial)
const exit = yield* Effect.acquireUseRelease(
@@ -370,21 +373,18 @@ export const make = Effect.fnUntraced(function* (input: {
return {
prompt: Effect.fnUntraced(function* (params, signal) {
const attached = yield* input.sessions.require(params.sessionId)
const catalog = yield* input.catalog.get(attached.cwd)
const prompt = yield* ACPPrompt.prepare(catalog, params.prompt)
// Synchronous, so concurrent prompts for one session cannot both register.
// Synchronous and before setup, so concurrent prompts cannot both register and an early cancel still applies.
const turn = yield* Effect.withFiber((fiber) => {
if (FiberMap.hasUnsafe(turns, attached.id)) {
if (FiberMap.hasUnsafe(turns, params.sessionId)) {
return Effect.fail(
new ACPError.ServiceFailureError({
safeMessage: `Session already has an active ACP prompt: ${attached.id}`,
safeMessage: `Session already has an active ACP prompt: ${params.sessionId}`,
service: "session",
}),
)
}
const forked = Effect.runForkWith(fiber.context)(run(attached, prompt), { uninterruptible: true })
FiberMap.setUnsafe(turns, attached.id, forked)
const forked = Effect.runForkWith(fiber.context)(run(params), { uninterruptible: true })
FiberMap.setUnsafe(turns, params.sessionId, forked)
return Effect.succeed(forked)
})
// A `$/cancel_request` for this prompt cancels its turn like `session/cancel`, rather than failing the request.
packages/cli/src/commands/commands.ts
+9 -1
View File
@@ -96,7 +96,15 @@ const Root = Spec.make(typeof OPENCODE_CLI_NAME === "string" ? OPENCODE_CLI_NAME
),
},
}),
Spec.make("acp", { description: "Start an Agent Client Protocol server" }),
Spec.make("acp", {
description: "Start an Agent Client Protocol server",
params: {
login: Flag.boolean("login").pipe(
Flag.withDescription("Run auth login instead of starting the server"),
Flag.withDefault(false),
),
},
}),
Spec.make("api", {
description: "Make a request to the running server",
params: {
packages/cli/src/commands/handlers/acp.ts
+12 -2
View File
@@ -2,7 +2,7 @@ import { MessageTooLargeError, ndJsonStream } from "@agentclientprotocol/sdk"
import { OpenCode } from "@opencode/client/effect"
import { Service } from "@opencode/client/effect/service"
import { CrossSpawnSpawner } from "@opencode/util/cross-spawn-spawner"
import { Effect } from "effect"
import { Effect, Option } from "effect"
import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http"
import { Writable } from "node:stream"
import { ACP } from "../../acp/agent"
@@ -12,7 +12,17 @@ import { Standalone } from "../../services/standalone"
export default Runtime.handler(
Commands.commands.acp,
Effect.fn("cli.acp")(function* () {
Effect.fn("cli.acp")(function* (input) {
if (input.login) {
const login = yield* Effect.promise(() => import("./auth/login"))
return yield* login.default({
target: Option.none(),
method: Option.none(),
answer: [],
server: Option.none(),
standalone: false,
})
}
process.env.OPENCODE_CLIENT = "acp"
const endpoint = yield* Standalone.start()
const client = yield* OpenCode.make({ baseUrl: endpoint.url }).pipe(
packages/cli/test/acp/prompt.test.ts
+59 -29
View File
@@ -33,10 +33,25 @@ import {
type WireOptions,
} from "./wire-fixture"
// A "hold" prompt is admitted and starts streaming, then only finishes when interrupted.
const lost = { type: "provider.transport", message: "stream connection lost" }
// A "hold" prompt is admitted, streams, and fails a step, then waits on its retry until interrupted.
const held = {
onPrompt: ({ sessionID, id, text }) =>
text === "hold" ? [delivered(sessionID, id), textDelta(sessionID, "msg_held", "working")] : turn(sessionID, id),
text === "hold"
? [
delivered(sessionID, id),
textDelta(sessionID, "msg_held", "working"),
durableEvent("session.step.failed", { sessionID, assistantMessageID: "msg_held", error: lost }),
durableEvent("session.retry.scheduled", {
sessionID,
assistantMessageID: "msg_held",
attempt: 1,
at: 0,
error: lost,
}),
]
: turn(sessionID, id),
onInterrupt: ({ sessionID }) => [interrupted(sessionID)],
} satisfies WireOptions
@@ -350,7 +365,9 @@ describe("acp prompt turns over the wire", () => {
expect(await rpcError(acp.prompt(acp.sessionId, "hello"))).toMatchObject(expected)
})
test("session/cancel before admission aborts the submission and returns cancelled", async () => {
test("session/cancel during prompt setup or before admission returns cancelled", async () => {
await using dir = await tmpdir()
await Bun.write(path.join(dir.path, "notes.md"), "# notes\n")
const aborted = Promise.withResolvers<void>()
await using acp = await startSession({
onPrompt: ({ signal }) =>
@@ -366,6 +383,15 @@ describe("acp prompt turns over the wire", () => {
}),
})
// The linked file's readability check holds the prompt in setup, before it submits.
const preparing = acp.prompt(acp.sessionId, [
{ type: "resource_link", uri: pathToFileURL(path.join(dir.path, "notes.md")).href, name: "notes.md" },
])
await acp.notify("session/cancel", { sessionId: acp.sessionId })
expect(await preparing).toEqual({ stopReason: "cancelled", _meta: {} })
expect(acp.server.submissions).toEqual([])
const prompt = acp.prompt(acp.sessionId, "hello")
await acp.until(() => acp.server.submissions.length === 1, "prompt submission")
await acp.notify("session/cancel", { sessionId: acp.sessionId })
@@ -479,35 +505,39 @@ describe("acp prompt turns over the wire", () => {
expect((await acp.prompt(acp.sessionId, "again")).stopReason).toBe("end_turn")
})
test("session/close settles the active turn before responding and detaches only that session", async () => {
await using acp = await startSession(held)
const other = await acp.newSession()
test.each(["session/close", "session/delete"] as const)(
"%s settles the active turn before responding and detaches only that session",
async (method) => {
await using acp = await startSession(held)
const other = await acp.newSession()
const order: string[] = []
const prompt = acp.prompt(acp.sessionId, "hold").then((response) => {
order.push("prompt")
return response
})
await admitted(acp, acp.sessionId)
const close = await acp.request("session/close", { sessionId: acp.sessionId }).then((response) => {
order.push("close")
return response
})
const order: string[] = []
const prompt = acp.prompt(acp.sessionId, "hold").then((result) => {
order.push("prompt")
return result
})
await admitted(acp, acp.sessionId)
const response = await acp.request(method, { sessionId: acp.sessionId }).then((result) => {
order.push(method)
return result
})
expect(close).toEqual({})
expect(await prompt).toMatchObject({ stopReason: "cancelled" })
expect(order).toEqual(["prompt", "close"])
expect(acp.server.interrupts).toEqual([acp.sessionId])
expect(await rpcError(acp.prompt(acp.sessionId, "again"))).toMatchObject({
code: -32602,
data: { sessionId: acp.sessionId },
})
expect((await acp.prompt(other.sessionId, "still here")).stopReason).toBe("end_turn")
expect(response).toEqual({})
expect(await prompt).toMatchObject({ stopReason: "cancelled" })
expect(order).toEqual(["prompt", method])
expect(acp.server.interrupts).toEqual([acp.sessionId])
expect(acp.server.sessions.has(acp.sessionId)).toBe(method === "session/close")
expect(await rpcError(acp.prompt(acp.sessionId, "again"))).toMatchObject({
code: -32602,
data: { sessionId: acp.sessionId },
})
expect((await acp.prompt(other.sessionId, "still here")).stopReason).toBe("end_turn")
expect((await acp.prompt(other.sessionId, "/review now")).stopReason).toBe("end_turn")
await acp.request("session/close", { sessionId: other.sessionId })
expect(acp.server.interrupts).toEqual([acp.sessionId, other.sessionId])
})
expect((await acp.prompt(other.sessionId, "/review now")).stopReason).toBe("end_turn")
await acp.request("session/close", { sessionId: other.sessionId })
expect(acp.server.interrupts).toEqual([acp.sessionId, other.sessionId])
},
)
})
// The server answered admission before streaming the chunk, and this request round-trips through the server after it,
packages/cli/test/acp/session.test.ts
+28 -1
View File
@@ -10,6 +10,10 @@ describe("acp session lifecycle over the wire", () => {
const plain = await acp.initialize()
const terminal = await acp.initialize({ terminalAuth: true, childSessionUpdates: true })
const standard = await acp.request("initialize", {
protocolVersion: 1,
clientCapabilities: { auth: { terminal: true } },
})
expect(plain).toMatchObject({
protocolVersion: 1,
@@ -28,6 +32,15 @@ describe("acp session lifecycle over the wire", () => {
expect(terminal.authMethods?.[0]?._meta).toEqual({
"terminal-auth": { command: "opencode", args: ["auth", "login"], label: "OpenCode Login" },
})
expect(standard.authMethods).toEqual([
{
id: "opencode-login",
name: "Login with opencode",
description: "Run `opencode auth login` in the terminal",
type: "terminal",
args: ["--login"],
},
])
expect(await acp.request("authenticate", { methodId: "opencode-login" })).toEqual({})
expect(await rpcError(acp.request("authenticate", { methodId: "missing" }))).toMatchObject({
code: -32602,
@@ -165,7 +178,7 @@ describe("acp session lifecycle over the wire", () => {
config: { type: "remote", url: "https://example.com/mcp", headers: { Authorization: "Bearer x" }, oauth: false },
})
})
test("rejects MCP-over-ACP and SSE servers before creating or loading a session", async () => {
test("rejects relative cwds, MCP-over-ACP, and SSE servers before creating or loading a session", async () => {
await using acp = await startWire()
acp.server.sessions.set("ses_saved", makeSession("ses_saved"))
await acp.initialize()
@@ -177,12 +190,26 @@ describe("acp session lifecycle over the wire", () => {
message: "Invalid params: Only stdio and HTTP MCP servers are supported",
data: { field: "mcpServers" },
}
const relative = {
code: -32602,
message: "Invalid params: cwd must be an absolute path: workspace",
data: { field: "cwd" },
}
expect(await rpcError(acp.newSession("/workspace", mcpServers))).toEqual(invalid)
expect(await rpcError(acp.newSession("/workspace", sse))).toEqual(invalid)
expect(
await rpcError(acp.request("session/load", { cwd: "/workspace", sessionId: "ses_saved", mcpServers })),
).toEqual(invalid)
expect(await rpcError(acp.newSession("workspace"))).toEqual(relative)
expect(
await rpcError(acp.request("session/load", { cwd: "workspace", sessionId: "ses_saved", mcpServers: [] })),
).toEqual(relative)
expect(await rpcError(acp.request("session/list", { cwd: "workspace" }))).toEqual(relative)
expect(await rpcError(acp.request("session/list", { cwd: "" }))).toMatchObject({
code: -32602,
data: { field: "cwd" },
})
expect(new Set(acp.server.sessions.keys())).toEqual(existing)
expect(acp.server.requests.filter((request) => request.path.includes("ses_saved"))).toEqual([])
expect(acp.logs).toEqual([])
packages/cli/test/acp/wire-fixture.ts
+3
View File
@@ -751,6 +751,9 @@ function startServer(options: WireOptions, changed: () => void) {
})
return noContent()
}),
DELETE: route((req) =>
fake.sessions.delete(req.params.sessionID) ? noContent() : notFound(req.params.sessionID),
),
},
"/api/session/:sessionID/fork": {
POST: route((req) => {
packages/cli/test/auth.test.ts
+5 -2
View File
@@ -289,11 +289,14 @@ describe("auth command", () => {
expect(removed).toEqual(["cred_test"])
})
test("requires a target outside an interactive terminal", async () => {
const result = await cli(["auth", "login"])
test("requires a target outside an interactive terminal, including through acp --login", async () => {
const [result, acp] = await Promise.all([cli(["auth", "login"]), cli(["acp", "--login"])])
expect(result.exitCode).toBe(1)
expect(result.stdout).toContain("Pass an integration ID or name")
expect(result.stdout).not.toContain("Background service failed to start")
expect(acp.exitCode).toBe(1)
expect(acp.stdout).toContain("Pass an integration ID or name when running without an interactive terminal")
expect(acp.stdout).not.toContain("Background service failed to start")
})
test("reports list connection failures without a stack trace", async () => {
services/www/src/docs/content/cli/acp.mdx
+23 -7
View File
@@ -28,9 +28,25 @@ OpenCode uses the provider credentials it already has. Sign in from a terminal b
opencode auth login
```
OpenCode offers one auth method, `opencode-login`. The `authenticate` request accepts only that method and doesn't
collect credentials. If a prompt fails because a provider needs authentication, OpenCode returns ACP's `auth_required`
error.
OpenCode offers one auth method, `opencode-login`. By default it's an agent auth method. The `authenticate` request
accepts only that method and doesn't collect credentials. If a prompt fails because a provider needs authentication,
OpenCode returns ACP's `auth_required` error.
If the client sets `clientCapabilities.auth.terminal` to `true`, the auth method is a terminal auth method:
```json
{
"id": "opencode-login",
"name": "Login with opencode",
"description": "Run `opencode auth login` in the terminal",
"type": "terminal",
"args": ["--login"]
}
```
The client must not send this method to `authenticate`. Instead, it adds `--login` to the command that starts OpenCode,
for example `opencode acp --login`, and runs it in a terminal. OpenCode runs the `opencode auth login` flow and exits.
Exit status 0 means the login succeeded. The client then reconnects and sends `initialize` again.
If the client sets `clientCapabilities._meta["terminal-auth"]` to `true`, the auth method includes the login command:
@@ -43,9 +59,6 @@ If the client sets `clientCapabilities._meta["terminal-auth"]` to `true`, the au
}
```
OpenCode doesn't support ACP's standard terminal auth yet. That is the `clientCapabilities.auth.terminal` capability
with a `type: "terminal"` auth method.
## Troubleshooting
`opencode acp` waits for ACP messages on stdin, so it looks stuck when you run it directly in a terminal. Start it from
@@ -97,6 +110,7 @@ OpenCode reads these client capabilities:
| Capability | Effect |
| -------------------------------------- | -------------------------------------------------------------- |
| `auth.terminal` set to `true` | The auth method is a [terminal auth method](#authentication). |
| `elicitation.form` | OpenCode asks [questions](#questions) as forms. |
| `session.compaction` | OpenCode reports [compaction](#compaction) with ACP's updates. |
| `_meta["terminal-auth"]` set to `true` | The auth method includes the [login command](#authentication). |
@@ -106,6 +120,7 @@ OpenCode reads these client capabilities:
The client sets the working directory when it creates a session. OpenCode uses that directory's configuration,
including its models, agents, commands, skills, instructions, plugins, and MCP servers.
- `cwd` must be an absolute path. OpenCode rejects a relative `cwd` with an invalid params error.
- `session/load` replays the saved messages. `session/resume` and `session/fork` don't replay them.
- To load or resume a session, the client must pass the session's own directory as `cwd`. OpenCode rejects a different
`cwd` with an invalid params error.
@@ -113,7 +128,8 @@ including its models, agents, commands, skills, instructions, plugins, and MCP s
- `session/list` returns up to 100 sessions per page. It accepts a `cwd` filter and a `cursor`.
- `session/close` interrupts active work and detaches the session from the ACP process. OpenCode keeps the saved
session.
- `session/delete` removes the session. Deleting a session that doesn't exist succeeds.
- `session/delete` ends an active prompt like `session/cancel`, then removes the session. Deleting a session that
doesn't exist succeeds.
- A session runs one prompt at a time. OpenCode returns an error for a second `session/prompt` while the first is
active.