From 1010efcfc8800d9b59aa724b676aeae272604582 Mon Sep 17 00:00:00 2001 From: literally-dan <9935415+literally-dan@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:24:21 +0100 Subject: [PATCH] fix(session): route nested blockers to ancestor clients A permission raised inside a subagent never reached a client. `ACPSession` only holds sessions the client created, so `tryGet(permission.sessionID)` missed every subagent and the handler returned without replying. Since `Permission.ask` waits on a deferred with no timeout, any tool needing permission at depth >= 1 hung the run outright. Run mode had the same gap: it compared the event's session against the root id, so a descendant's prompt was treated as somebody else's. Both now resolve the owning session by walking the parent chain and route the prompt to the nearest ancestor the client actually knows about, at any depth. Run mode additionally seeds the transitive descendant set up front so attach mode surfaces prompts that were already pending, and extends it from `session.created`/`session.updated`. Resolution retries are bounded. The lookup fails transiently for a server restart or a grandchild whose `session.created` has not projected yet, but it also fails permanently for a parent chain cycle or a parentID pointing at a deleted session. Retrying those forever converts the hang this change removes into a polling hang, so `src/session/ancestry.ts` caps the retry at roughly seven seconds and reports exhaustion, and both callers then reject the request rather than leave the nested tool call waiting. That module also gives the two callers one implementation instead of the two near-identical copies they would otherwise keep in step by hand. Replies tolerate a missing request. The server drops pending permissions when a session aborts and rejects the cascade itself, so a reply that lost that race is already answered and must not fail the run. The descendant walk is bounded on every axis: fan-out concurrency, a per-request timeout, a total deadline, a size cap, and cycle detection, so a corrupt parent pointer cannot spin or fan out without limit. The `session.deleted` subscription drops cached parent ids and aborts prompts for the removed session, and stopping the subscription tears down every retained retry. Run mode no longer awaits the descendant seed before subscribing to `/event`. That route has no replay, so the round trip risked losing events published in the gap, including the `session.status: idle` the loop breaks on. The seed resolves in the background and prompt routing awaits it. The two end-to-end tests set `subagent_depth: 2`. Without it the default of 1 makes the second delegation fail, so the prompt comes from the child and the tests pass on depth-1 routing while appearing to cover nesting. Both now assert the grandchild was really prompted. --- packages/opencode/src/acp/event.ts | 4 + packages/opencode/src/acp/permission.ts | 234 ++++++++++++-- packages/opencode/src/cli/cmd/run.ts | 271 ++++++++++------ .../src/cli/cmd/run/stream.transport.ts | 30 +- .../opencode/src/cli/cmd/run/subagent-data.ts | 37 ++- packages/opencode/src/cli/cmd/run/tree.ts | 215 +++++++++++++ packages/opencode/src/session/ancestry.ts | 66 ++++ packages/opencode/test/acp/permission.test.ts | 295 +++++++++++++++++- .../test/cli/acp/nested-permission.test.ts | 99 ++++++ .../opencode/test/cli/run/run-process.test.ts | 58 ++++ .../test/cli/run/stream.transport.test.ts | 110 +++++++ .../test/cli/run/subagent-data.test.ts | 73 +++++ packages/opencode/test/cli/run/tree.test.ts | 295 ++++++++++++++++++ packages/tui/src/routes/session/footer.tsx | 10 +- packages/tui/src/routes/session/index.tsx | 13 +- packages/tui/src/routes/session/tree.ts | 36 +++ packages/tui/test/routes/session/tree.test.ts | 29 ++ 17 files changed, 1719 insertions(+), 156 deletions(-) create mode 100644 packages/opencode/src/cli/cmd/run/tree.ts create mode 100644 packages/opencode/src/session/ancestry.ts create mode 100644 packages/opencode/test/cli/acp/nested-permission.test.ts create mode 100644 packages/opencode/test/cli/run/tree.test.ts create mode 100644 packages/tui/src/routes/session/tree.ts create mode 100644 packages/tui/test/routes/session/tree.test.ts diff --git a/packages/opencode/src/acp/event.ts b/packages/opencode/src/acp/event.ts index 7d05fa6ee58c..58772e657134 100644 --- a/packages/opencode/src/acp/event.ts +++ b/packages/opencode/src/acp/event.ts @@ -63,6 +63,7 @@ export class Subscription { stop() { this.abort.abort() + this.permission.stop() } async handle(event: Event) { @@ -74,6 +75,9 @@ export class Subscription { return this.handlePartUpdated(event) case "message.part.delta": return this.handlePartDelta(event) + case "session.deleted": + this.permission.forgetSession(event.properties.sessionID) + return } } diff --git a/packages/opencode/src/acp/permission.ts b/packages/opencode/src/acp/permission.ts index 4eeca28f09f5..285795673883 100644 --- a/packages/opencode/src/acp/permission.ts +++ b/packages/opencode/src/acp/permission.ts @@ -11,6 +11,7 @@ import { applyPatch } from "diff" import { exists, readText } from "@/util/filesystem" import type { ACPSession } from "./session" import { pendingToolCall, toLocations, type ToolInput } from "./tool" +import { SessionAncestry } from "@/session/ancestry" import { Effect } from "effect" type PermissionEvent = Extract @@ -23,8 +24,17 @@ const permissionOptions: PermissionOption[] = [ { optionId: "reject", kind: "reject_once", name: "Reject" }, ] +type Ownership = SessionAncestry.Ownership + export class Handler { private readonly queues = new Map>() + private readonly pending = new Map() + private stopped = false + // Memoise sessionID -> parentID lookups so events streaming from a + // deeply nested subagent don't re-fetch the whole chain per event. + // `parentID` is immutable per session row, so cache entries never go + // stale; we drop them on `session.deleted` to bound memory. + private readonly parentIDCache = new Map() constructor( private readonly input: { @@ -34,46 +44,170 @@ export class Handler { }, ) {} + // Invoked from the event subscription on `session.deleted` so the + // parentID cache tracks live sessions only. + forgetSession(sessionID: string) { + this.parentIDCache.delete(sessionID) + for (const pending of this.pending.values()) { + if (pending.sessionID === sessionID) pending.controller.abort() + } + } + + stop() { + this.stopped = true + for (const pending of this.pending.values()) pending.controller.abort() + this.pending.clear() + this.queues.clear() + } + handle(event: PermissionEvent) { const permission = event.properties - const previous = this.queues.get(permission.sessionID) ?? Promise.resolve() + if (this.stopped || this.pending.has(permission.id)) return + const controller = new AbortController() + this.pending.set(permission.id, { sessionID: permission.sessionID, controller }) + void this.route(event, controller.signal) + .catch((error) => + Effect.runSync( + Effect.logError("failed to route permission request", { + requestID: permission.id, + sessionID: permission.sessionID, + error, + }), + ), + ) + .finally(() => { + if (this.pending.get(permission.id)?.controller === controller) this.pending.delete(permission.id) + }) + } + + private async route(event: PermissionEvent, signal: AbortSignal) { + const permission = event.properties + const ownership = await this.resolveOwnership(permission, signal) + if (ownership.type === "foreign" || ownership.type === "cancelled") return + if (ownership.type === "exhausted") { + // A cycle or a deleted parent never resolves. Without a reply the nested + // tool call waits on an untimed deferred forever, so answer it. + Effect.runSync( + Effect.logError("permission ancestry unresolvable; rejecting", { + requestID: permission.id, + sessionID: permission.sessionID, + error: ownership.error, + }), + ) + await this.reply(permission.id, "reject").catch(() => {}) + return + } + const session = ownership.value + + // The ACP client displays nested prompts against the managed ancestor. + // Queue by that same ID so sibling leaves cannot open competing prompts + // for one displayed session. + const previous = this.queues.get(session.id) ?? Promise.resolve() const next = previous - .then(() => this.process(event)) - .catch(() => {}) + .then(() => (signal.aborted ? undefined : this.process(event, session))) + .catch((error) => this.rejectOwnedPermission(event, error, session.cwd)) .finally(() => { - if (this.queues.get(permission.sessionID) === next) { - this.queues.delete(permission.sessionID) + if (this.queues.get(session.id) === next) { + this.queues.delete(session.id) } }) - this.queues.set(permission.sessionID, next) + this.queues.set(session.id, next) + await next } - private async process(event: PermissionEvent) { - const permission = event.properties - const session = await Effect.runPromise(this.input.session.tryGet(permission.sessionID)) - if (!session) return + private resolveOwnership(permission: PermissionEvent["properties"], signal: AbortSignal) { + return SessionAncestry.resolve({ + signal, + lookup: (current) => this.lookupOwnership(permission.sessionID, current), + onRetry: (retry) => { + if (retry.attempt !== 1 && retry.attempt % 5 !== 0) return + Effect.runSync( + Effect.logWarning("permission ownership unresolved; retrying ancestry lookup", { + requestID: permission.id, + sessionID: permission.sessionID, + attempt: retry.attempt, + retryIn: retry.retryIn, + error: retry.error, + }), + ) + }, + }) + } + private async lookupOwnership(sessionID: string, signal: AbortSignal): Promise { + try { + const session = await this.resolveManagedAncestor(sessionID, signal) + return session ? { type: "owned", value: session } : { type: "foreign" } + } catch (error) { + return { type: "unknown", error } + } + } + + // Climb the parent chain from `sessionID` and return the first ACP + // session the client created (root or load). Used to route permission + // prompts for nested subagent sessions — the ACP client never sees the + // leaf id, so we report against the managed ancestor instead. Returns + // undefined if the chain reaches the root without hitting a managed + // session, in which case the prompt is foreign (e.g. another ACP + // agent's session). + private async resolveManagedAncestor(sessionID: string, signal: AbortSignal) { + const direct = await Effect.runPromise(this.input.session.tryGet(sessionID)) + if (direct) return direct + let current: string | undefined = sessionID + const seen = new Set() + while (current && !seen.has(current)) { + seen.add(current) + const parentID = await this.lookupParentID(current, signal) + if (!parentID) return undefined + const parent = await Effect.runPromise(this.input.session.tryGet(parentID)) + if (parent) return parent + current = parentID + } + throw new Error(`parent chain cycle detected for ${sessionID}: ${Array.from(seen).join(" -> ")}`) + } + + private async lookupParentID(sessionID: string, signal: AbortSignal): Promise { + if (this.parentIDCache.has(sessionID)) return this.parentIDCache.get(sessionID) + // Only cache successful lookups. Caching `undefined` on a transient + // SDK failure (network blip, server restart mid-flight) would poison + // the chain — a retained permission would re-use the false negative, + // fail to find an ancestor, and the tool call would hang forever. That + // is exactly the symptom this fix is meant to prevent, so each retry + // pays the SDK round-trip again. `throwOnError` + // is required so an HTTP-level failure (5xx during a server restart) + // rejects into the catch instead of resolving to an empty-data envelope + // whose `undefined` parentID would otherwise be cached as a false + // negative. + const res = await this.input.sdk.session.get({ sessionID }, { throwOnError: true, signal }) + if (!res.data) throw new Error(`session lookup returned no data for ${sessionID}`) + const parentID = res.data.parentID + this.parentIDCache.set(sessionID, parentID) + return parentID + } + + private async process(event: PermissionEvent, session: ACPSession.Info) { + const permission = event.properties + // Permissions raised inside a nested subagent carry the deepest + // session's ID, which the ACP client never explicitly created. Walk + // up to the managed ancestor so the prompt is routed somewhere the + // client knows about; otherwise the deferred on the server never + // resolves and the nested tool call (including MCP calls) hangs. if (!this.input.connection.requestPermission) { await this.reply(permission.id, "reject", session.cwd) return } - const result = await this.input.connection - .requestPermission({ - sessionId: permission.sessionID, - toolCall: await permissionToolCall({ - toolCallId: permission.tool?.callID ?? permission.id, - toolName: permission.permission, - input: permission.metadata, - }), - options: permissionOptions, - }) - .catch(async () => { - await this.reply(permission.id, "reject", session.cwd) - return undefined - }) - - if (!result) return + const result = await this.input.connection.requestPermission({ + // Route to the managed ancestor's id (resolved above), not the + // leaf subagent id the client never created. + sessionId: session.id, + toolCall: await permissionToolCall({ + toolCallId: permission.tool?.callID ?? permission.id, + toolName: permission.permission, + input: permission.metadata, + }), + options: permissionOptions, + }) const reply = selectedReply(result) if (reply !== "once" && reply !== "always") { @@ -88,14 +222,48 @@ export class Handler { await this.reply(permission.id, reply, session.cwd) } - private async reply(requestID: string, reply: Reply, directory: string) { - await this.input.sdk.permission.reply({ - requestID, - reply, - directory, + private async rejectOwnedPermission(event: PermissionEvent, error: unknown, directory: string) { + const permission = event.properties + Effect.runSync( + Effect.logError("failed to process owned permission request", { + requestID: permission.id, + sessionID: permission.sessionID, + error, + }), + ) + await this.reply(permission.id, "reject", directory).catch((replyError) => { + Effect.runSync( + Effect.logError("failed to reject owned permission request", { + requestID: permission.id, + sessionID: permission.sessionID, + error: replyError, + }), + ) }) } + private async reply(requestID: string, reply: Reply, directory?: string) { + try { + const result: unknown = await this.input.sdk.permission.reply( + { + requestID, + reply, + ...(directory ? { directory } : {}), + }, + { throwOnError: true }, + ) + if (typeof result === "object" && result !== null && "error" in result && result.error !== undefined) { + throw result.error + } + } catch (error) { + // The server drops a pending request when its session is aborted and + // rejects the cascade itself, so a reply that lost that race is already + // answered. Nothing left to do. + if (isPermissionNotFound(error)) return + throw error + } + } + private async writeProposedEdit(sessionId: string, metadata: ToolInput) { const filepath = stringValue(metadata.filepath) const diff = stringValue(metadata.diff) @@ -115,6 +283,10 @@ export class Handler { } } +function isPermissionNotFound(error: unknown) { + return typeof error === "object" && error !== null && "_tag" in error && error._tag === "PermissionNotFoundError" +} + async function permissionToolCall(input: { readonly toolCallId: string readonly toolName: string diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 3927f615a080..bad48ba2dbc6 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -25,6 +25,7 @@ import { Filesystem } from "@/util/filesystem" import { createOpencodeClient, type OpencodeClient, type ToolPart } from "@opencode-ai/sdk/v2" import { FormatError, FormatUnknownError } from "../error" import { INTERACTIVE_INPUT_ERROR, resolveInteractiveStdin } from "./run/runtime.stdin" +import { DescendantFetchError, fetchDescendantIDs, replyPermission, resolveSessionTreeOwnership } from "./run/tree" type ModelInput = Parameters[0]["model"] @@ -697,123 +698,205 @@ export const RunCommand = effectCmd({ async function loop(client: OpencodeClient, events: Awaited>) { const toggles = new Map() let error: string | undefined + // `tree` is the transitive set of sessions under this run. It is + // the membership oracle for "should this permission prompt count + // as ours" — a nested subagent's permission event carries the + // deepest session's id, and equality against the root id would + // miss it. + // + // Maintenance: + // - seeded with the entire descendant chain so attach mode + // surfaces prompts pending from before the subscription; + // - extended on `session.created`/`session.updated` whenever a + // new session's parent is already in the tree; + // - on out-of-order events (grandchild seen before parent), + // `resolveSessionTreeOwnership` walks the chain via the SDK as a fallback. + const ancestryAbort = new AbortController() + const tree = new Set([sessionID]) + // Resolved in the background. Awaiting it here would delay the `/event` + // subscription below by a round trip, and that route has no replay, so + // anything published in the gap — including the `session.status: idle` + // this loop breaks on — would be lost. + const seeded = fetchDescendantIDs(client, sessionID, { signal: ancestryAbort.signal }) + .catch((error) => { + UI.error( + `failed to seed complete session tree; permission ancestry will be checked on demand (${String(error)})`, + ) + return new Set(error instanceof DescendantFetchError ? error.partial.map((item) => item.id) : []) + }) + .then((ids) => { + for (const id of ids) tree.add(id) + }) + const pendingPermissions = new Map>() - for await (const event of events.stream) { - if ( - event.type === "message.updated" && - event.properties.sessionID === sessionID && - event.properties.info.role === "assistant" && - args.format !== "json" && - toggles.get("start") !== true - ) { - UI.empty() - UI.println(`> ${event.properties.info.agent} · ${event.properties.info.modelID}`) - UI.empty() - toggles.set("start", true) - } - - if (event.type === "message.part.updated") { - const part = event.properties.part - if (part.sessionID !== sessionID) continue - - if (part.type === "tool" && (part.state.status === "completed" || part.state.status === "error")) { - if (emit("tool_use", { part })) continue - if (part.state.status === "completed") { - await tool(part) - continue - } - await toolError(part) - UI.error(part.state.error) + try { + for await (const event of events.stream) { + if (event.type === "session.created" || event.type === "session.updated") { + const info = event.properties.info + if (info.parentID && tree.has(info.parentID)) tree.add(info.id) } if ( - part.type === "tool" && - part.tool === "task" && - part.state.status === "running" && - args.format !== "json" + event.type === "message.updated" && + event.properties.sessionID === sessionID && + event.properties.info.role === "assistant" && + args.format !== "json" && + toggles.get("start") !== true ) { - if (toggles.get(part.id) === true) continue - await tool(part) - toggles.set(part.id, true) + UI.empty() + UI.println(`> ${event.properties.info.agent} · ${event.properties.info.modelID}`) + UI.empty() + toggles.set("start", true) } - if (part.type === "step-start") { - if (emit("step_start", { part })) continue - } + if (event.type === "message.part.updated") { + const part = event.properties.part + if (part.sessionID !== sessionID) continue + + if (part.type === "tool" && (part.state.status === "completed" || part.state.status === "error")) { + if (emit("tool_use", { part })) continue + if (part.state.status === "completed") { + await tool(part) + continue + } + await toolError(part) + UI.error(part.state.error) + } - if (part.type === "step-finish") { - if (emit("step_finish", { part })) continue - } + if ( + part.type === "tool" && + part.tool === "task" && + part.state.status === "running" && + args.format !== "json" + ) { + if (toggles.get(part.id) === true) continue + await tool(part) + toggles.set(part.id, true) + } - if (part.type === "text" && part.time?.end) { - if (emit("text", { part })) continue - const text = part.text.trim() - if (!text) continue - if (!process.stdout.isTTY) { - process.stdout.write(text + EOL) - continue + if (part.type === "step-start") { + if (emit("step_start", { part })) continue + } + + if (part.type === "step-finish") { + if (emit("step_finish", { part })) continue } - UI.empty() - UI.println(text) - UI.empty() - } - if (part.type === "reasoning" && part.time?.end && thinking) { - if (emit("reasoning", { part })) continue - const text = part.text.trim() - if (!text) continue - const line = `Thinking: ${text}` - if (process.stdout.isTTY) { + if (part.type === "text" && part.time?.end) { + if (emit("text", { part })) continue + const text = part.text.trim() + if (!text) continue + if (!process.stdout.isTTY) { + process.stdout.write(text + EOL) + continue + } UI.empty() - UI.println(`${UI.Style.TEXT_DIM}\u001b[3m${line}\u001b[0m${UI.Style.TEXT_NORMAL}`) + UI.println(text) UI.empty() - continue } - process.stdout.write(line + EOL) - } - } - if (event.type === "session.error") { - const props = event.properties - if (props.sessionID !== sessionID || !props.error) continue - let err = String(props.error.name) - if ("data" in props.error && props.error.data && "message" in props.error.data) { - err = String(props.error.data.message) + if (part.type === "reasoning" && part.time?.end && thinking) { + if (emit("reasoning", { part })) continue + const text = part.text.trim() + if (!text) continue + const line = `Thinking: ${text}` + if (process.stdout.isTTY) { + UI.empty() + UI.println(`${UI.Style.TEXT_DIM}\u001b[3m${line}\u001b[0m${UI.Style.TEXT_NORMAL}`) + UI.empty() + continue + } + process.stdout.write(line + EOL) + } } - error = error ? error + EOL + err : err - if (emit("error", { error: props.error })) continue - UI.error(err) - } - if ( - event.type === "session.status" && - event.properties.sessionID === sessionID && - event.properties.status.type === "idle" - ) { - break - } + if (event.type === "session.error") { + const props = event.properties + if (props.sessionID !== sessionID || !props.error) continue + let err = String(props.error.name) + if ("data" in props.error && props.error.data && "message" in props.error.data) { + err = String(props.error.data.message) + } + error = error ? error + EOL + err : err + if (emit("error", { error: props.error })) continue + UI.error(err) + } - if (event.type === "permission.asked") { - const permission = event.properties - if (permission.sessionID !== sessionID) continue + if ( + event.type === "session.status" && + event.properties.sessionID === sessionID && + event.properties.status.type === "idle" + ) { + break + } - if (auto) { - await client.permission.reply({ - requestID: permission.id, - reply: "once", + if (event.type === "permission.asked") { + const permission = event.properties + // Tree membership covers any depth; we still honor "not ours" + // for prompts that belong to a parallel run on the same + // server instance. If a grandchild's `session.created` race + // arrived before this permission, fall back to an SDK ancestry + // walk so we don't silently drop our own prompt. + if (pendingPermissions.has(permission.id)) continue + const task = (async () => { + // The seed may still be in flight, and it is what makes an + // already-pending prompt from before the subscription ours. + await seeded + const ownership = tree.has(permission.sessionID) + ? ({ type: "owned" } as const) + : await resolveSessionTreeOwnership(client, tree, permission.sessionID, { + signal: ancestryAbort.signal, + onUnknown: (retry) => { + if (retry.attempt !== 1 && retry.attempt % 5 !== 0) return + UI.error( + `permission ownership unresolved; retrying ${permission.id} in ${retry.retryIn}ms (${String(retry.error)})`, + ) + }, + }) + if (ownership.type === "foreign" || ownership.type === "cancelled") return + if (ownership.type === "exhausted") { + // A cycle or deleted parent never resolves, and the server + // waits on an untimed deferred, so reject rather than hang. + UI.error( + `permission ancestry unresolvable for ${permission.id}; rejecting (${String(ownership.error)})`, + ) + await replyPermission(client, { requestID: permission.id, reply: "reject" }) + return + } + + // Auto-accept/reject behaviour applies to every descendant + // because they are all part of this run. + if (auto) { + await replyPermission(client, { + requestID: permission.id, + reply: "once", + }) + return + } + + UI.println( + UI.Style.TEXT_WARNING_BOLD + "!", + UI.Style.TEXT_NORMAL + + `permission requested: ${permission.permission} (${permission.patterns.join(", ")}); auto-rejecting`, + ) + await replyPermission(client, { + requestID: permission.id, + reply: "reject", + }) + })().catch((failure) => { + const message = `failed to route permission ${permission.id}: ${String(failure)}` + error = error ? error + EOL + message : message + UI.error(message) }) - } else { - UI.println( - UI.Style.TEXT_WARNING_BOLD + "!", - UI.Style.TEXT_NORMAL + - `permission requested: ${permission.permission} (${permission.patterns.join(", ")}); auto-rejecting`, - ) - await client.permission.reply({ - requestID: permission.id, - reply: "reject", + const pending = task.finally(() => { + if (pendingPermissions.get(permission.id) === pending) pendingPermissions.delete(permission.id) }) + pendingPermissions.set(permission.id, pending) } } + } finally { + ancestryAbort.abort() + await Promise.allSettled(pendingPermissions.values()) } return error } diff --git a/packages/opencode/src/cli/cmd/run/stream.transport.ts b/packages/opencode/src/cli/cmd/run/stream.transport.ts index e4817f514dca..3d0571249ed2 100644 --- a/packages/opencode/src/cli/cmd/run/stream.transport.ts +++ b/packages/opencode/src/cli/cmd/run/stream.transport.ts @@ -42,6 +42,7 @@ import { SUBAGENT_CALL_BOOTSTRAP_LIMIT, type SubagentData, } from "./subagent-data" +import { DescendantFetchError, fetchDescendants } from "./tree" import { traceFooterOutput, writeSessionOutput } from "./stream" import type { FooterApi, @@ -674,7 +675,7 @@ function createLayer(input: StreamInput) { }) const bootstrap = Effect.fn("RunStreamTransport.bootstrap")(function* () { - const [messagesList, children, permissions, questions] = yield* Effect.all( + const [messagesList, descendants, permissions, questions] = yield* Effect.all( [ messages( input.sessionID, @@ -684,13 +685,26 @@ function createLayer(input: StreamInput) { : Math.max(input.replayLimit, SUBAGENT_BOOTSTRAP_LIMIT) : SUBAGENT_BOOTSTRAP_LIMIT, ), - Effect.promise(() => - input.sdk.session.children({ - sessionID: input.sessionID, + // Bootstrap with the full descendant tree, not just direct + // children. `bootstrapSubagentData` only registers blocker + // tabs for sessions in the `children` set, so a one-level + // fetch silently drops grandchild prompts that were already + // pending when the transport started. + Effect.tryPromise({ + try: () => fetchDescendants(input.sdk, input.sessionID, { signal: abort.signal }), + catch: (error) => ({ error }), + }).pipe( + Effect.catch((failure) => { + const error = failure.error + if (!(error instanceof DescendantFetchError)) return Effect.fail(error) + input.trace?.write("bootstrap.descendants.partial", { + failures: error.failures.map((item) => item.sessionID), + descendants: error.partial.map((item) => item.id), + }) + return Effect.logWarning("using partial descendant data after bootstrap lookup failure", { + failures: error.failures.map((item) => item.sessionID), + }).pipe(Effect.as(error.partial)) }), - ).pipe( - Effect.map((item) => item.data ?? []), - Effect.orElseSucceed(() => []), ), Effect.promise(() => input.sdk.permission.list()).pipe( Effect.map((item) => item.data ?? []), @@ -750,7 +764,7 @@ function createLayer(input: StreamInput) { bootstrapSubagentData({ data: state.subagent, messages: messagesList, - children, + children: descendants, permissions, questions, }) diff --git a/packages/opencode/src/cli/cmd/run/subagent-data.ts b/packages/opencode/src/cli/cmd/run/subagent-data.ts index 172741d3b3cc..9567a92d5914 100644 --- a/packages/opencode/src/cli/cmd/run/subagent-data.ts +++ b/packages/opencode/src/cli/cmd/run/subagent-data.ts @@ -798,14 +798,27 @@ export function reduceSubagentData(input: { }) { const event = input.event + // Sub-effect: a task tool part can register a new tab in addition to + // any per-session detail mutation the downstream routing performs. We + // OR this into the eventual return so callers see the tab as a change. + let tabChanged = false + if (event.type === "message.part.updated") { const part = event.properties.part if (part.sessionID === input.sessionID) { - if (part.type !== "tool") { - return false - } + if (part.type !== "tool") return tabChanged + return syncTaskTab(input.data, part) || tabChanged + } - return syncTaskTab(input.data, part) + // A subagent at any depth that spawns another subagent emits its + // task tool part with sessionID = the spawning subagent. We register + // the grandchild's tab here so its later blocker events pass the + // `knownSession` gate below. We rely on the spawning subagent itself + // already being known — the parent's task tool part (state=running) + // is emitted before the spawned child can produce any events, so + // this ordering holds in practice. + if (part.type === "tool" && part.tool === "task" && knownSession(input.data, part.sessionID)) { + tabChanged = syncTaskTab(input.data, part) } } @@ -825,7 +838,7 @@ export function reduceSubagentData(input: { : undefined if (!sessionID || !knownSession(input.data, sessionID)) { - return false + return tabChanged } const detail = ensureDetail(input.data, sessionID) @@ -835,7 +848,7 @@ export function reduceSubagentData(input: { : false if (event.type === "session.status") { if (event.properties.status.type !== "retry") { - return cancelled + return cancelled || tabChanged } return ( @@ -847,7 +860,9 @@ export function reduceSubagentData(input: { source: "system", messageID: `retry:${event.properties.status.attempt}`, }, - ]) || cancelled + ]) || + cancelled || + tabChanged ) } @@ -861,7 +876,9 @@ export function reduceSubagentData(input: { source: "system", messageID: `session.error:${event.properties.sessionID}:${formatError(event.properties.error)}`, }, - ]) || cancelled + ]) || + cancelled || + tabChanged ) } @@ -871,6 +888,8 @@ export function reduceSubagentData(input: { event, thinking: input.thinking, limits: input.limits, - }) || cancelled + }) || + cancelled || + tabChanged ) } diff --git a/packages/opencode/src/cli/cmd/run/tree.ts b/packages/opencode/src/cli/cmd/run/tree.ts new file mode 100644 index 000000000000..7772d86da9f4 --- /dev/null +++ b/packages/opencode/src/cli/cmd/run/tree.ts @@ -0,0 +1,215 @@ +// Shared helpers for walking a session's descendant tree from the SDK. +// The server only exposes a one-level `children` endpoint, so consumers +// that need the transitive set (run-mode permission routing, blocker +// bootstrap, etc.) walk the tree client-side. Centralising the BFS here +// keeps the depth invariant consistent across call sites — see +// `run.ts:loop` and `stream.transport.ts:bootstrap`. + +import type { OpencodeClient } from "@opencode-ai/sdk/v2" +import { SessionAncestry } from "@/session/ancestry" + +export type DescendantInfo = { id: string; title?: string } + +export type SessionTreeOwnership = SessionAncestry.Ownership +export type SessionTreeResolution = SessionAncestry.Resolution + +export type DescendantFetchFailure = { + sessionID: string + error: unknown +} + +export type DescendantFetchOptions = { + signal?: AbortSignal + concurrency?: number + timeoutMs?: number + totalTimeoutMs?: number + limit?: number +} + +export class DescendantFetchError extends Error { + readonly partial: DescendantInfo[] + readonly failures: DescendantFetchFailure[] + + constructor(partial: DescendantInfo[], failures: DescendantFetchFailure[]) { + super(`failed to fetch children for ${failures.map((item) => item.sessionID).join(", ")}`) + this.name = "DescendantFetchError" + this.partial = partial + this.failures = failures + } +} + +export async function fetchDescendants( + sdk: OpencodeClient, + sessionID: string, + input: DescendantFetchOptions = {}, +): Promise { + const out: DescendantInfo[] = [] + const failures: DescendantFetchFailure[] = [] + const seen = new Set([sessionID]) + const concurrency = positiveInteger(input.concurrency, 4) + const timeoutMs = positiveInteger(input.timeoutMs, 10_000) + const totalTimeoutMs = positiveInteger(input.totalTimeoutMs, 30_000) + const limit = positiveInteger(input.limit, 1_000) + const deadline = Date.now() + totalTimeoutMs + let limited = false + let expired = false + let frontier = [sessionID] + while (frontier.length) { + input.signal?.throwIfAborted() + const current = frontier + const next: string[] = [] + for (let offset = 0; offset < current.length; offset += concurrency) { + input.signal?.throwIfAborted() + const group = current.slice(offset, offset + concurrency) + const remaining = deadline - Date.now() + if (remaining <= 0) { + failures.push({ sessionID: group[0]!, error: new Error(`session tree lookup exceeded ${totalTimeoutMs}ms`) }) + expired = true + break + } + const batches = await Promise.allSettled( + group.map(async (id) => { + const timeout = new AbortController() + const timer = setTimeout( + () => timeout.abort(new Error(`children lookup timed out for ${id}`)), + Math.min(timeoutMs, remaining), + ) + const signal = input.signal ? AbortSignal.any([input.signal, timeout.signal]) : timeout.signal + try { + const result = await sdk.session.children({ sessionID: id }, { throwOnError: true, signal }) + if (!result.data) throw new Error(`children lookup returned no data for ${id}`) + return result.data + } finally { + clearTimeout(timer) + } + }), + ) + input.signal?.throwIfAborted() + for (const [index, batch] of batches.entries()) { + if (batch.status === "rejected") { + failures.push({ sessionID: group[index]!, error: batch.reason }) + continue + } + for (const child of batch.value) { + if (seen.has(child.id)) continue + if (seen.size >= limit) { + if (!limited) { + failures.push({ sessionID: child.id, error: new Error(`session tree exceeds ${limit} entries`) }) + limited = true + } + continue + } + seen.add(child.id) + out.push(child) + next.push(child.id) + } + } + } + if (expired) break + frontier = next + } + if (failures.length) throw new DescendantFetchError(out, failures) + return out +} + +function positiveInteger(value: number | undefined, fallback: number) { + if (value === undefined || !Number.isFinite(value) || value <= 0) return fallback + return Math.floor(value) +} + +// Like `fetchDescendants` but returns just the id set including the root. +// Useful when the caller wants membership tests rather than the full info. +export async function fetchDescendantIDs( + sdk: OpencodeClient, + sessionID: string, + input: DescendantFetchOptions = {}, +): Promise> { + const ids = new Set([sessionID]) + for (const child of await fetchDescendants(sdk, sessionID, input)) { + ids.add(child.id) + } + return ids +} + +export function resolveSessionTreeOwnership( + sdk: OpencodeClient, + tree: Set, + sessionID: string, + input: { + signal: AbortSignal + onUnknown?: (input: { error: unknown; attempt: number; retryIn: number }) => void + attempts?: number + }, +): Promise { + return SessionAncestry.resolve({ + signal: input.signal, + onRetry: input.onUnknown, + attempts: input.attempts, + lookup: (signal) => lookupSessionTreeOwnership(sdk, tree, sessionID, signal), + }) +} + +export async function lookupSessionTreeOwnership( + sdk: OpencodeClient, + tree: Set, + sessionID: string, + signal?: AbortSignal, +): Promise { + try { + const owned = await addAncestryToTree(sdk, tree, sessionID, signal) + return owned ? { type: "owned", value: undefined } : { type: "foreign" } + } catch (error) { + return { type: "unknown", error } + } +} + +async function addAncestryToTree(sdk: OpencodeClient, tree: Set, sessionID: string, signal?: AbortSignal) { + if (tree.has(sessionID)) return true + + const chain: string[] = [] + const seen = new Set() + let current: string | undefined = sessionID + while (current) { + if (seen.has(current)) { + throw new Error(`parent chain cycle detected for ${sessionID}: ${[...chain, current].join(" -> ")}`) + } + seen.add(current) + chain.push(current) + if (tree.has(current)) { + for (const id of chain) tree.add(id) + return true + } + + const result: { data?: { parentID?: string }; error?: unknown } = await sdk.session.get( + { sessionID: current }, + { throwOnError: true, ...(signal ? { signal } : {}) }, + ) + if (result.error !== undefined) throw result.error + if (!result.data) throw new Error(`session lookup returned no data for ${current}`) + current = result.data.parentID + } + return false +} + +export async function replyPermission( + sdk: OpencodeClient, + input: { requestID: string; reply: "once" | "always" | "reject"; directory?: string }, +) { + try { + const result: unknown = await sdk.permission.reply(input, { throwOnError: true }) + if (typeof result === "object" && result !== null && "error" in result && result.error !== undefined) { + throw result.error + } + } catch (error) { + // The server drops a pending request when its session is aborted or the + // turn tears down, and it also rejects the whole cascade itself. A reply + // that lost that race is already answered, so treat it as done rather than + // failing the run. + if (isPermissionNotFound(error)) return + throw error + } +} + +function isPermissionNotFound(error: unknown) { + return typeof error === "object" && error !== null && "_tag" in error && error._tag === "PermissionNotFoundError" +} diff --git a/packages/opencode/src/session/ancestry.ts b/packages/opencode/src/session/ancestry.ts new file mode 100644 index 000000000000..2e59ae354869 --- /dev/null +++ b/packages/opencode/src/session/ancestry.ts @@ -0,0 +1,66 @@ +// Shared bounded retry for "which session that I own does this descendant +// belong to" lookups. Both the ACP permission handler and run mode need it, and +// keeping one implementation stops the two from drifting apart. +// +// The lookup fails transiently for a server restart mid-flight, a 5xx, or a +// grandchild whose `session.created` has not been projected yet — all worth +// retrying. It also fails permanently for a parent chain cycle or a `parentID` +// pointing at a deleted session. Retrying forever turns those into a permanent +// hang, which is the exact symptom this routing exists to remove, so the retry +// is bounded and exhaustion is reported so callers can reject the request +// instead of leaving the nested tool call waiting. + +export type Ownership = { type: "owned"; value: T } | { type: "foreign" } | { type: "unknown"; error: unknown } + +export type Resolution = + | { type: "owned"; value: T } + | { type: "foreign" } + | { type: "cancelled" } + | { type: "exhausted"; error: unknown } + +// 100, 200, 400, 800, 1600, 2000, 2000 -> about 7s before giving up. Long enough +// to ride out a server restart, short enough that a cycle surfaces promptly. +export const MAX_ATTEMPTS = 8 + +export function retryDelay(attempt: number) { + return Math.min(100 * 2 ** Math.min(attempt - 1, 4), 2_000) +} + +export async function resolve(input: { + signal: AbortSignal + lookup: (signal: AbortSignal) => Promise> + onRetry?: (input: { error: unknown; attempt: number; retryIn: number }) => void + attempts?: number +}): Promise> { + const attempts = input.attempts ?? MAX_ATTEMPTS + let last: unknown + for (let attempt = 1; attempt <= attempts; attempt++) { + if (input.signal.aborted) return { type: "cancelled" } + const ownership = await input.lookup(input.signal) + if (input.signal.aborted) return { type: "cancelled" } + if (ownership.type !== "unknown") return ownership + last = ownership.error + if (attempt === attempts) break + const retryIn = retryDelay(attempt) + input.onRetry?.({ error: ownership.error, attempt, retryIn }) + if (!(await wait(retryIn, input.signal))) return { type: "cancelled" } + } + return { type: "exhausted", error: last } +} + +function wait(delay: number, signal: AbortSignal) { + if (signal.aborted) return Promise.resolve(false) + return new Promise((resolve) => { + const finish = (ready: boolean) => { + clearTimeout(timer) + signal.removeEventListener("abort", onAbort) + resolve(ready) + } + const onAbort = () => finish(false) + const timer = setTimeout(() => finish(true), delay) + signal.addEventListener("abort", onAbort, { once: true }) + if (signal.aborted) onAbort() + }) +} + +export * as SessionAncestry from "./ancestry" diff --git a/packages/opencode/test/acp/permission.test.ts b/packages/opencode/test/acp/permission.test.ts index 99da5e6ac13f..aab96860f1cb 100644 --- a/packages/opencode/test/acp/permission.test.ts +++ b/packages/opencode/test/acp/permission.test.ts @@ -13,6 +13,7 @@ import { mkdtemp, rm } from "node:fs/promises" import { tmpdir } from "node:os" import path from "node:path" import { ACPEvent } from "@/acp/event" +import { ACPPermission } from "@/acp/permission" import { ACPSession } from "@/acp/session" type PermissionEvent = Extract @@ -44,8 +45,11 @@ function makeSessionService() { } function createHarness( - requestPermission: (params: RequestPermissionRequest) => Promise = () => - Promise.resolve({ outcome: { outcome: "selected", optionId: "once" } }), + input: { + requestPermission?: (params: RequestPermissionRequest) => Promise + sessionGet?: (sessionID: string) => Promise<{ parentID?: string }> + permissionReply?: (params: PermissionReplyParams) => Promise<{ data?: boolean; error?: unknown }> + } = {}, ) { const replies: PermissionReplyParams[] = [] const requests: RequestPermissionRequest[] = [] @@ -55,17 +59,24 @@ function createHarness( permission: { reply: (params: PermissionReplyParams) => { replies.push(params) - return Promise.resolve({ data: true }) + return input.permissionReply?.(params) ?? Promise.resolve({ data: true }) }, }, session: { message: () => Promise.resolve({ data: undefined }), + get: (params: { sessionID: string }) => + ( + input.sessionGet?.(params.sessionID) ?? + Promise.reject(new Error(`unexpected session lookup: ${params.sessionID}`)) + ).then((data) => ({ data })), }, } as unknown as OpencodeClient const connection = { requestPermission: (params: RequestPermissionRequest) => { requests.push(params) - return requestPermission(params) + return ( + input.requestPermission?.(params) ?? Promise.resolve({ outcome: { outcome: "selected", optionId: "once" } }) + ) }, sessionUpdate: (params: SessionUpdateParams) => { updates.push(params) @@ -73,8 +84,9 @@ function createHarness( }, } satisfies Pick const subscription = new ACPEvent.Subscription({ sdk, connection, session }) + const handler = new ACPPermission.Handler({ sdk, connection, session }) - return { connection, replies, requests, sdk, session, subscription, updates } + return { connection, handler, replies, requests, sdk, session, subscription, updates } } async function createSession(session: ACPSession.Interface, sessionId: string, cwd = "/workspace") { @@ -330,7 +342,9 @@ describe("acp permissions", () => { }) it("rejects non-selected outcomes", async () => { - const harness = createHarness(() => Promise.resolve({ outcome: { outcome: "cancelled" } })) + const harness = createHarness({ + requestPermission: () => Promise.resolve({ outcome: { outcome: "cancelled" } }), + }) await createSession(harness.session, "ses_a") harness.subscription.handle(permissionAsked("ses_a", "perm_cancelled")) @@ -341,7 +355,9 @@ describe("acp permissions", () => { }) it("rejects when requestPermission fails", async () => { - const harness = createHarness(() => Promise.reject(new Error("client permission UI failed"))) + const harness = createHarness({ + requestPermission: () => Promise.reject(new Error("client permission UI failed")), + }) await createSession(harness.session, "ses_a") harness.subscription.handle(permissionAsked("ses_a", "perm_failed")) @@ -351,12 +367,184 @@ describe("acp permissions", () => { expect(harness.replies[0]).toMatchObject({ requestID: "perm_failed", reply: "reject" }) }) + it("routes a nested permission through its managed ancestor", async () => { + const parents = new Map([ + ["ses_leaf", "ses_middle"], + ["ses_middle", "ses_root"], + ]) + const harness = createHarness({ + sessionGet: async (sessionID) => { + if (!parents.has(sessionID)) throw new Error(`unexpected session lookup: ${sessionID}`) + return { parentID: parents.get(sessionID) } + }, + }) + await createSession(harness.session, "ses_root") + + harness.subscription.handle(permissionAsked("ses_leaf", "perm_nested")) + + await pollUntil(() => harness.replies.length === 1, "nested permission was never replied") + expect(harness.requests).toHaveLength(1) + expect(harness.requests[0]?.sessionId).toBe("ses_root") + expect(harness.replies[0]).toMatchObject({ requestID: "perm_nested", reply: "once" }) + }) + + it("rejects a permission whose ancestry never resolves", async () => { + // Permission.ask waits on a deferred with no timeout, so an ancestry that + // can never resolve — a parent chain cycle, or a parentID pointing at a + // deleted session — hangs the nested tool call forever unless the retry is + // bounded and exhaustion answers the request. + const harness = createHarness({ + sessionGet: (sessionID) => Promise.resolve({ parentID: sessionID === "ses_leaf" ? "ses_mid" : "ses_leaf" }), + }) + await createSession(harness.session, "ses_root") + + harness.handler.handle(permissionAsked("ses_leaf", "perm_cycle")) + + await pollUntil(() => harness.replies.length === 1, "unresolvable permission was never answered", { + timeoutMs: 20_000, + }) + expect(harness.replies[0]).toMatchObject({ requestID: "perm_cycle", reply: "reject" }) + expect(harness.requests).toHaveLength(0) + }, 30_000) + + it("treats a missing permission request as already answered", async () => { + // The server drops pending requests when a session aborts and rejects the + // cascade itself, so losing that race must not surface as a failure. + const harness = createHarness({ + sessionGet: () => Promise.resolve({ parentID: undefined }), + permissionReply: () => + Promise.reject({ _tag: "PermissionNotFoundError", requestID: "perm_gone", message: "gone" }), + }) + await createSession(harness.session, "ses_root") + + harness.handler.handle(permissionAsked("ses_root", "perm_gone")) + + await pollUntil(() => harness.replies.length === 1, "permission was never replied") + // One reply attempt, and no reject cascade from a thrown handler. + await Bun.sleep(50) + expect(harness.replies).toHaveLength(1) + }) + + it("cancels a retained unknown ownership retry without replying", async () => { + let lookups = 0 + const harness = createHarness({ + sessionGet: () => { + lookups += 1 + return Promise.reject(new Error("session service unavailable")) + }, + }) + + harness.handler.handle(permissionAsked("ses_leaf", "perm_lookup_failed")) + + await pollUntil(() => lookups >= 2, "ancestor lookup was not retained for retry") + harness.handler.stop() + const stoppedAt = lookups + await Bun.sleep(250) + expect(lookups).toBe(stoppedAt) + expect(harness.requests).toHaveLength(0) + expect(harness.replies).toHaveLength(0) + }) + + it("stops retained retries with the event subscription", async () => { + let lookups = 0 + const harness = createHarness({ + sessionGet: () => { + lookups += 1 + return Promise.reject(new Error("session service unavailable")) + }, + }) + + harness.subscription.handle(permissionAsked("ses_leaf", "perm_subscription_stop")) + await pollUntil(() => lookups >= 2, "ancestor lookup was not retained for retry") + harness.subscription.stop() + const stoppedAt = lookups + await Bun.sleep(250) + + expect(lookups).toBe(stoppedAt) + expect(harness.requests).toHaveLength(0) + }) + + it("does not let a retained unknown ownership retry block an owned permission", async () => { + let lookups = 0 + const harness = createHarness({ + sessionGet: () => { + lookups += 1 + return Promise.reject(new Error("session service unavailable")) + }, + }) + await createSession(harness.session, "ses_owned") + + harness.handler.handle(permissionAsked("ses_unknown", "perm_unknown")) + await pollUntil(() => lookups >= 1, "unknown ancestry lookup did not start") + harness.handler.handle(permissionAsked("ses_owned", "perm_owned")) + + await pollUntil( + () => harness.replies.some((reply) => reply.requestID === "perm_owned"), + "owned permission was blocked", + ) + harness.handler.stop() + expect(harness.requests).toHaveLength(1) + expect(harness.replies).toEqual([ + expect.objectContaining({ requestID: "perm_owned", reply: "once", directory: expect.any(String) }), + ]) + }) + + it("does not reply to a foreign session permission", async () => { + let lookups = 0 + const harness = createHarness({ + sessionGet: async () => { + lookups += 1 + return { parentID: undefined } + }, + }) + + harness.subscription.handle(permissionAsked("ses_foreign", "perm_foreign")) + + await pollUntil(() => lookups === 1, "foreign ancestry was never resolved") + expect(harness.requests).toHaveLength(0) + expect(harness.replies).toHaveLength(0) + }) + + it("eventually routes an owned permission after prolonged ancestry failures", async () => { + let lookups = 0 + const harness = createHarness({ + sessionGet: async () => { + lookups += 1 + if (lookups < 4) throw new Error("session service unavailable") + return { parentID: "ses_root" } + }, + }) + await createSession(harness.session, "ses_root") + + harness.subscription.handle(permissionAsked("ses_leaf", "perm_retry_owned")) + + await pollUntil(() => harness.replies.length === 1, "owned permission was not routed after ancestry retry", { + timeoutMs: 3000, + }) + expect(lookups).toBe(4) + expect(harness.requests[0]?.sessionId).toBe("ses_root") + expect(harness.replies[0]).toMatchObject({ requestID: "perm_retry_owned", reply: "once" }) + }) + + it("rejects an owned permission when the SDK returns a reply error envelope", async () => { + const harness = createHarness({ + permissionReply: async (params) => + params.reply === "once" ? { error: new Error("reply failed") } : { data: true }, + }) + await createSession(harness.session, "ses_a") + + harness.subscription.handle(permissionAsked("ses_a", "perm_reply_error")) + + await pollUntil(() => harness.replies.length === 2, "reply error envelope was not handled") + expect(harness.replies.map((item) => item.reply)).toEqual(["once", "reject"]) + }) + it("does not let a blocked session A permission block session B message updates", async () => { let releasePermission: (() => void) | undefined const blocked = new Promise((resolve) => { releasePermission = () => resolve({ outcome: { outcome: "selected", optionId: "once" } }) }) - const harness = createHarness(() => blocked) + const harness = createHarness({ requestPermission: () => blocked }) await createSession(harness.session, "ses_a") await createSession(harness.session, "ses_b") await createKnownTextPart(harness.session, "ses_b", "msg_b", "part_b") @@ -378,9 +566,12 @@ describe("acp permissions", () => { const first = new Promise((resolve) => { releaseFirst = () => resolve({ outcome: { outcome: "selected", optionId: "once" } }) }) - const harness = createHarness(() => - harness.requests.length === 1 ? first : Promise.resolve({ outcome: { outcome: "selected", optionId: "always" } }), - ) + const harness = createHarness({ + requestPermission: () => + harness.requests.length === 1 + ? first + : Promise.resolve({ outcome: { outcome: "selected", optionId: "always" } }), + }) await createSession(harness.session, "ses_a") harness.subscription.handle(permissionAsked("ses_a", "perm_1")) @@ -398,4 +589,86 @@ describe("acp permissions", () => { ["perm_2", "always"], ]) }) + + it("ignores a duplicate event while its permission is pending", async () => { + let release: (() => void) | undefined + const pending = new Promise((resolve) => { + release = () => resolve({ outcome: { outcome: "selected", optionId: "once" } }) + }) + const harness = createHarness({ requestPermission: () => pending }) + await createSession(harness.session, "ses_a") + const event = permissionAsked("ses_a", "perm_duplicate") + + harness.handler.handle(event) + harness.handler.handle(event) + await pollUntil(() => harness.requests.length === 1, "permission was never requested") + release?.() + await pollUntil(() => harness.replies.length === 1, "permission was never replied") + + expect(harness.requests).toHaveLength(1) + expect(harness.replies).toHaveLength(1) + }) + + it("does not start queued permissions after stop", async () => { + let release: (() => void) | undefined + const first = new Promise((resolve) => { + release = () => resolve({ outcome: { outcome: "selected", optionId: "once" } }) + }) + const harness = createHarness({ + requestPermission: () => + harness.requests.length === 1 ? first : Promise.resolve({ outcome: { outcome: "selected", optionId: "once" } }), + }) + await createSession(harness.session, "ses_a") + + harness.handler.handle(permissionAsked("ses_a", "perm_active")) + harness.handler.handle(permissionAsked("ses_a", "perm_queued")) + await pollUntil(() => harness.requests.length === 1, "first permission was never requested") + harness.handler.stop() + release?.() + await pollUntil(() => harness.replies.length === 1, "active permission was never replied") + await Bun.sleep(50) + + expect(harness.requests).toHaveLength(1) + expect(harness.replies[0]?.requestID).toBe("perm_active") + }) + + it("serializes sibling leaf permissions by managed ancestor", async () => { + let releaseFirst: (() => void) | undefined + const first = new Promise((resolve) => { + releaseFirst = () => resolve({ outcome: { outcome: "selected", optionId: "once" } }) + }) + let active = 0 + let maxActive = 0 + const harness = createHarness({ + sessionGet: async (sessionID) => { + if (sessionID !== "ses_leaf_a" && sessionID !== "ses_leaf_b") { + throw new Error(`unexpected session lookup: ${sessionID}`) + } + return { parentID: "ses_root" } + }, + requestPermission: async () => { + active += 1 + maxActive = Math.max(maxActive, active) + try { + if (harness.requests.length === 1) return await first + return { outcome: { outcome: "selected", optionId: "once" } } + } finally { + active -= 1 + } + }, + }) + await createSession(harness.session, "ses_root") + + harness.subscription.handle(permissionAsked("ses_leaf_a", "perm_leaf_a")) + harness.subscription.handle(permissionAsked("ses_leaf_b", "perm_leaf_b")) + + await pollUntil(() => harness.requests.length === 1, "first sibling permission was never requested") + expect(harness.requests[0]?.sessionId).toBe("ses_root") + releaseFirst?.() + await pollUntil(() => harness.replies.length === 2, "sibling permissions were not both replied") + + expect(harness.requests).toHaveLength(2) + expect(harness.requests.every((request) => request.sessionId === "ses_root")).toBe(true) + expect(maxActive).toBe(1) + }) }) diff --git a/packages/opencode/test/cli/acp/nested-permission.test.ts b/packages/opencode/test/cli/acp/nested-permission.test.ts new file mode 100644 index 000000000000..74876a8730fb --- /dev/null +++ b/packages/opencode/test/cli/acp/nested-permission.test.ts @@ -0,0 +1,99 @@ +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 } from "./acp-test-client" +import { initialize, newSession, verifierConfig } from "./helpers" + +type Message = { + id?: number + method?: string + params?: { sessionId?: string } + result?: unknown +} + +describe("acp nested permissions", () => { + cliIt.live( + "routes a nested permission through the managed session", + ({ home, llm, opencode }) => + Effect.gen(function* () { + const raw = yield* opencode.acp({ + env: { + OPENCODE_CONFIG_CONTENT: JSON.stringify({ + ...verifierConfig(llm.url), + // Required: the default of 1 stops the child from delegating, so + // the write permission would come from the child and the test would + // pass on depth-1 routing without ever reaching a grandchild. + subagent_depth: 2, + permission: { task: "allow", edit: "ask" }, + agent: { + nested: { + description: "Delegates nested test work.", + mode: "subagent", + permission: { task: "allow", edit: "ask" }, + }, + }, + }), + }, + }) + const acp = createAcpClient(raw) + yield* initialize(acp) + const session = yield* newSession(acp, home) + + yield* llm.tool("task", { + description: "delegate once", + prompt: "Delegate the file write to another agent.", + subagent_type: "nested", + }) + yield* llm.tool("task", { + description: "delegate twice", + prompt: "Write nested-denied.txt.", + subagent_type: "nested", + }) + yield* llm.tool("write", { filePath: "nested-denied.txt", content: "blocked" }) + yield* llm.text("grandchild done") + yield* llm.text("child done") + yield* llm.text("root done") + + const promptID = 9999 + yield* raw.send({ + jsonrpc: "2.0", + id: promptID, + method: "session/prompt", + params: { sessionId: session.sessionId, prompt: [{ type: "text", text: "delegate twice" }] }, + }) + + const permissionSessions: string[] = [] + const outcome = yield* Effect.gen(function* () { + while (true) { + const message = (yield* raw.receive.pipe(Effect.timeout(Duration.seconds(10)))) as Message + if (message.method === "session/request_permission" && message.id !== undefined) { + if (message.params?.sessionId) permissionSessions.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 === promptID) return message + } + }).pipe(Effect.timeout(Duration.seconds(20)), Effect.exit) + + expect(outcome._tag).toBe("Success") + if (outcome._tag !== "Success") return + expect((outcome.value.result as PromptResponse | undefined)?.stopReason).toBeDefined() + // The prompt is displayed against the managed root even though it + // originated two levels down, which is the whole point of the routing. + expect(permissionSessions).toEqual([session.sessionId]) + // Only a grandchild is ever prompted with the second delegation, so its + // presence proves the permission came from depth 2 rather than the child. + const inputs = JSON.stringify(yield* llm.inputs) + expect(inputs).toContain("Write nested-denied.txt.") + expect(inputs).not.toContain("depth limit") + expect(yield* Effect.promise(() => Bun.file(path.join(home, "nested-denied.txt")).exists())).toBe(false) + }), + 40_000, + ) +}) diff --git a/packages/opencode/test/cli/run/run-process.test.ts b/packages/opencode/test/cli/run/run-process.test.ts index bd5847e2723c..ed0bc43088b1 100644 --- a/packages/opencode/test/cli/run/run-process.test.ts +++ b/packages/opencode/test/cli/run/run-process.test.ts @@ -7,6 +7,7 @@ import { describe, expect } from "bun:test" import { Effect } from "effect" import { reply } from "../../lib/llm-server" import { cliIt } from "../../lib/cli-process" +import { testProviderConfig } from "../../lib/test-provider" describe("opencode run (non-interactive subprocess)", () => { // Happy path: prompt completes, output reaches stdout, process exits 0. @@ -277,6 +278,63 @@ describe("opencode run (non-interactive subprocess)", () => { 60_000, ) + cliIt.concurrent( + "answers permission requests from nested Task sessions", + ({ home, llm, opencode }) => + Effect.gen(function* () { + yield* llm.push( + reply().tool("task", { + description: "delegate once", + prompt: "Delegate the marker file to another agent.", + subagent_type: "nested", + }), + reply().tool("task", { + description: "delegate twice", + prompt: "Create the nested marker file.", + subagent_type: "nested", + }), + reply().tool("bash", { + command: `touch ${JSON.stringify(`${home}/nested-approved`)}`, + description: "Create the nested marker", + }), + reply().text("grandchild completed"), + reply().text("child completed"), + reply().text("root completed"), + ) + + const result = yield* opencode.run("delegate this task twice", { + extraArgs: ["--auto"], + timeoutMs: 30_000, + env: { + OPENCODE_CONFIG_CONTENT: JSON.stringify({ + ...testProviderConfig(llm.url), + // Required: the default of 1 stops the child from delegating, so + // the bash permission would come from the child and the test would + // never exercise a grandchild at all. + subagent_depth: 2, + permission: { task: "allow", bash: "ask" }, + agent: { + nested: { + description: "Delegates nested test work.", + mode: "subagent", + permission: { task: "allow", bash: "ask" }, + }, + }, + }), + }, + }) + + opencode.expectExit(result, 0) + expect(yield* Effect.promise(() => Bun.file(`${home}/nested-approved`).exists())).toBe(true) + // Only a grandchild is ever prompted with the second delegation, so its + // presence proves the permission came from depth 2 rather than the child. + const inputs = JSON.stringify(yield* llm.inputs) + expect(inputs).toContain("Create the nested marker file.") + expect(inputs).not.toContain("depth limit") + }), + 60_000, + ) + cliIt.live( "attach mode sends client-local file contents without a shared path", ({ home, llm, opencode }) => diff --git a/packages/opencode/test/cli/run/stream.transport.test.ts b/packages/opencode/test/cli/run/stream.transport.test.ts index 5bb578447f1c..37b00b1a0baf 100644 --- a/packages/opencode/test/cli/run/stream.transport.test.ts +++ b/packages/opencode/test/cli/run/stream.transport.test.ts @@ -1239,6 +1239,116 @@ describe("run stream transport", () => { } }) + test("continues bootstrap with partial descendants when one branch fails", async () => { + const src = eventFeed() + const ui = footer() + const trace = mock((_type: string, _data?: unknown) => {}) + const transport = await createSessionTransport({ + sdk: sdk({ + stream: src.stream, + children: async ({ sessionID }) => { + if (sessionID === "session-1") return ok([child("child-1")]) + throw new Error("child lookup unavailable") + }, + permissions: async () => + ok([ + { + id: "perm-partial", + sessionID: "child-1", + permission: "edit", + patterns: ["src/partial.ts"], + metadata: {}, + always: [], + }, + ]), + }), + sessionID: "session-1", + thinking: true, + limits: () => ({}), + footer: ui.api, + trace: { write: trace }, + }) + + try { + const state = await waitFor(() => { + const item = ui.events.findLast((event) => event.type === "stream.subagent") + const state = item?.type === "stream.subagent" ? item.state : undefined + return state?.permissions.some((permission) => permission.id === "perm-partial") ? state : undefined + }) + expect(state.tabs).toEqual([expect.objectContaining({ sessionID: "child-1" })]) + expect(trace).toHaveBeenCalledWith("bootstrap.descendants.partial", { + failures: ["child-1"], + descendants: ["child-1"], + }) + } finally { + src.close() + await transport.close() + } + }) + + test("bootstraps blockers from a grandchild session", async () => { + const src = eventFeed() + const ui = footer() + const transport = await createSessionTransport({ + sdk: sdk({ + stream: src.stream, + children: async ({ sessionID }) => { + if (sessionID === "session-1") return ok([child("child-1")]) + if (sessionID === "child-1") return ok([child("grandchild-1")]) + return ok([]) + }, + permissions: async () => + ok([ + { + id: "perm-grandchild", + sessionID: "grandchild-1", + permission: "edit", + patterns: ["src/nested.ts"], + metadata: {}, + always: [], + }, + ]), + questions: async () => + ok([ + { + id: "question-grandchild", + sessionID: "grandchild-1", + questions: [ + { + question: "Which file?", + header: "File", + options: [{ label: "nested.ts", description: "Use the nested file." }], + multiple: false, + }, + ], + }, + ]), + }), + sessionID: "session-1", + thinking: true, + limits: () => ({}), + footer: ui.api, + }) + + try { + const state = await waitFor(() => { + const item = ui.events.findLast((event) => event.type === "stream.subagent") + const state = item?.type === "stream.subagent" ? item.state : undefined + return state?.permissions.some((permission) => permission.id === "perm-grandchild") && + state.questions.some((question) => question.id === "question-grandchild") + ? state + : undefined + }) + + expect(state.tabs).toEqual([expect.objectContaining({ sessionID: "grandchild-1" })]) + expect(state.permissions[0]?.sessionID).toBe("grandchild-1") + expect(state.questions[0]?.sessionID).toBe("grandchild-1") + } finally { + src.close() + await transport.close() + } + }) + test("bootstraps child tabs and resumed blocker input", async () => { const src = eventFeed() const ui = footer() diff --git a/packages/opencode/test/cli/run/subagent-data.test.ts b/packages/opencode/test/cli/run/subagent-data.test.ts index 4dcbd09608bf..301d8aecf241 100644 --- a/packages/opencode/test/cli/run/subagent-data.test.ts +++ b/packages/opencode/test/cli/run/subagent-data.test.ts @@ -544,4 +544,77 @@ describe("run subagent data", () => { }), ]) }) + + // Regression for nested subagent permission hang: when a subagent spawns + // its own subagent via the task tool, the spawning subagent's message + // emits the task part with `part.sessionID = spawning subagent`, not the + // run's root. `reduceSubagentData` must register a tab for the spawned + // grandchild from that event, otherwise the grandchild's subsequent + // `permission.asked` event fails the `knownSession` gate and the prompt + // is silently dropped. + test("registers a tab for a grandchild spawned by an existing subagent", () => { + const data = createSubagentData() + + bootstrapSubagentData({ + data, + messages: [taskMessage("child-1", "running")], + children: [{ id: "child-1" }], + permissions: [], + questions: [], + }) + + const changed = reduce(data, { + type: "message.part.updated", + properties: { + part: { + id: "part-grandchild-1", + messageID: "msg-child-1-task", + sessionID: "child-1", + type: "tool", + callID: "call-grandchild-1", + tool: "task", + state: { + status: "running", + input: { + description: "Deep dive", + subagent_type: "explore", + }, + title: "Deep dive", + metadata: { + sessionId: "grandchild-1", + }, + time: { start: 4 }, + }, + }, + }, + }) + + expect(changed).toBe(true) + expect( + snapshotSubagentData(data) + .tabs.map((tab) => tab.sessionID) + .sort(), + ).toEqual(["child-1", "grandchild-1"]) + + // A permission asked on the grandchild must now route into the + // grandchild's detail view rather than being dropped. + const permChanged = reduce(data, { + type: "permission.asked", + properties: { + id: "perm-grandchild", + sessionID: "grandchild-1", + permission: "bash", + patterns: ["ls"], + metadata: {}, + always: [], + tool: { + messageID: "msg-grandchild-tool", + callID: "call-grandchild-tool", + }, + }, + }) + + expect(permChanged).toBe(true) + expect(snapshotSubagentData(data).permissions.map((p) => p.id)).toContain("perm-grandchild") + }) }) diff --git a/packages/opencode/test/cli/run/tree.test.ts b/packages/opencode/test/cli/run/tree.test.ts new file mode 100644 index 000000000000..13a66350bc4e --- /dev/null +++ b/packages/opencode/test/cli/run/tree.test.ts @@ -0,0 +1,295 @@ +import { describe, expect, test } from "bun:test" +import type { OpencodeClient } from "@opencode-ai/sdk/v2" +import { + DescendantFetchError, + fetchDescendants, + lookupSessionTreeOwnership, + replyPermission, + resolveSessionTreeOwnership, +} from "@/cli/cmd/run/tree" + +function client(input: { + children?: (sessionID: string, signal?: AbortSignal) => Promise> + get?: (sessionID: string) => Promise<{ parentID?: string }> + reply?: (input: { requestID: string; reply: "once" | "always" | "reject" }) => Promise<{ + data?: boolean + error?: unknown + }> +}) { + return { + permission: { + reply: (params: { requestID: string; reply: "once" | "always" | "reject" }) => input.reply?.(params), + }, + session: { + children: (params: { sessionID: string }, options?: { signal?: AbortSignal }) => + input.children?.(params.sessionID, options?.signal).then((data) => ({ data })), + get: (params: { sessionID: string }) => input.get?.(params.sessionID).then((data) => ({ data })), + }, + } as unknown as OpencodeClient +} + +describe("run session tree", () => { + test("surfaces descendant lookup failures with successfully traversed branches", async () => { + const sdk = client({ + children: async (sessionID) => { + if (sessionID === "root") return [{ id: "child-a" }, { id: "child-b" }] + if (sessionID === "child-a") throw new Error("child-a unavailable") + if (sessionID === "child-b") return [{ id: "grandchild-b" }] + return [] + }, + }) + + try { + await fetchDescendants(sdk, "root") + throw new Error("expected descendant traversal to fail") + } catch (error) { + expect(error).toBeInstanceOf(DescendantFetchError) + if (!(error instanceof DescendantFetchError)) return + expect(error.failures.map((item) => item.sessionID)).toEqual(["child-a"]) + expect(error.partial.map((item) => item.id)).toEqual(["child-a", "child-b", "grandchild-b"]) + } + }) + + test("does not revisit a cycle", async () => { + const sdk = client({ + children: async (sessionID) => { + if (sessionID === "root") return [{ id: "child" }] + if (sessionID === "child") return [{ id: "root" }] + return [] + }, + }) + + expect(await fetchDescendants(sdk, "root")).toEqual([{ id: "child" }]) + }) + + test("bounds concurrent descendant lookups", async () => { + let active = 0 + let maxActive = 0 + const sdk = client({ + children: async (sessionID) => { + if (sessionID === "root") return Array.from({ length: 6 }, (_, index) => ({ id: `child-${index}` })) + active += 1 + maxActive = Math.max(maxActive, active) + await Bun.sleep(20) + active -= 1 + return [] + }, + }) + + await fetchDescendants(sdk, "root", { concurrency: 2 }) + expect(maxActive).toBe(2) + }) + + test("uses bounded defaults for invalid options", async () => { + let active = 0 + let maxActive = 0 + const sdk = client({ + children: async (sessionID) => { + if (sessionID === "root") return Array.from({ length: 6 }, (_, index) => ({ id: `child-${index}` })) + active += 1 + maxActive = Math.max(maxActive, active) + await Bun.sleep(20) + active -= 1 + return [] + }, + }) + + await fetchDescendants(sdk, "root", { concurrency: Number.POSITIVE_INFINITY, limit: Number.NaN }) + expect(maxActive).toBe(4) + }) + + test("returns partial data when the session limit is reached", async () => { + const sdk = client({ + children: async (sessionID) => + sessionID === "root" ? [{ id: "child-a" }, { id: "child-b" }, { id: "child-c" }] : [], + }) + + try { + await fetchDescendants(sdk, "root", { limit: 2 }) + throw new Error("expected descendant traversal to reach its limit") + } catch (error) { + expect(error).toBeInstanceOf(DescendantFetchError) + if (!(error instanceof DescendantFetchError)) return + expect(error.partial).toEqual([{ id: "child-a" }]) + expect(String(error.failures[0]?.error)).toContain("session tree exceeds 2 entries") + } + }) + + test("cancels descendant lookups", async () => { + let started: (() => void) | undefined + const ready = new Promise((resolve) => { + started = resolve + }) + const sdk = client({ + children: async (sessionID, signal) => { + if (sessionID === "root") return [{ id: "child" }] + started?.() + return new Promise((_, reject) => { + signal?.addEventListener("abort", () => reject(signal.reason), { once: true }) + }) + }, + }) + const controller = new AbortController() + const pending = fetchDescendants(sdk, "root", { signal: controller.signal }) + + await ready + controller.abort(new Error("stopped")) + await expect(pending).rejects.toThrow("stopped") + }) + + test("times out stalled descendant lookups", async () => { + const sdk = client({ + children: async (sessionID, signal) => { + if (sessionID === "root") return [{ id: "child" }] + return new Promise((_, reject) => { + signal?.addEventListener("abort", () => reject(signal.reason), { once: true }) + }) + }, + }) + + await expect(fetchDescendants(sdk, "root", { timeoutMs: 20 })).rejects.toBeInstanceOf(DescendantFetchError) + }) + + test("bounds total traversal time", async () => { + const sdk = client({ + children: async (sessionID, signal) => { + if (sessionID === "root") return [{ id: "child" }] + return new Promise((_, reject) => { + signal?.addEventListener("abort", () => reject(signal.reason), { once: true }) + }) + }, + }) + + const started = Date.now() + await expect(fetchDescendants(sdk, "root", { timeoutMs: 1_000, totalTimeoutMs: 20 })).rejects.toBeInstanceOf( + DescendantFetchError, + ) + expect(Date.now() - started).toBeLessThan(500) + }) + + test("models a failed ancestry lookup as unknown", async () => { + let lookups = 0 + const sdk = client({ + get: () => { + lookups += 1 + return Promise.reject(new Error("session lookup unavailable")) + }, + }) + + const ownership = await lookupSessionTreeOwnership(sdk, new Set(["root"]), "leaf") + expect(ownership.type).toBe("unknown") + expect(lookups).toBe(1) + if (ownership.type === "unknown") expect(String(ownership.error)).toContain("session lookup unavailable") + }) + + test("cancels retained ancestry retries", async () => { + let lookups = 0 + let retryStarted: (() => void) | undefined + const started = new Promise((resolve) => { + retryStarted = resolve + }) + const sdk = client({ + get: () => { + lookups += 1 + return Promise.reject(new Error("session lookup unavailable")) + }, + }) + const controller = new AbortController() + const resolving = resolveSessionTreeOwnership(sdk, new Set(["root"]), "leaf", { + signal: controller.signal, + onUnknown: () => retryStarted?.(), + }) + + await started + controller.abort() + expect(await resolving).toEqual({ type: "cancelled" }) + const stoppedAt = lookups + await Bun.sleep(150) + expect(lookups).toBe(stoppedAt) + }) + + test("returns foreign only after reaching a known root", async () => { + const sdk = client({ + get: () => Promise.resolve({ parentID: undefined }), + }) + + expect(await lookupSessionTreeOwnership(sdk, new Set(["root"]), "foreign")).toEqual({ type: "foreign" }) + }) + + test("adds a resolved ancestry path to the known tree", async () => { + const parents = new Map([ + ["leaf", "middle"], + ["middle", "root"], + ]) + const sdk = client({ + get: async (sessionID) => { + if (!parents.has(sessionID)) throw new Error(`unexpected lookup: ${sessionID}`) + return { parentID: parents.get(sessionID) } + }, + }) + const tree = new Set(["root"]) + + expect(await resolveSessionTreeOwnership(sdk, tree, "leaf", { signal: new AbortController().signal })).toEqual({ + type: "owned", + value: undefined, + }) + expect(tree).toEqual(new Set(["root", "leaf", "middle"])) + }) + + test("gives up on an unresolvable ancestry instead of retrying forever", async () => { + // A parent chain cycle and a deleted parent both fail permanently. Retrying + // them forever leaves the server's permission deferred unanswered, so the + // walk has to surface exhaustion to the caller. + let lookups = 0 + const sdk = client({ + get: (sessionID) => { + lookups += 1 + return Promise.resolve({ parentID: sessionID === "leaf" ? "middle" : "leaf" }) + }, + }) + + const resolution = await resolveSessionTreeOwnership(sdk, new Set(["root"]), "leaf", { + signal: new AbortController().signal, + attempts: 3, + }) + + expect(resolution.type).toBe("exhausted") + if (resolution.type === "exhausted") expect(String(resolution.error)).toContain("cycle detected") + expect(lookups).toBeGreaterThan(0) + }) + + test("gives up when a parent points at a missing session", async () => { + const sdk = client({ + get: (sessionID) => { + if (sessionID === "leaf") return Promise.resolve({ parentID: "deleted" }) + return Promise.reject(new Error(`session not found: ${sessionID}`)) + }, + }) + + const resolution = await resolveSessionTreeOwnership(sdk, new Set(["root"]), "leaf", { + signal: new AbortController().signal, + attempts: 2, + }) + + expect(resolution.type).toBe("exhausted") + if (resolution.type === "exhausted") expect(String(resolution.error)).toContain("session not found") + }) + + test("treats a missing permission request as already answered", async () => { + // The server drops pending requests when a session aborts and rejects the + // cascade itself, so a reply that lost that race must not fail the run. + const sdk = client({ + reply: () => Promise.reject({ _tag: "PermissionNotFoundError", requestID: "perm-1", message: "gone" }), + }) + + await expect(replyPermission(sdk, { requestID: "perm-1", reply: "once" })).resolves.toBeUndefined() + }) + + test("rejects permission reply error envelopes", async () => { + const sdk = client({ + reply: () => Promise.resolve({ error: new Error("reply rejected") }), + }) + + await expect(replyPermission(sdk, { requestID: "perm-1", reply: "once" })).rejects.toThrow("reply rejected") + }) +}) diff --git a/packages/tui/src/routes/session/footer.tsx b/packages/tui/src/routes/session/footer.tsx index c3a96254e98b..a2639358145e 100644 --- a/packages/tui/src/routes/session/footer.tsx +++ b/packages/tui/src/routes/session/footer.tsx @@ -5,6 +5,7 @@ import { useDirectory } from "../../context/directory" import { useConnected } from "../../component/use-connected" import { createStore } from "solid-js/store" import { useRoute } from "../../context/route" +import { sessionTree } from "./tree" export function Footer() { const { theme } = useTheme() @@ -13,9 +14,16 @@ export function Footer() { const mcp = createMemo(() => Object.values(sync.data.mcp).filter((x) => x.status === "connected").length) const mcpError = createMemo(() => Object.values(sync.data.mcp).some((x) => x.status === "failed")) const lsp = createMemo(() => Object.keys(sync.data.lsp)) + // Counter mirrors the prompt UI: viewing the root sums every nested + // subagent's pending prompts; viewing a subagent shows only its own. + // Without this the chrome would undercount when a grandchild is the + // one waiting on input. const permissions = createMemo(() => { if (route.data.type !== "session") return [] - return sync.data.permission[route.data.sessionID] ?? [] + const viewed = sync.session.get(route.data.sessionID) + if (!viewed) return [] + if (viewed.parentID) return sync.data.permission[viewed.id] ?? [] + return sessionTree(sync.data.session, viewed.id).flatMap((x) => sync.data.permission[x.id] ?? []) }) const directory = useDirectory() const connected = useConnected() diff --git a/packages/tui/src/routes/session/index.tsx b/packages/tui/src/routes/session/index.tsx index 6d77b0ea58fd..60a92da68613 100644 --- a/packages/tui/src/routes/session/index.tsx +++ b/packages/tui/src/routes/session/index.tsx @@ -27,6 +27,7 @@ import { Spinner } from "../../component/spinner" import { createSyntaxStyleMemo, generateSubtleSyntax, selectedForeground, useTheme } from "../../context/theme" import { BoxRenderable, ScrollBoxRenderable, addDefaultParsers, TextAttributes, RGBA } from "@opentui/core" import { Prompt, type PromptRef } from "../../component/prompt" +import { sessionTree } from "./tree" import type { AssistantMessage, Part, @@ -210,6 +211,14 @@ export function Session() { .filter((x) => x.parentID === parentID || x.id === parentID) .toSorted((a, b) => (a.id < b.id ? -1 : a.id > b.id ? 1 : 0)) }) + // Root + every transitive subagent. The prompt UI scopes to descendants + // (not just direct children) so blockers raised inside a nested + // subagent surface against the root view the user is looking at. + const descendants = createMemo(() => { + const s = session() + if (!s || s.parentID) return [] + return sessionTree(sync.data.session, s.id) + }) const messages = createMemo(() => sync.data.message[route.sessionID] ?? []) const foregroundTasks = createMemo(() => sync.data.capabilities.experimentalBackgroundSubagents @@ -226,11 +235,11 @@ export function Session() { ) const permissions = createMemo(() => { if (session()?.parentID) return [] - return children().flatMap((x) => sync.data.permission[x.id] ?? []) + return descendants().flatMap((x) => sync.data.permission[x.id] ?? []) }) const questions = createMemo(() => { if (session()?.parentID) return [] - return children().flatMap((x) => sync.data.question[x.id] ?? []) + return descendants().flatMap((x) => sync.data.question[x.id] ?? []) }) const visible = createMemo(() => !session()?.parentID && permissions().length === 0 && questions().length === 0) const disabled = createMemo(() => permissions().length > 0 || questions().length > 0) diff --git a/packages/tui/src/routes/session/tree.ts b/packages/tui/src/routes/session/tree.ts new file mode 100644 index 000000000000..92147cb25c9c --- /dev/null +++ b/packages/tui/src/routes/session/tree.ts @@ -0,0 +1,36 @@ +import type { Session } from "@opencode-ai/sdk/v2" + +// Walk the in-memory session list and return the root together with every +// transitive subagent under it. Pure function over a session snapshot so callers +// can wrap it in `createMemo` and have it track only what changed. +// +// Returns `[]` when `rootID` is not in `sessions` so the caller can treat an +// unknown root the same as an empty tree without a special case. +export function sessionTree(sessions: readonly Session[], rootID: string): Session[] { + const byParent = new Map() + let root: Session | undefined + for (const s of sessions) { + if (s.id === rootID) root = s + if (!s.parentID) continue + const bucket = byParent.get(s.parentID) ?? [] + bucket.push(s) + byParent.set(s.parentID, bucket) + } + if (!root) return [] + const out: Session[] = [root] + const seen = new Set([rootID]) + // Appending to `queue` while iterating is the BFS frontier; `for...of` over a + // growing array visits the new entries and avoids shift()'s O(n^2). + const queue = [rootID] + for (const id of queue) { + const kids = byParent.get(id) + if (!kids) continue + for (const kid of kids) { + if (seen.has(kid.id)) continue + seen.add(kid.id) + out.push(kid) + queue.push(kid.id) + } + } + return out +} diff --git a/packages/tui/test/routes/session/tree.test.ts b/packages/tui/test/routes/session/tree.test.ts new file mode 100644 index 000000000000..4a4c53f76375 --- /dev/null +++ b/packages/tui/test/routes/session/tree.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, test } from "bun:test" +import type { Session } from "@opencode-ai/sdk/v2" +import { sessionTree } from "../../../src/routes/session/tree" + +const session = (id: string, parentID?: string) => ({ id, parentID, title: id }) as Session + +describe("sessionTree", () => { + test("returns the complete subtree", () => { + const sessions = [ + session("root"), + session("child-a", "root"), + session("child-b", "root"), + session("grandchild", "child-a"), + session("other"), + ] + + expect(sessionTree(sessions, "root").map((item) => item.id)).toEqual(["root", "child-a", "child-b", "grandchild"]) + }) + + test("returns an empty list for an unknown root", () => { + expect(sessionTree([session("child", "root")], "root")).toEqual([]) + }) + + test("visits each session once when parent links cycle", () => { + const sessions = [session("root", "child"), session("child", "root")] + + expect(sessionTree(sessions, "root").map((item) => item.id)).toEqual(["root", "child"]) + }) +})