From c07d0e495f0fdbbb794c978c32f4c43aa67914ad Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 25 Aug 2026 17:06:27 -0700 Subject: [PATCH] fix(tui): fence stale session actions and preserve tool output (#129681) --- src/tui/commands.test.ts | 15 ++++ src/tui/commands.ts | 19 ++--- src/tui/components/tool-execution.test.ts | 39 ++++++++++ src/tui/components/tool-execution.ts | 4 +- src/tui/tui-command-handlers.test.ts | 78 +++++++++++++++++++ src/tui/tui-command-handlers.ts | 45 +++++++---- .../tui-pty-command-surfaces-test-support.ts | 3 +- src/tui/tui-pty-rendering-test-support.ts | 16 +++- src/tui/tui-session-actions.test.ts | 30 ++++++- src/tui/tui-session-actions.ts | 10 ++- 10 files changed, 223 insertions(+), 36 deletions(-) diff --git a/src/tui/commands.test.ts b/src/tui/commands.test.ts index 2bb16d2ec0e9..a4fa65119962 100644 --- a/src/tui/commands.test.ts +++ b/src/tui/commands.test.ts @@ -240,6 +240,21 @@ describe("helpText", () => { }, ); + it("uses session-supported thinking levels in help before the provider fallback", () => { + const model = { provider: "minimax", model: "MiniMax-M3" }; + + expect( + helpText({ + ...model, + thinkingLevels: [ + { id: "off", label: "off" }, + { id: "max", label: "max" }, + ], + }), + ).toContain("/think "); + expect(helpText({ ...model, thinkingLevels: [] })).toContain("/think "); + }); + it("documents default reset values for model, thinking, and fast mode", () => { const output = helpText(); diff --git a/src/tui/commands.ts b/src/tui/commands.ts index 183881840397..7524cb381767 100644 --- a/src/tui/commands.ts +++ b/src/tui/commands.ts @@ -8,7 +8,6 @@ import { resolveTextCommand, } from "../auto-reply/commands-registry.js"; import { - formatThinkingLevels, listThinkingLevelLabels, type ReasoningLevel, type VerboseLevel, @@ -47,6 +46,12 @@ type SlashCommandOptions = { dynamicCommands?: CommandEntry[]; }; +function resolveThinkingLevelLabels(options: SlashCommandOptions): string[] { + return options.thinkingLevels?.length + ? options.thinkingLevels.map((level) => level.label) + : listThinkingLevelLabels(options.provider, options.model, undefined, options.agentRuntime); +} + function createLevelCompletion( levels: string[], ): NonNullable { @@ -243,9 +248,7 @@ export function isSharedTextCommand(input: string): boolean { } export function getSlashCommands(options: SlashCommandOptions = {}): SlashCommand[] { - const thinkLevels = options.thinkingLevels?.length - ? options.thinkingLevels.map((level) => level.label) - : listThinkingLevelLabels(options.provider, options.model, undefined, options.agentRuntime); + const thinkLevels = resolveThinkingLevelLabels(options); const commands: SlashCommand[] = []; const seen = new Map(); for (const command of TUI_COMMAND_DESCRIPTORS) { @@ -324,13 +327,7 @@ export function shouldSubmitExactArgumentCompletion( } export function helpText(options: SlashCommandOptions = {}): string { - const thinkLevels = formatThinkingLevels( - options.provider, - options.model, - "|", - undefined, - options.agentRuntime, - ); + const thinkLevels = resolveThinkingLevelLabels(options).join("|"); const commandHelp = TUI_COMMAND_DESCRIPTORS.flatMap((command) => { if (!command.help || !commandIsVisible(command, options.local === true)) { return []; diff --git a/src/tui/components/tool-execution.test.ts b/src/tui/components/tool-execution.test.ts index 41b8f42a649b..11ac75549755 100644 --- a/src/tui/components/tool-execution.test.ts +++ b/src/tui/components/tool-execution.test.ts @@ -13,6 +13,45 @@ function renderToolOutput(text: string, width: number) { } describe("ToolExecutionComponent", () => { + it.each( + [ + { source: " # heading\n command --flag", literal: "# heading" }, + { source: " > quoted source\n next line", literal: "> quoted source" }, + { source: " - source item\n nested", literal: "- source item" }, + ].flatMap(({ source, literal }) => [ + { source, literal, phase: "partial", complete: false }, + { source, literal, phase: "final", complete: true }, + ]), + )("preserves indented $literal in $phase tool output", ({ source, literal, complete }) => { + const component = new ToolExecutionComponent("read_file", { path: "example.txt" }); + const result = { content: [{ type: "text", text: source }] }; + if (complete) { + component.setResult(result); + } else { + component.setPartialResult(result); + } + + const rendered = component.render(80).map(normalizeTestText).join("\n"); + expect(rendered).toContain("```"); + expect(rendered).toContain(literal); + }); + + it.each([ + { phase: "partial", complete: false }, + { phase: "final", complete: true }, + ])("keeps whitespace-only $phase tool output visually empty", ({ complete }) => { + const component = new ToolExecutionComponent("read_file", { path: "example.txt" }); + const result = { content: [{ type: "text", text: " \n " }] }; + if (complete) { + component.setResult(result); + } else { + component.setPartialResult(result); + } + + const rendered = component.render(80).map(normalizeTestText).join("\n"); + expect(rendered.includes("...")).toBe(!complete); + }); + it.each([ { width: 20, characters: 8_192 }, { width: 20, characters: 16_384 }, diff --git a/src/tui/components/tool-execution.ts b/src/tui/components/tool-execution.ts index 7e4493b63416..548868c88dbe 100644 --- a/src/tui/components/tool-execution.ts +++ b/src/tui/components/tool-execution.ts @@ -105,7 +105,7 @@ function extractText(result?: ToolResult): string { lines.push(`[${mime}${size}${omitted}]`); } } - return lines.join("\n").trim(); + return lines.join("\n"); } /** Displays a running or completed tool call with optional expandable output. */ @@ -187,7 +187,7 @@ export class ToolExecutionComponent extends Container { this.argsLine.setText(argLine ? theme.dim(argLine) : theme.dim(" ")); const raw = extractText(this.result); - const text = raw || (this.isPartial ? "…" : ""); + const text = raw.trim() ? raw : this.isPartial ? "…" : ""; this.output.setExpanded(this.expanded); this.output.setText(text); } diff --git a/src/tui/tui-command-handlers.test.ts b/src/tui/tui-command-handlers.test.ts index 631fc6f40710..271f35c8a0f8 100644 --- a/src/tui/tui-command-handlers.test.ts +++ b/src/tui/tui-command-handlers.test.ts @@ -2071,6 +2071,52 @@ describe("tui command handlers", () => { }, ); + it.each([ + { command: "/think high", rejected: false }, + { command: "/verbose full", rejected: false }, + { command: "/usage reset", rejected: false }, + { command: "/model openai/gpt-5.6-luna", rejected: true }, + ])( + "ignores a stale $command patch after the same session is reset", + async ({ command, rejected }) => { + const deferred = createDeferred<{ + ok: true; + path: string; + key: string; + entry: Record; + }>(); + const harness = createHarness({ + currentSessionId: "session-before-reset", + sessionGeneration: 4, + sessionInfo: { responseUsage: "tokens", effectiveResponseUsage: "tokens" }, + patchSession: vi.fn(() => deferred.promise), + }); + + const pending = harness.handleCommand(command); + harness.state.currentSessionId = "session-after-reset"; + harness.state.sessionGeneration = 5; + if (rejected) { + deferred.reject(new Error("stale session setting")); + } else { + deferred.resolve({ + ok: true, + path: "/sessions/patch", + key: "agent:main:main", + entry: { model: "stale-model" }, + }); + } + await pending; + + expect(harness.state.sessionInfo.responseUsage).toBe("tokens"); + expect(harness.state.sessionInfo.effectiveResponseUsage).toBe("tokens"); + expect(harness.applySessionInfoFromPatch).not.toHaveBeenCalled(); + expect(harness.refreshSessionInfo).not.toHaveBeenCalled(); + expect(harness.loadHistory).not.toHaveBeenCalled(); + expect(harness.clearTools).not.toHaveBeenCalled(); + expect(harness.addSystem).not.toHaveBeenCalled(); + }, + ); + it("applies a model patch after its selected session becomes canonical", async () => { const deferred = createDeferred<{ ok: true; @@ -2119,6 +2165,11 @@ describe("tui command handlers", () => { it.each([ { name: "another session", initialKey: "agent:main:first", nextKey: "agent:main:second" }, { name: "another global agent", initialKey: "global", nextKey: "global" }, + { + name: "a replacement incarnation", + initialKey: "agent:main:main", + nextKey: "agent:main:main", + }, ])("ignores a stale reset after selecting $name", async ({ initialKey, nextKey }) => { const deferred = createDeferred<{ ok: true; @@ -2128,6 +2179,8 @@ describe("tui command handlers", () => { const harness = createHarness({ currentSessionKey: initialKey, currentAgentId: "main", + currentSessionId: "first-session", + sessionGeneration: 4, resetSession: vi.fn(() => deferred.promise), applySessionMutationResult: vi.fn().mockReturnValue(true), }); @@ -2142,6 +2195,9 @@ describe("tui command handlers", () => { if (initialKey === "global") { harness.state.currentAgentId = "work"; } + if (initialKey === nextKey && initialKey !== "global") { + harness.state.sessionGeneration += 1; + } harness.state.currentSessionId = "second-session"; deferred.resolve({ ok: true, @@ -2249,6 +2305,28 @@ describe("tui command handlers", () => { expect(openclaw.addSystem).toHaveBeenCalledWith(expect.stringContaining("ultra")); }); + it("uses the active session's supported thinking levels in help and command usage", async () => { + const { handleCommand, addSystem } = createHarness({ + sessionInfo: { + modelProvider: "minimax", + model: "MiniMax-M3", + thinkingLevels: [ + { id: "off", label: "off" }, + { id: "max", label: "max" }, + ], + }, + }); + + await handleCommand("/help"); + await handleCommand("/think"); + + expect(addSystem).toHaveBeenNthCalledWith( + 1, + expect.stringContaining("/think "), + ); + expect(addSystem).toHaveBeenNthCalledWith(2, "usage: /think "); + }); + it.each([ { command: "verbose", usage: "usage: /verbose " }, { command: "reasoning", usage: "usage: /reasoning " }, diff --git a/src/tui/tui-command-handlers.ts b/src/tui/tui-command-handlers.ts index 10e1afee0220..05e1195e4368 100644 --- a/src/tui/tui-command-handlers.ts +++ b/src/tui/tui-command-handlers.ts @@ -264,19 +264,33 @@ export function createCommandHandlers(context: CommandHandlerContext) { state.currentAgentId === selection.agentId && agentSessionKeysMatchByRequestKey(state.currentSessionKey, selection.sessionKey); + const captureSessionIncarnation = () => { + const selection = captureSessionSelection(); + const sessionId = state.currentSessionId; + const generation = state.sessionGeneration ?? 0; + return { + selection, + sessionId, + isCurrent: () => + isCurrentSessionSelection(selection) && + (state.sessionGeneration ?? 0) === generation && + (sessionId === null || state.currentSessionId === sessionId), + }; + }; + const patchCurrentSession = async ( patch: Omit[0], "key" | "agentId">, ): Promise => { - const selection = captureSessionSelection(); + const { selection, isCurrent } = captureSessionIncarnation(); try { const result = await client.patchSession({ key: selection.sessionKey, ...(!parseAgentSessionKey(selection.sessionKey) ? { agentId: selection.agentId } : {}), ...patch, }); - return isCurrentSessionSelection(selection) ? result : null; + return isCurrent() ? result : null; } catch (err) { - if (!isCurrentSessionSelection(selection)) { + if (!isCurrent()) { return null; } throw err; @@ -467,6 +481,7 @@ export function createCommandHandlers(context: CommandHandlerContext) { provider: state.sessionInfo.modelProvider, model: state.sessionInfo.model, agentRuntime: state.sessionInfo.agentRuntime?.id, + thinkingLevels: state.sessionInfo.thinkingLevels, }), ); }, @@ -787,8 +802,8 @@ export function createCommandHandlers(context: CommandHandlerContext) { if (rejectUnsafeSessionRollover("reset")) { return; } - const resetSelection = captureSessionSelection(); - let resetResultSelection = resetSelection; + let resetIncarnation = captureSessionIncarnation(); + const resetSelection = resetIncarnation.selection; const finishSessionTransition = beginSessionTransition("reset"); try { const result = await client.resetSession( @@ -798,7 +813,7 @@ export function createCommandHandlers(context: CommandHandlerContext) { ? { agentId: resetSelection.agentId } : undefined, ); - if (!isCurrentSessionSelection(resetSelection)) { + if (!resetIncarnation.isCurrent()) { return; } state.sessionInfo.inputTokens = null; @@ -806,17 +821,17 @@ export function createCommandHandlers(context: CommandHandlerContext) { state.sessionInfo.totalTokens = null; tui.requestRender(); if (applySessionMutationResult(result, resetSelection)) { - resetResultSelection = captureSessionSelection(); + resetIncarnation = captureSessionIncarnation(); await refreshSessionInfo(); } else { await loadHistory(); } - if (!isCurrentSessionSelection(resetResultSelection)) { + if (!resetIncarnation.isCurrent()) { return; } chatLog.addSystem(`session ${state.currentSessionKey} reset`); } catch (err) { - if (!isCurrentSessionSelection(resetResultSelection)) { + if (!resetIncarnation.isCurrent()) { return; } chatLog.addSystem(`reset failed: ${formatTuiErrorMessage(err)}`); @@ -875,13 +890,11 @@ export function createCommandHandlers(context: CommandHandlerContext) { // The Gateway owns queue policy. TUI only serializes pending RPC admission; // an already-active run must not suppress steer/followup/collect/interrupt. const runId = randomUUID(); - const sendSelection = captureSessionSelection(); - const sendSessionId = state.currentSessionId; - const sendSessionGeneration = state.sessionGeneration ?? 0; - const isCurrentSendViewport = () => - isCurrentSessionSelection(sendSelection) && - (state.sessionGeneration ?? 0) === sendSessionGeneration && - (sendSessionId === null || state.currentSessionId === sendSessionId); + const { + selection: sendSelection, + sessionId: sendSessionId, + isCurrent: isCurrentSendViewport, + } = captureSessionIncarnation(); const sendScope = readTuiSessionProjectionScope(state); try { if (!isBtw) { diff --git a/src/tui/tui-pty-command-surfaces-test-support.ts b/src/tui/tui-pty-command-surfaces-test-support.ts index 706ad93e9636..8b936837d975 100644 --- a/src/tui/tui-pty-command-surfaces-test-support.ts +++ b/src/tui/tui-pty-command-surfaces-test-support.ts @@ -6,7 +6,7 @@ import { import { objectFieldEquals, readFixtureLog } from "./tui-pty-harness-fixture-test-support.js"; const slashHelpMarkers = - "Slash commands:,/help,/verbose ,/reasoning ,/goal,/goal start ,/btw ,/queue,/stop,/exit".split( + "Slash commands:,/help,/think ,/verbose ,/reasoning ,/goal,/goal start ,/btw ,/queue,/stop,/exit".split( ",", ); const countHistoryLoads = async (logPath: string) => @@ -23,6 +23,7 @@ export async function exerciseTuiCommandSurface( env: { OPENCLAW_TUI_PTY_COLS: "100", OPENCLAW_TUI_PTY_ROWS: "24", + ...(surface === "slash-commands" ? { OPENCLAW_TUI_PTY_SAFE_THINKING_LABEL: "max" } : {}), ...(surface === "pickers" ? { OPENCLAW_TUI_PTY_PICKER_FIXTURE: "1" } : {}), }, }); diff --git a/src/tui/tui-pty-rendering-test-support.ts b/src/tui/tui-pty-rendering-test-support.ts index 8e1a0b2cdc63..ababffa43674 100644 --- a/src/tui/tui-pty-rendering-test-support.ts +++ b/src/tui/tui-pty-rendering-test-support.ts @@ -23,7 +23,15 @@ export function toolFrame(rows: string[], complete: boolean) { const before = frame.indexOf("PTY_BEFORE_TOOL"); const running = frame.indexOf("Read File (running)"); const partial = frame.indexOf("PTY_TOOL_PARTIAL"); - return before >= 0 && running >= 0 && partial >= 0 && before < running && running < partial; + return ( + before >= 0 && + running >= 0 && + partial >= 0 && + before < running && + running < partial && + frame.includes("# PTY_TOOL_PARTIAL") && + frame.includes("```") + ); } const markers = ["PTY_BEFORE_TOOL", "Read File", "PTY_TOOL_RESULT", "PTY_AFTER_TOOL"]; return ( @@ -34,6 +42,8 @@ export function toolFrame(rows: string[], complete: boolean) { ) && !frame.includes("(running)") && !frame.includes("PTY_TOOL_PARTIAL") && + frame.includes("> PTY_TOOL_RESULT") && + frame.includes("```") && frame.includes("idle") ); } @@ -113,9 +123,9 @@ export const TUI_PTY_RENDERING_FIXTURE_SCRIPT = ` const base = { toolCallId: "pty-rendering-tool", name: process.env.OPENCLAW_TUI_PTY_TOOL_NAME ?? "read_file" }; backend.onEvent?.({ event: "agent", payload: { runId, sessionKey, stream: "tool", data: { ...base, phase: "start", args: { path: "chronology-proof.txt" } } } }); if (process.env.OPENCLAW_TUI_PTY_VERBOSE_LEVEL === "full") { - backend.onEvent?.({ event: "agent", payload: { runId, sessionKey, stream: "tool", data: { ...base, phase: "update", partialResult: { content: [{ type: "text", text: "PTY_TOOL_PARTIAL" }] } } } }); + backend.onEvent?.({ event: "agent", payload: { runId, sessionKey, stream: "tool", data: { ...base, phase: "update", partialResult: { content: [{ type: "text", text: " # PTY_TOOL_PARTIAL" }] } } } }); record("toolPartialReady", { runId }); await waitForRenderingRelease("tool"); - backend.onEvent?.({ event: "agent", payload: { runId, sessionKey, stream: "tool", data: { ...base, phase: "result", result: { content: [{ type: "text", text: "PTY_TOOL_RESULT" }] } } } }); + backend.onEvent?.({ event: "agent", payload: { runId, sessionKey, stream: "tool", data: { ...base, phase: "result", result: { content: [{ type: "text", text: " > PTY_TOOL_RESULT" }] } } } }); } const finalText = "PTY_BEFORE_TOOL\\n\\nPTY_AFTER_TOOL"; emitAssistant(backend, runId, sessionKey, "delta", finalText); emitAssistant(backend, runId, sessionKey, "final", finalText); record("toolComplete", { runId }); record("toolChronologyComplete", { runId }); } diff --git a/src/tui/tui-session-actions.test.ts b/src/tui/tui-session-actions.test.ts index 6647f5cf25bf..aef29bd0ce2e 100644 --- a/src/tui/tui-session-actions.test.ts +++ b/src/tui/tui-session-actions.test.ts @@ -2459,6 +2459,27 @@ describe("tui session actions", () => { aborted: false, rejected: true, }, + { + name: "successful abort after the same session is replaced", + initialKey: "agent:main:main", + nextKey: "agent:main:main", + aborted: true, + rejected: false, + }, + { + name: "no-active-run abort after the same session is replaced", + initialKey: "agent:main:main", + nextKey: "agent:main:main", + aborted: false, + rejected: false, + }, + { + name: "rejected abort after the same session is replaced", + initialKey: "agent:main:main", + nextKey: "agent:main:main", + aborted: false, + rejected: true, + }, ])("ignores a $name", async ({ initialKey, nextKey, aborted, rejected }) => { const deferred = createDeferred>>(); const abortChat = vi.fn(() => deferred.promise); @@ -2476,6 +2497,8 @@ describe("tui session actions", () => { const state = createBaseState({ currentSessionKey: initialKey, currentAgentId: "main", + currentSessionId: "first-session", + sessionGeneration: 4, activeChatRunId: "first-active-run", pendingSubmit: acceptedSubmit("first-pending-run"), }); @@ -2495,7 +2518,12 @@ describe("tui session actions", () => { sessionKey: initialKey, ...(initialKey === "global" ? { agentId: "main" } : {}), }); - await setSession(nextKey, initialKey === "global" ? "work" : undefined); + if (initialKey === nextKey && initialKey !== "global") { + state.sessionGeneration = (state.sessionGeneration ?? 0) + 1; + state.currentSessionId = "second-session"; + } else { + await setSession(nextKey, initialKey === "global" ? "work" : undefined); + } state.activeChatRunId = "second-active-run"; state.pendingSubmit = acceptedSubmit("second-pending-run", "second draft"); addSystem.mockClear(); diff --git a/src/tui/tui-session-actions.ts b/src/tui/tui-session-actions.ts index d799d9f0def4..50d0cbf29bb7 100644 --- a/src/tui/tui-session-actions.ts +++ b/src/tui/tui-session-actions.ts @@ -659,8 +659,14 @@ export function createSessionActions(context: SessionActionContext) { return; } const selection = captureSessionSelection(); + const sessionId = state.currentSessionId; + const sessionGeneration = state.sessionGeneration ?? 0; const pendingRunId = submit.getPendingSubmitAcceptedRunId(state); const activeRunId = state.activeChatRunId; + const isCurrentAbort = () => + isCurrentSessionSelection(selection) && + (state.sessionGeneration ?? 0) === sessionGeneration && + (sessionId === null || state.currentSessionId === sessionId); const dropPendingRun = (runId: string) => { reduceTuiSessionProjection(state, { type: "sendFailed", @@ -677,7 +683,7 @@ export function createSessionActions(context: SessionActionContext) { sessionKey: selection.sessionKey, ...(!parseAgentSessionKey(selection.sessionKey) ? { agentId: selection.agentId } : {}), }); - if (!isCurrentSessionSelection(selection)) { + if (!isCurrentAbort()) { return; } if (!result.aborted) { @@ -704,7 +710,7 @@ export function createSessionActions(context: SessionActionContext) { } setActivityStatus("aborted"); } catch (err) { - if (!isCurrentSessionSelection(selection)) { + if (!isCurrentAbort()) { return; } chatLog.addSystem(`abort failed: ${formatTuiErrorMessage(err)}`);