From 4027dbb4d8c94d78131697ccd9db808de0dd9121 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 18 Jul 2026 00:09:44 -0400 Subject: [PATCH] refactor(tui): simplify quark migration after review - fix permission input losing reactivity (raw slot read; regression test) - BackgroundToolHint tracks structure + tool slots instead of whole-collection values - group views share mapArray-backed usePartSlots (per-ref node reuse) - dedupe streaming handlers behind variant-scoped modify/insert helpers - part ID scheme has one definition; hasText required in message navigation - expose read-only PartsView to view components; misc cleanups --- packages/quark/src/solid.ts | 2 +- packages/tui/src/context/data.tsx | 255 ++++++++++-------- packages/tui/src/routes/session/content.ts | 22 +- .../tui/src/routes/session/dialog-message.tsx | 8 +- packages/tui/src/routes/session/index.tsx | 90 +++---- .../src/routes/session/message-navigation.ts | 8 +- .../tui/src/routes/session/permission.tsx | 22 +- packages/tui/src/routes/session/rows.ts | 38 ++- packages/tui/src/routes/session/timeline.ts | 2 + .../test/cli/tui/message-navigation.test.ts | 15 ++ packages/tui/test/cli/tui/permission.test.tsx | 115 ++++++++ 11 files changed, 380 insertions(+), 197 deletions(-) create mode 100644 packages/tui/test/cli/tui/permission.test.tsx diff --git a/packages/quark/src/solid.ts b/packages/quark/src/solid.ts index 69c90360da..9acd6bcfff 100644 --- a/packages/quark/src/solid.ts +++ b/packages/quark/src/solid.ts @@ -11,7 +11,7 @@ export function useValue(readable: Readable): Accessor { * slot, while memo equality prevents an unchanged slot from propagating to the * consumer. Value changes flow through the slot itself. */ -export function useSlot(keyed: Keyed.Keyed, key: () => Key): Accessor { +export function useSlot(keyed: Pick, "slots" | "get">, key: () => Key): Accessor { const structure = useValue(keyed.slots) const slot = createMemo(() => { structure() diff --git a/packages/tui/src/context/data.tsx b/packages/tui/src/context/data.tsx index 504f99ac93..d0f47e0644 100644 --- a/packages/tui/src/context/data.tsx +++ b/packages/tui/src/context/data.tsx @@ -34,7 +34,7 @@ import { SessionContent } from "../routes/session/content" export type DataSessionStatus = "idle" | "running" -const messageIDFromEvent = (eventID: string) => eventID.replace(/^evt_/, "msg_") +export const messageIDFromEvent = (eventID: string) => eventID.replace(/^evt_/, "msg_") // Global MCP elicitations temporarily use "global" instead of a real session ID, so the // server cannot recover their Location when settling them. Preserve the event Location @@ -150,6 +150,44 @@ export const { use: useData, provider: DataProvider } = createSimpleContext({ // Assistant content lives in per-message keyed part slots, not the Solid // store: streaming deltas publish one slot instead of reconciling arrays. const content = SessionContent.make() + // Variant-scoped slot operations: each owns its part address scheme and + // type guard so the streaming handlers below read as pure transforms. + // A missing collection or key is a normal straggler race and no-ops. + const modifyText = ( + data: { sessionID: string; assistantMessageID: string; ordinal: number }, + f: (part: Extract) => SessionContent.Part, + ) => + content + .get(data.sessionID, data.assistantMessageID) + ?.modify(SessionContent.textID(data.ordinal), (part) => (part.type === "text" ? f(part) : part)) + const modifyReasoning = ( + data: { sessionID: string; assistantMessageID: string; ordinal: number }, + f: (part: Extract) => SessionContent.Part, + ) => + content + .get(data.sessionID, data.assistantMessageID) + ?.modify(SessionContent.reasoningID(data.ordinal), (part) => (part.type === "reasoning" ? f(part) : part)) + const modifyTool = ( + data: { sessionID: string; assistantMessageID: string; callID: string }, + f: (part: Extract) => SessionContent.Part, + ) => + content + .get(data.sessionID, data.assistantMessageID) + ?.modify(data.callID, (part) => (part.type === "tool" ? f(part) : part)) + const insertPart = (data: { sessionID: string; assistantMessageID: string }, part: SessionContent.Part) => { + const parts = content.ensure(data.sessionID, data.assistantMessageID) + if (!parts.has(part.partID)) parts.insert(part) + } + // Shared settlement trailer for tool success and failure. + const settled = ( + part: Extract, + data: { executed: boolean; resultState?: Extract["providerResultState"] }, + completed: number, + ) => ({ + executed: data.executed || part.executed === true, + providerResultState: data.resultState, + time: { ...part.time, completed }, + }) const sync = createSync() const pendingOperations = new Map() @@ -251,6 +289,8 @@ export const { use: useData, provider: DataProvider } = createSimpleContext({ ...item.data, time: { created: item.timeCreated }, } + // Placeholder row until the server projects the real compaction; + // pending compactions carry no reason, so "manual" is a stand-in. return { id: item.id, type: "compaction", @@ -566,142 +606,110 @@ export const { use: useData, provider: DataProvider } = createSimpleContext({ } }) break - case "session.text.started": { - const parts = content.ensure(event.data.sessionID, event.data.assistantMessageID) - const partID = SessionContent.textID(event.data.ordinal) - if (!parts.has(partID)) parts.insert({ type: "text", text: "", partID }) + case "session.text.started": + insertPart(event.data, { type: "text", text: "", partID: SessionContent.textID(event.data.ordinal) }) break - } case "session.text.delta": - content - .get(event.data.sessionID, event.data.assistantMessageID) - ?.modify(SessionContent.textID(event.data.ordinal), (part) => - part.type === "text" ? { ...part, text: part.text + event.data.delta } : part, - ) + modifyText(event.data, (part) => ({ ...part, text: part.text + event.data.delta })) break case "session.text.ended": - content - .get(event.data.sessionID, event.data.assistantMessageID) - ?.modify(SessionContent.textID(event.data.ordinal), (part) => - part.type === "text" ? { ...part, text: event.data.text } : part, - ) + modifyText(event.data, (part) => ({ ...part, text: event.data.text })) break - case "session.tool.input.started": { - const parts = content.ensure(event.data.sessionID, event.data.assistantMessageID) - if (!parts.has(event.data.callID)) - parts.insert({ - type: "tool", - id: event.data.callID, - name: event.data.name, - time: { created: event.created }, - state: { status: "streaming", input: "" }, - partID: event.data.callID, - }) - break - } - case "session.tool.input.delta": - content.get(event.data.sessionID, event.data.assistantMessageID)?.modify(event.data.callID, (part) => { - if (part.type !== "tool" || part.state.status !== "streaming") return part - return { ...part, state: { ...part.state, input: part.state.input + event.data.delta } } + case "session.tool.input.started": + insertPart(event.data, { + type: "tool", + id: event.data.callID, + name: event.data.name, + time: { created: event.created }, + state: { status: "streaming", input: "" }, + partID: event.data.callID, }) break + case "session.tool.input.delta": + modifyTool(event.data, (part) => + part.state.status !== "streaming" + ? part + : { ...part, state: { ...part.state, input: part.state.input + event.data.delta } }, + ) + break case "session.tool.input.ended": - content.get(event.data.sessionID, event.data.assistantMessageID)?.modify(event.data.callID, (part) => { - if (part.type !== "tool" || part.state.status !== "streaming") return part - return { ...part, state: { ...part.state, input: event.data.text } } - }) + modifyTool(event.data, (part) => + part.state.status !== "streaming" ? part : { ...part, state: { ...part.state, input: event.data.text } }, + ) break case "session.tool.called": - content.get(event.data.sessionID, event.data.assistantMessageID)?.modify(event.data.callID, (part) => { - if (part.type !== "tool") return part - return { - ...part, - time: { ...part.time, ran: event.created }, - executed: event.data.executed, - providerState: event.data.state, - state: { status: "running", input: event.data.input, structured: {}, content: [] }, - } - }) + modifyTool(event.data, (part) => ({ + ...part, + time: { ...part.time, ran: event.created }, + executed: event.data.executed, + providerState: event.data.state, + state: { status: "running", input: event.data.input, structured: {}, content: [] }, + })) break case "session.tool.progress": - content.get(event.data.sessionID, event.data.assistantMessageID)?.modify(event.data.callID, (part) => { - if (part.type !== "tool" || part.state.status !== "running") return part - return { - ...part, - state: { ...part.state, structured: event.data.structured, content: [...event.data.content] }, - } - }) + modifyTool(event.data, (part) => + part.state.status !== "running" + ? part + : { + ...part, + state: { ...part.state, structured: event.data.structured, content: [...event.data.content] }, + }, + ) break case "session.tool.success": - content.get(event.data.sessionID, event.data.assistantMessageID)?.modify(event.data.callID, (part) => { - if (part.type !== "tool" || part.state.status !== "running") return part - return { - ...part, - state: { - status: "completed", - input: part.state.input, - structured: event.data.structured, - content: [...event.data.content], - result: event.data.result, - }, - executed: event.data.executed || part.executed === true, - providerResultState: event.data.resultState, - time: { ...part.time, completed: event.created }, - } - }) + modifyTool(event.data, (part) => + part.state.status !== "running" + ? part + : { + ...part, + state: { + status: "completed", + input: part.state.input, + structured: event.data.structured, + content: [...event.data.content], + result: event.data.result, + }, + ...settled(part, event.data, event.created), + }, + ) break case "session.tool.failed": - content.get(event.data.sessionID, event.data.assistantMessageID)?.modify(event.data.callID, (part) => { - if (part.type !== "tool" || (part.state.status !== "streaming" && part.state.status !== "running")) - return part - return { - ...part, - state: { - status: "error", - error: event.data.error, - input: typeof part.state.input === "string" ? {} : part.state.input, - structured: part.state.status === "running" ? part.state.structured : {}, - content: part.state.status === "running" ? part.state.content : [], - result: event.data.result, - }, - executed: event.data.executed || part.executed === true, - providerResultState: event.data.resultState, - time: { ...part.time, completed: event.created }, - } + modifyTool(event.data, (part) => + part.state.status !== "streaming" && part.state.status !== "running" + ? part + : { + ...part, + state: { + status: "error", + error: event.data.error, + input: typeof part.state.input === "string" ? {} : part.state.input, + structured: part.state.status === "running" ? part.state.structured : {}, + content: part.state.status === "running" ? part.state.content : [], + result: event.data.result, + }, + ...settled(part, event.data, event.created), + }, + ) + break + case "session.reasoning.started": + insertPart(event.data, { + type: "reasoning", + text: "", + state: event.data.state, + time: { created: event.created }, + partID: SessionContent.reasoningID(event.data.ordinal), }) break - case "session.reasoning.started": { - const parts = content.ensure(event.data.sessionID, event.data.assistantMessageID) - const partID = SessionContent.reasoningID(event.data.ordinal) - if (!parts.has(partID)) - parts.insert({ - type: "reasoning", - text: "", - state: event.data.state, - time: { created: event.created }, - partID, - }) - break - } case "session.reasoning.delta": - content - .get(event.data.sessionID, event.data.assistantMessageID) - ?.modify(SessionContent.reasoningID(event.data.ordinal), (part) => - part.type === "reasoning" ? { ...part, text: part.text + event.data.delta } : part, - ) + modifyReasoning(event.data, (part) => ({ ...part, text: part.text + event.data.delta })) break case "session.reasoning.ended": - content - .get(event.data.sessionID, event.data.assistantMessageID) - ?.modify(SessionContent.reasoningID(event.data.ordinal), (part) => { - if (part.type !== "reasoning") return part - return { - ...part, - text: event.data.text, - time: { created: part.time?.created ?? event.created, completed: event.created }, - state: event.data.state !== undefined ? event.data.state : part.state, - } - }) + modifyReasoning(event.data, (part) => ({ + ...part, + text: event.data.text, + time: { created: part.time?.created ?? event.created, completed: event.created }, + state: event.data.state !== undefined ? event.data.state : part.state, + })) break case "session.retry.scheduled": message.update(event.data.sessionID, (draft, index) => { @@ -981,12 +989,17 @@ export const { use: useData, provider: DataProvider } = createSimpleContext({ sync(sessionID: string) { return sync.run(`session.message:${sessionID}`, async () => { await syncPending(sessionID) - const localInputs = pendingInputs(sessionID).map((item) => item.id) - const pendingMessages = [...(store.session.pending[sessionID] ?? [])].map(message.fromPending) + const inputIDs = () => pendingInputs(sessionID).map((item) => item.id) + // Snapshot pending state before the fetch: inputs admitted before + // the fetch must survive even if the server promotes them while + // the list request is in flight. The post-fetch snapshot covers + // inputs admitted during the fetch. + const localInputs = inputIDs() + const pendingMessages = (store.session.pending[sessionID] ?? []).map(message.fromPending) const projected = await client.api.message.list({ sessionID, limit: 200, order: "desc" }) const next = projected.data.toReversed() const index = new Map(next.map((message, index) => [message.id, index])) - const localInputIDs = new Set([...localInputs, ...pendingInputs(sessionID).map((item) => item.id)]) + const localInputIDs = new Set([...localInputs, ...inputIDs()]) localInputIDs.forEach((messageID) => { const position = messageIndex.get(sessionID)?.get(messageID) const item = position === undefined ? undefined : store.session.message[sessionID]?.[position] @@ -1004,6 +1017,10 @@ export const { use: useData, provider: DataProvider } = createSimpleContext({ }) }, parts(sessionID: string, messageID: string) { + // Deliberately ensure-on-read: consumers capture the collection + // for their lifetime (see useSlot), so identity per (session, + // message) must be stable even when reads precede streaming. + // Stray collections are reclaimed by prune on the next sync. return content.ensure(sessionID, messageID) }, invalidate(sessionID: string) { diff --git a/packages/tui/src/routes/session/content.ts b/packages/tui/src/routes/session/content.ts index f4c0a8de20..eaff9c6560 100644 --- a/packages/tui/src/routes/session/content.ts +++ b/packages/tui/src/routes/session/content.ts @@ -13,6 +13,8 @@ export namespace SessionContent { type ContentPart = SessionMessageAssistant["content"][number] export type Part = ContentPart & { readonly partID: string } export type Parts = Keyed.Keyed + /** Read surface for view components; mutation stays with the data layer. */ + export type PartsView = Pick // Streamed sub-objects are replaced immutably on change, so reference // equality is the correct (and cheapest) field comparator for them. @@ -52,10 +54,28 @@ export namespace SessionContent { const ordinals = { text: 0, reasoning: 0 } return content.map((part) => { if (part.type === "tool") return { ...part, partID: part.id } - return { ...part, partID: `${part.type}:${ordinals[part.type]++}` } + const id = part.type === "text" ? textID : reasoningID + return { ...part, partID: id(ordinals[part.type]++) } }) } + /** Non-empty text parts of one message, in slot order. */ + export function textParts(parts: PartsView) { + return parts + .values() + .filter((part): part is Extract => part.type === "text" && part.text.trim().length > 0) + } + + export function hasText(parts: PartsView) { + return textParts(parts).length > 0 + } + + export function text(parts: PartsView) { + return textParts(parts) + .map((part) => part.text) + .join("\n") + } + export function make(options?: { readonly metrics?: Keyed.Metrics }) { const collections = new Map() const id = (sessionID: string, messageID: string) => `${sessionID}\u0000${messageID}` diff --git a/packages/tui/src/routes/session/dialog-message.tsx b/packages/tui/src/routes/session/dialog-message.tsx index e221619177..edadfcdd77 100644 --- a/packages/tui/src/routes/session/dialog-message.tsx +++ b/packages/tui/src/routes/session/dialog-message.tsx @@ -6,6 +6,7 @@ import { useToast } from "../../ui/toast" import { useClient } from "../../context/client" import { errorMessage } from "../../util/error" import { DialogFork } from "./dialog-fork" +import { SessionContent } from "./content" import type { PromptInfo } from "../../prompt/history" export function DialogMessage(props: { @@ -62,12 +63,7 @@ export function DialogMessage(props: { value.type === "user" ? value.text : value.type === "assistant" - ? data.session.message - .parts(props.sessionID, props.messageID) - .values() - .filter((content) => content.type === "text") - .map((content) => content.text) - .join("\n") + ? SessionContent.text(data.session.message.parts(props.sessionID, props.messageID)) : "text" in value ? value.text : "" diff --git a/packages/tui/src/routes/session/index.tsx b/packages/tui/src/routes/session/index.tsx index 1f77617219..a791600905 100644 --- a/packages/tui/src/routes/session/index.tsx +++ b/packages/tui/src/routes/session/index.tsx @@ -5,6 +5,7 @@ import { createMemo, createSignal, For, + mapArray, Match, on, onCleanup, @@ -208,6 +209,10 @@ export function Session() { }) const editor = useEditorContext() const rows = createSessionRows(() => route.sessionID) + const partsOf = (messageID: string) => data.session.message.parts(route.sessionID, messageID) + // Slot reads here are deliberately untracked: messageBoundaryIDs depends + // only on structurally-immutable row fields (id, type, origin), so this memo + // re-runs on structural changes (rows.slots) and message list changes only. const boundaries = createMemo(() => messageBoundaryIDs( rows.slots().map((slot) => slot()), @@ -313,11 +318,7 @@ export function Session() { direction, children: scroll.getChildren(), messages: messages(), - hasText: (messageID) => - data.session.message - .parts(route.sessionID, messageID) - .values() - .some((part) => part.type === "text" && part.text.trim()), + hasText: (messageID) => SessionContent.hasText(partsOf(messageID)), scrollTop: scroll.scrollTop, viewportY: scroll.viewport.y, currentID: navigationMessage(), @@ -687,8 +688,7 @@ export function Session() { return } - const textParts = data.session.message - .parts(route.sessionID, lastAssistantMessage.id) + const textParts = partsOf(lastAssistantMessage.id) .values() .filter((part) => part.type === "text") if (textParts.length === 0) { @@ -729,7 +729,7 @@ export function Session() { const sessionData = session() if (!sessionData) return const transcript = formatSessionTranscript(sessionData, messages(), showThinking(), (messageID) => - data.session.message.parts(route.sessionID, messageID).values(), + partsOf(messageID).values(), ) await clipboard.write?.(transcript) toast.show({ message: "Session transcript copied to clipboard!", variant: "success" }) @@ -758,7 +758,7 @@ export function Session() { const content = options.format === "markdown" ? formatSessionTranscript(sessionData, messages(), options.thinking, (messageID) => - data.session.message.parts(route.sessionID, messageID).values(), + partsOf(messageID).values(), ) : await (async () => { if (options.debug) { @@ -947,15 +947,12 @@ export function Session() { data.session.message.get(route.sessionID, messageID)} - parts={(messageID) => data.session.message.parts(route.sessionID, messageID)} + parts={partsOf} boundaryID={boundaries()[index()]} /> )} - data.session.message.parts(route.sessionID, messageID)} - /> + SessionMessageInfo | undefined - parts: (messageID: string) => SessionContent.Parts + parts: (messageID: string) => SessionContent.PartsView boundaryID?: string }) { return ( @@ -1101,7 +1098,7 @@ function SessionRowView(props: { function BackgroundToolHint(props: { messages: SessionMessageInfo[] - parts: (messageID: string) => SessionContent.Parts + parts: (messageID: string) => SessionContent.PartsView }) { const { themeV2 } = useTheme() const shortcut = Keymap.useShortcut("session.background") @@ -1110,17 +1107,23 @@ function BackgroundToolHint(props: { (message): message is SessionMessageAssistant => message.type === "assistant" && !message.time.completed, ), ) - const parts = createMemo(() => { + // Track the part structure (publishes only on part insert/remove) and the + // individual tool slots; text and reasoning deltas never reach this memo. + const toolSlots = createMemo(() => { const message = current() - return message ? useValue(props.parts(message.id).values) : undefined + if (!message) return [] + const parts = props.parts(message.id) + return useValue(parts.slots)() + .filter((slot) => slot().type === "tool") + .map((slot) => useValue(slot)) }) - const visible = createMemo( - () => - parts()?.()?.some((part) => { - if (part.type !== "tool" || part.state.status !== "running") return false - const display = toolDisplay(part.name) - return display === "shell" || display === "subagent" - }) ?? false, + const visible = createMemo(() => + toolSlots().some((tool) => { + const part = tool() + if (part.type !== "tool" || part.state.status !== "running") return false + const display = toolDisplay(part.name) + return display === "shell" || display === "subagent" + }), ) return ( @@ -1161,10 +1164,18 @@ function SessionMessageView(props: { message: SessionMessageInfo }) { ) } +// Per-ref slot accessors for group views. mapArray reuses entries by ref +// identity, so appending one ref to a group creates one new slot subscription +// instead of rebuilding the whole accessor list; value changes flow through +// the individual slots. +function usePartSlots(parts: (messageID: string) => SessionContent.PartsView, refs: () => readonly PartRef[]) { + return mapArray(refs, (ref) => ({ ref, part: useSlot(parts(ref.messageID), () => ref.partID) })) +} + function SessionPartView(props: { partRef: PartRef message: (messageID: string) => SessionMessageInfo | undefined - parts: (messageID: string) => SessionContent.Parts + parts: (messageID: string) => SessionContent.PartsView }) { const message = createMemo(() => props.message(props.partRef.messageID)) // One reactive slot per part: unrelated deltas in the same message cannot @@ -1197,21 +1208,14 @@ function SessionReasoningGroupView(props: { refs: readonly PartRef[] completed: boolean message: (messageID: string) => SessionMessageInfo | undefined - parts: (messageID: string) => SessionContent.Parts + parts: (messageID: string) => SessionContent.PartsView }) { const ctx = use() const { themeV2, syntax } = useTheme() const renderer = useRenderer() const [expanded, setExpanded] = createSignal(false) const [hover, setHover] = createSignal(false) - // Slot accessors are created per ref list; value changes flow through the - // individual slots without re-running the accessor construction. - const accessors = createMemo(() => - props.refs.map((ref) => ({ - ref, - part: useSlot(props.parts(ref.messageID), () => ref.partID), - })), - ) + const accessors = usePartSlots(props.parts, () => props.refs) const parts = createMemo(() => accessors().flatMap((entry) => { const message = props.message(entry.ref.messageID) @@ -1331,22 +1335,18 @@ function SessionGroupView(props: { pending: readonly PartRef[] completed: boolean message: (messageID: string) => SessionMessageInfo | undefined - parts: (messageID: string) => SessionContent.Parts + parts: (messageID: string) => SessionContent.PartsView }) { const { themeV2 } = useTheme() const ctx = use() const renderer = useRenderer() const [expanded, setExpanded] = createSignal(false) const [hover, setHover] = createSignal(false) - // Slot accessors per ref list: tool state changes flow through individual - // slots; the accessor lists rebuild only when the refs themselves change. - const slots = (refs: () => readonly PartRef[]) => - createMemo(() => refs().map((ref) => useSlot(props.parts(ref.messageID), () => ref.partID))) - const groupedSlots = slots(() => props.refs) - const pendingSlots = slots(() => props.pending) - const parts = (accessors: readonly (() => SessionContent.Part | undefined)[]) => - accessors.flatMap((accessor) => { - const part = accessor() + const groupedSlots = usePartSlots(props.parts, () => props.refs) + const pendingSlots = usePartSlots(props.parts, () => props.pending) + const parts = (entries: readonly { readonly part: () => SessionContent.Part | undefined }[]) => + entries.flatMap((entry) => { + const part = entry.part() if (part?.type !== "tool") return [] return [part] }) diff --git a/packages/tui/src/routes/session/message-navigation.ts b/packages/tui/src/routes/session/message-navigation.ts index 29c2125440..e77c549f3f 100644 --- a/packages/tui/src/routes/session/message-navigation.ts +++ b/packages/tui/src/routes/session/message-navigation.ts @@ -19,7 +19,9 @@ export function findMessageBoundary(input: { direction: "next" | "prev" children: readonly MessageChild[] messages: readonly SessionMessageInfo[] - hasText?: (messageID: string) => boolean + // Assistant text lives in SessionContent slots; the store `content` field is + // fetch-time only, so callers must supply the live text predicate. + hasText: (messageID: string) => boolean scrollTop: number viewportY: number currentID?: string @@ -36,9 +38,7 @@ export function findMessageBoundary(input: { return [{ id: child.id, y, top: y }] } if (input.userOnly || message.type !== "assistant") return [] - const hasText = - input.hasText?.(message.id) ?? message.content.some((content) => content.type === "text" && content.text.trim()) - if (!hasText) return [] + if (!input.hasText(message.id)) return [] const y = input.scrollTop + child.y - input.viewportY return [{ id: child.id, y, top: Math.max(0, y - 1) }] }) diff --git a/packages/tui/src/routes/session/permission.tsx b/packages/tui/src/routes/session/permission.tsx index 97c3852276..9f596a017e 100644 --- a/packages/tui/src/routes/session/permission.tsx +++ b/packages/tui/src/routes/session/permission.tsx @@ -15,9 +15,23 @@ import { getScrollAcceleration } from "../../util/scroll" import { useConfig } from "../../config" import { Keymap } from "../../context/keymap" import { usePathFormatter } from "../../context/path-format" +import { useSlot } from "@opencode-ai/quark/solid" type PermissionStage = "permission" | "always" | "reject" +/** Resolved tool input for a permission request, once input streaming settles. */ +export function usePermissionInput(request: PermissionV2Request): () => Record { + const data = useData() + const tool = request.source + if (!tool) return () => ({}) + const part = useSlot(data.session.message.parts(request.sessionID, tool.messageID), () => tool.callID) + return createMemo(() => { + const item = part() + if (item?.type === "tool" && item.state.status !== "streaming") return item.state.input + return {} + }) +} + function EditBody(props: { request: PermissionV2Request; patch?: string }) { const themeState = useTheme() const themeV2 = themeState.themeV2 @@ -143,13 +157,7 @@ export function PermissionPrompt(props: { request: PermissionV2Request; director const pathFormatter = usePathFormatter() const session = createMemo(() => data.session.get(props.request.sessionID)) - const input = createMemo(() => { - const tool = props.request.source - if (!tool) return {} - const part = data.session.message.parts(props.request.sessionID, tool.messageID).get(tool.callID)?.() - if (part?.type === "tool" && part.state.status !== "streaming") return part.state.input - return {} - }) + const input = usePermissionInput(props.request) const { themeV2 } = useTheme() diff --git a/packages/tui/src/routes/session/rows.ts b/packages/tui/src/routes/session/rows.ts index 931dc7946c..c8c697a1c8 100644 --- a/packages/tui/src/routes/session/rows.ts +++ b/packages/tui/src/routes/session/rows.ts @@ -1,8 +1,9 @@ import { Keyed } from "@opencode-ai/quark" import { useValue } from "@opencode-ai/quark/solid" import { batch, createEffect, on, onCleanup, type Accessor } from "solid-js" -import { useData } from "../../context/data" +import { messageIDFromEvent, useData } from "../../context/data" import { useClient } from "../../context/client" +import { SessionContent } from "./content" import { SessionTimeline, compactionQueuedRow, @@ -36,7 +37,6 @@ export function createSessionRows(sessionID: Accessor, options?: { reado state.replace(value, isPending, pendingPermissions()) }) } - const mutate = (f: () => void) => batch(f) function reduce() { const messages = data.session.message.list(sessionID()) @@ -69,7 +69,7 @@ export function createSessionRows(sessionID: Accessor, options?: { reado createEffect(() => { const pending = pendingPermissions() - mutate(() => state.repartition(pending)) + batch(() => state.repartition(pending)) }) createEffect( @@ -132,20 +132,24 @@ export function createSessionRows(sessionID: Accessor, options?: { reado ) const appendMessage = (messageID: string) => - mutate(() => { + batch(() => { const pending = isPending(messageID) const message = data.session.message.get(sessionID(), messageID) state.appendMessage(messageID, { pending, compaction: message?.type === "compaction" }) }) - const appendPart = (ref: PartRef, part: AppendPart) => mutate(() => state.appendPart(ref, part)) + const appendPart = (ref: PartRef, part: AppendPart) => { + // Streaming deltas after the first are no-ops; skip the batch machinery. + if (state.hasPart(ref)) return + batch(() => state.appendPart(ref, part)) + } - const appendFooter = (messageID: string) => mutate(() => state.appendFooter(messageID)) + const appendFooter = (messageID: string) => batch(() => state.appendFooter(messageID)) - const removeFooter = (messageID: string) => mutate(() => state.removeFooter(messageID)) + const removeFooter = (messageID: string) => batch(() => state.removeFooter(messageID)) const message = (event: { id: string; data: { sessionID: string } }) => { - if (event.data.sessionID === sessionID()) appendMessage(event.id.replace(/^evt_/, "msg_")) + if (event.data.sessionID === sessionID()) appendMessage(messageIDFromEvent(event.id)) } const input = (event: { data: { @@ -163,35 +167,41 @@ export function createSessionRows(sessionID: Accessor, options?: { reado const subscriptions = [ data.on("session.input.admitted", input), data.on("session.compaction.started", (event) => { - if (event.data.sessionID === sessionID()) appendMessage(event.data.inputID ?? event.id.replace(/^evt_/, "msg_")) + if (event.data.sessionID === sessionID()) appendMessage(event.data.inputID ?? messageIDFromEvent(event.id)) }), data.on("session.instructions.updated", message), data.on("session.synthetic", (event) => { if (event.data.sessionID === sessionID() && event.data.description?.trim()) - appendMessage(event.id.replace(/^evt_/, "msg_")) + appendMessage(messageIDFromEvent(event.id)) }), data.on("session.shell.started", message), data.on("session.agent.selected", message), data.on("session.model.selected", message), data.on("session.text.delta", (event) => { if (event.data.sessionID === sessionID() && event.data.delta.trim()) - appendPart({ messageID: event.data.assistantMessageID, partID: `text:${event.data.ordinal}` }, { type: "text" }) + appendPart( + { messageID: event.data.assistantMessageID, partID: SessionContent.textID(event.data.ordinal) }, + { type: "text" }, + ) }), data.on("session.text.ended", (event) => { if (event.data.sessionID === sessionID() && event.data.text.trim()) - appendPart({ messageID: event.data.assistantMessageID, partID: `text:${event.data.ordinal}` }, { type: "text" }) + appendPart( + { messageID: event.data.assistantMessageID, partID: SessionContent.textID(event.data.ordinal) }, + { type: "text" }, + ) }), data.on("session.reasoning.delta", (event) => { if (event.data.sessionID === sessionID() && event.data.delta.trim()) appendPart( - { messageID: event.data.assistantMessageID, partID: `reasoning:${event.data.ordinal}` }, + { messageID: event.data.assistantMessageID, partID: SessionContent.reasoningID(event.data.ordinal) }, { type: "reasoning" }, ) }), data.on("session.reasoning.ended", (event) => { if (event.data.sessionID === sessionID() && event.data.text.trim()) appendPart( - { messageID: event.data.assistantMessageID, partID: `reasoning:${event.data.ordinal}` }, + { messageID: event.data.assistantMessageID, partID: SessionContent.reasoningID(event.data.ordinal) }, { type: "reasoning" }, ) }), diff --git a/packages/tui/src/routes/session/timeline.ts b/packages/tui/src/routes/session/timeline.ts index 924db1e5b3..8a47763a77 100644 --- a/packages/tui/src/routes/session/timeline.ts +++ b/packages/tui/src/routes/session/timeline.ts @@ -177,6 +177,8 @@ export namespace SessionTimeline { appendFooter, removeFooter, repartition, + /** Cheap no-op check so per-delta callers can skip batching machinery. */ + hasPart: (ref: PartRef) => state.hasMember("parts", partRowID(ref)), } } } diff --git a/packages/tui/test/cli/tui/message-navigation.test.ts b/packages/tui/test/cli/tui/message-navigation.test.ts index 97a29458f3..8f4936bc64 100644 --- a/packages/tui/test/cli/tui/message-navigation.test.ts +++ b/packages/tui/test/cli/tui/message-navigation.test.ts @@ -7,6 +7,10 @@ const messages: SessionMessageInfo[] = [ assistant("assistant-1", "Response"), { type: "user", id: "user-2", text: "Second", time: { created: 2 } }, ] +const hasText = (messageID: string) => { + const message = messages.find((item) => item.id === messageID) + return message?.type === "assistant" && message.content.some((part) => part.type === "text" && part.text.trim()) +} const children = [ { id: "user-1", y: 0 }, { id: "assistant-1", y: 20 }, @@ -24,6 +28,7 @@ test("finds the next user message without stopping at an assistant message", () direction: "next", children, messages, + hasText, scrollTop: 0, viewportY: 0, userOnly: true, @@ -37,6 +42,7 @@ test("finds the previous user message without stopping at an assistant message", direction: "prev", children: children.map((child) => ({ ...child, y: child.y - 35 })), messages, + hasText, scrollTop: 35, viewportY: 0, userOnly: true, @@ -50,6 +56,7 @@ test("preserves navigation across both user and assistant messages", () => { direction: "next", children, messages, + hasText, scrollTop: 0, viewportY: 0, }), @@ -59,6 +66,7 @@ test("preserves navigation across both user and assistant messages", () => { direction: "prev", children: children.map((child) => ({ ...child, y: child.y - 35 })), messages, + hasText, scrollTop: 35, viewportY: 0, }), @@ -71,6 +79,7 @@ test("uses the selected message when the viewport is too tall to scroll", () => direction: "next", children, messages, + hasText, scrollTop: 0, viewportY: 0, currentID: "user-1", @@ -82,6 +91,7 @@ test("uses the selected message when the viewport is too tall to scroll", () => direction: "prev", children, messages, + hasText, scrollTop: 0, viewportY: 0, currentID: "user-2", @@ -96,6 +106,7 @@ test("stops at the first and last selected user message", () => { direction: "next", children, messages, + hasText, scrollTop: 0, viewportY: 0, currentID: "user-2", @@ -107,6 +118,7 @@ test("stops at the first and last selected user message", () => { direction: "prev", children, messages, + hasText, scrollTop: 0, viewportY: 0, currentID: "user-1", @@ -121,6 +133,7 @@ test("keeps the logical boundary when layout temporarily moves it outside the vi direction: "next", children, messages, + hasText, scrollTop: 0, viewportY: 0, currentID: "user-2", @@ -135,6 +148,7 @@ test("stops at the first and last message", () => { direction: "next", children, messages, + hasText, scrollTop: 0, viewportY: 0, currentID: "user-2", @@ -145,6 +159,7 @@ test("stops at the first and last message", () => { direction: "prev", children, messages, + hasText, scrollTop: 0, viewportY: 0, currentID: "user-1", diff --git a/packages/tui/test/cli/tui/permission.test.tsx b/packages/tui/test/cli/tui/permission.test.tsx new file mode 100644 index 0000000000..fe89af4cb0 --- /dev/null +++ b/packages/tui/test/cli/tui/permission.test.tsx @@ -0,0 +1,115 @@ +/** @jsxImportSource @opentui/solid */ +import { expect, test } from "bun:test" +import { testRender } from "@opentui/solid" +import type { OpenCodeEvent, PermissionV2Request } from "@opencode-ai/client" +import { createEffect, type ParentProps } from "solid-js" +import { ClientProvider, useClient } from "../../../src/context/client" +import { DataProvider as DataProviderBase, useData } from "../../../src/context/data" +import { LocationProvider, useLocation } from "../../../src/context/location" +import { usePermissionInput } from "../../../src/routes/session/permission" +import { createApi, createEventStream, createFetch, directory, json } from "../../fixture/tui-client" +import { TestTuiContexts } from "../../fixture/tui-environment" + +async function wait(fn: () => boolean, timeout = 2000) { + const start = Date.now() + while (!fn()) { + if (Date.now() - start > timeout) throw new Error("timed out waiting for condition") + await Bun.sleep(10) + } +} + +function SyncLocation() { + const data = useData() + const location = useLocation() + createEffect(() => location.set(data.location.default())) + return null +} + +function DataProvider(props: ParentProps) { + return ( + + + + {props.children} + + + ) +} + +function emitEvent(events: ReturnType, event: OpenCodeEvent) { + events.emit({ ...event, location: { directory } }) +} + +test("permission input tracks the tool part slot as input settles", async () => { + const events = createEventStream() + const sessionID = "session-permission-input" + const calls = createFetch((url) => { + if (url.pathname === `/api/session/${sessionID}/message`) return json({ data: [], cursor: {} }) + }, events) + let data!: ReturnType + let client!: ReturnType + let input!: () => unknown + + const request = { + id: "perm_1", + sessionID, + permission: "shell", + resources: [], + metadata: {}, + source: { messageID: "message-assistant", callID: "call-permission" }, + time: { created: 1 }, + } as unknown as PermissionV2Request + + function Probe() { + data = useData() + client = useClient() + // Mounted like the permission dialog: before the tool call has settled. + input = usePermissionInput(request) + return + } + + const app = await testRender(() => ( + + + + + + + + )) + + try { + await wait(() => client.connection.status() === "connected") + expect(input()).toEqual({}) + + emitEvent(events, { + id: "evt_perm_tool_started", + created: 1, + type: "session.tool.input.started", + durable: { aggregateID: `session_${sessionID}`, seq: 0, version: 1 }, + data: { sessionID, assistantMessageID: "message-assistant", callID: "call-permission", name: "shell" }, + } as unknown as OpenCodeEvent) + emitEvent(events, { + id: "evt_perm_tool_called", + created: 2, + type: "session.tool.called", + durable: { aggregateID: `session_${sessionID}`, seq: 1, version: 1 }, + data: { + sessionID, + assistantMessageID: "message-assistant", + callID: "call-permission", + input: { command: "rm -rf ./dist" }, + timestamp: 2, + }, + } as unknown as OpenCodeEvent) + + // The prompt is already mounted; the resolved input must flow through the + // part slot once the tool call settles out of input streaming. + await wait(() => { + const value = input() + return typeof value === "object" && value !== null && (value as { command?: string }).command === "rm -rf ./dist" + }) + } finally { + app.renderer.destroy() + } +})