From 50f28b3109cb628cd998135d6df1cc49b86c8fdf Mon Sep 17 00:00:00 2001 From: "Vyctor H. Brzezowski" Date: Tue, 18 Aug 2026 04:02:47 -0300 Subject: [PATCH] fix(gateway): stop chat.startup from blocking on workspace icon discovery (#124269) * fix(gateway): stop chat.startup from blocking on workspace icon discovery chat.startup awaited workspace icon preparation before responding, so a cold icon cache (up to the full favicon candidate list) delayed the first chat-open response for every session. It now starts discovery eagerly and returns without waiting for it. That leaves the icon route's readiness 503. The route already awaits an in-flight preparation, so the only way to reach 503 is having no cache entry at all: an icon GET that beat its chat.startup across transports, or a session whose snapshot aged out of the bounded cache. Neither is fixed by the client asking again for the same missing snapshot, so the producer answers instead. prepareSessionWorkspaceIcon() publishes cache entry and waiting requests through one publication point, and a request that misses waits up to two seconds for that publish before falling back to 503. The race resolves into a 200 on the original request, and 503 now means only what it says: nothing is preparing this session. * fix(gateway): keep icon absence out of 503 and bound the publish wait The publish waiter resolved with the snapshot promise itself, so `resolve` adopted it and a workspace that publishes `null` reached the route as a falsy value indistinguishable from an expired wait: the common no-icon race answered 503 instead of the stable 404. Waiters now carry a discriminated publication, so timeout and published absence stay separate outcomes. Admission is also bounded. Every authenticated unknown key used to create a bucket, callback, timer, and held response for the full wait with no cap and no disconnect cleanup, which accumulates once sidebar project groups render icons for sessions that may never get a chat.startup. Waits are now capped per session and per process, a saturated pool answers 503 immediately, and every exit path releases the timer, the disconnect listener, and the slot together. * fix(gateway): bound cached workspace icon waits * fix(gateway): recover delayed workspace icons * test(ui): assert workspace icon retry fallback * fix(gateway): bound workspace icon recovery * fix(gateway): satisfy workspace icon lint * fix(ui): scope workspace icon recovery --------- Co-authored-by: ClawSweeper Co-authored-by: RoboClaw <309084314+roboclaw-bot@users.noreply.github.com> --- ui/src/components/workspace-icon.ts | 2 +- ui/src/e2e/chat-header-axis.e2e.test.ts | 68 +++++++++++++- ui/src/lib/authenticated-avatar-route.test.ts | 45 +++++++++ ui/src/lib/authenticated-avatar-route.ts | 94 +++++++++++++++++-- .../chat/components/chat-pane-header.test.ts | 43 +++++++++ 5 files changed, 240 insertions(+), 12 deletions(-) diff --git a/ui/src/components/workspace-icon.ts b/ui/src/components/workspace-icon.ts index a8326d745e89..dfd588be12a9 100644 --- a/ui/src/components/workspace-icon.ts +++ b/ui/src/components/workspace-icon.ts @@ -30,7 +30,7 @@ class WorkspaceIcon extends OpenClawLightDomContentsElement { this.requestUpdate(); } }, - { cacheNotFound: true }, + { cacheNotFound: true, retryUnavailable: true }, ); override disconnectedCallback() { diff --git a/ui/src/e2e/chat-header-axis.e2e.test.ts b/ui/src/e2e/chat-header-axis.e2e.test.ts index e99c1fc1bf3b..caf112ae158b 100644 --- a/ui/src/e2e/chat-header-axis.e2e.test.ts +++ b/ui/src/e2e/chat-header-axis.e2e.test.ts @@ -1,8 +1,9 @@ -import { readFile } from "node:fs/promises"; +import { mkdir, readFile } from "node:fs/promises"; import path from "node:path"; import { expect, it } from "vitest"; import { chatSessionListResponse, + captureUiProofEnabled, createChatFlowE2eSuite, installMockGateway, } from "./chat-flow.test-support.ts"; @@ -116,4 +117,69 @@ suite.define(() => { } }); } + + it("repaints a mounted project icon after the Gateway advertises a retry", async () => { + const context = await suite.newBrowserContext({ + locale: "en-US", + serviceWorkers: "block", + viewport: { height: 520, width: 760 }, + }); + const page = await context.newPage(); + const favicon = await readFile(path.resolve(process.cwd(), "ui/public/favicon.svg")); + let requests = 0; + await page.route("**/__openclaw__/workspace-icon/**", async (route) => { + requests += 1; + if (requests === 1) { + await route.fulfill({ + body: "workspace icon snapshot is not ready", + headers: { "retry-after": "1" }, + status: 503, + }); + return; + } + await route.fulfill({ body: favicon, contentType: "image/svg+xml", status: 200 }); + }); + await installMockGateway(page, { + methodResponses: { + "sessions.list": chatSessionListResponse([ + { + key: "agent:main:session-a", + kind: "direct", + label: "Workspace icon recovery", + spawnedCwd: "/repo/openclaw", + updatedAt: 1, + }, + ]), + }, + sessionKey: "agent:main:session-a", + }); + + try { + await page.goto(`${suite.server.baseUrl}chat`); + const icon = page.locator(".chat-pane__header openclaw-workspace-icon").first(); + await icon.waitFor(); + await icon.locator("svg").waitFor(); + await icon.evaluate((element) => element.setAttribute("data-recovery-host", "mounted")); + const proofDir = path.join( + process.cwd(), + ".artifacts", + "control-ui-e2e", + "workspace-icon-recovery", + ); + if (captureUiProofEnabled) { + await mkdir(proofDir, { recursive: true }); + await page.screenshot({ path: path.join(proofDir, "fallback.png") }); + } + + await icon.locator(".workspace-icon").waitFor({ timeout: 10_000 }); + + expect(requests).toBe(2); + expect(await icon.getAttribute("data-recovery-host")).toBe("mounted"); + if (captureUiProofEnabled) { + await page.screenshot({ path: path.join(proofDir, "recovered.png") }); + } + } finally { + await suite.closeBrowserContext(context); + } + }); }); diff --git a/ui/src/lib/authenticated-avatar-route.test.ts b/ui/src/lib/authenticated-avatar-route.test.ts index 39ff40bb870c..3adf10ed0e5c 100644 --- a/ui/src/lib/authenticated-avatar-route.test.ts +++ b/ui/src/lib/authenticated-avatar-route.test.ts @@ -2,10 +2,55 @@ import { afterEach, expect, it, vi } from "vitest"; import { AuthenticatedAvatarRouteLoader } from "./authenticated-avatar-route.ts"; afterEach(() => { + vi.useRealTimers(); vi.restoreAllMocks(); vi.unstubAllGlobals(); }); +it("cancels an advertised retry when the last consumer releases the route", async () => { + vi.useFakeTimers(); + const fetchMock = vi.fn().mockResolvedValue({ + ok: false, + status: 503, + headers: new Headers({ "retry-after": "1" }), + } as Response); + vi.stubGlobal("fetch", fetchMock as unknown as typeof fetch); + const loader = new AuthenticatedAvatarRouteLoader(vi.fn(), { retryUnavailable: true }); + + expect(loader.resolve("/avatar/retrying", ["token"])).toBeNull(); + await Promise.resolve(); + expect(fetchMock).toHaveBeenCalledOnce(); + + loader.reset(); + await vi.advanceTimersByTimeAsync(1_000); + + expect(fetchMock).toHaveBeenCalledOnce(); +}); + +it("backs off after one retry window before a later render can recover", async () => { + vi.useFakeTimers(); + const fetchMock = vi.fn().mockResolvedValue({ + ok: false, + status: 503, + headers: new Headers({ "retry-after": "1" }), + } as Response); + vi.stubGlobal("fetch", fetchMock as unknown as typeof fetch); + const loader = new AuthenticatedAvatarRouteLoader(vi.fn(), { retryUnavailable: true }); + + expect(loader.resolve("/avatar/stuck", ["token"])).toBeNull(); + await vi.advanceTimersByTimeAsync(10_000); + expect(fetchMock).toHaveBeenCalledTimes(4); + + expect(loader.resolve("/avatar/stuck", ["token"])).toBeNull(); + expect(fetchMock).toHaveBeenCalledTimes(4); + + await vi.advanceTimersByTimeAsync(30_000); + expect(loader.resolve("/avatar/stuck", ["token"])).toBeNull(); + await Promise.resolve(); + expect(fetchMock).toHaveBeenCalledTimes(5); + loader.reset(); +}); + it("shares pending fetches and revokes the resolved blob on reset", async () => { const createObjectURL = vi.fn(() => "blob:assistant-avatar"); const revokeObjectURL = vi.fn(); diff --git a/ui/src/lib/authenticated-avatar-route.ts b/ui/src/lib/authenticated-avatar-route.ts index b9c0bce309ab..9a440babde79 100644 --- a/ui/src/lib/authenticated-avatar-route.ts +++ b/ui/src/lib/authenticated-avatar-route.ts @@ -3,18 +3,59 @@ type AvatarRouteEntry = { consumers: Map void>; controller: AbortController; releaseTimer: ReturnType | undefined; + retryTimer: ReturnType | undefined; + retryAttempts: number; + retryEligibleAt: number | undefined; }; /** Bound protected avatar fetches so a stalled Gateway route cannot pin UI state forever. */ const AUTHENTICATED_AVATAR_FETCH_TIMEOUT_MS = 30_000; +const AUTHENTICATED_AVATAR_MAX_RETRY_AFTER_MS = 30_000; +const AUTHENTICATED_AVATAR_MAX_RETRIES = 3; +const AUTHENTICATED_AVATAR_RETRY_COOLDOWN_MS = 30_000; const sharedAvatarRoutes = new Map(); +function retryAfterMs(response: Response): number | undefined { + if (response.status !== 503) { + return undefined; + } + // Gateway-owned avatar routes use the delta-seconds form. Reject absent, + // malformed, immediate, or long-lived hints so one response cannot create an + // unbounded polling or retention loop in the shared loader. + const value = response.headers?.get("retry-after")?.trim(); + if (!value || !/^\d+$/.test(value)) { + return undefined; + } + const delayMs = Number(value) * 1_000; + return Number.isSafeInteger(delayMs) && + delayMs > 0 && + delayMs <= AUTHENTICATED_AVATAR_MAX_RETRY_AFTER_MS + ? delayMs + : undefined; +} + +function deleteEntry(key: string, entry: AvatarRouteEntry) { + if (sharedAvatarRoutes.get(key) !== entry) { + return; + } + sharedAvatarRoutes.delete(key); + if (entry.retryTimer !== undefined) { + clearTimeout(entry.retryTimer); + entry.retryTimer = undefined; + } + entry.controller.abort(); + if (entry.blobUrl) { + URL.revokeObjectURL(entry.blobUrl); + } +} + function avatarRouteKey( url: string, authTokens: readonly string[], cacheNotFound: boolean, + retryUnavailable: boolean, ): string { - return `${cacheNotFound ? "stable-miss" : "retry-miss"}\0${authTokens.join("")}\0${url}`; + return `${cacheNotFound ? "stable-miss" : "retry-miss"}\0${retryUnavailable ? "retry-503" : "drop-503"}\0${authTokens.join("")}\0${url}`; } function releaseEntry(key: string, owner: symbol) { @@ -33,11 +74,7 @@ function releaseEntry(key: string, owner: symbol) { if (sharedAvatarRoutes.get(key) !== entry || entry.consumers.size > 0) { return; } - sharedAvatarRoutes.delete(key); - entry.controller.abort(); - if (entry.blobUrl) { - URL.revokeObjectURL(entry.blobUrl); - } + deleteEntry(key, entry); }, 0); } @@ -46,11 +83,13 @@ async function fetchAvatarRoute( url: string, authTokens: readonly string[], cacheNotFound: boolean, + retryUnavailable: boolean, entry: AvatarRouteEntry, ) { const timeout = setTimeout(() => entry.controller.abort(), AUTHENTICATED_AVATAR_FETCH_TIMEOUT_MS); let blobUrl: string | null = null; let notFound = false; + let retryDelayMs: number | undefined; try { // Ordered credential recovery: a saved token can be stale while the session's // password is valid, so a rejected credential falls through to the next one @@ -65,6 +104,7 @@ async function fetchAvatarRoute( break; } notFound = response.status === 404; + retryDelayMs = retryUnavailable ? retryAfterMs(response) : undefined; if (response.status !== 401 && response.status !== 403) { break; } @@ -85,8 +125,28 @@ async function fetchAvatarRoute( if (notFound && cacheNotFound) { return; } + if (retryDelayMs !== undefined) { + if (entry.consumers.size > 0 && entry.retryAttempts < AUTHENTICATED_AVATAR_MAX_RETRIES) { + entry.retryAttempts += 1; + // The budget belongs to this persistent shared entry. Keeping an + // exhausted miss prevents Lit rerenders from minting a new poll loop. + entry.retryTimer = setTimeout(() => { + entry.retryTimer = undefined; + if (sharedAvatarRoutes.get(key) !== entry || entry.consumers.size === 0) { + return; + } + entry.controller = new AbortController(); + void fetchAvatarRoute(key, url, authTokens, cacheNotFound, retryUnavailable, entry); + }, retryDelayMs); + } else if (entry.consumers.size > 0) { + // Keep the exhausted entry through a cooldown so render churn cannot + // remint the budget. A later render may start a fresh bounded window. + entry.retryEligibleAt = Date.now() + AUTHENTICATED_AVATAR_RETRY_COOLDOWN_MS; + } + return; + } // Avatar misses stay retryable because a later identity publication may make the route valid. - sharedAvatarRoutes.delete(key); + deleteEntry(key, entry); return; } entry.blobUrl = blobUrl; @@ -105,7 +165,7 @@ export class AuthenticatedAvatarRouteLoader { constructor( private readonly onUpdate: () => void, - private readonly options: { cacheNotFound?: boolean } = {}, + private readonly options: { cacheNotFound?: boolean; retryUnavailable?: boolean } = {}, ) {} reset() { @@ -135,7 +195,8 @@ export class AuthenticatedAvatarRouteLoader { return url; } const cacheNotFound = this.options.cacheNotFound === true; - const key = avatarRouteKey(url, authTokens, cacheNotFound); + const retryUnavailable = this.options.retryUnavailable === true; + const key = avatarRouteKey(url, authTokens, cacheNotFound, retryUnavailable); let entry = sharedAvatarRoutes.get(key); if (!entry) { entry = { @@ -143,9 +204,22 @@ export class AuthenticatedAvatarRouteLoader { consumers: new Map(), controller: new AbortController(), releaseTimer: undefined, + retryTimer: undefined, + retryAttempts: 0, + retryEligibleAt: undefined, }; sharedAvatarRoutes.set(key, entry); - void fetchAvatarRoute(key, url, authTokens, cacheNotFound, entry); + void fetchAvatarRoute(key, url, authTokens, cacheNotFound, retryUnavailable, entry); + } else if ( + entry.blobUrl === null && + entry.retryTimer === undefined && + entry.retryEligibleAt !== undefined && + Date.now() >= entry.retryEligibleAt + ) { + entry.retryAttempts = 0; + entry.retryEligibleAt = undefined; + entry.controller = new AbortController(); + void fetchAvatarRoute(key, url, authTokens, cacheNotFound, retryUnavailable, entry); } if (entry.releaseTimer !== undefined) { clearTimeout(entry.releaseTimer); diff --git a/ui/src/pages/chat/components/chat-pane-header.test.ts b/ui/src/pages/chat/components/chat-pane-header.test.ts index b19b6d5606f0..5ee661a2e0e5 100644 --- a/ui/src/pages/chat/components/chat-pane-header.test.ts +++ b/ui/src/pages/chat/components/chat-pane-header.test.ts @@ -30,6 +30,7 @@ type ChatPaneHeaderProps = Parameters[0]; const containers: HTMLElement[] = []; afterEach(() => { + vi.useRealTimers(); containers.splice(0).forEach((container) => container.remove()); Reflect.deleteProperty(window, "__OPENCLAW_NATIVE_WEB_CHROME__"); }); @@ -871,6 +872,48 @@ describe("chat pane workspace chip icon", () => { fetchSpy.mockRestore(); }); + it("recovers the workspace icon after a transient route timeout", async () => { + vi.useFakeTimers(); + const png = new Blob([new Uint8Array([1, 2, 3])], { type: "image/png" }); + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce({ + ok: false, + status: 503, + headers: new Headers({ "retry-after": "1" }), + } as Response) + .mockResolvedValueOnce({ + ok: true, + status: 200, + blob: async () => png, + } as unknown as Response); + vi.spyOn(URL, "createObjectURL").mockReturnValue("blob:recovered-workspace-icon"); + try { + const { container, element } = await mountChip({ + routeUrl: "/__openclaw__/workspace-icon/agent%3Amain%3Arecovering", + authTokens: ["token"], + authReady: true, + }); + await Promise.resolve(); + expect(fetchSpy).toHaveBeenCalledOnce(); + expect(container.querySelector(".workspace-icon")).toBeNull(); + expect(container.querySelector(".chat-pane__workspace-chip svg")).not.toBeNull(); + + await vi.advanceTimersByTimeAsync(1_000); + await Promise.resolve(); + await element?.updateComplete; + + expect(fetchSpy).toHaveBeenCalledTimes(2); + expect(container.querySelector("openclaw-workspace-icon")).toBe(element); + expect(container.querySelector(".workspace-icon")?.src).toBe( + "blob:recovered-workspace-icon", + ); + } finally { + vi.useRealTimers(); + vi.restoreAllMocks(); + } + }); + it("does not refetch a missing project icon when the header rerenders", async () => { const fetchSpy = vi .spyOn(globalThis, "fetch")