From b8a95e082bbf229ce0d97cb528bb8739b8f49c8f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 16 Aug 2026 16:15:24 -0700 Subject: [PATCH] fix(ui): make terminal open failures retryable (#124890) --- .../terminal/terminal-connection.ts | 2 +- .../terminal/terminal-panel-chrome.ts | 33 +++-- .../terminal/terminal-panel-readiness.test.ts | 13 +- .../terminal-panel-session-controller.ts | 21 ++-- .../terminal/terminal-panel-styles.ts | 13 ++ ui/src/components/terminal/terminal-panel.ts | 23 +++- .../terminal/terminal-pending-actions.ts | 42 +++++++ ui/src/e2e/terminal-open.e2e.test.ts | 119 ++++++++++++++++++ ui/src/i18n/locales/en.ts | 2 + ui/src/test-helpers/control-ui-e2e.ts | 107 ++++++++++++++++ 10 files changed, 349 insertions(+), 26 deletions(-) create mode 100644 ui/src/e2e/terminal-open.e2e.test.ts diff --git a/ui/src/components/terminal/terminal-connection.ts b/ui/src/components/terminal/terminal-connection.ts index 32767a48d54c..cb591b2f7745 100644 --- a/ui/src/components/terminal/terminal-connection.ts +++ b/ui/src/components/terminal/terminal-connection.ts @@ -101,7 +101,7 @@ export class TerminalOpenTimeoutError extends Error { * tab label, uploads, or a replay. Fail here so the panel reports an unusable * gateway instead of surfacing a downstream TypeError as its only content. */ export class TerminalOpenUnusableSessionError extends Error { - constructor(field: string) { + constructor(readonly field: string) { super(`terminal session response is missing ${field}`); this.name = "TerminalOpenUnusableSessionError"; } diff --git a/ui/src/components/terminal/terminal-panel-chrome.ts b/ui/src/components/terminal/terminal-panel-chrome.ts index bc353ecba20b..772f1676e794 100644 --- a/ui/src/components/terminal/terminal-panel-chrome.ts +++ b/ui/src/components/terminal/terminal-panel-chrome.ts @@ -17,6 +17,12 @@ import { } from "./terminal-panel-upload.ts"; type TerminalDock = Exclude; +type TerminalPanelViewportParams = { + activeId: string | null; + connecting: boolean; + error: { text: string; retry?: () => void } | null; + uploadController: TerminalPanelUploadController; +}; export function renderTerminalPanelToolbar( fullscreen: boolean, @@ -62,14 +68,23 @@ export function renderTerminalPanelHeader( `; } -export function renderTerminalPanelViewport( - activeId: string | null, - connecting: boolean, - errorText: string | null, - uploadController: TerminalPanelUploadController, -): TemplateResult { +export function renderTerminalPanelViewport({ + activeId, + connecting, + error, + uploadController, +}: TerminalPanelViewportParams): TemplateResult { return html` - ${errorText ? html`` : nothing} + ${error + ? html`` + : nothing} ${t("terminal.connecting")} ` : nothing} - ${!activeId && !connecting && !errorText + ${!activeId && !connecting && !error ? renderPanelEmptyState({ icon: icons.terminal, heading: t("chat.sidePanel.terminal"), @@ -105,7 +120,7 @@ export function terminalOpenErrorText(error: unknown): string { return t("terminal.connectionTimedOut"); } if (error instanceof TerminalOpenUnusableSessionError) { - return t("terminal.unavailable"); + return t("terminal.unusableSession", { field: error.field }); } return formatUiError(error); } diff --git a/ui/src/components/terminal/terminal-panel-readiness.test.ts b/ui/src/components/terminal/terminal-panel-readiness.test.ts index be2b78d6a59c..b7cdf86b992e 100644 --- a/ui/src/components/terminal/terminal-panel-readiness.test.ts +++ b/ui/src/components/terminal/terminal-panel-readiness.test.ts @@ -281,10 +281,11 @@ describe("terminal panel readiness", () => { panel.available = true; (panel as unknown as { catalogReadyTimeoutMs: number }).catalogReadyTimeoutMs = 5; document.body.append(panel); + const catalog = { catalogId: "anthropic", hostId: "node:mac", threadId: "thread" }; panel.handleToggleRequest( new CustomEvent("openclaw:terminal-toggle", { - detail: { catalog: { catalogId: "anthropic", hostId: "node:mac", threadId: "thread" } }, + detail: { catalog }, }), ); @@ -298,5 +299,15 @@ describe("terminal panel readiness", () => { params: { sessionId: "catalog-terminal-1" }, }); expect(panel.renderRoot.querySelector(".tabstrip-tab")).toBeNull(); + const retry = panel.renderRoot.querySelector(".tp-error button"); + expect(retry?.textContent?.trim()).toBe("Retry"); + + retry?.click(); + await waitForFast(() => { + expect(requests.filter((request) => request.method === "terminal.open")).toHaveLength(2); + }); + expect(requests.findLast((request) => request.method === "terminal.open")?.params).toEqual( + expect.objectContaining({ catalog }), + ); }); }); diff --git a/ui/src/components/terminal/terminal-panel-session-controller.ts b/ui/src/components/terminal/terminal-panel-session-controller.ts index 7e7c2af8ca63..901388740fda 100644 --- a/ui/src/components/terminal/terminal-panel-session-controller.ts +++ b/ui/src/components/terminal/terminal-panel-session-controller.ts @@ -23,7 +23,8 @@ import { type TerminalPanelSessionControllerState, type TerminalPanelSessionTab, } from "./terminal-panel-session-types.ts"; -import { terminalIntentQueue, type TerminalIntentHost } from "./terminal-pending-actions.ts"; +import { TerminalOpenRetry, terminalIntentQueue } from "./terminal-pending-actions.ts"; +import type { TerminalIntentHost } from "./terminal-pending-actions.ts"; import { loadPersistedTerminalSessionIds, persistLiveTerminalSessions, @@ -33,8 +34,6 @@ import { TerminalTabReadinessController } from "./terminal-tab-readiness.ts"; import { TerminalTaskQueue } from "./terminal-task-queue.ts"; import { terminalDynamicColors, terminalTheme } from "./terminal-theme.ts"; -type TerminalSessionSink = Parameters[1]; - /** Owns gateway PTY sessions and the Ghostty controllers bound to them. */ export class TerminalPanelSessionController implements ReactiveController, TerminalPanelSessionControllerState @@ -52,6 +51,7 @@ export class TerminalPanelSessionController private lifecycleAbortController = new AbortController(); private lifecycleSyncToken = 0; private tabSequence = 0; + readonly openRetry = new TerminalOpenRetry(); private readonly bootQueue = new TerminalTaskQueue(); private readonly intentHost: TerminalIntentHost; private readonly readiness: TerminalTabReadinessController; @@ -70,17 +70,14 @@ export class TerminalPanelSessionController requestUpdate: () => this.host.requestUpdate(), setBooting: (booting) => this.updateControllerState("booting", booting), timeoutMs: () => this.host.catalogReadyTimeoutMs, - showTimeout: () => { - this.host.terminalPanelErrorText = t("terminal.refreshRequired"); - }, - clearTimeout: () => { - this.host.terminalPanelErrorText = null; - }, + showTimeout: () => (this.host.terminalPanelErrorText = t("terminal.refreshRequired")), + clearTimeout: () => (this.host.terminalPanelErrorText = null), }; this.readiness = new TerminalTabReadinessController({ timeoutMs: () => this.host.catalogReadyTimeoutMs, isCurrent: (tab) => this.tabs.includes(tab), onReady: () => { + this.openRetry.clear(); this.updateControllerState("tabs", [...this.tabs]); persistLiveTerminalSessions(this.tabs); }, @@ -318,6 +315,7 @@ export class TerminalPanelSessionController return false; } this.updateControllerState("booting", true); + this.openRetry.clear(); this.host.terminalPanelErrorText = null; try { const attached = await this.attachSession(sessionId, operation, agentOwned); @@ -413,7 +411,7 @@ export class TerminalPanelSessionController readyTimer: null, }; tabReference.current = tab; - const sink: TerminalSessionSink = { + const sink: Parameters[1] = { // The cancelled guard also protects the buffered-event replay inside // connection.open/attach from writing to an already-disposed terminal. onData: (data: string) => { @@ -520,6 +518,7 @@ export class TerminalPanelSessionController return false; } this.updateControllerState("booting", true); + this.openRetry.remember(catalog, agentId); this.host.terminalPanelErrorText = null; // Freeze the selection for this tab; later agent changes affect only new tabs. const ownerAgentId = agentId ?? undefined; @@ -561,6 +560,7 @@ export class TerminalPanelSessionController if (!this.isTerminalOperationCurrent(operation)) { return false; } + this.openRetry.clearUnlessRetryable(error); this.host.terminalPanelErrorText = terminalOpenErrorText(error); return true; } finally { @@ -766,6 +766,7 @@ export class TerminalPanelSessionController this.lifecycleAbortController.abort(); this.lifecycleAbortController = new AbortController(); this.bootQueue.reset(); + this.openRetry.clear(); this.updateControllerState("booting", false); this.host.terminalPanelUploadController.dispose(); this.host.clearTerminalPanelResizeListeners(); diff --git a/ui/src/components/terminal/terminal-panel-styles.ts b/ui/src/components/terminal/terminal-panel-styles.ts index f2f3dd7b387c..0b0182e85e4d 100644 --- a/ui/src/components/terminal/terminal-panel-styles.ts +++ b/ui/src/components/terminal/terminal-panel-styles.ts @@ -182,10 +182,23 @@ export const terminalPanelStyles = css` animation: tp-spin 0.8s linear infinite; } .tp-error { + display: flex; + align-items: center; + justify-content: space-between; + gap: 12px; padding: 10px 12px; font-size: 12px; color: var(--danger, #ff6b6b); } + .tp-error .btn { + flex: 0 0 auto; + border: 1px solid var(--border); + border-radius: var(--radius-md); + background: var(--bg-elevated); + color: var(--text); + padding: 6px 10px; + font: inherit; + } @keyframes tp-spin { to { transform: rotate(360deg); diff --git a/ui/src/components/terminal/terminal-panel.ts b/ui/src/components/terminal/terminal-panel.ts index 44f8b3af09c8..f0e0879a6cad 100644 --- a/ui/src/components/terminal/terminal-panel.ts +++ b/ui/src/components/terminal/terminal-panel.ts @@ -378,6 +378,11 @@ export class OpenClawTerminalPanel extends OpenClawLitElement { return this.renderRoot.querySelector(".tp-viewport"); } + private retryTerminalOpen(): void { + this.terminalPanelErrorText = null; + this.terminalSessions.openRetry.run(); + } + override render() { if ( !this.available || @@ -400,6 +405,14 @@ export class OpenClawTerminalPanel extends OpenClawLitElement { this.terminalSessions.waitingForRefresh || (this.terminalSessions.booting && this.terminalSessions.tabs.length === 0) || activeTab?.status === "connecting"; + const terminalError = this.terminalPanelErrorText + ? { + text: this.terminalPanelErrorText, + retry: this.terminalSessions.openRetry.available + ? () => this.retryTerminalOpen() + : undefined, + } + : null; const sessionPicker = renderTerminalSessionPicker({ open: this.sessionPickerOpen, loading: this.sessionPickerTask.status === TaskStatus.PENDING, @@ -443,12 +456,12 @@ export class OpenClawTerminalPanel extends OpenClawLitElement { }, () => void this.terminalSessions.openSession(), )} - ${renderTerminalPanelViewport( - this.terminalSessions.activeId, + ${renderTerminalPanelViewport({ + activeId: this.terminalSessions.activeId, connecting, - this.terminalPanelErrorText, - this.terminalPanelUploadController, - )} + error: terminalError, + uploadController: this.terminalPanelUploadController, + })} `; } diff --git a/ui/src/components/terminal/terminal-pending-actions.ts b/ui/src/components/terminal/terminal-pending-actions.ts index e55eab19afd4..471b21865bfa 100644 --- a/ui/src/components/terminal/terminal-pending-actions.ts +++ b/ui/src/components/terminal/terminal-pending-actions.ts @@ -1,4 +1,8 @@ import type { TerminalPanelToggleDetail } from "../panel-toggle-contract.ts"; +import { + TerminalOpenTimeoutError, + TerminalOpenUnusableSessionError, +} from "./terminal-connection.ts"; import type { TerminalPanelAction, TerminalPanelCatalogReference, @@ -9,6 +13,44 @@ import { } from "./terminal-session-storage.ts"; import type { TerminalTaskQueue } from "./terminal-task-queue.ts"; +type RetryOpenAction = Extract; + +/** Retains the exact failed open intent until the operator retries or the tab becomes ready. */ +export class TerminalOpenRetry { + private action: RetryOpenAction | null = null; + + remember(catalog: TerminalPanelCatalogReference | undefined, agentId: string | null): void { + this.action = catalog ? { kind: "catalog", agentId, catalog } : { kind: "open", agentId }; + } + + clearUnlessRetryable(error: unknown): void { + if ( + !( + error instanceof TerminalOpenTimeoutError || + error instanceof TerminalOpenUnusableSessionError + ) + ) { + this.clear(); + } + } + + clear(): void { + this.action = null; + } + + get available(): boolean { + return this.action !== null; + } + + run(): void { + const action = this.action; + this.clear(); + if (action) { + void terminalIntentQueue.queue(action); + } + } +} + export type TerminalIntentHost = { bootQueue: Pick; currentGeneration: () => number; diff --git a/ui/src/e2e/terminal-open.e2e.test.ts b/ui/src/e2e/terminal-open.e2e.test.ts new file mode 100644 index 000000000000..4cdefa52a1e5 --- /dev/null +++ b/ui/src/e2e/terminal-open.e2e.test.ts @@ -0,0 +1,119 @@ +import { createHash } from "node:crypto"; +import type { Locator, Page } from "playwright"; +import { expect, it } from "vitest"; +import { waitForControlUiGatewayReady } from "../test-helpers/control-ui-e2e-readiness.ts"; +import { installMockGateway } from "../test-helpers/control-ui-e2e.ts"; +import { openChatSidePanelType } from "./chat-side-panel.test-support.ts"; +import { createControlUiE2eSuite } from "./control-ui-e2e-suite.test-support.ts"; + +const suite = createControlUiE2eSuite({ + name: "Control UI terminal open", + startServerBeforeBrowser: true, + unavailableMessage: (executablePath) => + `Playwright Chromium is not installed or cannot start at ${executablePath}. Run \`pnpm --dir ui exec playwright install --with-deps chromium\`.`, +}); + +async function openTerminalSidePanel(page: Page): Promise { + await page.goto(`${suite.server.baseUrl}chat`); + await waitForControlUiGatewayReady(page); + await openChatSidePanelType(page, "Terminal"); + return page.locator(".sidebar-region__right-runtime openclaw-terminal-panel"); +} + +async function canvasDigest(canvas: Locator): Promise { + const png = await canvas.screenshot({ animations: "disabled", caret: "hide" }); + return createHash("sha256").update(png).digest("hex"); +} + +async function settleTerminalPaint(page: Page): Promise { + await page.evaluate( + () => + new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }), + ); +} + +suite.define(() => { + it("opens against the shared mock and renders echoed terminal output", async () => { + await suite.withPage({ serviceWorkers: "block" }, async ({ page }) => { + const gateway = await installMockGateway(page, { + featureMethods: ["chat.startup", "terminal.open"], + terminalEnabled: true, + }); + + const panel = await openTerminalSidePanel(page); + await gateway.waitForRequest("terminal.open"); + await panel.locator(".tabstrip-tab.is-live").waitFor(); + const canvas = panel.locator(".tp-host canvas"); + await canvas.waitFor({ state: "visible" }); + await settleTerminalPaint(page); + const bannerDigest = await canvasDigest(canvas); + + const sentinel = "PROVABLE_MOCK_TERMINAL"; + await canvas.click(); + await page.keyboard.type(sentinel); + await expect + .poll(async () => + (await gateway.getRequests("terminal.input")) + .map((request) => (request.params as { data?: string }).data ?? "") + .join(""), + ) + .toContain(sentinel); + await expect.poll(() => canvasDigest(canvas)).not.toBe(bannerDigest); + }); + }); + + it("names a missing field and retries the open successfully", async () => { + await suite.withPage({ serviceWorkers: "block" }, async ({ page }) => { + const gateway = await installMockGateway(page, { + featureMethods: ["chat.startup", "terminal.open"], + methodResponses: { + "terminal.open": { + sequence: [ + { + agentId: "main", + confined: false, + sessionId: "terminal-missing-cwd", + shell: "/bin/zsh", + }, + { + agentId: "main", + confined: false, + cwd: "/workspace/openclaw", + sessionId: "terminal-retry-ready", + shell: "/bin/zsh", + }, + ], + }, + }, + terminalEnabled: true, + }); + + const panel = await openTerminalSidePanel(page); + const error = + "The Gateway returned an unusable terminal session (missing cwd). The Gateway is likely older than this Control UI — update it, then retry."; + await panel.getByText(error, { exact: true }).waitFor(); + const retry = panel.getByRole("button", { name: "Retry", exact: true }); + await retry.waitFor(); + expect(await gateway.getRequests("terminal.open")).toHaveLength(1); + + await retry.click(); + await expect.poll(async () => (await gateway.getRequests("terminal.open")).length).toBe(2); + await panel.locator(".tabstrip-tab.is-live").waitFor(); + const canvas = panel.locator(".tp-host canvas"); + await canvas.waitFor({ state: "visible" }); + await settleTerminalPaint(page); + const blankDigest = await canvasDigest(canvas); + const retryOutput = "Retry opened terminal\r\n"; + await gateway.emitGatewayEvent("terminal.data", { + sessionId: "terminal-retry-ready", + seq: retryOutput.length, + data: retryOutput, + }); + + await expect.poll(() => canvasDigest(canvas)).not.toBe(blankDigest); + expect(await panel.getByText(error, { exact: true }).count()).toBe(0); + }); + }); +}); diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index 15059074e319..6e820bbbe18c 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -2014,6 +2014,8 @@ export const en: TranslationMap = { dockMain: "Fill main content area", dockMode: "Terminal panel position", unavailable: "The terminal is not available on this gateway.", + unusableSession: + "The Gateway returned an unusable terminal session (missing {field}). The Gateway is likely older than this Control UI — update it, then retry.", uploadTooLarge: "File exceeds the 16 MiB terminal upload limit: {file}", uploadUnsafeCmdPath: "Cannot safely insert an uploaded path containing % or ! into cmd.exe", uploadUnsupportedShell: "Cannot safely insert an uploaded path into unsupported shell: {shell}", diff --git a/ui/src/test-helpers/control-ui-e2e.ts b/ui/src/test-helpers/control-ui-e2e.ts index e7755117c5b7..f54c57e344a9 100644 --- a/ui/src/test-helpers/control-ui-e2e.ts +++ b/ui/src/test-helpers/control-ui-e2e.ts @@ -948,6 +948,18 @@ function installControlUiMockGateway( method: string; match?: Record; }; + type MockTerminalSession = { + sessionId: string; + agentId: string; + shell: string; + cwd: string; + confined: boolean; + attached: boolean; + owner: "conn"; + createdAtMs: number; + buffer: string; + seq: number; + }; type ExposedGateway = { closeLatest: (code?: number, reason?: string) => void; deliverLatest: (frame: unknown) => void; @@ -1006,6 +1018,8 @@ function installControlUiMockGateway( const methodResponseSequenceIndexes = new Map(); const sessionPatches = new Map>(); const createdSessions = new Map>(); + const terminalSessions = new Map(); + let terminalSessionSequence = 0; const sessionMessageSubscriptions = new Set(); const sockets: Array<{ readonly readyState: number; @@ -1883,11 +1897,101 @@ function installControlUiMockGateway( }; case "sessions.messages.unsubscribe": return { ok: true }; + case "terminal.open": { + const sessionId = `control-ui-mock-terminal-${++terminalSessionSequence}`; + const session: MockTerminalSession = { + sessionId, + agentId: + isRecord(params) && typeof params.agentId === "string" + ? params.agentId + : scenario.defaultAgentId, + shell: "/bin/zsh", + cwd: scenario.workspace || "/workspace/openclaw", + confined: false, + attached: true, + owner: "conn", + createdAtMs: Date.now(), + buffer: "", + seq: 0, + }; + terminalSessions.set(sessionId, session); + return { + sessionId: session.sessionId, + agentId: session.agentId, + shell: session.shell, + cwd: session.cwd, + confined: session.confined, + }; + } + case "terminal.attach": { + const sessionId = isRecord(params) ? params.sessionId : undefined; + const session = typeof sessionId === "string" ? terminalSessions.get(sessionId) : null; + return session + ? { + sessionId: session.sessionId, + agentId: session.agentId, + shell: session.shell, + cwd: session.cwd, + confined: session.confined, + buffer: session.buffer, + seq: session.seq, + } + : {}; + } + case "terminal.list": + return { + sessions: [...terminalSessions.values()].map( + ({ buffer: _buffer, seq: _seq, ...session }) => session, + ), + }; + case "terminal.input": + case "terminal.resize": + return { ok: true }; + case "terminal.close": { + const sessionId = isRecord(params) ? params.sessionId : undefined; + if (typeof sessionId === "string") { + terminalSessions.delete(sessionId); + } + return { ok: true }; + } default: return {}; } } + function emitTerminalOutput( + socket: { deliver: (frame: unknown) => void }, + method: string, + params: unknown, + response: unknown, + ): void { + let data = ""; + let session: MockTerminalSession | undefined; + if ( + method === "terminal.open" && + isRecord(response) && + typeof response.sessionId === "string" + ) { + session = terminalSessions.get(response.sessionId); + data = "OpenClaw mock terminal\r\nType anything and the mock Gateway will echo it.\r\n$ "; + } else if (method === "terminal.input" && isRecord(params)) { + session = + typeof params.sessionId === "string" ? terminalSessions.get(params.sessionId) : undefined; + data = typeof params.data === "string" ? params.data : ""; + } + if (!session || !data) { + return; + } + session.buffer += data; + session.seq += data.length; + socket.deliver({ + event: "terminal.data", + payload: { sessionId: session.sessionId, seq: session.seq, data }, + seq: ++seq, + type: "event", + }); + } + function shouldDefer(method: string, params: unknown): boolean { const index = deferredMethods.findIndex( (candidate) => candidate.method === method && paramsMatch(params, candidate.match), @@ -2005,6 +2109,9 @@ function installControlUiMockGateway( ? { id, ok: false, error: mockError, type: "res" } : { id, ok: true, payload, type: "res" }, ); + if (!mockError) { + emitTerminalOutput(this, method, frame.params, payload); + } if (!mockError && method === "connect" && this.readyState === MockWebSocket.OPEN) { this.tickTimer = window.setInterval(() => { this.deliver({ event: "tick", payload: {}, seq: ++seq, type: "event" });