From 6f56d797d7be33bc3d8252c41fb4856180c8a779 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 9 Aug 2026 02:57:54 -0700 Subject: [PATCH] fix(ui): survive route notFound at startup and stop silent chat draft loss (#120960) --- ui/src/app-route-paths.test.ts | 116 +++++++++++++- ui/src/app-routes.ts | 24 ++- ui/src/app/app-host.test.ts | 6 +- ui/src/app/app-host.ts | 2 +- ui/src/app/app-shell-navigation.ts | 15 +- ui/src/app/app-shell-view.ts | 3 +- ui/src/app/bootstrap-location.ts | 8 +- ui/src/app/bootstrap.test.ts | 168 ++++++++++++-------- ui/src/app/bootstrap.ts | 1 - ui/src/app/router-outlet-controller.test.ts | 28 ++++ ui/src/app/router-outlet-controller.ts | 22 ++- ui/src/app/router-outlet.ts | 7 +- ui/src/pages/chat/chat-send-submit.test.ts | 31 ++++ ui/src/pages/chat/chat-send-submit.ts | 8 +- 14 files changed, 348 insertions(+), 91 deletions(-) diff --git a/ui/src/app-route-paths.test.ts b/ui/src/app-route-paths.test.ts index 0bab61903248..30f5cca5fd96 100644 --- a/ui/src/app-route-paths.test.ts +++ b/ui/src/app-route-paths.test.ts @@ -1,5 +1,5 @@ // @vitest-environment node -import type { RouteLocation, RouterHistory } from "@openclaw/uirouter"; +import { notFound, type RouteLocation, type RouterHistory } from "@openclaw/uirouter"; import { describe, expect, it, vi } from "vitest"; import { agentRouteFromPath, @@ -194,6 +194,120 @@ describe("Dynamic route startup bridge", () => { route.component = originalComponent; } }); + + it("keeps a loader not-found state without rejecting startup", async () => { + let location: RouteLocation = { pathname: "/", search: "", hash: "" }; + const history: RouterHistory = { + location: () => location, + push: vi.fn(), + replace: vi.fn((next: RouteLocation) => { + location = next; + }), + listen: () => () => undefined, + }; + const router = createApplicationRouter(); + const route = router.getRoute("chat"); + if (!route) { + throw new Error("Chat route missing"); + } + const originalLoader = route.loader; + const originalComponent = route.component; + try { + route.loader = () => notFound({ routeId: "chat" }); + route.component = async () => ({ render: () => null }); + + await expect( + startApplicationRouter(router, history, "", { + basePath: "", + } as unknown as ApplicationContext), + ).resolves.toBeUndefined(); + + expect(location.pathname).toBe("/chat"); + expect(router.getState().status).toBe("notFound"); + expect(router.getState().matches[0]).toMatchObject({ + routeId: "chat", + status: "notFound", + error: { type: "notFound", data: { routeId: "chat" } }, + }); + } finally { + router.stop(); + route.loader = originalLoader; + route.component = originalComponent; + } + }); + + it("tolerates not-found from both dynamic startup navigations", async () => { + const location: RouteLocation = { + pathname: "/chat/main/01JSESSIONA", + search: "", + hash: "", + }; + const history: RouterHistory = { + location: () => location, + push: vi.fn(), + replace: vi.fn(), + listen: () => () => undefined, + }; + const router = createApplicationRouter(); + const route = router.getRoute("chat"); + if (!route) { + throw new Error("Chat route missing"); + } + const loader = vi.fn(() => notFound({ routeId: "chat" })); + const originalLoader = route.loader; + const originalComponent = route.component; + try { + route.loader = loader; + route.component = async () => ({ render: () => null }); + + await expect( + startApplicationRouter(router, history, "", { + basePath: "", + } as unknown as ApplicationContext), + ).resolves.toBeUndefined(); + + expect(loader).toHaveBeenCalledTimes(2); + expect(router.getState().status).toBe("notFound"); + expect(router.getState().location).toEqual(location); + } finally { + router.stop(); + route.loader = originalLoader; + route.component = originalComponent; + } + }); + + it("still rejects non-not-found startup failures", async () => { + const failure = new Error("chat loader failed"); + const history: RouterHistory = { + location: () => ({ pathname: "/chat", search: "", hash: "" }), + push: vi.fn(), + replace: vi.fn(), + listen: () => () => undefined, + }; + const router = createApplicationRouter(); + const route = router.getRoute("chat"); + if (!route) { + throw new Error("Chat route missing"); + } + const originalLoader = route.loader; + const originalComponent = route.component; + try { + route.loader = () => { + throw failure; + }; + route.component = async () => ({ render: () => null }); + + await expect( + startApplicationRouter(router, history, "", { + basePath: "", + } as unknown as ApplicationContext), + ).rejects.toBe(failure); + } finally { + router.stop(); + route.loader = originalLoader; + route.component = originalComponent; + } + }); }); describe("Agent panel route paths", () => { diff --git a/ui/src/app-routes.ts b/ui/src/app-routes.ts index 851b4507af92..62efd60b237a 100644 --- a/ui/src/app-routes.ts +++ b/ui/src/app-routes.ts @@ -3,6 +3,7 @@ import type { PageDefinition, RouteLocation, RouteMatch, + RouteNotFound, Router, RouterHistory, } from "@openclaw/uirouter"; @@ -163,6 +164,23 @@ function sameRouteLocation(left: RouteLocation, right: RouteLocation): boolean { ); } +function isRouteNotFound(error: unknown): error is RouteNotFound { + return ( + typeof error === "object" && error !== null && "type" in error && error.type === "notFound" + ); +} + +async function tolerateRouteNotFound(navigation: Promise): Promise { + try { + await navigation; + } catch (error) { + // uirouter commits not-found state before rethrowing; the outlet owns its recovery UI. + if (!isRouteNotFound(error)) { + throw error; + } + } +} + export async function startApplicationRouter( router: ApplicationRouter, history: RouterHistory, @@ -206,12 +224,14 @@ export async function startApplicationRouter( listener(next); }), }; - await router.start(applicationHistory, basePath, context); + await tolerateRouteNotFound(router.start(applicationHistory, basePath, context)); if (initialDynamicRoute && sameRouteLocation(history.location(), location)) { // Replace the synthetic exact-match location with the real browser path // before the shell renders. A loader-visible redirect wins if it already // moved history while startup was still resolving. - await router.navigate(initialDynamicRoute[0], context, { history: "none" }, location); + await tolerateRouteNotFound( + router.navigate(initialDynamicRoute[0], context, { history: "none" }, location), + ); } } diff --git a/ui/src/app/app-host.test.ts b/ui/src/app/app-host.test.ts index db3a98e4cfa9..6e6d31af9714 100644 --- a/ui/src/app/app-host.test.ts +++ b/ui/src/app/app-host.test.ts @@ -240,7 +240,7 @@ type ShellSessionNavigationState = { routeState: { routeId?: RouteId }; navigate: (routeId: RouteId) => void; handleCommandPaletteSlashCommand: (command: string) => void; - replaceChatWithCurrentSession: () => void; + replaceChatWithCurrentSession: () => boolean; }; function committedRouterState( @@ -494,11 +494,11 @@ describe("OpenClaw shell route session commits", () => { shell.activeSessionKey = "main"; shell.routeState = { routeId: "chat" }; - shell.replaceChatWithCurrentSession(); + expect(shell.replaceChatWithCurrentSession()).toBe(false); expect(replace).not.toHaveBeenCalled(); snapshot.phase = "connected"; - shell.replaceChatWithCurrentSession(); + expect(shell.replaceChatWithCurrentSession()).toBe(true); expect(replace).toHaveBeenCalledWith("chat", { pathname: "/chat/research" }); }); diff --git a/ui/src/app/app-host.ts b/ui/src/app/app-host.ts index 72bce6d1a792..a4c52e5030b9 100644 --- a/ui/src/app/app-host.ts +++ b/ui/src/app/app-host.ts @@ -424,7 +424,7 @@ class OpenClawShell } replaceChatWithCurrentSession() { - this.shellNavigation.replaceChatWithCurrentSession(); + return this.shellNavigation.replaceChatWithCurrentSession(); } recoverDeletedActiveSession(sessionState: ApplicationContext["sessions"]["state"]) { diff --git a/ui/src/app/app-shell-navigation.ts b/ui/src/app/app-shell-navigation.ts index c8824e6656fe..c4360831113a 100644 --- a/ui/src/app/app-shell-navigation.ts +++ b/ui/src/app/app-shell-navigation.ts @@ -87,14 +87,14 @@ export class ShellNavigationOwner { ); } - replaceChatWithCurrentSession(): void { + replaceChatWithCurrentSession(): boolean { const context = this.host.context; const sessionKey = this.host.activeSessionKey.trim(); - if ( - !context || - (!parseAgentSessionKey(sessionKey) && context.gateway.snapshot.phase !== "connected") - ) { - return; + if (!context) { + return true; + } + if (!parseAgentSessionKey(sessionKey) && context.gateway.snapshot.phase !== "connected") { + return false; } const face = this.host.routeState.routeId === "dashboard" ? "dashboard" : "chat"; const sessionWasDeleted = (context.sessions.state.deletedSessions ?? []).some( @@ -138,7 +138,7 @@ export class ShellNavigationOwner { // Gateway rejects deletion of a live main session. If an orphaned event // still names that fallback, replacing the same route would retry forever. if (sessionWasDeleted && replacementSessionKey === sessionKey) { - return; + return true; } if (replacementSessionKey !== sessionKey) { // Commit the replacement to both selection owners before navigating; @@ -154,6 +154,7 @@ export class ShellNavigationOwner { face, sessionNavigationTarget({ context, face, sessionKey: replacementSessionKey }).options, ); + return true; } recoverDeletedActiveSession(sessionState: ApplicationContext["sessions"]["state"]): void { diff --git a/ui/src/app/app-shell-view.ts b/ui/src/app/app-shell-view.ts index b9dbe2fbdd6e..8b3d82492f03 100644 --- a/ui/src/app/app-shell-view.ts +++ b/ui/src/app/app-shell-view.ts @@ -76,7 +76,7 @@ export interface ShellViewHost { openNewSession(agentId: string, target?: NewSessionTarget): void; openPalette(): void; refreshControlUi(): void; - replaceChatWithCurrentSession(): void; + replaceChatWithCurrentSession(): boolean; resizeNavigation(splitRatio: number): void; selectChatSession(sessionKey: string, agentId?: string | null): void; storedOutboxScopeHost(context: ApplicationContext): StoredOutboxScopeHost; @@ -457,6 +457,7 @@ export function renderApplicationShell(host: ShellViewHost) { .router=${runtime.router} .retryContext=${context} .onNotFound=${() => host.replaceChatWithCurrentSession()} + .notFoundRecoveryReady=${gatewayConnected} > { ); }); - it("waits for the configured default agent before normalizing a persisted alias", async () => { - type GatewayListener = Parameters["gateway"]["subscribe"]>[0]; - let listener: GatewayListener | null = null; - let snapshot = { - phase: "connecting", - client: null, - hello: null, - } as unknown as ApplicationContext["gateway"]["snapshot"]; - const gateway = { - get snapshot() { - return snapshot; - }, - subscribe: (next: GatewayListener) => { - listener = next; - return () => undefined; - }, - }; - const pending = resolveInitialApplicationLocation({ - location: { pathname: "/", search: "", hash: "" }, - basePath: "", - sessionKey: "main", - gateway, - agentsList: () => null, - signal: new AbortController().signal, - }); - let settled = false; - void pending.then(() => { - settled = true; - }); - await Promise.resolve(); - expect(settled).toBe(false); - - snapshot = { - phase: "connected", - client: {}, - hello: { - snapshot: { - sessionDefaults: { defaultAgentId: "research", mainKey: "workspace" }, + it.each([ + { persistedSessionKey: "main", connectedSessionKey: "main" }, + { persistedSessionKey: "", connectedSessionKey: "agent:research:workspace" }, + ])( + "waits for gateway defaults before normalizing '$persistedSessionKey'", + async ({ persistedSessionKey, connectedSessionKey }) => { + type GatewayListener = Parameters["gateway"]["subscribe"]>[0]; + let listener: GatewayListener | null = null; + let snapshot = { + phase: "connecting", + client: null, + hello: null, + } as unknown as ApplicationContext["gateway"]["snapshot"]; + const gateway = { + get snapshot() { + return snapshot; }, - }, - } as unknown as ApplicationContext["gateway"]["snapshot"]; - const connectedListener = listener as GatewayListener | null; - if (!connectedListener) { - throw new Error("expected gateway readiness subscription"); - } - connectedListener(snapshot); - - await expect(pending).resolves.toEqual({ pathname: "/chat/research", search: "", hash: "" }); - }); - - it("does not wait for gateway defaults on an explicit startup route", async () => { - const subscribe = vi.fn(() => () => undefined); - const location = { pathname: "/settings/appearance", search: "", hash: "" }; - - await expect( - resolveInitialApplicationLocation({ - location, + subscribe: (next: GatewayListener) => { + listener = next; + return () => undefined; + }, + }; + const pending = resolveInitialApplicationLocation({ + location: { pathname: "/", search: "", hash: "" }, basePath: "", - sessionKey: "main", - gateway: { - snapshot: { phase: "connecting", client: null, hello: null }, - subscribe, - } as unknown as ApplicationContext["gateway"], + sessionKey: persistedSessionKey, + gateway, agentsList: () => null, signal: new AbortController().signal, - }), - ).resolves.toBe(location); - expect(subscribe).not.toHaveBeenCalled(); - }); + }); + let settled = false; + void pending.then(() => { + settled = true; + }); + await Promise.resolve(); + expect(settled).toBe(false); + + snapshot = { + phase: "connected", + client: {}, + sessionKey: connectedSessionKey, + hello: { + snapshot: { + sessionDefaults: { defaultAgentId: "research", mainKey: "workspace" }, + }, + }, + } as unknown as ApplicationContext["gateway"]["snapshot"]; + const connectedListener = listener as GatewayListener | null; + if (!connectedListener) { + throw new Error("expected gateway readiness subscription"); + } + connectedListener(snapshot); + + await expect(pending).resolves.toEqual({ + pathname: "/chat/research", + search: "", + hash: "", + }); + }, + ); + + it.each(["main", ""])( + "does not wait for gateway defaults on an explicit startup route with '%s'", + async (sessionKey) => { + const subscribe = vi.fn(() => () => undefined); + const location = { pathname: "/settings/appearance", search: "", hash: "" }; + + await expect( + resolveInitialApplicationLocation({ + location, + basePath: "", + sessionKey, + gateway: { + snapshot: { phase: "connecting", client: null, hello: null }, + subscribe, + } as unknown as ApplicationContext["gateway"], + agentsList: () => null, + signal: new AbortController().signal, + }), + ).resolves.toBe(location); + expect(subscribe).not.toHaveBeenCalled(); + }, + ); it("canonicalizes a scoped persisted main key when defaults are already known", async () => { const subscribe = vi.fn(() => () => undefined); @@ -721,4 +735,28 @@ describe("normalizeInitialApplicationLocation", () => { window.history.replaceState({}, "", previousUrl); } }); + + it("resolves runtime startup when the bare default route is not found", async () => { + const previousSettings = loadSettings(); + const previousUrl = window.location.href; + saveSettings({ + ...previousSettings, + sessionKey: "main", + lastActiveSessionKey: "main", + }); + window.history.replaceState({}, "", "/"); + const runtime = bootstrapApplication({ sessionPathBuilderReady: Promise.resolve() }); + const routerStart = vi + .spyOn(runtime.router, "start") + .mockRejectedValue({ type: "notFound", data: { routeId: "chat" } }); + + try { + await expect(runtime.start()).resolves.toBeUndefined(); + expect(routerStart).toHaveBeenCalledOnce(); + } finally { + runtime.stop(); + saveSettings(previousSettings); + window.history.replaceState({}, "", previousUrl); + } + }); }); diff --git a/ui/src/app/bootstrap.ts b/ui/src/app/bootstrap.ts index fe2e8d8b1ca0..b8991f5c8c4e 100644 --- a/ui/src/app/bootstrap.ts +++ b/ui/src/app/bootstrap.ts @@ -295,7 +295,6 @@ export function bootstrapApplication( documentMode === null && !releasedSessionQuery && firstRunDefaultLanding && - settings.sessionKey.trim() !== "" && !parseAgentSessionKey(settings.sessionKey); const initialLocationReady = ( documentMode diff --git a/ui/src/app/router-outlet-controller.test.ts b/ui/src/app/router-outlet-controller.test.ts index c61c0bdc8447..79e526b8527b 100644 --- a/ui/src/app/router-outlet-controller.test.ts +++ b/ui/src/app/router-outlet-controller.test.ts @@ -250,4 +250,32 @@ describe("RouterOutletController not-found boundary", () => { controller.disconnect(); router.stop(); }); + + it("retries a declined fallback once recovery becomes ready", async () => { + const router = createTestRouter(); + const onNotFound = vi.fn(() => false); + const controller = new RouterOutletController( + vi.fn(), + ); + controller.setInputs({ router, onNotFound, notFoundRecoveryReady: false }); + controller.connect(); + + await router.navigateLocation(location("/missing"), { label: "test" }); + await flushPromises(); + expect(onNotFound).toHaveBeenCalledTimes(1); + + controller.setInputs({ router, onNotFound, notFoundRecoveryReady: false }); + await flushPromises(); + expect(onNotFound).toHaveBeenCalledTimes(1); + + controller.setInputs({ router, onNotFound, notFoundRecoveryReady: true }); + await flushPromises(); + expect(onNotFound).toHaveBeenCalledTimes(2); + + controller.setInputs({ router, onNotFound, notFoundRecoveryReady: true }); + await flushPromises(); + expect(onNotFound).toHaveBeenCalledTimes(2); + controller.disconnect(); + router.stop(); + }); }); diff --git a/ui/src/app/router-outlet-controller.ts b/ui/src/app/router-outlet-controller.ts index 6968ada8ec55..6486026cbdfc 100644 --- a/ui/src/app/router-outlet-controller.ts +++ b/ui/src/app/router-outlet-controller.ts @@ -23,7 +23,8 @@ export type RouterOutletSnapshot< type RouterOutletInputs = { router?: Router; - onNotFound?: () => void; + onNotFound?: () => boolean | void; + notFoundRecoveryReady?: boolean; }; type RouterOutletControllerOptions = { @@ -86,7 +87,7 @@ export class RouterOutletController< TData = unknown, > { private router?: Router; - private onNotFound?: () => void; + private onNotFound?: () => boolean | void; private connected = false; private unsubscribe?: () => void; private selection: RouterOutletStateSlice = idleSnapshot(); @@ -96,8 +97,10 @@ export class RouterOutletController< private pendingTimer?: ReturnType; private showPending = false; private notFoundActive = false; + private notFoundDeclined = false; private notFoundQueued = false; private notFoundGeneration = 0; + private notFoundRecoveryReady = true; private readonly pendingDelayMs: number; constructor( @@ -113,7 +116,15 @@ export class RouterOutletController< setInputs(inputs: RouterOutletInputs): void { this.onNotFound = inputs.onNotFound; + const nextNotFoundRecoveryReady = inputs.notFoundRecoveryReady ?? true; + const recoveryBecameReady = + !this.notFoundRecoveryReady && nextNotFoundRecoveryReady && this.notFoundDeclined; + this.notFoundRecoveryReady = nextNotFoundRecoveryReady; if (this.router === inputs.router) { + if (recoveryBecameReady && this.selection.status === "notFound") { + this.cancelNotFoundEffect(); + this.updateNotFoundEffect(this.selection.status); + } return; } @@ -253,13 +264,18 @@ export class RouterOutletController< return; } this.notFoundQueued = false; - this.onNotFound?.(); + // A disconnected shell declines transiently. Keep the latch until its + // readiness input changes so unrelated renders cannot spin retries. + if (this.onNotFound?.() === false) { + this.notFoundDeclined = true; + } }); } private cancelNotFoundEffect(): void { this.notFoundGeneration += 1; this.notFoundActive = false; + this.notFoundDeclined = false; this.notFoundQueued = false; } diff --git a/ui/src/app/router-outlet.ts b/ui/src/app/router-outlet.ts index bae739b03ffb..f430aed87ee3 100644 --- a/ui/src/app/router-outlet.ts +++ b/ui/src/app/router-outlet.ts @@ -195,7 +195,8 @@ function renderRouterOutlet = { router?: Router; - onNotFound?: () => void; + onNotFound?: () => boolean | void; + notFoundRecoveryReady?: boolean; }; class LitRouterOutletController< @@ -240,10 +241,12 @@ class OpenClawRouterOutlet< > extends OpenClawLightDomElement { @property({ attribute: false }) router?: Router; @property({ attribute: false }) retryContext?: TLoadContext; - @property({ attribute: false }) onNotFound?: () => void; + @property({ attribute: false }) onNotFound?: () => boolean | void; + @property({ attribute: false }) notFoundRecoveryReady?: boolean; private readonly outlet = new LitRouterOutletController(this, () => ({ router: this.router, onNotFound: this.onNotFound, + notFoundRecoveryReady: this.notFoundRecoveryReady, })); private readonly mcpAppUnmountGate = new McpAppUnmountGate(this); diff --git a/ui/src/pages/chat/chat-send-submit.test.ts b/ui/src/pages/chat/chat-send-submit.test.ts index da1c2dd6938a..c4aa71762767 100644 --- a/ui/src/pages/chat/chat-send-submit.test.ts +++ b/ui/src/pages/chat/chat-send-submit.test.ts @@ -126,3 +126,34 @@ describe("handleSendChat immediate local commands", () => { expect(getChatAttachmentDataUrl(host.chatAttachments[0]!)).toBe(attachmentDataUrl); }); }); + +describe("handleSendChat session ownership", () => { + it("keeps the composer intact when no visible session owns the send", async () => { + const attachment = createStagedAttachment("unscoped-att"); + const request = vi.fn(); + const host = createImmediateCommandHost("keep this draft", attachment, { + client: { request } as unknown as ChatHost["client"], + sessionKey: "", + chatReplyTarget: { + messageId: "reply-1", + sourceMessageId: "source-1", + text: "original message", + }, + }); + + await handleSendChat(host); + + expect(request).not.toHaveBeenCalled(); + expect(host.chatMessage).toBe("keep this draft"); + expect(host.chatAttachments).toEqual([attachment]); + expect(getChatAttachmentDataUrl(attachment)).toBe(attachmentDataUrl); + expect(host.chatReplyTarget).toEqual({ + messageId: "reply-1", + sourceMessageId: "source-1", + text: "original message", + }); + expect(host.chatQueue).toEqual([]); + expect(host.lastError).toBe("The active session is unavailable; refresh and try again."); + expect(host.chatError).toBe(host.lastError); + }); +}); diff --git a/ui/src/pages/chat/chat-send-submit.ts b/ui/src/pages/chat/chat-send-submit.ts index 4957ac96a685..88469f26d2ec 100644 --- a/ui/src/pages/chat/chat-send-submit.ts +++ b/ui/src/pages/chat/chat-send-submit.ts @@ -1,10 +1,11 @@ import { shouldForwardModelCommandToServer } from "../../../../src/auto-reply/commands-registry.shared.js"; import { normalizeChatFollowUpModeOverride, setLastActiveSessionKey } from "../../app/settings.ts"; +import { t } from "../../i18n/index.ts"; import type { ChatAttachment, ChatQueueSkillWorkshopRevision } from "../../lib/chat/chat-types.ts"; import { parseSlashCommand } from "../../lib/chat/commands.ts"; import { extractCompanionCommandQuestion } from "../../lib/chat/companion-question.ts"; import { resolveCurrentUserIdentity } from "../../lib/chat/current-user-identity.ts"; -import { visibleSessionMatches } from "../../lib/sessions/index.ts"; +import { scopedAgentIdForSession, visibleSessionMatches } from "../../lib/sessions/index.ts"; import { getChatAttachmentDataUrl, releaseChatAttachmentPayloads, @@ -417,6 +418,11 @@ export async function handleSendChat( if (host.sessionKey !== submittedSessionKey) { return; } + const submittedAgentId = scopedAgentIdForSession(host, submittedSessionKey); + if (!visibleSessionMatches(host, submittedSessionKey, submittedAgentId)) { + setChatError(host, t("mcpServers.sessionUnavailable")); + return; + } const cleared = messageOverride == null ? clearSubmittedComposerState(host, previousDraft, attachmentsToSend)