Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions packages/opencode/src/acp/permission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {
import type { Event, OpencodeClient } from "@opencode-ai/sdk/v2"
import { applyPatch } from "diff"
import { exists, readText } from "@/util/filesystem"
import type { ACPSession } from "./session"
import { ACPSession } from "./session"
import { pendingToolCall, toLocations, type ToolInput } from "./tool"
import { Effect } from "effect"

Expand Down Expand Up @@ -50,8 +50,16 @@ export class Handler {

private async process(event: PermissionEvent) {
const permission = event.properties
const session = await Effect.runPromise(this.input.session.tryGet(permission.sessionID))
if (!session) return
const session = await this.resolveSession(permission.sessionID)
if (!session) {
// Unresolvable even after walking the parentID chain (e.g. a child
// session whose ancestor was itself never registered). Reply with an
// active reject instead of silently dropping the event — silence
// leaves the underlying Permission.ask Deferred unresolved forever,
// hanging whichever session/prompt call is waiting on it (G1).
await this.rejectUnresolvable(permission.id)
return
}

if (!this.input.connection.requestPermission) {
await this.reply(permission.id, "reject", session.cwd)
Expand Down Expand Up @@ -96,6 +104,41 @@ export class Handler {
})
}

// Child/subagent sessions spawned server-side by the `task` tool are never
// registered as their own ACP session (only session/new|load|resume|fork
// register one). Walk the SDK's `parentID` chain to find the nearest
// ancestor that IS registered, so permission asks from those sessions
// still route through this connection instead of being dropped.
private resolveSession(sessionID: string): Promise<ACPSession.Info | undefined> {
return Effect.runPromise(
ACPSession.resolveAncestor({
tryGet: this.input.session.tryGet,
sessionId: sessionID,
fetchParentID: (id) => this.fetchParentID(id),
}),
)
}

private async fetchParentID(sessionID: string): Promise<string | undefined> {
const roots = await Effect.runPromise(this.input.session.list())
const directories = [...new Set(roots.map((root) => root.cwd))]
for (const directory of directories) {
const info = await this.input.sdk.session
.get({ directory, sessionID }, { throwOnError: true })
.then((response) => response.data)
.catch(() => undefined)
if (info) return info.parentID
}
return undefined
}

private async rejectUnresolvable(requestID: string) {
const roots = await Effect.runPromise(this.input.session.list())
const directory = roots[0]?.cwd
if (!directory) return
await this.reply(requestID, "reject", directory).catch(() => {})
}

private async writeProposedEdit(sessionId: string, metadata: ToolInput) {
const filepath = stringValue(metadata.filepath)
const diff = stringValue(metadata.diff)
Expand Down
30 changes: 30 additions & 0 deletions packages/opencode/src/acp/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,4 +229,34 @@ function partMetadataKey(input: { messageId: string; partId: string }) {
return `${input.messageId}:${input.partId}`
}

const MAX_ANCESTOR_DEPTH = 8

// Resolves a session id that was never registered as its own ACP session —
// e.g. a subagent session spawned server-side by the `task` tool — back to
// the nearest ancestor that IS registered (a root session/new/load/resume/
// fork). Without this, permission asks and tool-call updates from child
// sessions hit `tryGet` -> undefined and get silently dropped by callers,
// which (for permission asks) leaves the underlying Permission.ask Deferred
// unresolved forever (see ACP G1: child session permission hang).
//
// `fetchParentID` is supplied by the caller since walking the ancestor chain
// requires an SDK round-trip (this module has no SDK dependency of its own).
export function resolveAncestor(input: {
readonly tryGet: (sessionId: string) => Effect.Effect<Info | undefined>
readonly fetchParentID: (sessionId: string) => Promise<string | undefined>
readonly sessionId: string
}): Effect.Effect<Info | undefined> {
return Effect.gen(function* () {
let current = input.sessionId
for (let depth = 0; depth < MAX_ANCESTOR_DEPTH; depth++) {
const known = yield* input.tryGet(current)
if (known) return known
const parentID = yield* Effect.promise(() => input.fetchParentID(current))
if (!parentID || parentID === current) return undefined
current = parentID
}
return undefined
})
}

export * as ACPSession from "./session"
94 changes: 94 additions & 0 deletions packages/opencode/test/cli/acp/child-session-permission.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// Regression test for G1: child/subagent sessions spawned by the `task` tool
// were never registered in the ACP session store, so their `permission.asked`
// events hit `session.tryGet` -> undefined -> early return in
// acp/permission.ts. Under an "ask" ruleset this left the underlying
// Permission.ask Deferred unresolved forever, hanging the whole
// `session/prompt` call. The fix resolves the child session back to its
// registered root ACP session via the SDK's `parentID` chain, so the
// existing `session/request_permission` round trip fires as normal instead
// of the event being silently dropped.
import { describe, expect } from "bun:test"
import type { PromptResponse, RequestPermissionResponse } from "@agentclientprotocol/sdk"
import { Duration, Effect } from "effect"
import path from "node:path"
import { cliIt } from "../../lib/cli-process"
import { createAcpClient as createJsonRpcAcpClient } from "./acp-test-client"
import { initialize, newSession, verifierConfig } from "./helpers"

type JsonRpcMessage = {
readonly jsonrpc: "2.0"
readonly id?: number
readonly method?: string
readonly params?: { sessionId?: string }
readonly result?: unknown
}

describe("acp child session permission (G1)", () => {
cliIt.live(
"child/subagent session edit:ask surfaces session/request_permission for the child session instead of hanging",
({ home, llm, opencode }) =>
Effect.gen(function* () {
const raw = yield* opencode.acp({
env: { OPENCODE_CONFIG_CONTENT: JSON.stringify({ ...verifierConfig(llm.url), permission: { edit: "ask" } }) },
})
const acp = createJsonRpcAcpClient(raw)
yield* initialize(acp)
const session = yield* newSession(acp, home)

yield* llm.tool("task", {
description: "write a file",
prompt: "write child-fix-check.txt",
subagent_type: "general",
})
yield* llm.tool("write", { filePath: "child-fix-check.txt", content: "child wrote this" })
yield* llm.text("child done")
yield* llm.text("parent done")

const promptRequestId = 9999
yield* raw.send({
jsonrpc: "2.0",
id: promptRequestId,
method: "session/prompt",
params: { sessionId: session.sessionId, prompt: [{ type: "text", text: "delegate to subagent" }] },
})

// Manually drive the duplex channel: reply to any incoming
// session/request_permission with "reject" (simulating a real but
// permission-unaware client), while watching for the final
// session/prompt response. Captures the sessionId the server used
// for the permission request so we can assert it's the child's real
// id, not the resolved root's.
const capturedPermissionSessionIds: string[] = []
const outcome = yield* Effect.gen(function* () {
while (true) {
const message = (yield* raw.receive.pipe(Effect.timeout(Duration.seconds(10)))) as JsonRpcMessage
if (message.method === "session/request_permission" && message.id !== undefined) {
if (message.params?.sessionId) capturedPermissionSessionIds.push(message.params.sessionId)
yield* raw.send({
jsonrpc: "2.0",
id: message.id,
result: { outcome: { outcome: "selected", optionId: "reject" } } satisfies RequestPermissionResponse,
})
continue
}
if (message.id === promptRequestId) return message
}
}).pipe(Effect.timeout(Duration.seconds(15)), Effect.exit)

expect(outcome._tag).toBe("Success")
if (outcome._tag !== "Success") return
const response = outcome.value as { result?: PromptResponse }
expect(response.result?.stopReason).toBeDefined()

// The permission request must have been raised for the CHILD's own
// session id (subagent action honestly attributed), not silently
// dropped and not impersonating the root session.
expect(capturedPermissionSessionIds.length).toBeGreaterThan(0)
expect(capturedPermissionSessionIds).not.toContain(session.sessionId)

const exists = yield* Effect.promise(() => Bun.file(path.join(home, "child-fix-check.txt")).exists())
expect(exists).toBe(false)
}),
30_000,
)
})
Loading