From ee7fe7be28d1cb74ed4f99acbce1d9c36159db75 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 23 Aug 2026 20:59:16 -0700 Subject: [PATCH] fix(ui): surface child session loading failures (#128418) --- .../app-sidebar-session-list-render.ts | 3 + .../app-sidebar-session-navigation.ts | 4 +- .../app-sidebar-session-row-render.ts | 26 +++ ui/src/components/app-sidebar.test.ts | 1 + ui/src/components/session-data-controller.ts | 26 ++- .../app-sidebar-cases/child-session-errors.ts | 151 ++++++++++++++++++ .../app-sidebar-cases/child-sessions.ts | 6 + 7 files changed, 201 insertions(+), 16 deletions(-) create mode 100644 ui/src/test-helpers/app-sidebar-cases/child-session-errors.ts diff --git a/ui/src/components/app-sidebar-session-list-render.ts b/ui/src/components/app-sidebar-session-list-render.ts index 6ce1b570ef00..d93c3b9f226c 100644 --- a/ui/src/components/app-sidebar-session-list-render.ts +++ b/ui/src/components/app-sidebar-session-list-render.ts @@ -8,6 +8,7 @@ import { openCatalogSessionInTerminal } from "../lib/sessions/catalog-terminal.t import type { SidebarSessionSection } from "../lib/sessions/grouping.ts"; import type { SessionCatalogGroupsRenderer } from "./app-sidebar-session-catalog-render.ts"; import { + renderChildSessionLoadError, renderRecentSession, renderSessionTree, type SessionListHost, @@ -451,6 +452,7 @@ export function renderSessionList(params: { catalogRenderer: SessionCatalogGroupsRenderer | null; }) { const { host } = params; + const hiddenMainSessionKey = host.mainSessionRow()?.key; return html`
; @@ -100,6 +102,7 @@ export interface SessionListHost { sessionKey: string, worktreeId: string, ): SessionPullRequestIndicatorState; + mainSessionRow(): { key: string } | null; isSessionChildrenExpanded(session: SidebarRecentSession): boolean; startSessionDrag(session: SidebarRecentSession): void; finishSessionDrag(): void; @@ -459,6 +462,28 @@ export function renderRecentSession(params: { return keyed(session.key, row); } +export function renderChildSessionLoadError(host: SessionListHost, parentKey: string) { + const error = host.sessionData.childSessionErrorsByParent.get(parentKey); + if (!error) { + return nothing; + } + return html``; +} + export function renderSessionTree(params: { host: SessionListHost; session: SidebarRecentSession; @@ -503,6 +528,7 @@ export function renderSessionTree(params: { ${t("sessionsView.showMoreChildren", { count: String(hiddenChildCount) })} ` : nothing} + ${renderChildSessionLoadError(host, session.key)} ${session.loadingChildren && session.children.length === 0 ? html`${t("common.loading")}` : nothing} diff --git a/ui/src/components/app-sidebar.test.ts b/ui/src/components/app-sidebar.test.ts index 5e4bc418387d..c589f5e3a6af 100644 --- a/ui/src/components/app-sidebar.test.ts +++ b/ui/src/components/app-sidebar.test.ts @@ -16,6 +16,7 @@ import "../test-helpers/app-sidebar-cases/catalog-ownership.ts"; import "../test-helpers/app-sidebar-cases/catalog-terminal-owner.ts"; import "../test-helpers/app-sidebar-cases/catalog-pages.ts"; import "../test-helpers/app-sidebar-cases/categorized-child-sessions.ts"; +import "../test-helpers/app-sidebar-cases/child-session-errors.ts"; import "../test-helpers/app-sidebar-cases/child-sessions-cap.ts"; import "../test-helpers/app-sidebar-cases/child-sessions.ts"; import "../test-helpers/app-sidebar-cases/group-mutations.ts"; diff --git a/ui/src/components/session-data-controller.ts b/ui/src/components/session-data-controller.ts index 4e1f8c620142..05873dbcef3d 100644 --- a/ui/src/components/session-data-controller.ts +++ b/ui/src/components/session-data-controller.ts @@ -65,7 +65,7 @@ export class SessionDataController implements ReactiveController, SessionCatalog sessionsLoading = false; childSessionRowsByParent: Readonly> = {}; loadedChildSessionKeys: ReadonlySet = new Set(); - failedChildSessionKeys: ReadonlySet = new Set(); + childSessionErrorsByParent: ReadonlyMap = new Map(); loadingChildSessionKeys: ReadonlySet = new Set(); activeSessionLineageRoot: GatewaySessionRow | null = null; activeSessionLineageSelectedRow: GatewaySessionRow | null = null; @@ -383,7 +383,7 @@ export class SessionDataController implements ReactiveController, SessionCatalog ) : {}; this.loadedChildSessionKeys = new Set(); - this.failedChildSessionKeys = new Set(); + this.childSessionErrorsByParent = new Map(); this.loadingChildSessionKeys = new Set(); if (options.preserveActiveLineage !== true) { this.activeSessionLineageRoot = null; @@ -560,7 +560,7 @@ export class SessionDataController implements ReactiveController, SessionCatalog if ( !parentKey || this.loadedChildSessionKeys.has(parentKey) || - this.failedChildSessionKeys.has(parentKey) || + this.childSessionErrorsByParent.has(parentKey) || this.loadingChildSessionKeys.has(parentKey) ) { return; @@ -586,13 +586,8 @@ export class SessionDataController implements ReactiveController, SessionCatalog } this.childSessionRowsByParent = { ...this.childSessionRowsByParent, [parentKey]: rows }; this.loadedChildSessionKeys = new Set([...this.loadedChildSessionKeys, parentKey]); - if (this.failedChildSessionKeys.has(parentKey)) { - const failedKeys = new Set(this.failedChildSessionKeys); - failedKeys.delete(parentKey); - this.failedChildSessionKeys = failedKeys; - } this.notify(); - } catch { + } catch (error) { if (generation !== this.childSessionGeneration || sessions !== this.context?.sessions) { return; } @@ -602,7 +597,10 @@ export class SessionDataController implements ReactiveController, SessionCatalog ...this.childSessionRowsByParent, [parentKey]: this.childSessionRowsByParent[parentKey] ?? [], }; - this.failedChildSessionKeys = new Set([...this.failedChildSessionKeys, parentKey]); + this.childSessionErrorsByParent = new Map(this.childSessionErrorsByParent).set( + parentKey, + formatUiError(error), + ); this.notify(); } finally { if (generation === this.childSessionGeneration && sessions === this.context?.sessions) { @@ -718,10 +716,10 @@ export class SessionDataController implements ReactiveController, SessionCatalog } retryChildSessions(sessionKey: string): void { - if (this.failedChildSessionKeys.has(sessionKey)) { - const failedKeys = new Set(this.failedChildSessionKeys); - failedKeys.delete(sessionKey); - this.failedChildSessionKeys = failedKeys; + if (this.childSessionErrorsByParent.has(sessionKey)) { + const errors = new Map(this.childSessionErrorsByParent); + errors.delete(sessionKey); + this.childSessionErrorsByParent = errors; this.notify(); } void this.loadChildSessions(sessionKey); diff --git a/ui/src/test-helpers/app-sidebar-cases/child-session-errors.ts b/ui/src/test-helpers/app-sidebar-cases/child-session-errors.ts new file mode 100644 index 000000000000..9e9ed720c716 --- /dev/null +++ b/ui/src/test-helpers/app-sidebar-cases/child-session-errors.ts @@ -0,0 +1,151 @@ +import { describe, expect, it } from "vitest"; +import type { GatewayBrowserClient } from "../../api/gateway.ts"; +import type { SessionsListResult } from "../../api/types.ts"; +import { createGateway, createSessionsHarness, mountSidebar } from "../app-sidebar.ts"; +import { waitForFast } from "../wait-for.ts"; +import "../../components/app-sidebar.ts"; + +function sessionResult(sessions: SessionsListResult["sessions"]): SessionsListResult { + return { + ts: 2, + path: "", + count: sessions.length, + defaults: { modelProvider: null, model: null, contextTokens: null }, + sessions, + }; +} + +function parentSession(key: string, childKey: string) { + return { key, kind: "direct" as const, updatedAt: 1, childSessions: [childKey] }; +} + +function recoveredChild(key: string, parentKey: string, label: string) { + return { key, spawnedBy: parentKey, kind: "direct" as const, label, updatedAt: 2 }; +} + +describe("AppSidebar child-session load errors", () => { + it.each([ + { + failure: "temporary list failure", + visibleError: "temporary list failure", + }, + { + failure: "OPENAI_API_KEY=sk-1234567890abcdef", + visibleError: "OPENAI_API_KEY=sk-123...cdef", + }, + ])( + "surfaces $visibleError with an accessible retry action", + async ({ failure, visibleError }) => { + const parentKey = "agent:main:parent"; + const childKey = "agent:worker:child"; + const gateway = createGateway({} as GatewayBrowserClient); + const harness = createSessionsHarness("main", [parentKey]); + harness.list + .mockRejectedValueOnce(new Error(failure)) + .mockResolvedValueOnce( + sessionResult([recoveredChild(childKey, parentKey, "Recovered child")]), + ); + const { sidebar } = await mountSidebar(gateway, harness.sessions); + harness.publishList({ result: sessionResult([parentSession(parentKey, childKey)]) }); + await sidebar.updateComplete; + sidebar.querySelector("[data-child-session-toggle]")?.click(); + + await waitForFast(() => { + const alert = sidebar.querySelector(`[data-child-session-error="${parentKey}"]`); + expect(alert?.getAttribute("role")).toBe("alert"); + expect(alert?.textContent).toContain(visibleError); + }); + if (failure !== visibleError) { + expect(sidebar.textContent).not.toContain(failure); + } + + const retry = sidebar.querySelector( + `[data-retry-child-sessions="${parentKey}"]`, + ); + expect(retry?.textContent).toContain("Retry"); + retry?.click(); + + await waitForFast(() => expect(sidebar.textContent).toContain("Recovered child")); + expect(harness.list).toHaveBeenCalledTimes(2); + expect(sidebar.querySelector("[data-child-session-error]")).toBeNull(); + }, + ); + + it("keeps failures and retry state scoped to their parent", async () => { + const firstParent = "agent:main:first-parent"; + const secondParent = "agent:main:second-parent"; + const gateway = createGateway({} as GatewayBrowserClient); + const harness = createSessionsHarness("main", [firstParent, secondParent]); + let firstAttempts = 0; + harness.list.mockImplementation(async (options) => { + const parentKey = options?.spawnedBy; + if (parentKey === secondParent || firstAttempts++ === 0) { + throw new Error(`${parentKey} temporarily unavailable`); + } + return sessionResult([ + recoveredChild("agent:worker:first-child", firstParent, "Recovered first child"), + ]); + }); + const { sidebar } = await mountSidebar(gateway, harness.sessions); + harness.publishList({ + result: sessionResult( + [firstParent, secondParent].map((parentKey) => + parentSession(parentKey, `${parentKey}:child`), + ), + ), + }); + await sidebar.updateComplete; + for (const parentKey of [firstParent, secondParent]) { + sidebar + .querySelector(`[data-child-session-toggle="${parentKey}"]`) + ?.click(); + } + await waitForFast(() => + expect(sidebar.querySelectorAll("[data-child-session-error]")).toHaveLength(2), + ); + + sidebar + .querySelector(`[data-retry-child-sessions="${firstParent}"]`) + ?.click(); + + await waitForFast(() => expect(sidebar.textContent).toContain("Recovered first child")); + expect(sidebar.querySelector(`[data-child-session-error="${firstParent}"]`)).toBeNull(); + expect( + sidebar.querySelector(`[data-child-session-error="${secondParent}"]`)?.textContent, + ).toContain(secondParent); + }); + + it.each(["main", "agent:main:main"])( + "surfaces and retries child failures for the hidden %s main session", + async (parentKey) => { + const childKey = "agent:main:subagent:recovered"; + const gateway = createGateway({} as GatewayBrowserClient); + const harness = createSessionsHarness("main", [parentKey]); + harness.list + .mockRejectedValueOnce(new Error("main session children temporarily unavailable")) + .mockResolvedValueOnce( + sessionResult([recoveredChild(childKey, parentKey, "Recovered main-session child")]), + ); + const { sidebar } = await mountSidebar(gateway, harness.sessions); + harness.publishList({ result: sessionResult([parentSession(parentKey, childKey)]) }); + + await waitForFast(() => expect(harness.list).toHaveBeenCalledOnce()); + expect(sidebar.querySelector(`[data-session-key="${parentKey}"]`)).toBeNull(); + await waitForFast(() => { + const alert = sidebar.querySelector(`[data-child-session-error="${parentKey}"]`); + expect(alert?.getAttribute("role")).toBe("alert"); + expect(alert?.textContent).toContain("main session children temporarily unavailable"); + }); + + sidebar + .querySelector(`[data-retry-child-sessions="${parentKey}"]`) + ?.click(); + + await waitForFast(() => + expect(sidebar.textContent).toContain("Recovered main-session child"), + ); + expect(harness.list).toHaveBeenCalledTimes(2); + expect(sidebar.querySelector("[data-child-session-error]")).toBeNull(); + }, + ); +}); diff --git a/ui/src/test-helpers/app-sidebar-cases/child-sessions.ts b/ui/src/test-helpers/app-sidebar-cases/child-sessions.ts index 4cee5ab7a906..8e659cc05dc5 100644 --- a/ui/src/test-helpers/app-sidebar-cases/child-sessions.ts +++ b/ui/src/test-helpers/app-sidebar-cases/child-sessions.ts @@ -350,6 +350,11 @@ describe("AppSidebar agent chip", () => { await waitForFast(() => expect(harness.list).toHaveBeenCalledTimes(2)); expect(sidebar.querySelector(".sidebar-recent-session--child")).toBeNull(); + await waitForFast(() => + expect( + sidebar.querySelector('[data-child-session-error="agent:main:parent"]')?.textContent, + ).toContain("child session list returned no result"), + ); publishParent(11); await waitForFast(() => expect(harness.list).toHaveBeenCalledTimes(3)); @@ -427,6 +432,7 @@ describe("AppSidebar agent chip", () => { expect(sidebar.querySelector('[data-session-key="agent:worker:child"]')?.textContent).toContain( "Replacement child", ); + expect(sidebar.querySelector("[data-child-session-error]")).toBeNull(); }); it("nests the selected child under its parent and reveals the active path", async () => {