From 9c39e75ce2bf60284229cd13059d74c7dd0af6bb Mon Sep 17 00:00:00 2001 From: Shoubhit Dash Date: Mon, 31 Aug 2026 14:38:05 +0530 Subject: [PATCH] fix(tui): surface subagent permissions and questions (#44976) --- .../feature-plugins/system/notifications.ts | 3 +- packages/tui/src/routes/session/attention.ts | 29 +++++++ packages/tui/src/routes/session/form.tsx | 24 +++++- packages/tui/src/routes/session/index.tsx | 76 ++++++++++++------- .../tui/src/routes/session/permission.tsx | 36 ++++++++- packages/tui/src/util/session.ts | 27 ++++++- .../test/cli/cmd/tui/notifications.test.ts | 15 +++- packages/tui/test/cli/tui/permission.test.ts | 3 + .../test/cli/tui/session-attention.test.ts | 39 ++++++++++ packages/tui/test/util/session.test.ts | 35 ++++++++- 10 files changed, 247 insertions(+), 40 deletions(-) create mode 100644 packages/tui/src/routes/session/attention.ts create mode 100644 packages/tui/test/cli/tui/session-attention.test.ts diff --git a/packages/tui/src/feature-plugins/system/notifications.ts b/packages/tui/src/feature-plugins/system/notifications.ts index 03b6b626515..b878381a457 100644 --- a/packages/tui/src/feature-plugins/system/notifications.ts +++ b/packages/tui/src/feature-plugins/system/notifications.ts @@ -10,10 +10,11 @@ function notify( ) { const session = sessionID ? context.data.session.get(sessionID) : undefined const isSubagent = session?.parentID !== undefined + const actionable = sound === "permission" || sound === "question" void context.attention.notify({ title: title ?? session?.title, message, - notification: isSubagent ? false : { when: "blurred" }, + notification: isSubagent && !actionable ? false : { when: "blurred" }, sound: { name: sound, when: "always" }, }) } diff --git a/packages/tui/src/routes/session/attention.ts b/packages/tui/src/routes/session/attention.ts new file mode 100644 index 00000000000..94fc559000d --- /dev/null +++ b/packages/tui/src/routes/session/attention.ts @@ -0,0 +1,29 @@ +import type { PermissionRequest } from "@opencode-ai/client" +import type { FormWithLocation } from "../../context/data" + +export type SessionAttention = + | { type: "permission"; request: PermissionRequest } + | { type: "form"; request: FormWithLocation } + +export function selectSessionAttention( + permissions: readonly PermissionRequest[], + forms: readonly FormWithLocation[], + previous?: SessionAttention, +): SessionAttention | undefined { + if (previous?.type === "permission") { + const current = permissions.find((request) => request.id === previous.request.id) + if (current) return { type: "permission", request: current } + } + + if (previous?.type === "form") { + const current = forms.find((request) => request.id === previous.request.id) + if (current) return { type: "form", request: current } + } + + const permission = permissions[0] + if (permission) return { type: "permission", request: permission } + + const form = forms[0] + if (form) return { type: "form", request: form } + return undefined +} diff --git a/packages/tui/src/routes/session/form.tsx b/packages/tui/src/routes/session/form.tsx index 2e45791a682..6343c5ec1de 100644 --- a/packages/tui/src/routes/session/form.tsx +++ b/packages/tui/src/routes/session/form.tsx @@ -19,6 +19,7 @@ import { useToast } from "../../ui/toast" import { Keymap } from "../../context/keymap" import { useConfig } from "../../config" import { errorMessage } from "../../util/error" +import { subagentLabel } from "../../util/session" import { formCustom, formDisplayValue, @@ -40,7 +41,7 @@ function truncate(label: string, max: number) { return label.length > max ? label.slice(0, max - 1).trimEnd() + "…" : label } -export function FormPrompt(props: { form: FormWithLocation }) { +export function FormPrompt(props: { form: FormWithLocation; pending?: { current: number; total: number } }) { const data = useData() const themes = useThemes() const theme = useTheme("elevated") @@ -51,6 +52,10 @@ export function FormPrompt(props: { form: FormWithLocation }) { const config = useConfig().data const clipboard = useClipboard() const toast = useToast() + const owner = createMemo(() => { + const session = data.session.get(props.form.sessionID) + return session?.parentID ? session : undefined + }) const configuredFields = props.form.fields.filter(isFormAnswerField) const initial = formInitialValues(props.form.fields) @@ -749,7 +754,22 @@ export function FormPrompt(props: { form: FormWithLocation }) { > - {props.form.title} + + {props.form.title} + 1}> + + + {props.pending?.current} of {props.pending?.total} + + + + + {(current) => ( + + {subagentLabel(current())} + + )} + diff --git a/packages/tui/src/routes/session/index.tsx b/packages/tui/src/routes/session/index.tsx index ff05284da17..4412858f7a3 100644 --- a/packages/tui/src/routes/session/index.tsx +++ b/packages/tui/src/routes/session/index.tsx @@ -103,6 +103,7 @@ import { import { switchLabel } from "../../util/model" import { findMessageBoundary, messageNavigationSlack } from "./message-navigation" import { stringWidth } from "../../util/string-width" +import { sessionDescendants } from "../../util/session" import { useArgs } from "../../context/args" import { withTimestampedFallback } from "@opencode-ai/util/session-title-fallback" import { useSessionTabs } from "../../context/session-tabs" @@ -111,6 +112,7 @@ import type { SessionInbox } from "@opencode-ai/schema/session-inbox" import { generateThinkingSyntax } from "./thinking-syntax" import { createDelayedPresence } from "../../util/delayed-presence" import { SessionLocationMissing } from "./location-missing" +import { selectSessionAttention, type SessionAttention } from "./attention" import { isRecord } from "../../util/record" import { createHistoryPrepend } from "./history" import { useSessionTerminals } from "../../context/session-terminals" @@ -202,23 +204,29 @@ export function Session(props: { setEpilogue(sessionEpilogue({ title, sessionID: session()?.id })) }) onCleanup(() => setEpilogue()) - const descendantSessionIDs = createMemo(() => { - if (session()?.parentID) return [] - return data.session.family(route.sessionID).filter((id) => id !== route.sessionID) - }) - const permissions = createMemo(() => { - if (session()?.parentID) return [] - return [route.sessionID, ...descendantSessionIDs()].flatMap( - (sessionID) => data.session.permission.list(sessionID) ?? [], - ) - }) + const descendantSessionIDs = createMemo(() => + session() ? sessionDescendants(data.session.list(), route.sessionID).map((item) => item.id) : [], + ) + const permissions = createMemo(() => + [route.sessionID, ...descendantSessionIDs()].flatMap((sessionID) => data.session.permission.list(sessionID) ?? []), + ) const promptedPermissions = createMemo(() => (local.permission.mode === "auto" ? [] : permissions())) - const forms = createMemo(() => { - const global = data.session.form.list("global", location()) ?? [] - if (session()?.parentID) return global - return [route.sessionID, ...descendantSessionIDs()] + const forms = createMemo(() => + [route.sessionID, ...descendantSessionIDs()] .flatMap((sessionID) => data.session.form.list(sessionID) ?? []) - .concat(global) + .concat(data.session.form.list("global", location()) ?? []), + ) + const attention = createMemo( + (previous: SessionAttention | undefined) => selectSessionAttention(promptedPermissions(), forms(), previous), + undefined, + ) + const requestCount = createMemo(() => promptedPermissions().length + forms().length) + const requestPosition = createMemo(() => { + const current = attention() + if (!current) return 0 + if (current.type === "permission") + return promptedPermissions().findIndex((item) => item.id === current.request.id) + 1 + return promptedPermissions().length + forms().findIndex((item) => item.id === current.request.id) + 1 }) const pendingUsers = createMemo(() => data.session.pending.list(route.sessionID).flatMap((item) => (item.type === "user" ? [item] : [])), @@ -235,6 +243,7 @@ export function Session(props: { if (props.promptMuted && composer.open) setComposer("open", false) }) const disabled = createMemo(() => promptedPermissions().length > 0 || forms().length > 0) + const composerVisible = createMemo(() => !disabled() && (composer.open || !!session()?.parentID)) const lastAssistant = createMemo(() => { return messages().findLast((x) => x.type === "assistant") @@ -324,7 +333,11 @@ export function Session(props: { on([descendantSessionIDs, () => client.connection.status()], ([sessionIDs, status]) => { if (status !== "connected") return void Promise.allSettled( - sessionIDs.flatMap((sessionID) => [data.session.permission.sync(sessionID), data.session.form.sync(sessionID)]), + sessionIDs.flatMap((sessionID) => [ + data.session.sync(sessionID, { children: true }), + data.session.permission.sync(sessionID), + data.session.form.sync(sessionID), + ]), ) }), ) @@ -1440,7 +1453,7 @@ export function Session(props: { { const parent = session()?.parentID @@ -1453,22 +1466,31 @@ export function Session(props: { visibleTerminalID={props.visibleTerminalID} /> - {null} - 0}> - + {null} + + {(_) => { - const request = promptedPermissions()[0] - return request ? ( - + const current = attention() + return current?.type === "permission" ? ( + ) : null }} - 0}> - + + {(_) => { - const form = forms()[0] - return form ? : null + const current = attention() + return current?.type === "form" ? ( + + ) : null }} diff --git a/packages/tui/src/routes/session/permission.tsx b/packages/tui/src/routes/session/permission.tsx index 4b46d829bfd..dd2c8e12d62 100644 --- a/packages/tui/src/routes/session/permission.tsx +++ b/packages/tui/src/routes/session/permission.tsx @@ -8,6 +8,7 @@ import { SplitBorder } from "../../ui/border" import { useData } from "../../context/data" import { filetype } from "../../util/filetype" import { permissionAlwaysLines, permissionOptionLabel, permissionPresentation } from "../../util/permission" +import { subagentLabel } from "../../util/session" import { getScrollAcceleration } from "../../util/scroll" import { useConfig } from "../../config" import { Keymap } from "../../context/keymap" @@ -109,7 +110,11 @@ function EditBody(props: { file?: string; diff?: string; patch?: string }) { ) } -export function PermissionPrompt(props: { request: PermissionRequest; directory?: string }) { +export function PermissionPrompt(props: { + request: PermissionRequest + directory?: string + pending?: { current: number; total: number } +}) { const data = useData() const toast = useToast() const [store, setStore] = createStore({ @@ -117,6 +122,10 @@ export function PermissionPrompt(props: { request: PermissionRequest; directory? }) const pathFormatter = usePathFormatter() const session = createMemo(() => data.session.get(props.request.sessionID)) + const owner = createMemo(() => { + const current = session() + return current?.parentID ? current : undefined + }) const source = createMemo(() => { const tool = props.request.source @@ -222,7 +231,22 @@ export function PermissionPrompt(props: { request: PermissionRequest; directory? {"△"} Permission required + 1}> + + + {props.pending?.current} of {props.pending?.total} + + + + {(current) => ( + + + {subagentLabel(current())} + + + )} + @@ -237,7 +261,11 @@ export function PermissionPrompt(props: { request: PermissionRequest; directory? const body = ( (sessions: readonly T[], ses return walk(root(current).id, []) } +export function sessionDescendants(sessions: readonly T[], sessionID: string) { + const children = new Map() + sessions.forEach((session) => { + if (!session.parentID) return + const group = children.get(session.parentID) + if (group) group.push(session) + else children.set(session.parentID, [session]) + }) + + const visited = new Set([sessionID]) + function walk(parentID: string): T[] { + return (children.get(parentID) ?? []).flatMap((session) => { + if (visited.has(session.id)) return [] + visited.add(session.id) + return [session, ...walk(session.id)] + }) + } + + return walk(sessionID) +} + +export function subagentLabel(session: Pick) { + return [Locale.titlecase(session.agent ?? "Subagent"), session.title].filter(Boolean).join(" · ") +} + export function lastAssistantWithUsage(messages: ReadonlyArray, boundary?: string) { const boundaryIndex = boundary ? messages.findIndex((message) => message.id === boundary) : -1 if (boundary && boundaryIndex === -1) return undefined diff --git a/packages/tui/test/cli/cmd/tui/notifications.test.ts b/packages/tui/test/cli/cmd/tui/notifications.test.ts index a0a246105ce..32869890636 100644 --- a/packages/tui/test/cli/cmd/tui/notifications.test.ts +++ b/packages/tui/test/cli/cmd/tui/notifications.test.ts @@ -221,7 +221,7 @@ describe("internal notifications TUI plugin", () => { ]) }) - test("uses sound-only notifications and subagent_done sound for subagent sessions", async () => { + test("notifies for subagent requests while keeping completions sound-only", async () => { const harness = await setup() harness.emit({ @@ -230,16 +230,23 @@ describe("internal notifications TUI plugin", () => { type: "form.created", data: { form: { ...form("form-1", "subagent"), title: "Questions" } }, }) - harness.emit(executionStarted("event-2", "subagent")) - harness.emit(executionSucceeded("event-3", "subagent")) + harness.emit({ id: "event-2", created: 0, type: "permission.asked", data: permission("permission-1", "subagent") }) + harness.emit(executionStarted("event-3", "subagent")) + harness.emit(executionSucceeded("event-4", "subagent")) expect(harness.notifications).toEqual([ { title: "Questions", message: "Input needs response", - notification: false, + notification: { when: "blurred" }, sound: { name: "question", when: "always" }, }, + { + title: "Subagent session", + message: "Permission needs input", + notification: { when: "blurred" }, + sound: { name: "permission", when: "always" }, + }, { title: "Subagent session", message: "Session done", diff --git a/packages/tui/test/cli/tui/permission.test.ts b/packages/tui/test/cli/tui/permission.test.ts index d4da46ad680..0ddd05adb0d 100644 --- a/packages/tui/test/cli/tui/permission.test.ts +++ b/packages/tui/test/cli/tui/permission.test.ts @@ -4,4 +4,7 @@ import { permissionSemanticLabel } from "../../../src/routes/session/permission" test("uses the permission action when a surface has no display title", () => { expect(permissionSemanticLabel("shell")).toBe("Permission required: shell") expect(permissionSemanticLabel("edit", "Edit fixture.txt")).toBe("Permission required: Edit fixture.txt") + expect(permissionSemanticLabel("shell", "Run git status", "Explore · Inspect permissions")).toBe( + "Permission required from Explore · Inspect permissions: Run git status", + ) }) diff --git a/packages/tui/test/cli/tui/session-attention.test.ts b/packages/tui/test/cli/tui/session-attention.test.ts new file mode 100644 index 00000000000..8375e9bb575 --- /dev/null +++ b/packages/tui/test/cli/tui/session-attention.test.ts @@ -0,0 +1,39 @@ +import { expect, test } from "bun:test" +import type { PermissionRequest } from "@opencode-ai/client" +import type { FormWithLocation } from "../../../src/context/data" +import { selectSessionAttention } from "../../../src/routes/session/attention" + +function permission(id: string, sessionID = "child"): PermissionRequest { + return { id, sessionID, action: "shell", resources: ["git status"] } +} + +function form(id: string, sessionID = "child"): FormWithLocation { + return { + id, + sessionID, + title: "Questions", + fields: [{ key: "answer", type: "string", description: "Which strategy should I use?" }], + } +} + +test("prefers a permission when selecting an initial pending request", () => { + const approval = permission("permission-one") + expect(selectSessionAttention([approval], [form("form-one")])).toEqual({ type: "permission", request: approval }) +}) + +test("keeps an active question mounted when another subagent requests permission", () => { + const question = form("form-one", "child-a") + const current = selectSessionAttention([], [question]) + const approval = permission("permission-one", "child-b") + + expect(selectSessionAttention([approval], [question], current)).toEqual({ type: "form", request: question }) +}) + +test("advances to the next request after the current owner responds", () => { + const approval = permission("permission-one") + const question = form("form-one", "child-b") + const current = selectSessionAttention([approval], [question]) + + expect(selectSessionAttention([], [question], current)).toEqual({ type: "form", request: question }) + expect(selectSessionAttention([], [], { type: "form", request: question })).toBeUndefined() +}) diff --git a/packages/tui/test/util/session.test.ts b/packages/tui/test/util/session.test.ts index b3e32aa6340..fae71cc486e 100644 --- a/packages/tui/test/util/session.test.ts +++ b/packages/tui/test/util/session.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test" import type { SessionMessageInfo } from "@opencode-ai/client" -import { lastAssistantWithUsage, sessionFamily } from "../../src/util/session" +import { lastAssistantWithUsage, sessionDescendants, sessionFamily, subagentLabel } from "../../src/util/session" const assistant = (id: string, input: number): SessionMessageInfo => ({ id, @@ -34,6 +34,39 @@ describe("util.session", () => { ]) }) + test("limits descendants to the selected subagent branch", () => { + const sessions = [ + { id: "root" }, + { id: "child-a", parentID: "root" }, + { id: "grandchild-a", parentID: "child-a" }, + { id: "child-b", parentID: "root" }, + { id: "grandchild-b", parentID: "child-b" }, + ] + + expect(sessionDescendants(sessions, "root").map((session) => session.id)).toEqual([ + "child-a", + "grandchild-a", + "child-b", + "grandchild-b", + ]) + expect(sessionDescendants(sessions, "child-a").map((session) => session.id)).toEqual(["grandchild-a"]) + }) + + test("does not revisit sessions while collecting a descendant cycle", () => { + const sessions = [ + { id: "root", parentID: "child" }, + { id: "child", parentID: "root" }, + ] + + expect(sessionDescendants(sessions, "root").map((session) => session.id)).toEqual(["child"]) + }) + + test("labels requesting subagents with their agent and task", () => { + expect(subagentLabel({ agent: "explore", title: "Inspect permissions" })).toBe("Explore · Inspect permissions") + expect(subagentLabel({ agent: undefined, title: "Inspect permissions" })).toBe("Subagent · Inspect permissions") + expect(subagentLabel({ agent: "general", title: undefined })).toBe("General") + }) + test("tracks usage across undo and redo boundaries", () => { const messages = [assistant("msg_z", 10), assistant("msg_a", 30)]